[Extensions c2s] Show reload message in menu for site settings update

When an user updates site settings, the changes are immediate.
However, sometimes a page reload is needed for the changes to be applied
to the page (e.g extension was already injected and the user blocked
extensions). Therefore, we show a reload message in the extensions
menu when a reload needs to apply the updated user site settings. The
reload message is only shown in the tab where the site settings were updated until the user refreshes the page or navigates to another
site. [1]

Known issue:
If another tab was open with the same origin, the site setting will be
updated but the extensions menu will not show the refresh message even
if it needs it (crbug.com/1449066) [2]. Ideally we should cover those
cases, but for now we are matching current behavior for multiple tabs
open for <site> and one of them changes site access. The other tabs
toolbar/menu strings are updated but doesn't match with visible
changes and user is not aware it needs to reload (crbug.com/1449066
[3].

Screencasts:
[1] https://drive.google.com/file/d/1dFRvRMbF8SLQE_-iPg8zpk17ybjm2KpP/view?usp=sharing
[2] https://drive.google.com/file/d/1ijk8TmVU5oEnRtXSOt6W9aPhi1RyHjwj/view?usp=sharing
[3]
https://drive.google.com/file/d/1_sQ38sK6Xx3LA_uUrsno8Hh_2V1Qaq-V/view?usp=sharing

Bug: 1390952
Change-Id: Ifed63131d30afa7262f9416fdcbad8c0932199a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4564373
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Emilia Paz <emiliapaz@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1150635}

chrome/app/generated_resources.grd

@@ -5840,6 +5840,15 @@ Keep your key file in a safe place. You will need it to create new versions of y
       <message name="IDS_EXTENSIONS_MENU_MESSAGE_SECTION_USER_BLOCKED_ACCESS_TEXT" desc="Text of the message section when the user blocked access to this site to all extensions.">
         Extensions are blocked on this site
       </message>
+      <message name="IDS_EXTENSIONS_MENU_MESSAGE_SECTION_USER_CUSTOMIZED_ACCESS_TEXT" desc="Text of the message section when the user can customize access to this site by extensions.">
+        Extensions can request access to this site
+      </message>
+      <message name="IDS_EXTENSIONS_MENU_MESSAGE_SECTION_RELOAD_CONTAINER_DESCRIPTION_TEXT" desc="Text of the message section when the user has to reload the page to apply the updated user site settings.">
+        To apply your updated settings to this site, reload this page
+      </message>
+       <message name="IDS_EXTENSIONS_MENU_MESSAGE_SECTION_RELOAD_CONTAINER_BUTTON_TEXT" desc="Text of the button in the message section to reload the page when the user has to reload the page to apply the updated user site settings.">
+        Reload
+      </message>
       <message name="IDS_EXTENSIONS_MENU_REQUESTS_ACCESS_SECTION_TITLE" desc="Title of the requests access section that lists the extensions requesting site access">
         Requests access
       </message>

chrome/app/generated_resources_grd/IDS_EXTENSIONS_MENU_MESSAGE_SECTION_RELOAD_CONTAINER_BUTTON_TEXT.png.sha1

@@ -0,0 +1 @@
+20fba0bc5e51b92f8433737e6f4ee799352e96f3

chrome/app/generated_resources_grd/IDS_EXTENSIONS_MENU_MESSAGE_SECTION_RELOAD_CONTAINER_DESCRIPTION_TEXT.png.sha1

@@ -0,0 +1 @@
+20fba0bc5e51b92f8433737e6f4ee799352e96f3

chrome/app/generated_resources_grd/IDS_EXTENSIONS_MENU_MESSAGE_SECTION_USER_CUSTOMIZED_ACCESS_TEXT.png.sha1

@@ -0,0 +1 @@
+87119df8ee3a5ddc56d8883270e478579acd7545

chrome/browser/extensions/tab_helper.cc

@@ -8,6 +8,7 @@
 
 #include "base/check_op.h"
 #include "base/functional/bind.h"
+#include "base/notreached.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -20,6 +21,7 @@
 #include "chrome/browser/extensions/install_observer.h"
 #include "chrome/browser/extensions/install_tracker.h"
 #include "chrome/browser/extensions/install_tracker_factory.h"
+#include "chrome/browser/extensions/site_permissions_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_tab_helper_factory.h"
 #include "chrome/browser/shell_integration.h"
@@ -250,6 +252,46 @@ SkBitmap* TabHelper::GetExtensionAppIcon() {
   return &extension_app_icon_;
 }
 
+void TabHelper::SetReloadRequired(
+    PermissionsManager::UserSiteSetting site_setting) {
+  switch (site_setting) {
+    case PermissionsManager::UserSiteSetting::kGrantAllExtensions: {
+      // Granting access to all extensions is allowed iff feature is
+      // enabled, and it shouldn't be enabled anywhere where this is called.
+      NOTREACHED_NORETURN();
+    }
+    case PermissionsManager::UserSiteSetting::kBlockAllExtensions: {
+      // A reload is required if any extension that had site access will lose
+      // it.
+      content::WebContents* web_contents = GetVisibleWebContents();
+      SitePermissionsHelper permissions_helper(profile_);
+      const extensions::ExtensionSet& extensions =
+          extensions::ExtensionRegistry::Get(profile_)->enabled_extensions();
+      reload_required_ = base::ranges::any_of(
+          extensions, [&permissions_helper,
+                       web_contents](scoped_refptr<const Extension> extension) {
+            return permissions_helper.GetSiteInteraction(*extension,
+                                                         web_contents) ==
+                   SitePermissionsHelper::SiteInteraction::kGranted;
+          });
+      break;
+    }
+    case PermissionsManager::UserSiteSetting::kCustomizeByExtension:
+      // When the user selects "customize by extension" it means previously all
+      // extensions were blocked and each extension's page access is set as
+      // "denied". Blocked actions in the ExtensionActionRunner are computed by
+      // checking if a page access is "withheld". Therefore, we always need a
+      // refresh since we don't know if there are any extensions that would have
+      // wanted to run if the page had not been restricted by the user.
+      reload_required_ = true;
+      break;
+  }
+}
+
+bool TabHelper::IsReloadRequired() {
+  return reload_required_;
+}
+
 void TabHelper::OnWatchedPageChanged(
     const std::vector<std::string>& css_selectors) {
   InvokeForContentRulesRegistries(
@@ -316,6 +358,10 @@ void TabHelper::DidFinishNavigation(
     UpdateExtensionAppIcon(
         enabled_extensions.GetExtensionOrAppByURL(navigation_handle->GetURL()));
   }
+
+  // Reset the `reload_required_` data member, since a page navigation acts as a
+  // page refresh.
+  reload_required_ = false;
 }
 
 bool TabHelper::OnMessageReceived(const IPC::Message& message,
@@ -344,6 +390,8 @@ void TabHelper::WebContentsDestroyed() {
   InvokeForContentRulesRegistries([this](ContentRulesRegistry* registry) {
     registry->WebContentsDestroyed(web_contents());
   });
+
+  reload_required_ = false;
 }
 
 void TabHelper::OnContentScriptsExecuting(
@@ -416,6 +464,13 @@ void TabHelper::OnExtensionUnloaded(content::BrowserContext* browser_context,
     return;
   if (extension == extension_app_)
     SetExtensionApp(nullptr);
+
+  // Technically, the refresh is no longer needed if the unloaded extension was
+  // the only one causing `refresh_required`. However, we would need to track
+  // which are the extensions causing the reload, and sometimes it is not
+  // specific to an extensions. Also, this is a very edge case  (site settings
+  // changed and then extension is installed externally), so it's fine to not
+  // handle it.
 }
 
 void TabHelper::SetTabId(content::RenderFrameHost* render_frame_host) {

chrome/browser/extensions/tab_helper.h

@@ -20,6 +20,7 @@
 #include "extensions/browser/extension_function_dispatcher.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_observer.h"
+#include "extensions/browser/permissions_manager.h"
 #include "extensions/browser/script_executor.h"
 #include "extensions/common/extension_id.h"
 #include "extensions/common/stack_frame.h"
@@ -76,6 +77,14 @@ class TabHelper : public content::WebContentsObserver,
   // extension_misc::EXTENSION_ICON_SMALLISH).
   SkBitmap* GetExtensionAppIcon();
 
+  // Sets whether the tab will require a page reload for applying
+  // `site_setting`.
+  void SetReloadRequired(PermissionsManager::UserSiteSetting site_setting);
+
+  // Returns whether a page reload is required to apply the user site settings
+  // in the tab.
+  bool IsReloadRequired();
+
   ScriptExecutor* script_executor() {
     return script_executor_.get();
   }
@@ -158,6 +167,9 @@ class TabHelper : public content::WebContentsObserver,
 
   std::unique_ptr<ActiveTabPermissionGranter> active_tab_permission_granter_;
 
+  // Whether the tab needs a page reload to apply the user site settings.
+  bool reload_required_ = false;
+
   base::ScopedObservation<ExtensionRegistry, ExtensionRegistryObserver>
       registry_observation_{this};
 

chrome/browser/extensions/tab_helper_unittest.cc

@@ -8,26 +8,154 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_with_install.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/test_browser_window.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/web_contents_tester.h"
+#include "extensions/browser/permissions_manager.h"
+#include "extensions/test/test_extension_dir.h"
+#include "url/origin.h"
 
 namespace extensions {
 
-TEST_F(ExtensionServiceTestWithInstall, TabHelperClearsExtensionOnUnload) {
-  InitializeEmptyExtensionService();
+class TabHelperUnitTest : public ExtensionServiceTestWithInstall {
+ public:
+  void SetUp() override {
+    ExtensionServiceTestWithInstall::SetUp();
+    InitializeEmptyExtensionService();
+
+    std::unique_ptr<content::WebContents> web_contents(
+        content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
+    web_contents_tester_ = content::WebContentsTester::For(web_contents.get());
+    TabHelper::CreateForWebContents(web_contents.get());
+    tab_helper_ = TabHelper::FromWebContents(web_contents.get());
+    browser()->tab_strip_model()->AppendWebContents(std::move(web_contents),
+                                                    true);
+
+    permissions_manager_ = PermissionsManager::Get(profile());
+  }
+
+  void TearDown() override {
+    // Remove any tabs in the tab strip to avoid test crashes.
+    if (browser_) {
+      while (!browser_->tab_strip_model()->empty()) {
+        browser_->tab_strip_model()->DetachAndDeleteWebContentsAt(0);
+      }
+    }
+
+    ExtensionServiceTestBase::TearDown();
+  }
+
+  Browser* browser() {
+    if (!browser_) {
+      Browser::CreateParams params(profile(), true);
+      browser_window_ = std::make_unique<TestBrowserWindow>();
+      params.window = browser_window_.get();
+      browser_.reset(Browser::Create(params));
+    }
+    return browser_.get();
+  }
+
+  content::WebContentsTester* web_contents_tester() {
+    return web_contents_tester_;
+  }
+
+  TabHelper* tab_helper() { return tab_helper_; }
+
+  PermissionsManager* permissions_manager() { return permissions_manager_; }
+
+ private:
+  // The browser and accompaying window.
+  std::unique_ptr<Browser> browser_;
+  std::unique_ptr<TestBrowserWindow> browser_window_;
+
+  raw_ptr<content::WebContentsTester> web_contents_tester_;
+  raw_ptr<TabHelper> tab_helper_;
+  raw_ptr<PermissionsManager> permissions_manager_;
+};
+
+TEST_F(TabHelperUnitTest, ClearsExtensionOnUnload) {
   const Extension* extension =
       PackAndInstallCRX(data_dir().AppendASCII("hosted_app"), INSTALL_NEW);
   ASSERT_TRUE(extension);
-  std::unique_ptr<content::WebContents> web_contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
-  TabHelper::CreateForWebContents(web_contents.get());
-  TabHelper* tab_helper = TabHelper::FromWebContents(web_contents.get());
-  tab_helper->SetExtensionApp(extension);
-  EXPECT_EQ(extension->id(), tab_helper->GetExtensionAppId());
-  EXPECT_TRUE(tab_helper->is_app());
+
+  tab_helper()->SetExtensionApp(extension);
+  EXPECT_EQ(extension->id(), tab_helper()->GetExtensionAppId());
+  EXPECT_TRUE(tab_helper()->is_app());
   service()->UnloadExtension(extension->id(),
                              UnloadedExtensionReason::TERMINATE);
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(ExtensionId(), tab_helper->GetExtensionAppId());
+  EXPECT_EQ(ExtensionId(), tab_helper()->GetExtensionAppId());
+}
+
+TEST_F(TabHelperUnitTest, ReloadRequired_BlockAllExtensions) {
+  static constexpr char kManifest[] =
+      R"({
+           "name": "Extension",
+           "manifest_version": 3,
+           "version": "0.1",
+           "host_permissions": ["<all_urls>"]
+         })";
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+
+  const Extension* extension =
+      PackAndInstallCRX(test_dir.UnpackedPath(), INSTALL_NEW);
+  ASSERT_TRUE(extension);
+
+  const GURL url("http://www.example.com");
+  web_contents_tester()->NavigateAndCommit(url);
+
+  // By default, user can customize extension's site access.
+  EXPECT_EQ(permissions_manager()->GetUserSiteSetting(url::Origin::Create(url)),
+            PermissionsManager::UserSiteSetting::kCustomizeByExtension);
+
+  // Reload is required when user wants to block all extensions and any
+  // extension loses site access.
+  tab_helper()->SetReloadRequired(
+      PermissionsManager::UserSiteSetting::kBlockAllExtensions);
+  EXPECT_TRUE(tab_helper()->IsReloadRequired());
+
+  // Navigating to another url restores the reload required value.
+  const GURL other_url("http://www.example.com");
+  web_contents_tester()->NavigateAndCommit(other_url);
+  EXPECT_FALSE(tab_helper()->IsReloadRequired());
+}
+
+TEST_F(TabHelperUnitTest, ReloadRequired_CustomizeByExtension) {
+  static constexpr char kManifest[] =
+      R"({
+           "name": "Extension",
+           "manifest_version": 3,
+           "version": "0.1"
+         })";
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(kManifest);
+
+  const Extension* extension =
+      PackAndInstallCRX(test_dir.UnpackedPath(), INSTALL_NEW);
+  ASSERT_TRUE(extension);
+
+  // Change site setting to "block all extensions", so we can test whether a
+  // reload will be required for "customize by extension".
+  const GURL url("http://www.example.com");
+  auto origin = url::Origin::Create(url);
+  permissions_manager()->UpdateUserSiteSetting(
+      origin, PermissionsManager::UserSiteSetting::kBlockAllExtensions);
+
+  web_contents_tester()->NavigateAndCommit(url);
+
+  // Reload is required when user wants to customize by extension, regardless
+  // of whether the extension requires site access.
+  tab_helper()->SetReloadRequired(
+      PermissionsManager::UserSiteSetting::kCustomizeByExtension);
+  EXPECT_TRUE(tab_helper()->IsReloadRequired());
+
+  // Navigating to another url restores the reload required value.
+  const GURL other_url("http://www.example.com");
+  web_contents_tester()->NavigateAndCommit(other_url);
+  EXPECT_FALSE(tab_helper()->IsReloadRequired());
 }
 
 }  // namespace extensions

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

@@ -37,6 +37,9 @@ class ExtensionsMenuHandler {
   virtual void OnExtensionToggleSelected(extensions::ExtensionId extension_id,
                                          bool is_on) = 0;
 
+  // Reload the current web contents.
+  virtual void OnReloadPageButtonClicked() = 0;
+
   // Grants one time site access to `extension_id` on the current web contents.
   virtual void OnAllowExtensionClicked(
       const extensions::ExtensionId& extension_id) = 0;