[top-chrome-md] Made extension buttons show the highlight when focused.

chrome/browser/ui/views/toolbar/toolbar_action_view.cc

Index: chrome/browser/ui/views/toolbar/toolbar_action_view.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
index 9968433c774838f490295daa411611a248a00183..5225c601e2316576f675ce6eedcaef2292bd7485 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
@@ -23,6 +23,7 @@
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/image/image_skia_source.h"
+#include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/controls/menu/menu_controller.h"
@@ -110,9 +111,24 @@ std::unique_ptr<views::InkDrop> ToolbarActionView::CreateInkDrop() {
   std::unique_ptr<views::InkDropImpl> ink_drop =
       CustomButton::CreateDefaultInkDropImpl();
   ink_drop->SetShowHighlightOnHover(!delegate_->ShownInsideMenu());
+  ink_drop->SetShowHighlightOnFocus(true);
   return std::move(ink_drop);
 }
 
+std::unique_ptr<views::InkDropRipple> ToolbarActionView::CreateInkDropRipple()
+    const {
+  if (delegate_->ShownInsideMenu()) {
+    // The ripple should not be visible when |this| is shown inside of a menu.
+    // TODO(bruthig): Use a lighter weight InkDropRipple type that doesn't
+    // consume as many the resources as the FloodFillInkDropRipple. See
+    // https://crbug.com/665214.
+    return base::MakeUnique<views::FloodFillInkDropRipple>(
+        gfx::Rect(), gfx::Point(), SK_ColorTRANSPARENT, 0.f);

[Peter Kasting, 2016/11/15 00:38:39]

Can we just return null, and have the caller handle that correctly?

[bruthig, 2016/11/15 01:25:12]

Short answer, returning null is a possible long term solution but isn't that
straight forward to do since the InkDropImpl can kick off highlight animations
based on the starting/ending of ripple animations. This is a quick fix which is
strictly better. Issue 665214 will remain open afterwards to track the work
required for a better solution.

[Peter Kasting, 2016/11/15 01:47:36]

I commented there about the overall route here.  I'm concerned that removing the
ripple isn't the right behavior change to begin with.

How bad is this bug?  Can we just leave the ripple for items inside menus for
now, until (assuming we want to make this change) we have the right framework to
do this better, e.g. some kind of null ripple class that just kicks off
subsequent animations, or callers that kick off those animations themselves when
there's no ripple, or something?  It seems like the addition on line 14 is the
most critical one to get in soon?

[bruthig, 2016/11/15 03:47:50]

Done.

+  }
+
+  return MenuButton::CreateInkDropRipple();
+}
+
 content::WebContents* ToolbarActionView::GetCurrentWebContents() const {
   return delegate_->GetCurrentWebContents();
 }