Add Manage Extensions footer button to menu

This makes the Extensions menu use a standard title and close-x. The
footer entry is easier to notice and understand.

Removes work that's been done for custom headers. First-iteration work
is not looking to introduce panes to this dialog.

Bug: chromium:943702
Change-Id: Ic42fa05953be92f0a53448dc1a7f1c628a511dca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566619
Commit-Queue: Peter Boström <pbos@chromium.org>
Commit-Queue: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: Taylor Bergquist <tbergquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650519}

chrome/app/generated_resources.grd

@@ -4269,9 +4269,6 @@ Keep your key file in a safe place. You will need it to create new versions of y
       <message name="IDS_EXTENSIONS_MENU_TITLE" desc="Title of the Extensions Menu">
         Extensions
       </message>
-      <message name="IDS_EXTENSIONS_MENU_SETTINGS_TOOLTIP" desc="Tooltip for the 'manage extensions' button inside the Extensions Menu">
-        Manage extensions
-      </message>
 
       <!-- Settings API bubble -->
       <message name="IDS_EXTENSIONS_SETTINGS_API_TITLE_HOME_PAGE_BUBBLE" desc="Title of a bubble warning users that an extension has overridden their home page setting">

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

@@ -15,8 +15,10 @@
 #include "chrome/grit/generated_resources.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/image_button_factory.h"
+#include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/flex_layout.h"
 #include "ui/views/view_class_properties.h"
@@ -26,30 +28,14 @@ ExtensionsMenuView* g_extensions_dialog = nullptr;
 
 constexpr int EXTENSIONS_SETTINGS_ID = 42;
 
-std::unique_ptr<views::View> CreateHeaderView(const base::string16& title) {
-  auto container = std::make_unique<views::View>();
-
-  auto* layout_manager =
-      container->SetLayoutManager(std::make_unique<views::FlexLayout>());
-  layout_manager->SetOrientation(views::LayoutOrientation::kHorizontal)
-      .SetCrossAxisAlignment(views::LayoutAlignment::kCenter);
-
-  gfx::Insets header_insets =
-      ChromeLayoutProvider::Get()->GetInsetsMetric(views::INSETS_DIALOG);
-  container->SetProperty(views::kMarginsKey, new gfx::Insets(header_insets));
-
-  views::Label* title_label =
-      new views::Label(title, views::style::CONTEXT_DIALOG_TITLE);
-  title_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  title_label->SetFocusBehavior(views::View::FocusBehavior::ACCESSIBLE_ONLY);
-
-  container->AddChildView(title_label);
-  layout_manager->SetFlexForView(title_label,
-                                 views::FlexSpecification::ForSizeRule(
-                                     views::MinimumFlexSizeRule::kPreferred,
-                                     views::MaximumFlexSizeRule::kUnbounded));
-  return container;
+gfx::ImageSkia CreateVectorIcon(const gfx::VectorIcon& icon) {
+  return gfx::CreateVectorIcon(
+      icon, 16,
+      ui::NativeTheme::GetInstanceForNativeUi()->SystemDarkModeEnabled()
+          ? gfx::kGoogleGrey500
+          : gfx::kChromeIconGrey);
 }
+
 }  // namespace
 
 ExtensionsMenuView::ExtensionsMenuView(views::View* anchor_view,
@@ -79,17 +65,18 @@ ExtensionsMenuView::~ExtensionsMenuView() {
 
 void ExtensionsMenuView::ButtonPressed(views::Button* sender,
                                        const ui::Event& event) {
-  if (sender->id() != EXTENSIONS_SETTINGS_ID)
-    return;
+  DCHECK_EQ(sender->id(), EXTENSIONS_SETTINGS_ID);
   chrome::ShowExtensions(browser_, std::string());
 }
 
-base::string16 ExtensionsMenuView::GetAccessibleWindowTitle() const {
-  // TODO(pbos): Revisit this when subpanes exist so that the title read by a11y
-  // tools are in sync with the current visuals.
+base::string16 ExtensionsMenuView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_EXTENSIONS_MENU_TITLE);
 }
 
+bool ExtensionsMenuView::ShouldShowCloseButton() const {
+  return true;
+}
+
 bool ExtensionsMenuView::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
   if (accelerator.key_code() != ui::VKEY_DOWN &&
@@ -112,25 +99,19 @@ bool ExtensionsMenuView::ShouldSnapFrameWidth() const {
 void ExtensionsMenuView::Repopulate() {
   RemoveAllChildViews(true);
 
-  auto header =
-      CreateHeaderView(l10n_util::GetStringUTF16(IDS_EXTENSIONS_MENU_TITLE));
-  header->AddChildView(CreateImageButtonForHeader(
-      kSettingsIcon, EXTENSIONS_SETTINGS_ID,
-      l10n_util::GetStringUTF16(IDS_EXTENSIONS_MENU_SETTINGS_TOOLTIP)));
-  AddChildView(std::move(header));
-
   for (auto action_id : model_->action_ids()) {
     AddChildView(std::make_unique<ExtensionsMenuButton>(
         browser_, model_->CreateActionForId(browser_, toolbar_actions_bar_,
                                             false, action_id)));
   }
 
-  // TODO(pbos): This is a placeholder until we have proper UI treatment of the
-  // no-extensions case.
-  if (model_->action_ids().empty()) {
-    AddChildView(std::make_unique<views::Label>(
-        l10n_util::GetStringUTF16(IDS_EXTENSIONS_MENU_TITLE)));
-  }
+  AddChildView(std::make_unique<views::Separator>());
+  auto footer = std::make_unique<HoverButton>(
+      this, CreateVectorIcon(kSettingsIcon),
+      l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION));
+  footer->set_id(EXTENSIONS_SETTINGS_ID);
+  manage_extensions_button_for_testing_ = footer.get();
+  AddChildView(std::move(footer));
 }
 
 // TODO(pbos): Revisit observed events below.
@@ -189,25 +170,3 @@ void ExtensionsMenuView::Hide() {
 ExtensionsMenuView* ExtensionsMenuView::GetExtensionsMenuViewForTesting() {
   return g_extensions_dialog;
 }
-
-std::unique_ptr<views::ImageButton>
-ExtensionsMenuView::CreateImageButtonForHeader(const gfx::VectorIcon& icon,
-                                               int id,
-                                               const base::string16& tooltip) {
-  views::ImageButton* image_button = views::CreateVectorImageButton(this);
-  views::SetImageFromVectorIconWithColor(
-      image_button, icon,
-      GetNativeTheme()->SystemDarkModeEnabled()
-          ? SkColorSetA(SK_ColorWHITE, 0xDD)
-          : gfx::kGoogleGrey700);
-  image_button->set_id(id);
-  image_button->SetTooltipText(tooltip);
-  image_button->SizeToPreferredSize();
-
-  // Let the settings button use a circular inkdrop shape.
-  auto highlight_path = std::make_unique<SkPath>();
-  highlight_path->addOval(gfx::RectToSkRect(gfx::Rect(image_button->size())));
-  image_button->SetProperty(views::kHighlightPathKey, highlight_path.release());
-
-  return base::WrapUnique<views::ImageButton>(image_button);
-}

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

@@ -11,12 +11,8 @@
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 #include "ui/views/controls/button/button.h"
 
-namespace gfx {
-struct VectorIcon;
-}  // namespace gfx
-
 namespace views {
-class ImageButton;
+class Button;
 }  // namespace views
 
 class ToolbarActionsBar;
@@ -44,7 +40,8 @@ class ExtensionsMenuView : public views::ButtonListener,
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // views::BubbleDialogDelegateView:
-  base::string16 GetAccessibleWindowTitle() const override;
+  base::string16 GetWindowTitle() const override;
+  bool ShouldShowCloseButton() const override;
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
   int GetDialogButtons() const override;
   bool ShouldSnapFrameWidth() const override;
@@ -63,20 +60,21 @@ class ExtensionsMenuView : public views::ButtonListener,
   void OnToolbarHighlightModeChanged(bool is_highlighting) override;
   void OnToolbarModelInitialized() override;
 
+  views::Button* manage_extensions_button_for_testing() {
+    return manage_extensions_button_for_testing_;
+  }
+
  private:
   void Repopulate();
 
-  std::unique_ptr<views::ImageButton> CreateImageButtonForHeader(
-      const gfx::VectorIcon& icon,
-      int id,
-      const base::string16& tooltip);
-
   Browser* const browser_;
   ToolbarActionsBar* const toolbar_actions_bar_;
   ToolbarActionsModel* const model_;
   ScopedObserver<ToolbarActionsModel, ToolbarActionsModel::Observer>
       model_observer_;
 
+  views::Button* manage_extensions_button_for_testing_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionsMenuView);
 };
 

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

@@ -14,6 +14,8 @@
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/webui_url_constants.h"
+#include "ui/views/test/button_test_api.h"
 
 class ExtensionsMenuViewBrowserTest : public DialogBrowserTest {
  protected:
@@ -74,3 +76,23 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
   EXPECT_EQ(extensions_.size(), GetExtensionMenuButtons().size());
   DismissUi();
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
+                       ManageExtensionsOpensExtensionsPage) {
+  ShowUi("");
+  VerifyUi();
+
+  EXPECT_TRUE(ExtensionsMenuView::IsShowing());
+
+  ui::MouseEvent click_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                             base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON, 0);
+  views::test::ButtonTestApi test_api(
+      ExtensionsMenuView::GetExtensionsMenuViewForTesting()
+          ->manage_extensions_button_for_testing());
+  test_api.NotifyClick(click_event);
+
+  // Clicking the Manage Extensions button should open chrome://extensions.
+  EXPECT_EQ(
+      chrome::kChromeUIExtensionsURL,
+      browser()->tab_strip_model()->GetActiveWebContents()->GetVisibleURL());
+}