Enabled ink drop highlight for focus on bookmark bar buttons.

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

 // 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.
 
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <limits>
@@ skipping 199 matching lines @@
   }
 
   bool IsTriggerableEvent(const ui::Event& e) override {
     return e.type() == ui::ET_GESTURE_TAP ||
            e.type() == ui::ET_GESTURE_TAP_DOWN ||
            event_utils::IsPossibleDispositionEvent(e);
   }
 
   // LabelButton:
   std::unique_ptr<views::InkDrop> CreateInkDrop() override {
-    return CreateDefaultFloodFillInkDropImpl();
+    std::unique_ptr<views::InkDropImpl> ink_drop =
+        CreateDefaultFloodFillInkDropImpl();
+    ink_drop->SetShowHighlightOnFocus(true);

[Peter Kasting, 2016/11/17 19:31:57]

Should CreateDefaultFloodFillInkDropImpl() do this by default?  Should it take
an arg as to whether the created ink drop should show highlight on focus, so
callers have to explicitly choose?

[bruthig, 2016/11/17 19:43:33]

At a high level there is a lot of rework opportunities with how the InkDrop
parts are constructed.  I'm hoping to replace the virtual CreateInkDrop*()
functions in InkDropHost/InkDropHostView with a factory design pattern. This
will hopefully make it more obvious/explicit/etc to clients as to how the ink
drop is configured.

With respect to the specific request tho, UI elements that use the ink drop
highlight to show keyboard focus are quite special and there are really only 2
or 3 surfaces that do it, namely toolbar buttons and bookmark bar buttons.  I
wouldn't want to over burden all the other elements with an extra param for such
a special case.

I plan to leave it as is unless you have strong objections.

+    return std::move(ink_drop);
   }
 
   std::unique_ptr<views::InkDropRipple> CreateInkDropRipple() const override {
     return base::MakeUnique<views::FloodFillInkDropRipple>(
         CalculateInkDropBounds(size()), GetInkDropCenterBasedOnLastEvent(),
         GetInkDropBaseColor(), ink_drop_visible_opacity());
   }
 
   std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
       const override {
@@ skipping 99 matching lines @@
   BookmarkMenuButtonBase(const base::string16& title,
                          views::MenuButtonListener* menu_button_listener,
                          bool show_menu_marker)
       : MenuButton(title, menu_button_listener, show_menu_marker) {
     SetInkDropMode(InkDropMode::ON);
     SetFocusPainter(nullptr);
   }
 
   // MenuButton:
   std::unique_ptr<views::InkDrop> CreateInkDrop() override {
-    return CreateDefaultFloodFillInkDropImpl();
+    std::unique_ptr<views::InkDropImpl> ink_drop =
+        CreateDefaultFloodFillInkDropImpl();
+    ink_drop->SetShowHighlightOnFocus(true);
+    return std::move(ink_drop);
   }
 
   std::unique_ptr<views::InkDropRipple> CreateInkDropRipple() const override {
     return base::MakeUnique<views::FloodFillInkDropRipple>(
         CalculateInkDropBounds(size()), GetInkDropCenterBasedOnLastEvent(),
         GetInkDropBaseColor(), ink_drop_visible_opacity());
   }
 
   std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
       const override {
@@ skipping 1784 matching lines @@
     return;
   apps_page_shortcut_->SetVisible(visible);
   UpdateBookmarksSeparatorVisibility();
   LayoutAndPaint();
 }
 
 void BookmarkBarView::OnShowManagedBookmarksPrefChanged() {
   if (UpdateOtherAndManagedButtonsVisibility())
     LayoutAndPaint();
 }