Add pin button to extensions menu button.

Pin button is visible when hoving the extensions menu button, when the extension's context menu is open, or when pinned.

Bug: 959920
Change-Id: I73296cfd4502eb963954fa2e0c315ff57972478c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1720862
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683260}

chrome/app/generated_resources.grd

@@ -4311,6 +4311,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
       <message name="IDS_EXTENSIONS_MENU_CONTEXT_MENU_TOOLTIP" desc="Title of the context menu for individual extensions in the Extensions Menu">
         More actions
       </message>
+      <message name="IDS_EXTENSIONS_MENU_PIN_BUTTON_TOOLTIP" desc="The tooltip for the pinning button for individual extensions in the Extensions Menu">
+        Pin extension
+      </message>
+      <message name="IDS_EXTENSIONS_MENU_UNPIN_BUTTON_TOOLTIP" desc="The tooltip for the pinning button for individual extensions in the Extensions Menu">
+        Unpin extension
+      </message>
       <message name="IDS_EXTENSIONS_MENU_ACCESSING_SITE_DATA" desc="Header for section of extensions that are able to access the current site's data">
         Accessing this site's data
       </message>

chrome/browser/ui/views/extensions/extensions_menu_button.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_view.h"
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -15,9 +16,11 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/button/image_button_factory.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/layout_provider.h"
+#include "ui/views/vector_icons.h"
 #include "ui/views/view_class_properties.h"
 
 const char ExtensionsMenuButton::kClassName[] = "ExtensionsMenuButton";
@@ -25,6 +28,15 @@ const char ExtensionsMenuButton::kClassName[] = "ExtensionsMenuButton";
 namespace {
 
 constexpr int EXTENSION_CONTEXT_MENU = 13;
+constexpr int EXTENSION_PINNING = 14;
+
+constexpr int kSecondaryIconSizeDp = 16;
+
+void SetSecondaryButtonHighlightPath(views::Button* button) {
+  auto highlight_path = std::make_unique<SkPath>();
+  highlight_path->addOval(gfx::RectToSkRect(gfx::Rect(button->size())));
+  button->SetProperty(views::kHighlightPathKey, highlight_path.release());
+}
 
 }  // namespace
 
@@ -39,7 +51,11 @@ ExtensionsMenuButton::ExtensionsMenuButton(
                   true,
                   true),
       browser_(browser),
-      controller_(std::move(controller)) {
+      controller_(std::move(controller)),
+      model_(ToolbarActionsModel::Get(browser_->profile())) {
+  // Set so the extension button receives enter/exit on children to retain hover
+  // status when hovering child views.
+  set_notify_enter_exit_on_child(true);
   ConfigureSecondaryView();
   set_auto_compute_tooltip(false);
   controller_->SetDelegate(this);
@@ -48,6 +64,42 @@ ExtensionsMenuButton::ExtensionsMenuButton(
 
 ExtensionsMenuButton::~ExtensionsMenuButton() = default;
 
+void ExtensionsMenuButton::UpdatePinButton() {
+  pin_button_->SetTooltipText(l10n_util::GetStringUTF16(
+      IsPinned() ? IDS_EXTENSIONS_MENU_UNPIN_BUTTON_TOOLTIP
+                 : IDS_EXTENSIONS_MENU_PIN_BUTTON_TOOLTIP));
+  SkColor unpinned_icon_color =
+      ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeEnabled()
+          ? gfx::kGoogleGrey500
+          : gfx::kChromeIconGrey;
+  SkColor icon_color = IsPinned()
+                           ? GetNativeTheme()->GetSystemColor(
+                                 ui::NativeTheme::kColorId_ProminentButtonColor)
+                           : unpinned_icon_color;
+  views::SetImageFromVectorIcon(
+      pin_button_, IsPinned() ? views::kUnpinIcon : views::kPinIcon,
+      kSecondaryIconSizeDp, icon_color);
+  pin_button_->SetVisible(IsPinned() || IsMouseHovered() || IsMenuRunning());
+}
+
+void ExtensionsMenuButton::OnMouseEntered(const ui::MouseEvent& event) {
+  UpdatePinButton();
+  // The layout manager does not get notified of visibility changes and the pin
+  // buttons has not be laid out before if it was invisible.
+  pin_button_->InvalidateLayout();
+  views::Button::OnMouseEntered(event);
+}
+
+void ExtensionsMenuButton::OnMouseExited(const ui::MouseEvent& event) {
+  UpdatePinButton();
+  views::Button::OnMouseExited(event);
+}
+
+void ExtensionsMenuButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  UpdatePinButton();
+  HoverButton::OnBoundsChanged(previous_bounds);
+}
+
 const char* ExtensionsMenuButton::GetClassName() const {
   return kClassName;
 }
@@ -57,6 +109,9 @@ void ExtensionsMenuButton::ButtonPressed(Button* sender,
   if (sender->GetID() == EXTENSION_CONTEXT_MENU) {
     RunExtensionContextMenu(ui::MENU_SOURCE_MOUSE);
     return;
+  } else if (sender->GetID() == EXTENSION_PINNING) {
+    model_->SetActionVisibility(controller_->GetId(), !IsPinned());
+    return;
   }
   DCHECK_EQ(this, sender);
   controller_->ExecuteAction(true);
@@ -123,6 +178,10 @@ void ExtensionsMenuButton::OnMenuClosed() {
   menu_runner_.reset();
   controller_->OnContextMenuClosed();
   menu_adapter_.reset();
+  // OnMouseExited is triggered when the context menu is opened. Since we don't
+  // hide the pin button OnMouseExited if the context menu is open we must
+  // update its state to hide it when the context menu is closed.
+  UpdatePinButton();
 }
 
 void ExtensionsMenuButton::ConfigureSecondaryView() {
@@ -136,6 +195,16 @@ void ExtensionsMenuButton::ConfigureSecondaryView() {
           ? gfx::kGoogleGrey500
           : gfx::kChromeIconGrey;
 
+  auto pin_button = views::CreateVectorImageButton(this);
+  pin_button->SetID(EXTENSION_PINNING);
+  pin_button->set_ink_drop_base_color(icon_color);
+  pin_button->SizeToPreferredSize();
+
+  pin_button_ = pin_button.get();
+  SetSecondaryButtonHighlightPath(pin_button_);
+  UpdatePinButton();
+  container->AddChildView(std::move(pin_button));
+
   auto context_menu_button =
       std::make_unique<views::MenuButton>(base::string16(), this);
   context_menu_button->SetID(EXTENSION_CONTEXT_MENU);
@@ -144,7 +213,9 @@ void ExtensionsMenuButton::ConfigureSecondaryView() {
 
   context_menu_button->SetImage(
       views::Button::STATE_NORMAL,
-      gfx::CreateVectorIcon(kBrowserToolsIcon, 16, icon_color));
+      gfx::CreateVectorIcon(kBrowserToolsIcon, kSecondaryIconSizeDp,
+                            icon_color));
+
   context_menu_button->set_ink_drop_base_color(icon_color);
   context_menu_button->SetBorder(
       views::CreateEmptyBorder(views::LayoutProvider::Get()->GetInsetsMetric(
@@ -153,12 +224,15 @@ void ExtensionsMenuButton::ConfigureSecondaryView() {
 
   context_menu_button->SetInkDropMode(InkDropMode::ON);
   context_menu_button->set_has_ink_drop_action_on_click(true);
-  auto highlight_path = std::make_unique<SkPath>();
-  highlight_path->addOval(
-      gfx::RectToSkRect(gfx::Rect(context_menu_button->size())));
-  context_menu_button->SetProperty(views::kHighlightPathKey,
-                                   highlight_path.release());
 
   context_menu_button_ = context_menu_button.get();
+  SetSecondaryButtonHighlightPath(context_menu_button_);
   container->AddChildView(std::move(context_menu_button));
 }
+
+bool ExtensionsMenuButton::IsPinned() {
+  // |model_| can be null in unit tests.
+  if (!model_)
+    return false;
+  return model_->IsActionPinned(controller_->GetId());
+}

chrome/browser/ui/views/extensions/extensions_menu_button.h

@@ -15,6 +15,7 @@
 
 namespace views {
 class Button;
+class ImageButton;
 class MenuButton;
 class MenuModelAdapter;
 class MenuRunner;
@@ -28,9 +29,18 @@ class ExtensionsMenuButton : public HoverButton,
                        std::unique_ptr<ToolbarActionViewController> controller);
   ~ExtensionsMenuButton() override;
 
+  // Update pin button icon, color, tooltip, and visibility based on pinned
+  // state.
+  void UpdatePinButton();
+
   static const char kClassName[];
 
  private:
+  // views::Button:
+  void OnMouseEntered(const ui::MouseEvent& event) override;
+  void OnMouseExited(const ui::MouseEvent& event) override;
+  void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
+
   // views::ButtonListener:
   const char* GetClassName() const override;
   void ButtonPressed(Button* sender, const ui::Event& event) override;
@@ -51,8 +61,11 @@ class ExtensionsMenuButton : public HoverButton,
   // Configures the secondary (right-hand-side) view of this HoverButton.
   void ConfigureSecondaryView();
 
+  bool IsPinned();
+
   Browser* const browser_;
   const std::unique_ptr<ToolbarActionViewController> controller_;
+  ToolbarActionsModel* const model_;
 
   // TODO(pbos): There's complicated configuration code in place since menus
   // can't be triggered from ImageButtons. When MenuRunner::RunMenuAt accepts
@@ -60,6 +73,8 @@ class ExtensionsMenuButton : public HoverButton,
   // image_button_factory.h methods to configure it.
   views::MenuButton* context_menu_button_ = nullptr;
 
+  views::ImageButton* pin_button_ = nullptr;
+
   // Responsible for converting the context menu model into |menu_|.
   std::unique_ptr<views::MenuModelAdapter> menu_adapter_;
 

chrome/browser/ui/views/extensions/extensions_menu_button_unittest.cc

@@ -5,8 +5,10 @@
 #include "chrome/browser/ui/views/extensions/extensions_menu_button.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/native_widget_factory.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "ui/events/event.h"
@@ -19,6 +21,7 @@ class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
       : initial_extension_name_(base::ASCIIToUTF16("Initial Extension Name")),
         initial_tooltip_(base::ASCIIToUTF16("Initial tooltip")) {}
   void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(features::kExtensionsToolbarMenu);
     BrowserWithTestWindowTest::SetUp();
 
     widget_ = std::make_unique<views::Widget>();
@@ -64,6 +67,7 @@ class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
     test_api.NotifyClick(click_event);
   }
 
+  base::test::ScopedFeatureList scoped_feature_list_;
   const base::string16 initial_extension_name_;
   const base::string16 initial_tooltip_;
   std::unique_ptr<views::Widget> widget_;

chrome/browser/ui/views/extensions/extensions_menu_view.cc

@@ -63,6 +63,7 @@ ExtensionsMenuView::ExtensionsMenuView(
 ExtensionsMenuView::~ExtensionsMenuView() {
   DCHECK_EQ(g_extensions_dialog, this);
   g_extensions_dialog = nullptr;
+  extension_menu_buttons_.clear();
 }
 
 void ExtensionsMenuView::ButtonPressed(views::Button* sender,
@@ -113,6 +114,7 @@ void ExtensionsMenuView::Repopulate() {
 
 std::unique_ptr<views::View>
 ExtensionsMenuView::CreateExtensionButtonsContainer() {
+  extension_menu_buttons_.clear();
   content::WebContents* const web_contents =
       browser_->tab_strip_model()->GetActiveWebContents();
 
@@ -139,7 +141,6 @@ ExtensionsMenuView::CreateExtensionButtonsContainer() {
   }
 
   auto extension_buttons = std::make_unique<views::View>();
-  extension_menu_button_container_for_testing_ = extension_buttons.get();
   extension_buttons->SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kVertical));
 
@@ -179,9 +180,11 @@ ExtensionsMenuView::CreateExtensionButtonsContainer() {
             });
 
         for (auto& controller : *controller_group) {
-          extension_buttons->AddChildView(
+          std::unique_ptr<ExtensionsMenuButton> extension_button =
               std::make_unique<ExtensionsMenuButton>(browser_,
-                                                     std::move(controller)));
+                                                     std::move(controller));
+          extension_menu_buttons_.push_back(extension_button.get());
+          extension_buttons->AddChildView(std::move(extension_button));
         }
         controller_group->clear();
       };
@@ -233,7 +236,9 @@ void ExtensionsMenuView::OnToolbarModelInitialized() {
 }
 
 void ExtensionsMenuView::OnToolbarPinnedActionsChanged() {
-  Repopulate();
+  for (auto* button : extension_menu_buttons_) {
+    button->UpdatePinButton();
+  }
 }
 
 // static

chrome/browser/ui/views/extensions/extensions_menu_view.h

@@ -16,6 +16,7 @@ class Button;
 class ImageView;
 }  // namespace views
 
+class ExtensionsMenuButton;
 class ExtensionsContainer;
 
 // This bubble view displays a list of user extensions.
@@ -62,8 +63,8 @@ class ExtensionsMenuView : public views::ButtonListener,
   void OnToolbarModelInitialized() override;
   void OnToolbarPinnedActionsChanged() override;
 
-  views::View* extension_menu_button_container_for_testing() {
-    return extension_menu_button_container_for_testing_;
+  std::vector<ExtensionsMenuButton*> extension_menu_buttons_for_testing() {
+    return extension_menu_buttons_;
   }
   views::Button* manage_extensions_button_for_testing() {
     return manage_extensions_button_for_testing_;
@@ -78,8 +79,8 @@ class ExtensionsMenuView : public views::ButtonListener,
   ToolbarActionsModel* const model_;
   ScopedObserver<ToolbarActionsModel, ToolbarActionsModel::Observer>
       model_observer_;
+  std::vector<ExtensionsMenuButton*> extension_menu_buttons_;
 
-  views::View* extension_menu_button_container_for_testing_ = nullptr;
   views::Button* manage_extensions_button_for_testing_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionsMenuView);

chrome/browser/ui/views/extensions/extensions_menu_view_browsertest.cc

@@ -58,14 +58,8 @@ class ExtensionsMenuViewBrowserTest : public DialogBrowserTest {
   }
 
   static std::vector<ExtensionsMenuButton*> GetExtensionMenuButtons() {
-    std::vector<ExtensionsMenuButton*> buttons;
-    for (auto* view : ExtensionsMenuView::GetExtensionsMenuViewForTesting()
-                          ->extension_menu_button_container_for_testing()
-                          ->children()) {
-      if (view->GetClassName() == ExtensionsMenuButton::kClassName)
-        buttons.push_back(static_cast<ExtensionsMenuButton*>(view));
-    }
-    return buttons;
+    return ExtensionsMenuView::GetExtensionsMenuViewForTesting()
+        ->extension_menu_buttons_for_testing();
   }
 
   std::vector<ToolbarActionView*> GetToolbarActionViews() const {