Updates Zoom bubble layout and adds +/- buttons

chrome/browser/ui/views/location_bar/zoom_bubble_view.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_ZOOM_BUBBLE_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_ZOOM_BUBBLE_VIEW_H_
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_bubble_delegate_view.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/extension_icon_image.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/label.h"
 
 namespace content {
 class WebContents;
 }
 
+namespace gfx {
+struct VectorIcon;
+}
+
 namespace views {
 class ImageButton;
 }  // namespace views
 
 // View used to display the zoom percentage when it has changed.
 class ZoomBubbleView : public LocationBarBubbleDelegateView,
                        public views::ButtonListener,
                        public ImmersiveModeController::Observer,
                        public extensions::IconImage::Observer {
  public:
@@ skipping 47 matching lines @@
                  ImmersiveModeController* immersive_mode_controller);
   ~ZoomBubbleView() override;
 
   // LocationBarBubbleDelegateView:
   void OnGestureEvent(ui::GestureEvent* event) override;
   void OnMouseEntered(const ui::MouseEvent& event) override;
   void OnMouseExited(const ui::MouseEvent& event) override;
   void Init() override;
   void WindowClosing() override;
   void CloseBubble() override;
+  bool ShouldSnapFrameWidth() const override;
+  gfx::Size GetPreferredSize() const override;

[tapted, 2017/05/02 01:28:02]

nit: GetPreferredSize() comes from views::View, so I'd order it at the top,
before the OnFooEvent methods

[varkha, 2017/05/03 01:50:28]

Done.

 
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // ImmersiveModeController::Observer:
   void OnImmersiveRevealStarted() override;
   void OnImmersiveModeControllerDestroyed() override;
 
   // extensions::IconImage::Observer:
   void OnExtensionIconImageChanged(extensions::IconImage* /* image */) override;
 
   // Sets information about the extension that initiated the zoom change.
   // Calling this method asserts that the extension |extension| did initiate
   // the zoom change.
   void SetExtensionInfo(const extensions::Extension* extension);
 
+  // Creates an ImageButton using vector |icon|, sets a tooltip with
+  // |tooltip_id|. Returns the button.
+  std::unique_ptr<views::Button> CreateZoomButton(const gfx::VectorIcon& icon,

[tapted, 2017/05/02 01:28:02]

This doesn't manipulate any members - can it be put in an anonymous namespace in
the .cc?

std::unique_ptr<views::Button> CreateZoomButton(ButtonListener* listener, const
gfx::VectorIcon& icon, int tooltip_id);

The gfx::VectorIcon forward declare might not be needed then either

[varkha, 2017/05/03 01:50:28]

Done.

+                                                  int tooltip_id);
+
+  // Updates |label_| with the up to date zoom.
+  void UpdateZoomPercent();
+
   // Starts a timer which will close the bubble if |auto_close_| is true.
   void StartTimerIfNecessary();
 
   // Stops the auto-close timer.
   void StopTimer();
 
   ZoomBubbleExtensionInfo extension_info_;
 
   // Singleton instance of the zoom bubble. The zoom bubble can only be shown on
   // the active browser window, so there is no case in which it will be shown
   // twice at the same time.
   static ZoomBubbleView* zoom_bubble_;
 
   // Timer used to auto close the bubble.
   base::OneShotTimer timer_;
 
+  // Timer duration that is made longer if a user presses + or - buttons.
+  int timer_duration_;

[tapted, 2017/05/02 01:28:02]

Add units? (e.g. timer_duration_ms_), or use a base::TimeDelta?

Or this could also be a bool `any_button_pressed_` and an `if` in
StartTimerIfNecessary

[varkha, 2017/05/03 01:50:28]

I like the self documenting base::TimeDelta consts. Done.

+
   // Image button in the zoom bubble that will show the |extension_icon_| image
   // if an extension initiated the zoom change, and links to that extension at
   // "chrome://extensions".
   views::ImageButton* image_button_;
 
   // Label displaying the zoom percentage.
   views::Label* label_;
 
+  // Action buttons that can change zoom.
+  views::Button* zoom_out_button_;
+  views::Button* zoom_in_button_;
+  views::Button* reset_button_;
+
   // The WebContents for the page whose zoom has changed.
   content::WebContents* web_contents_;
 
   // Whether the currently displayed bubble will automatically close.
   bool auto_close_;
 
+  // True when handling a button click event.
+  bool ignore_close_bubble_;
+
   // The immersive mode controller for the BrowserView containing
   // |web_contents_|.
   // Not owned.
   ImmersiveModeController* immersive_mode_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(ZoomBubbleView);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_ZOOM_BUBBLE_VIEW_H_