Adds MD ink ripple animations to buttons within location bar

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

Index: chrome/browser/ui/views/location_bar/bubble_icon_view.h
diff --git a/chrome/browser/ui/views/location_bar/bubble_icon_view.h b/chrome/browser/ui/views/location_bar/bubble_icon_view.h
index 30df952f1dfa97caaac57a679b883faa55b58bf4..d08f968d3ae2cda19d77973ff89a0909cfc7f1c6 100644
--- a/chrome/browser/ui/views/location_bar/bubble_icon_view.h
+++ b/chrome/browser/ui/views/location_bar/bubble_icon_view.h
@@ -6,8 +6,10 @@
 #define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_BUBBLE_ICON_VIEW_H_
 
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/vector_icons_public.h"
+#include "ui/views/animation/ink_drop_host.h"
 #include "ui/views/controls/image_view.h"
 
 class CommandUpdater;
@@ -18,10 +20,11 @@ enum class VectorIconId;
 
 namespace views {
 class BubbleDelegateView;
+class InkDropDelegate;
 }
 
 // Represents an icon on the omnibox that shows a bubble when clicked.
-class BubbleIconView : public views::ImageView {
+class BubbleIconView : public views::View, public views::InkDropHost {
  protected:
   enum ExecuteSource {
     EXECUTE_SOURCE_MOUSE,
@@ -29,22 +32,38 @@ class BubbleIconView : public views::ImageView {
     EXECUTE_SOURCE_GESTURE,
   };
 
+  // The bubble icon's class name.
+  static const char kViewClassName[];

[Peter Kasting, 2016/01/26 16:59:35]

Why does this need to be a public static?  Why can't it just be a string
constant inside GetClassName()?

[varkha, 2016/01/26 17:45:45]

Done.

+
   BubbleIconView(CommandUpdater* command_updater, int command_id);
   ~BubbleIconView() override;
 
   // Returns true if a related bubble is showing.
   bool IsBubbleShowing() const;
 
+  // Sets the image that should be displayed in |image_|.
+  void SetImage(const gfx::ImageSkia* image_skia);
+
+  // Returns the image currently displayed or NULL of none is currently set.

[bruthig, 2016/01/26 16:47:25]

Can you update this comment, and the one in ImageView, because it can't actually
return NULL?

[varkha, 2016/01/26 17:45:45]

Done.

+  // The returned image is owned by the ImageView.

[Peter Kasting, 2016/01/26 16:59:35]

Nit: "the ImageView" is vague.  I would explicitly say "|image_|".

[varkha, 2016/01/26 17:45:45]

Done.

+  const gfx::ImageSkia& GetImage();
+
+  // Sets the tooltip text.
+  void SetTooltipText(const base::string16& tooltip);
+
   // Invoked prior to executing the command.
   virtual void OnExecuting(ExecuteSource execute_source) = 0;
 
-  // views::ImageView:
+  // views::View:
   void GetAccessibleState(ui::AXViewState* state) override;
   bool GetTooltipText(const gfx::Point& p, base::string16* tooltip) const
       override;
+  gfx::Size GetPreferredSize() const override;
+  void Layout() override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
   bool OnKeyPressed(const ui::KeyEvent& event) override;
+  void OnBubbleClosing() override;
   void ViewHierarchyChanged(
       const ViewHierarchyChangedDetails& details) override;
   void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
@@ -52,6 +71,10 @@ class BubbleIconView : public views::ImageView {
   // ui::EventHandler:
   void OnGestureEvent(ui::GestureEvent* event) override;
 
+  // views::InkDropHost:
+  void AddInkDropLayer(ui::Layer* ink_drop_layer) override;
+  void RemoveInkDropLayer(ui::Layer* ink_drop_layer) override;
+
  protected:
   // Calls OnExecuting and runs |command_id_| with a valid |command_updater_|.
   virtual void ExecuteCommand(ExecuteSource source);
@@ -61,7 +84,7 @@ class BubbleIconView : public views::ImageView {
 
   // Gets the given vector icon in the correct color and size based on |active|
   // and whether Chrome's in material design mode.
-  virtual gfx::VectorIconId GetVectorIcon() const = 0;
+  virtual gfx::VectorIconId GetVectorIcon() const;

[Peter Kasting, 2016/01/26 16:59:35]

Why can't we leave this pure virtual?

[varkha, 2016/01/26 17:45:45]

Because it gets called for the first time from inside a derived class
constructor (via UpdateIcon) and that would cause a pure virtual call.

 
   // Sets the image using a PNG from the resource bundle. Returns true if an
   // image was set, or false if the icon should use a vector asset. This only
@@ -69,8 +92,12 @@ class BubbleIconView : public views::ImageView {
   virtual bool SetRasterIcon();
 
   // views::View:
+  const char* GetClassName() const override;

[Peter Kasting, 2016/01/26 16:59:35]

Do we really need to override this?  Seems like nothing checks for this view's
particular class name.

[varkha, 2016/01/26 17:45:45]

We don't. I used it for debugging. Removed.

   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
 
+  // views::InkDropHost:
+  gfx::Point CalculateInkDropCenter() const override;
+
   // Updates the icon after some state has changed.
   void UpdateIcon();
 
@@ -81,6 +108,9 @@ class BubbleIconView : public views::ImageView {
   bool active() const { return active_; }
 
  private:
+  // The image shown in the button.
+  views::ImageView* image_;
+
   // The CommandUpdater for the Browser object that owns the location bar.
   CommandUpdater* command_updater_;
 
@@ -97,6 +127,9 @@ class BubbleIconView : public views::ImageView {
   // prevent the bubble from reshowing.
   bool suppress_mouse_released_action_;
 
+  // Animation delegate for the ink drop ripple effect.
+  scoped_ptr<views::InkDropDelegate> ink_drop_delegate_;
+
   DISALLOW_COPY_AND_ASSIGN(BubbleIconView);
 };