DPWA: update file handlers on permission change

Unregister file handlers when file handling permission is changed to BLOCK
by the user, and re-register file handlers when the permission settings is
changed to ASK/ALLOW. Make WebAppInstallFinalizer observe file handling
permission changes and update app db state as well as OS state. Make
both install and update task checks file handling permission during
|WebAppInstallFinalizer::SetWebAppManifestFieldsAndWriteData|.

Bug: 1194163
Change-Id: Ia24030018243571b6ed020de205cf346742c0169
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2868925
Commit-Queue: Phillis Tang <phillis@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880576}

chrome/browser/ui/web_applications/web_app_file_handling_browsertest.cc

@@ -11,6 +11,7 @@
 #include "base/test/bind.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/values.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
@@ -25,10 +26,16 @@
 #include "chrome/browser/web_applications/components/web_app_prefs_utils.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/components/web_application_info.h"
+#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/embedder_support/switches.h"
+#include "components/policy/core/browser/browser_policy_connector.h"
+#include "components/policy/core/common/mock_configuration_policy_provider.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/policy_constants.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
@@ -663,6 +670,65 @@ IN_PROC_BROWSER_TEST_F(WebAppFileHandlingOriginTrialTest,
             content::EvalJs(web_content, "window.launchParams.files[0].name"));
 }
 
+class WebAppFileHandlingPolicyBrowserTest
+    : public WebAppFileHandlingBrowserTest {
+ public:
+  // Set the file handling policy to BLOCK the app between the PRE test and the
+  // actual test.
+  void SetUpInProcessBrowserTestFixture() override {
+    if (GetTestPreCount() == 0) {
+      SetFileHandlingBlockPolicy();
+    }
+  }
+
+ private:
+  void SetFileHandlingBlockPolicy() {
+    ON_CALL(provider_, IsInitializationComplete(testing::_))
+        .WillByDefault(testing::Return(true));
+    ON_CALL(provider_, IsFirstPolicyLoadComplete(testing::_))
+        .WillByDefault(testing::Return(true));
+
+    policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
+
+    policy::PolicyMap values;
+    base::Value list(base::Value::Type::LIST);
+    list.Append(base::Value("https://app.com"));
+    policy::PolicyMap::Entry entry_list(
+        policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_MACHINE,
+        policy::POLICY_SOURCE_CLOUD, std::move(list), nullptr);
+
+    values.Set(policy::key::kFileHandlingBlockedForUrls, std::move(entry_list));
+    provider_.UpdateChromePolicy(values);
+  }
+  testing::NiceMock<policy::MockConfigurationPolicyProvider> provider_;
+};
+
+IN_PROC_BROWSER_TEST_F(WebAppFileHandlingPolicyBrowserTest,
+                       PRE_PolicySettingsBlockedUrl) {
+  InstallFileHandlingPWA();
+  EXPECT_EQ(registrar().GetAppIds().size(), 1u);
+  EXPECT_FALSE(registrar()
+                   .AsWebAppRegistrar()
+                   ->GetAppById(app_id())
+                   ->file_handler_permission_blocked());
+}
+
+// Test that the app's `file_handler_permission_blocked` state should be updated
+// on WebAppProvider system setup based on current permission settings.
+IN_PROC_BROWSER_TEST_F(WebAppFileHandlingPolicyBrowserTest,
+                       PolicySettingsBlockedUrl) {
+  auto* provider = web_app::WebAppProvider::Get(profile());
+  DCHECK(provider);
+  web_app::test::WaitUntilReady(provider);
+
+  std::vector<web_app::AppId> app_ids = registrar().GetAppIds();
+  EXPECT_EQ(app_ids.size(), 1u);
+  EXPECT_TRUE(registrar()
+                  .AsWebAppRegistrar()
+                  ->GetAppById(app_ids[0])
+                  ->file_handler_permission_blocked());
+}
+
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 
 // End-to-end test to ensure the file handler is registered on ChromeOS when the

chrome/browser/web_applications/components/app_registrar.h

@@ -108,6 +108,7 @@ class AppRegistrar {
       const AppId& app_id) const = 0;
   virtual const apps::FileHandlers* GetAppFileHandlers(
       const AppId& app_id) const = 0;
+  virtual bool IsAppFileHandlerPermissionBlocked(const AppId& app_id) const = 0;
 
   // Returns the start_url with launch_query_params appended to the end if any.
   GURL GetAppLaunchUrl(const AppId& app_id) const;

chrome/browser/web_applications/components/file_handler_manager.cc

@@ -173,7 +173,8 @@ void FileHandlerManager::DisableForceEnabledFileHandlingOriginTrial(
 
 const apps::FileHandlers* FileHandlerManager::GetEnabledFileHandlers(
     const AppId& app_id) {
-  if (AreFileHandlersEnabled(app_id) && IsFileHandlingAPIAvailable(app_id))
+  if (AreFileHandlersEnabled(app_id) && IsFileHandlingAPIAvailable(app_id) &&
+      !registrar_->IsAppFileHandlerPermissionBlocked(app_id))
     return GetAllFileHandlers(app_id);
 
   return nullptr;

chrome/browser/web_applications/components/os_integration_manager.cc

@@ -234,13 +234,13 @@ void OsIntegrationManager::UpdateOsHooks(
     const AppId& app_id,
     base::StringPiece old_name,
     std::unique_ptr<ShortcutInfo> old_shortcut,
-    bool file_handlers_need_os_update,
+    FileHandlerUpdateAction file_handlers_need_os_update,
     const WebApplicationInfo& web_app_info) {
   if (g_suppress_os_hooks_for_testing_)
     return;
 
-  if (file_handlers_need_os_update)
-    UpdateFileHandlers(app_id, std::move(old_shortcut));
+  UpdateFileHandlersWithShortcutInfo(app_id, file_handlers_need_os_update,
+                                     std::move(old_shortcut));
 
   UpdateShortcuts(app_id, old_name);
   UpdateShortcutsMenu(app_id, web_app_info);
@@ -552,18 +552,43 @@ void OsIntegrationManager::UpdateUrlHandlers(
 
 void OsIntegrationManager::UpdateFileHandlers(
     const AppId& app_id,
+    FileHandlerUpdateAction file_handlers_need_os_update) {
+  GetShortcutInfoForApp(
+      app_id,
+      base::BindOnce(&OsIntegrationManager::UpdateFileHandlersWithShortcutInfo,
+                     weak_ptr_factory_.GetWeakPtr(), app_id,
+                     file_handlers_need_os_update));
+}
+
+void OsIntegrationManager::UpdateFileHandlersWithShortcutInfo(
+    const AppId& app_id,
+    FileHandlerUpdateAction file_handlers_need_os_update,
     std::unique_ptr<ShortcutInfo> info) {
   if (!IsFileHandlingAPIAvailable(app_id))
     return;
 
-  // Update file handlers via complete uninstallation, then reinstallation.
-  auto callback = base::BindOnce(&OsIntegrationManager::RegisterFileHandlers,
-                                 weak_ptr_factory_.GetWeakPtr(), app_id,
-                                 base::DoNothing::Once<bool>());
+  base::OnceClosure callback_after_removal;
+  switch (file_handlers_need_os_update) {
+    case FileHandlerUpdateAction::kNoUpdate:
+      return;
+    case FileHandlerUpdateAction::kUpdate:
+      callback_after_removal =
+          base::BindOnce(&OsIntegrationManager::RegisterFileHandlers,
+                         weak_ptr_factory_.GetWeakPtr(), app_id,
+                         base::DoNothing::Once<bool>());
+      break;
+    case FileHandlerUpdateAction::kRemove:
+      callback_after_removal = base::DoNothing::Once();
+      break;
+  }
+
+  // Update file handlers via complete uninstallation, then potential
+  // reinstallation.
   content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(&OsIntegrationManager::UnregisterFileHandlers,
-                                weak_ptr_factory_.GetWeakPtr(), app_id,
-                                std::move(info), std::move(callback)));
+      FROM_HERE,
+      base::BindOnce(&OsIntegrationManager::UnregisterFileHandlers,
+                     weak_ptr_factory_.GetWeakPtr(), app_id, std::move(info),
+                     std::move(callback_after_removal)));
 }
 
 std::unique_ptr<ShortcutInfo> OsIntegrationManager::BuildShortcutInfo(

chrome/browser/web_applications/components/os_integration_manager.h

@@ -110,11 +110,12 @@ class OsIntegrationManager {
 
   // Update all needed OS hooks for the web app.
   // virtual for testing
-  virtual void UpdateOsHooks(const AppId& app_id,
-                             base::StringPiece old_name,
-                             std::unique_ptr<ShortcutInfo> old_shortcut,
-                             bool file_handlers_need_os_update,
-                             const WebApplicationInfo& web_app_info);
+  virtual void UpdateOsHooks(
+      const AppId& app_id,
+      base::StringPiece old_name,
+      std::unique_ptr<ShortcutInfo> old_shortcut,
+      FileHandlerUpdateAction file_handlers_need_os_update,
+      const WebApplicationInfo& web_app_info);
 
   // Proxy calls for AppShortcutManager.
   // virtual for testing
@@ -163,6 +164,10 @@ class OsIntegrationManager {
       const AppId& app_id,
       base::OnceCallback<void(bool success)> callback);
 
+  virtual void UpdateFileHandlers(
+      const AppId& app_id,
+      FileHandlerUpdateAction file_handlers_need_os_update);
+
  protected:
   AppShortcutManager* shortcut_manager() { return shortcut_manager_.get(); }
   FileHandlerManager* file_handler_manager() {
@@ -238,8 +243,10 @@ class OsIntegrationManager {
   virtual void UpdateShortcuts(const AppId& app_id, base::StringPiece old_name);
   virtual void UpdateShortcutsMenu(const AppId& app_id,
                                    const WebApplicationInfo& web_app_info);
-  virtual void UpdateFileHandlers(const AppId& app_id,
-                                  std::unique_ptr<ShortcutInfo> info);
+  virtual void UpdateFileHandlersWithShortcutInfo(
+      const AppId& app_id,
+      FileHandlerUpdateAction file_handlers_need_os_update,
+      std::unique_ptr<ShortcutInfo> info);
 
   // Utility methods:
   virtual std::unique_ptr<ShortcutInfo> BuildShortcutInfo(const AppId& app_id);

chrome/browser/web_applications/components/os_integration_manager_unittest.cc

@@ -122,8 +122,10 @@ class MockOsIntegrationManager : public OsIntegrationManager {
 
   // Update:
   MOCK_METHOD(void,
-              UpdateFileHandlers,
-              (const AppId& app_id, std::unique_ptr<ShortcutInfo> info),
+              UpdateFileHandlersWithShortcutInfo,
+              (const AppId& app_id,
+               FileHandlerUpdateAction file_handlers_need_os_update,
+               std::unique_ptr<ShortcutInfo> info),
               (override));
   MOCK_METHOD(void,
               UpdateShortcuts,
@@ -306,12 +308,16 @@ TEST_F(OsIntegrationManagerTest, UpdateOsHooksEverything) {
   WebApplicationInfo web_app_info;
   base::StringPiece old_name = "test-name";
 
-  EXPECT_CALL(manager, UpdateFileHandlers(app_id, testing::_)).Times(1);
+  EXPECT_CALL(manager,
+              UpdateFileHandlersWithShortcutInfo(
+                  app_id, FileHandlerUpdateAction::kUpdate, testing::_))
+      .Times(1);
   EXPECT_CALL(manager, UpdateShortcuts(app_id, old_name)).Times(1);
   EXPECT_CALL(manager, UpdateShortcutsMenu(app_id, testing::_)).Times(1);
   EXPECT_CALL(manager, UpdateUrlHandlers(app_id, testing::_)).Times(1);
 
-  manager.UpdateOsHooks(app_id, old_name, nullptr, true, web_app_info);
+  manager.UpdateOsHooks(app_id, old_name, nullptr,
+                        FileHandlerUpdateAction::kUpdate, web_app_info);
 }
 
 }  // namespace

chrome/browser/web_applications/components/web_app_constants.h

@@ -265,6 +265,17 @@ constexpr int kIphFieldTrialParamDefaultSiteEngagementThreshold = 10;
 // chrome:// web applications are exempt from this limit.
 constexpr size_t kMaxFileHandlers = 10;
 
+// Expected file handler update actions to be taken by OsIntegrationManager
+// during UpdateOsHooks.
+enum class FileHandlerUpdateAction {
+  // Perform update, removing and re-adding all file handlers.
+  kUpdate = 0,
+  // Remove all file handlers.
+  kRemove = 1,
+  // Do not perform update.
+  kNoUpdate = 2,
+};
+
 }  // namespace web_app
 
 #endif  // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_CONSTANTS_H_

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.cc

@@ -297,7 +297,8 @@ void BookmarkAppInstallFinalizer::OnExtensionUpdated(
     std::unique_ptr<web_app::ShortcutInfo> old_shortcut;
     os_integration_manager().UpdateOsHooks(
         extension->id(), old_name, std::move(old_shortcut),
-        /*file_handlers_need_os_update=*/false, web_app_info);
+        /*file_handlers_need_os_update=*/
+        web_app::FileHandlerUpdateAction::kNoUpdate, web_app_info);
     registrar().NotifyWebAppManifestUpdated(extension->id(), old_name);
   }
   std::move(callback).Run(extension->id(),

chrome/browser/web_applications/extensions/bookmark_app_registrar.cc

@@ -178,6 +178,11 @@ const apps::FileHandlers* BookmarkAppRegistrar::GetAppFileHandlers(
   return nullptr;
 }
 
+bool BookmarkAppRegistrar::IsAppFileHandlerPermissionBlocked(
+    const web_app::AppId& app_id) const {
+  return false;
+}
+
 base::Optional<GURL> BookmarkAppRegistrar::GetAppScopeInternal(
     const web_app::AppId& app_id) const {
   const Extension* extension = GetBookmarkAppDchecked(app_id);

chrome/browser/web_applications/extensions/bookmark_app_registrar.h

@@ -54,6 +54,8 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar,
       const web_app::AppId& app_id) const override;
   const apps::FileHandlers* GetAppFileHandlers(
       const web_app::AppId& app_id) const override;
+  bool IsAppFileHandlerPermissionBlocked(
+      const web_app::AppId& app_id) const override;
   base::Optional<GURL> GetAppScopeInternal(
       const web_app::AppId& app_id) const override;
   web_app::DisplayMode GetAppDisplayMode(

chrome/browser/web_applications/manifest_update_manager_browsertest.cc

@@ -1555,6 +1555,66 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTestWithFileHandling,
                                    ContentSettingsType::FILE_HANDLING));
 }
 
+IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTestWithFileHandling,
+                       BlockPermissionRemoveFileHandlers) {
+  constexpr char kFileHandlerManifestTemplate[] = R"(
+    {
+      "name": "Test app name",
+      "start_url": ".",
+      "scope": "/",
+      "display": "minimal-ui",
+      "file_handlers": [
+        {
+          "action": "/?plaintext",
+          "name": "Plain Text",
+          "accept": {
+            "text/plain": ["$1"]
+          }
+        }
+      ],
+      "icons": $2
+    }
+  )";
+
+  OverrideManifest(kFileHandlerManifestTemplate,
+                   {".txt", kInstallableIconList});
+  AppId app_id = InstallWebApp();
+  const WebAppRegistrar* registrar =
+      GetProvider().registrar().AsWebAppRegistrar();
+  const WebApp* web_app = registrar->GetAppById(app_id);
+
+  EXPECT_FALSE(web_app->file_handler_permission_blocked());
+  ASSERT_FALSE(web_app->file_handlers().empty());
+  const auto& old_file_handler = web_app->file_handlers()[0];
+  ASSERT_FALSE(old_file_handler.accept.empty());
+  auto old_extensions = old_file_handler.accept[0].file_extensions;
+  EXPECT_TRUE(base::Contains(old_extensions, ".txt"));
+  auto* map =
+      HostContentSettingsMapFactory::GetForProfile(browser()->profile());
+  const GURL url = GetAppURL();
+  const GURL origin = url.GetOrigin();
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            map->GetContentSetting(origin, origin,
+                                   ContentSettingsType::FILE_HANDLING));
+  // Set permission to BLOCK.
+  map->SetContentSettingDefaultScope(origin, origin,
+                                     ContentSettingsType::FILE_HANDLING,
+                                     CONTENT_SETTING_BLOCK);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            map->GetContentSetting(origin, origin,
+                                   ContentSettingsType::FILE_HANDLING));
+
+  // App should be updated to permission blocked by
+  // `WebAppInstallFinalizer::OnContentSettingChanged`.
+  EXPECT_TRUE(registrar->GetAppById(app_id)->file_handler_permission_blocked());
+  // Update manifest.
+  OverrideManifest(kFileHandlerManifestTemplate, {".md", kInstallableIconList});
+  EXPECT_EQ(ManifestUpdateResult::kAppUpdated,
+            GetResultAfterPageLoad(url, &app_id));
+  // Manifest update task should preserve the permission blocked state.
+  EXPECT_TRUE(registrar->GetAppById(app_id)->file_handler_permission_blocked());
+}
+
 IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTestWithFileHandling,
                        CheckFindsDeletedFileHandler) {
   constexpr char kFileHandlerManifestTemplate[] = R"(

chrome/browser/web_applications/proto/web_app.proto

@@ -195,4 +195,6 @@ message WebAppProto {
   // Last time the Badging API was used.
   // ms since the Unix epoch. See sync/base/time.h.
   optional int64 last_badging_time = 31;
+
+  optional bool file_handler_permission_blocked = 32;
 }

chrome/browser/web_applications/test/test_app_registrar.cc

@@ -137,6 +137,11 @@ const apps::FileHandlers* TestAppRegistrar::GetAppFileHandlers(
   return nullptr;
 }
 
+bool TestAppRegistrar::IsAppFileHandlerPermissionBlocked(
+    const web_app::AppId& app_id) const {
+  return false;
+}
+
 base::Optional<GURL> TestAppRegistrar::GetAppScopeInternal(
     const AppId& app_id) const {
   const auto& result = installed_apps_.find(app_id);

chrome/browser/web_applications/test/test_app_registrar.h

@@ -70,6 +70,8 @@ class TestAppRegistrar : public AppRegistrar {
       const AppId& app_id) const override;
   const apps::FileHandlers* GetAppFileHandlers(
       const AppId& app_id) const override;
+  bool IsAppFileHandlerPermissionBlocked(
+      const web_app::AppId& app_id) const override;
   base::Optional<GURL> GetAppScopeInternal(const AppId& app_id) const override;
   DisplayMode GetAppDisplayMode(const AppId& app_id) const override;
   DisplayMode GetAppUserDisplayMode(const AppId& app_id) const override;