[Extensions c2s] Display request button when extensions request access

Adds a method in the action controller that returns whether the extension requests access to the given site. Using such method, display the request button when 1 or more extensions request access to the current site and update the button label with the number of extensions. Bug: 1239772 Change-Id: I793496ea1dbe9eb2bd01ed47f9a45c4853ea335c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3541459 Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org> Reviewed-by: Caroline Rising <corising@chromium.org> Commit-Queue: Emilia Paz <emiliapaz@chromium.org> Cr-Commit-Position: refs/heads/main@{#989713}
chromium · Apr 7, 2022 · b7d2f93 · b7d2f93
1 parent cd96beb
commit b7d2f93
Show file tree

Hide file tree

Showing 15 changed files with 254 additions and 40 deletions.
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
@@ -4879,7 +4879,7 @@ are declared in tools/grit/grit_rule.gni.
           Has access to this site
         </message>
         <message name="IDS_EXTENSIONS_REQUEST_ACCESS_BUTTON" desc="The text of the request acces button that appears on the toolbar when an extension requests access to the site">
-          Allow?
+          Allow <ph name="EXTENSIONS_REQUESTING_ACCESS_COUNT">$1<ex>3</ex></ph>?
         </message>
         <if expr="not use_titlecase">
           <message name="IDS_EXTENSIONS_CONTEXT_MENU_CANT_ACCESS_PAGE" desc="The label in an extension's context menu indicating the extension cannot access the current page. (sentence case)">

diff --git a/chrome/app/generated_resources_grd/IDS_EXTENSIONS_REQUEST_ACCESS_BUTTON.png.sha1 b/chrome/app/generated_resources_grd/IDS_EXTENSIONS_REQUEST_ACCESS_BUTTON.png.sha1
@@ -1 +1 @@
-608fcedc3b840c237b27968ce81e3f9ebaee0bb2
+ad553089adedea3971a955ffbd1131a26ee6323f
diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.cc b/chrome/browser/ui/extensions/extension_action_view_controller.cc
@@ -210,6 +210,12 @@ bool ExtensionActionViewController::IsShowingPopup() const {
   return popup_host_ != nullptr;
 }
 
+bool ExtensionActionViewController::IsRequestingSiteAccess(
+    content::WebContents* web_contents) const {
+  return GetSiteInteraction(web_contents) ==
+         extensions::SitePermissionsHelper::SiteInteraction::kPending;
+}
+
 void ExtensionActionViewController::HidePopup() {
   if (IsShowingPopup()) {
     popup_host_->Close();

diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.h b/chrome/browser/ui/extensions/extension_action_view_controller.h
@@ -73,6 +73,8 @@ class ExtensionActionViewController
       content::WebContents* web_contents) const override;
   bool IsEnabled(content::WebContents* web_contents) const override;
   bool IsShowingPopup() const override;
+  bool IsRequestingSiteAccess(
+      content::WebContents* web_contents) const override;
   void HidePopup() override;
   gfx::NativeView GetPopupNativeView() override;
   ui::MenuModel* GetContextMenu(

diff --git a/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.cc b/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.cc
@@ -60,6 +60,11 @@ bool TestToolbarActionViewController::IsShowingPopup() const {
   return popup_showing_;
 }
 
+bool TestToolbarActionViewController::IsRequestingSiteAccess(
+    content::WebContents* web_contents) const {
+  return false;
+}
+
 void TestToolbarActionViewController::HidePopup() {
   popup_showing_ = false;
   delegate_->OnPopupClosed();

diff --git a/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h b/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h
@@ -33,6 +33,8 @@ class TestToolbarActionViewController : public ToolbarActionViewController {
   std::u16string GetTooltip(content::WebContents* web_contents) const override;
   bool IsEnabled(content::WebContents* web_contents) const override;
   bool IsShowingPopup() const override;
+  bool IsRequestingSiteAccess(
+      content::WebContents* web_contents) const override;
   void HidePopup() override;
   gfx::NativeView GetPopupNativeView() override;
   ui::MenuModel* GetContextMenu(

diff --git a/chrome/browser/ui/toolbar/toolbar_action_view_controller.h b/chrome/browser/ui/toolbar/toolbar_action_view_controller.h
@@ -89,6 +89,10 @@ class ToolbarActionViewController {
   // Returns whether there is currently a popup visible.
   virtual bool IsShowingPopup() const = 0;
 
+  // Returns whether the action is requesting site access to `web_contents`.
+  virtual bool IsRequestingSiteAccess(
+      content::WebContents* web_contents) const = 0;
+
   // Hides the current popup, if one is visible.
   virtual void HidePopup() = 0;
 

diff --git a/chrome/browser/ui/views/extensions/extensions_request_access_button.cc b/chrome/browser/ui/views/extensions/extensions_request_access_button.cc
@@ -6,23 +6,30 @@
 #include <memory>
 
 #include "base/bind.h"
+#include "base/check_op.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 ExtensionsRequestAccessButton::ExtensionsRequestAccessButton()
     : ToolbarButton(
           base::BindRepeating(&ExtensionsRequestAccessButton::OnButtonPressed,
-                              base::Unretained(this))) {
+                              base::Unretained(this))) {}
+
+ExtensionsRequestAccessButton::~ExtensionsRequestAccessButton() = default;
+
+void ExtensionsRequestAccessButton::UpdateLabel(
+    int extensions_requesting_access_count) {
+  DCHECK_GT(extensions_requesting_access_count, 0);
   // TODO(crbug.com/1239772): Set the label and background color without borders
   // separately to match the mocks. For now, using SetHighlight to display that
   // adds a border and highlight color in addition to the label.
   absl::optional<SkColor> color;
-  SetHighlight(l10n_util::GetStringUTF16(IDS_EXTENSIONS_REQUEST_ACCESS_BUTTON),
-               color);
+  SetHighlight(
+      l10n_util::GetStringFUTF16Int(IDS_EXTENSIONS_REQUEST_ACCESS_BUTTON,
+                                    extensions_requesting_access_count),
+      color);
 }
 
-ExtensionsRequestAccessButton::~ExtensionsRequestAccessButton() = default;
-
 // TODO(crbug.com/1239772): Grant access to all the extensions requesting access
 // when the button is pressed.
 void ExtensionsRequestAccessButton::OnButtonPressed() {}
diff --git a/chrome/browser/ui/views/extensions/extensions_request_access_button.h b/chrome/browser/ui/views/extensions/extensions_request_access_button.h
@@ -17,6 +17,10 @@ class ExtensionsRequestAccessButton : public ToolbarButton {
       const ExtensionsRequestAccessButton&) = delete;
   ~ExtensionsRequestAccessButton() override;
 
+  // Updates the label based on the `extensions_requesting_access_count`. This
+  // should only be called if there is at least one extension requesting access.
+  void UpdateLabel(int extensions_requesting_access_count);
+
  private:
   void OnButtonPressed();
 };

diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc
@@ -848,14 +848,22 @@ void ExtensionsToolbarContainer::UpdateControlsVisibility() {
   if (!extensions_controls_)
     return;
 
+  content::WebContents* web_contents = GetCurrentWebContents();
+
   extensions_controls_->UpdateSiteAccessButtonVisibility(
       ExtensionActionViewController::AnyActionHasCurrentSiteAccess(
-          actions_, GetCurrentWebContents()));
-
-  // TODO(crbug.com/1239772): Get extensions that are requesting access to the
-  // current site.
-  const std::vector<std::unique_ptr<ToolbarActionViewController>> actions;
-  extensions_controls_->UpdateRequestAccessButton(actions);
+          actions_, web_contents));
+
+  // TODO(crbug.com/1239772): The request access button should only include
+  // extensions that are requesting access to a restricted site.
+  // `SiteInteraction::kPending` includes extensions with activeTab, that can
+  // request access to restricted or non-restricted sites. Need to update the
+  // method to not take into account activeTab extensions.
+  int count_requesting_extensions = std::count_if(
+      actions_.begin(), actions_.end(), [web_contents](const auto& action) {
+        return action->IsRequestingSiteAccess(web_contents);
+      });
+  extensions_controls_->UpdateRequestAccessButton(count_requesting_extensions);
 }
 
 BEGIN_METADATA(ExtensionsToolbarContainer, ToolbarIconContainerView)

diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_controls.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_controls.cc
@@ -20,7 +20,7 @@ ExtensionsToolbarControls::ExtensionsToolbarControls(
       site_access_button_(AddChildView(std::move(site_access_button))),
       extensions_button_(extensions_button.get()) {
   site_access_button_->SetVisible(false);
-  request_access_button_->SetVisible(true);
+  request_access_button_->SetVisible(false);
   // TODO(emiliapaz): Consider changing AddMainItem() to receive a unique_ptr.
   AddMainItem(extensions_button.release());
 }
@@ -33,20 +33,29 @@ void ExtensionsToolbarControls::UpdateSiteAccessButtonVisibility(
     bool visibility) {
   site_access_button_->SetVisible(visibility);
 
-  // Layout animation does not handle host view visibility changing; requires
-  // resetting.
-  // TODO(crbug.com/1239772): Consider moving this to a separate method, or
-  // merging both visibility updates under one method after setting the request
-  // access button visibility based on extensions requesting access.
-  GetAnimatingLayoutManager()->ResetLayout();
+  ResetLayout();
 }
 
 void ExtensionsToolbarControls::UpdateRequestAccessButton(
-    const std::vector<std::unique_ptr<ToolbarActionViewController>>& actions) {
-  // TODO(crbug.com/1239772): Display site access button, and add the action
-  // icons to the button and tooltip only when 1+ actions are given. For now,
-  // setting visible as true to see the button in the toolbar.
-  request_access_button_->SetVisible(true);
+    int count_requesting_extensions) {
+  if (count_requesting_extensions == 0) {
+    request_access_button_->SetVisible(false);
+  } else {
+    // TODO(crbug.com/1239772): Update icons, based on the number of extensions
+    // requesting access, once multiple icons in button is supported. Since we
+    // will need to access the extension information, this method may receive
+    // actions instead of actions count. For now, just show the number of
+    // actions.
+    request_access_button_->UpdateLabel(count_requesting_extensions);
+    request_access_button_->SetVisible(true);
+    // TODO(crbug.com/1239772): Update tooltip with the extension's names.
+  }
+
+  ResetLayout();
+}
+
+void ExtensionsToolbarControls::ResetLayout() {
+  GetAnimatingLayoutManager()->ResetLayout();
 }
 
 BEGIN_METADATA(ExtensionsToolbarControls, ToolbarIconContainerView)

diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_controls.h b/chrome/browser/ui/views/extensions/extensions_toolbar_controls.h
@@ -13,7 +13,6 @@
 
 class ExtensionsToolbarButton;
 class ExtensionsRequestAccessButton;
-class ToolbarActionViewController;
 
 class ExtensionsToolbarControls : public ToolbarIconContainerView {
  public:
@@ -32,17 +31,24 @@ class ExtensionsToolbarControls : public ToolbarIconContainerView {
     return extensions_button_;
   }
 
+  // Methods for testing.
   ExtensionsToolbarButton* site_access_button_for_testing() const {
     return site_access_button_;
   }
+  ExtensionsRequestAccessButton* request_access_button_for_testing() const {
+    return request_access_button_;
+  }
 
   // Updates `site_access_button_` visibility to the given one.
   void UpdateSiteAccessButtonVisibility(bool visibility);
 
   // Updates `request_access_button_` visibility and content based on the given
-  // `actions`.
-  void UpdateRequestAccessButton(
-      const std::vector<std::unique_ptr<ToolbarActionViewController>>& actions);
+  // `count_requesting_extensions`.
+  void UpdateRequestAccessButton(int count_requesting_extensions);
+
+  // Resets the layout since layout animation does not handle host view
+  // visibility changing. This should be called after any visibility changes.
+  void ResetLayout();
 
   // ToolbarIconContainerView:
   void UpdateAllIcons() override;