desktop-pwas: Hide the Extensions menu item when there are no Extensions

This CL does 3 things:
 - Hides the Extensions menu item from web app windows when no
   Extensions are installed.
 - Fixes a bug exposed by the updated test where the Extensions puzzle
   piece would show when an Extension was installed due to the
   layout animation.
 - Addresses comments from
   https://chromium-review.googlesource.com/c/chromium/src/+/2602305

Bug: 1164829
Change-Id: I75e3fc5e8a7262316947b7322b825a10be3bb848
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2621512
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842372}

chrome/browser/ui/extensions/extensions_container.h

@@ -76,7 +76,11 @@ class ExtensionsContainer {
   virtual void ShowToolbarActionBubbleAsync(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) = 0;
 
+  // Toggle the Extensions menu (as if the user clicked the puzzle piece icon).
   virtual void ToggleExtensionsMenu() = 0;
+
+  // Whether there are any Extensions registered with the ExtensionsContainer.
+  virtual bool HasAnyExtensions() const = 0;
 };
 
 #endif  // CHROME_BROWSER_UI_EXTENSIONS_EXTENSIONS_CONTAINER_H_

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

@@ -608,10 +608,17 @@ void ToolbarActionsBar::ShowToolbarActionBubbleAsync(
 
 void ToolbarActionsBar::ToggleExtensionsMenu() {
   // This is only implemented by |ExtensionsToolbarContainer|.
-  // TODO(crbug.com/943702): Remove this entire class.
+  // TODO(crbug.com/1165609): Remove this entire class.
   NOTREACHED();
 }
 
+bool ToolbarActionsBar::HasAnyExtensions() const {
+  // This is only implemented by |ExtensionsToolbarContainer|.
+  // TODO(crbug.com/1165609): Remove this entire class.
+  NOTREACHED();
+  return false;
+}
+
 bool ToolbarActionsBar::CloseOverflowMenuIfOpen() {
   return delegate_->CloseOverflowMenuIfOpen();
 }

chrome/browser/ui/toolbar/toolbar_actions_bar.h

@@ -249,6 +249,7 @@ class ToolbarActionsBar : public ExtensionsContainer,
   void ShowToolbarActionBubbleAsync(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override;
   void ToggleExtensionsMenu() override;
+  bool HasAnyExtensions() const override;
 
  private:
   // Returns the insets by which the icon area bounds (See GetIconAreaRect())

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

@@ -26,8 +26,8 @@ class ExtensionsToolbarButton : public ToolbarButton,
   ExtensionsToolbarButton& operator=(const ExtensionsToolbarButton&) = delete;
   ~ExtensionsToolbarButton() override;
 
-  // Activate the Extensions menu. If the ExtensionsToolbarContainer is in
-  // kAutoHide mode this will cause it to show.
+  // Toggle the Extensions menu. If the ExtensionsToolbarContainer is in
+  // kAutoHide mode and hidden this will cause it to show.
   void ToggleExtensionsMenu();
 
   bool IsExtensionsMenuShowing() const;

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

@@ -391,6 +391,10 @@ void ExtensionsToolbarContainer::ToggleExtensionsMenu() {
   extensions_button_->ToggleExtensionsMenu();
 }
 
+bool ExtensionsToolbarContainer::HasAnyExtensions() const {
+  return !actions_.empty();
+}
+
 void ExtensionsToolbarContainer::OnTabStripModelChanged(
     TabStripModel* tab_strip_model,
     const TabStripModelChange& change,
@@ -407,7 +411,11 @@ void ExtensionsToolbarContainer::OnToolbarActionAdded(
     int index) {
   CreateActionForId(action_id);
   ReorderViews();
-  UpdateContainerVisibility();
+
+  // Auto hide mode should not become visible due to extensions being added,
+  // only due to user interaction.
+  if (display_mode_ != DisplayMode::kAutoHide)
+    UpdateContainerVisibility();
 }
 
 void ExtensionsToolbarContainer::OnToolbarActionRemoved(
@@ -712,7 +720,7 @@ void ExtensionsToolbarContainer::UpdateContainerVisibility() {
 bool ExtensionsToolbarContainer::ShouldContainerBeVisible() const {
   // The container (and extensions-menu button) should not be visible if we have
   // no extensions.
-  if (actions_.empty())
+  if (!HasAnyExtensions())
     return false;
 
   // All other display modes are constantly visible.

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

@@ -54,13 +54,13 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
     // be hidden if the available space does not allow for them. Compact mode is
     // used in smaller windows (e.g. web apps) where
     // there may not be enough space to display the buttons.
+    // TODO(crbug.com/1155421): Remove kCompact in favour of kAutoHide once the
+    // |kDesktopPWAsElidedExtensionsMenu| flag is removed.
     kCompact,
     // In auto hide mode the menu icon is hidden until
     // extensions_button()->ToggleExtensionsMenu() is called by the embedder.
-    // This
-    // is used for windows that want to minimize the number of visible icons in
-    // their
-    // toolbar (e.g. web apps).
+    // This is used for windows that want to minimize the number of visible
+    // icons in their toolbar (e.g. web apps).
     kAutoHide,
   };
 
@@ -141,6 +141,7 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
   void ShowToolbarActionBubbleAsync(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override;
   void ToggleExtensionsMenu() override;
+  bool HasAnyExtensions() const override;
 
   // ToolbarActionView::Delegate:
   content::WebContents* GetCurrentWebContents() override;

chrome/browser/ui/views/web_apps/frame_toolbar/web_app_frame_toolbar_browsertest.cc

@@ -97,6 +97,19 @@ class WebAppFrameToolbarBrowserTest : public InProcessBrowserTest {
     return &web_app_frame_toolbar_helper_;
   }
 
+  bool IsMenuCommandEnabled(int command_id) {
+    auto app_menu_model = std::make_unique<WebAppMenuModel>(
+        /*provider=*/nullptr, helper()->app_browser());
+    app_menu_model->Init();
+    ui::MenuModel* model = app_menu_model.get();
+    int index = -1;
+    if (!app_menu_model->GetModelAndIndexForCommandId(command_id, &model,
+                                                      &index)) {
+      return false;
+    }
+    return model->IsEnabledAt(index);
+  }
+
  private:
   net::EmbeddedTestServer https_server_;
   WebAppFrameToolbarTestHelper web_app_frame_toolbar_helper_;
@@ -311,26 +324,27 @@ class WebAppFrameToolbarBrowserTest_ElidedExtensionsMenu
 
 IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest_ElidedExtensionsMenu,
                        Test) {
-  LoadTestPopUpExtension(browser()->profile());
   helper()->InstallAndLaunchWebApp(browser(), GURL("https://test.org"));
 
-  WebAppToolbarButtonContainer* toolbar_button_container =
-      helper()->web_app_frame_toolbar()->get_right_container_for_testing();
+  // There should be no menu entry for opening the Extensions menu prior to
+  // installing Extensions.
+  EXPECT_FALSE(IsMenuCommandEnabled(WebAppMenuModel::kExtensionsMenuCommandId));
+
+  // Install test Extension.
+  LoadTestPopUpExtension(browser()->profile());
 
   // There should be no visible Extensions icon.
+  WebAppToolbarButtonContainer* toolbar_button_container =
+      helper()->web_app_frame_toolbar()->get_right_container_for_testing();
   EXPECT_FALSE(toolbar_button_container->extensions_container()->GetVisible());
 
   // There should be a menu entry for opening the Extensions menu.
+  EXPECT_TRUE(IsMenuCommandEnabled(WebAppMenuModel::kExtensionsMenuCommandId));
+
+  // Trigger the Extensions menu entry.
   auto app_menu_model = std::make_unique<WebAppMenuModel>(
       /*provider=*/nullptr, helper()->app_browser());
   app_menu_model->Init();
-  ui::MenuModel* model = app_menu_model.get();
-  int index = -1;
-  const bool found = app_menu_model->GetModelAndIndexForCommandId(
-      WebAppMenuModel::kExtensionsMenuCommandId, &model, &index);
-  EXPECT_TRUE(found);
-  EXPECT_TRUE(model->IsEnabledAt(index));
-
   app_menu_model->ExecuteCommand(WebAppMenuModel::kExtensionsMenuCommandId,
                                  /*event_flags=*/0);
 
@@ -371,12 +385,5 @@ IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest_NoElidedExtensionsMenu,
   EXPECT_TRUE(toolbar_button_container->extensions_container()->GetVisible());
 
   // There should be no menu entry for opening the Extensions menu.
-  auto app_menu_model = std::make_unique<WebAppMenuModel>(
-      /*provider=*/nullptr, helper()->app_browser());
-  app_menu_model->Init();
-  ui::MenuModel* model = app_menu_model.get();
-  int index = -1;
-  const bool found = app_menu_model->GetModelAndIndexForCommandId(
-      WebAppMenuModel::kExtensionsMenuCommandId, &model, &index);
-  EXPECT_FALSE(found);
+  EXPECT_FALSE(IsMenuCommandEnabled(WebAppMenuModel::kExtensionsMenuCommandId));
 }

chrome/browser/ui/web_applications/web_app_menu_model.cc

@@ -47,7 +47,8 @@ bool WebAppMenuModel::IsCommandIdEnabled(int command_id) const {
     case kExtensionsMenuCommandId:
       return base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu) &&
              base::FeatureList::IsEnabled(
-                 features::kDesktopPWAsElidedExtensionsMenu);
+                 features::kDesktopPWAsElidedExtensionsMenu) &&
+             browser()->window()->GetExtensionsContainer()->HasAnyExtensions();
     default:
       return AppMenuModel::IsCommandIdEnabled(command_id);
   }