File Handling: Reset allowed permission on manifest update (M91).

To ensure that:
(1) A dPWA cannot request expanded access to file handlers without user
consent, when file handlers (ContentSettingsType::FILE_HANDLING) are in
the "ALLOW" state, update the permission during manifest update to reset
this status back to "ASK".
(2) A dPWA cannot register additional file handlers on the system when
the user has BLOCK'ed the API, disallow file handlers to update during
manifest update when the permission is set to "BLOCK".

Before, file handler permissions weren't affected by manifest update.

Notably, this also makes OS registered file handlers out of sync with
registrar file handlers. This will be fixed with crbug.com/1194163

As File Handling OS integration isn't currently testable via automated
tests, here's the manual test to verify that file handlers
don't update when BLOCK'ed after this patch (verified on Linux):
(1) Create a file with extension ".txt", and another file with extension
    ".docx". Verify that these files don't have in their "Open with"
    menu an option to open them using any not-yet-installed PWAs like
    https://morning-bubbly-chauffeur.glitch.me/
(2) Open and install a PWA with file handling support for ".txt" but not
    ".docx" extensions, like a fork of
    https://morning-bubbly-chauffeur.glitch.me/
(3) Use the "Open with" menu to open the ".txt" file created in (1)
    using the installed PWA from (2). Reject the permission to set the
    content setting for file handling to "BLOCK".
(4) Modify the PWA manifest to also support the ".asdf" extension.
(5) Close Chrome and reopen, triggering a manifest update.
(6) Use the "Open with" menu to open the ".txt" file created in (1)
    using the installed PWA from (2). The permission prompt should not
    come up again, and the file should not load, signalling that the
    permission has stayed at "BLOCK".
(7) Try to find the PWA from (2) using the "Open with" menu of the
    ".docx" file. The PWA should not appear in the "Open with" menu,
    because setting the content setting to "BLOCK" should have blocked
    manifest update of file handlers.
(8) (Optional Teardown) Uninstall the PWA, and revert the manifest
    change so that only ".txt" is supported again.
Similar tests may exist for verifying that manifests are still updated
for permission ALLOW/ASK states.

(cherry picked from commit dafa1fe)

Bug: 1194309
Change-Id: I49f8a05d0d02767894ea04402c90f76ca6aa004e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2803471
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#871133}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2822473
Cr-Commit-Position: refs/branch-heads/4472@{#21}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}

chrome/browser/web_applications/BUILD.gn

@@ -128,7 +128,10 @@ source_set("web_applications") {
     }
   }
 
-  public_deps = [ "//chrome/browser/web_applications/proto" ]
+  public_deps = [
+    "//chrome/browser/web_applications/proto",
+    "//components/permissions:permissions",
+  ]
 }
 
 # This test_support library doesn't use extensions.

chrome/browser/web_applications/components/install_finalizer.h

@@ -66,6 +66,7 @@ class InstallFinalizer {
   // if the app window needing update closes at the same time as Chrome.
   // Therefore, the manifest may not always update as expected.
   virtual void FinalizeUpdate(const WebApplicationInfo& web_app_info,
+                              content::WebContents* web_contents,
                               InstallFinalizedCallback callback) = 0;
 
   // Removes |external_install_source| from |app_id|. If no more interested

chrome/browser/web_applications/components/os_integration_manager.cc

@@ -234,11 +234,14 @@ void OsIntegrationManager::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) {
   if (g_suppress_os_hooks_for_testing_)
     return;
 
-  UpdateFileHandlers(app_id, std::move(old_shortcut));
+  if (file_handlers_need_os_update)
+    UpdateFileHandlers(app_id, std::move(old_shortcut));
+
   UpdateShortcuts(app_id, old_name);
   UpdateShortcutsMenu(app_id, web_app_info);
   UpdateUrlHandlers(app_id, base::DoNothing());

chrome/browser/web_applications/components/os_integration_manager.h

@@ -113,6 +113,7 @@ class OsIntegrationManager {
   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);
 
   // Proxy calls for AppShortcutManager.

chrome/browser/web_applications/components/os_integration_manager_unittest.cc

@@ -311,7 +311,7 @@ TEST_F(OsIntegrationManagerTest, UpdateOsHooksEverything) {
   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, web_app_info);
+  manager.UpdateOsHooks(app_id, old_name, nullptr, true, web_app_info);
 }
 
 }  // namespace

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.cc

@@ -126,6 +126,7 @@ void BookmarkAppInstallFinalizer::FinalizeUninstallAfterSync(
 
 void BookmarkAppInstallFinalizer::FinalizeUpdate(
     const WebApplicationInfo& web_app_info,
+    content::WebContents* web_contents,
     InstallFinalizedCallback callback) {
   web_app::AppId expected_app_id =
       web_app::GenerateAppIdFromURL(web_app_info.start_url);
@@ -288,12 +289,13 @@ void BookmarkAppInstallFinalizer::OnExtensionUpdated(
   }
 
   if (!is_legacy_finalizer()) {
-    // Using an empty ShortcutInfo here because BookmarkApp* is deprecated.
-    // Note that this code may not correctly update File Handlers on Linux, if
-    // un-deprecated.
+    // Using an empty `old_shortcut` and `file_handlers_need_os_update` here
+    // because BookmarkApp* is deprecated. Note that this code will not
+    // correctly update File Handlers on Linux, if un-deprecated.
     std::unique_ptr<web_app::ShortcutInfo> old_shortcut = nullptr;
     os_integration_manager().UpdateOsHooks(
-        extension->id(), old_name, std::move(old_shortcut), web_app_info);
+        extension->id(), old_name, std::move(old_shortcut),
+        /*file_handlers_need_os_update=*/false, web_app_info);
     registrar().NotifyWebAppManifestUpdated(extension->id(), old_name);
   }
   std::move(callback).Run(extension->id(),

chrome/browser/web_applications/extensions/bookmark_app_install_finalizer.h

@@ -40,6 +40,7 @@ class BookmarkAppInstallFinalizer : public web_app::InstallFinalizer {
   void FinalizeUninstallAfterSync(const web_app::AppId& app_id,
                                   UninstallWebAppCallback callback) override;
   void FinalizeUpdate(const WebApplicationInfo& web_app_info,
+                      content::WebContents* web_contents,
                       InstallFinalizedCallback callback) override;
   void UninstallExternalWebApp(
       const web_app::AppId& app_id,

chrome/browser/web_applications/extensions/pending_app_install_task_unittest.cc

@@ -170,6 +170,7 @@ class TestPendingAppInstallFinalizer : public InstallFinalizer {
   }
 
   void FinalizeUpdate(const WebApplicationInfo& web_app_info,
+                      content::WebContents* web_contents,
                       InstallFinalizedCallback callback) override {
     NOTREACHED();
   }

chrome/browser/web_applications/manifest_update_manager_browsertest.cc

@@ -20,6 +20,7 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -43,6 +44,8 @@
 #include "chrome/common/chrome_features.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/common/content_settings.h"
 #include "components/webapps/browser/installable/installable_metrics.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/url_loader_interceptor.h"
@@ -1427,6 +1430,63 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTestWithFileHandling,
   EXPECT_TRUE(base::Contains(new_extensions, ".md"));
 }
 
+IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTestWithFileHandling,
+                       AllowedPermissionResetToAskOnUpdate) {
+  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 WebApp* web_app =
+      GetProvider().registrar().AsWebAppRegistrar()->GetAppById(app_id);
+  const auto& old_file_handler = web_app->file_handlers()[0];
+  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 ALLOW.
+  map->SetContentSettingDefaultScope(origin, origin,
+                                     ContentSettingsType::FILE_HANDLING,
+                                     CONTENT_SETTING_ALLOW);
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            map->GetContentSetting(origin, origin,
+                                   ContentSettingsType::FILE_HANDLING));
+  // Update manifest.
+  OverrideManifest(kFileHandlerManifestTemplate, {".md", kInstallableIconList});
+  EXPECT_EQ(ManifestUpdateResult::kAppUpdated,
+            GetResultAfterPageLoad(url, &app_id));
+  const auto& new_file_handler = web_app->file_handlers()[0];
+  auto new_extensions = new_file_handler.accept[0].file_extensions;
+  EXPECT_TRUE(base::Contains(new_extensions, ".md"));
+
+  // Confirm permission is downgraded to ASK after manifest update.
+  EXPECT_EQ(CONTENT_SETTING_ASK,
+            map->GetContentSetting(origin, origin,
+                                   ContentSettingsType::FILE_HANDLING));
+}
+
 IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTestWithFileHandling,
                        CheckFindsDeletedFileHandler) {
   constexpr char kFileHandlerManifestTemplate[] = R"(

chrome/browser/web_applications/manifest_update_task.cc

@@ -63,8 +63,6 @@ bool HaveIconBitmapsChanged(const IconBitmaps& disk_icon_bitmaps,
 
 }  // namespace
 
-namespace internal {
-
 bool HaveFileHandlersChanged(
     const apps::FileHandlers* old_handlers,
     const std::vector<blink::Manifest::FileHandler>& new_handlers) {
@@ -116,8 +114,6 @@ bool HaveFileHandlersChanged(
   return false;
 }
 
-}  // namespace internal
-
 ManifestUpdateTask::ManifestUpdateTask(
     const GURL& url,
     const AppId& app_id,
@@ -284,7 +280,7 @@ bool ManifestUpdateTask::IsUpdateNeededForManifest() const {
   }
 
   if (base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI) &&
-      internal::HaveFileHandlersChanged(
+      HaveFileHandlersChanged(
           /*old_handlers=*/registrar_.GetAppFileHandlers(app_id_),
           /*new_handlers=*/web_application_info_->file_handlers)) {
     return true;

chrome/browser/web_applications/manifest_update_task.h

@@ -14,6 +14,7 @@
 #include "chrome/browser/web_applications/components/web_app_icon_downloader.h"
 #include "chrome/browser/web_applications/components/web_app_id.h"
 #include "chrome/browser/web_applications/components/web_application_info.h"
+#include "components/content_settings/core/common/content_settings.h"
 #include "components/services/app_service/public/cpp/file_handler.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "third_party/blink/public/common/manifest/manifest.h"
@@ -26,14 +27,12 @@ struct InstallableData;
 
 namespace web_app {
 
-namespace internal {
 // Checks for whether file handlers have changed. Ignores differences in names,
 // which aren't stored in the apps::FileHandlers, and ordering, which may
 // change after being inserted into a set or map.
 bool HaveFileHandlersChanged(
     const apps::FileHandlers* old_handlers,
     const std::vector<blink::Manifest::FileHandler>& new_handlers);
-}  // namespace internal
 
 class AppIconManager;
 class AppRegistrar;

chrome/browser/web_applications/manifest_update_task_unittest.cc

@@ -43,15 +43,15 @@ class ManifestUpdateTaskTest : public testing::Test {
   ~ManifestUpdateTaskTest() override = default;
 };
 
-// Below tests primarily test internal::HaveFileHandlersChanged.
+// Below tests primarily test HaveFileHandlersChanged.
 // Basic tests like added/removed/unchanged handlers are also in
 // functional tests at ManifestUpdateManagerBrowserTestWithFileHandling.
 TEST_F(ManifestUpdateTaskTest, TestFileHandlersUnchanged) {
   apps::FileHandlers old_handlers = GetDefaultAppsFileHandlers();
   std::vector<blink::Manifest::FileHandler> new_handlers =
       GetDefaultManifestFileHandlers();
 
-  EXPECT_FALSE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_FALSE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 TEST_F(ManifestUpdateTaskTest, TestSecondFileHandlerAdded) {
@@ -65,7 +65,7 @@ TEST_F(ManifestUpdateTaskTest, TestSecondFileHandlerAdded) {
   second_handler.accept.emplace(u"text/csv", extensions);
   new_handlers.push_back(second_handler);
 
-  EXPECT_TRUE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_TRUE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedName) {
@@ -75,7 +75,7 @@ TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedName) {
   new_handlers[0].name = u"Comma-Separated Values";
 
   // Ignore name changes, because the registrar doesn't store the name.
-  EXPECT_FALSE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_FALSE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedAction) {
@@ -84,7 +84,7 @@ TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedAction) {
       GetDefaultManifestFileHandlers();
   new_handlers[0].action = GURL("/?csvtext");
 
-  EXPECT_TRUE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_TRUE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 TEST_F(ManifestUpdateTaskTest, TestFileHandlerExtraAccept) {
@@ -94,7 +94,7 @@ TEST_F(ManifestUpdateTaskTest, TestFileHandlerExtraAccept) {
   std::vector<std::u16string> csv_extensions = {u".csv"};
   new_handlers[0].accept.emplace(u"text/csv", csv_extensions);
 
-  EXPECT_TRUE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_TRUE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedMimeType) {
@@ -103,7 +103,7 @@ TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedMimeType) {
       GetDefaultManifestFileHandlers();
   old_handlers[0].accept[0].mime_type = "text/csv";
 
-  EXPECT_TRUE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_TRUE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedExtension) {
@@ -112,7 +112,7 @@ TEST_F(ManifestUpdateTaskTest, TestFileHandlerChangedExtension) {
       GetDefaultManifestFileHandlers();
   old_handlers[0].accept[0].file_extensions.emplace(".csv");
 
-  EXPECT_TRUE(internal::HaveFileHandlersChanged(&old_handlers, new_handlers));
+  EXPECT_TRUE(HaveFileHandlersChanged(&old_handlers, new_handlers));
 }
 
 }  // namespace web_app

chrome/browser/web_applications/test/test_install_finalizer.cc

@@ -38,6 +38,7 @@ void TestInstallFinalizer::FinalizeInstall(
 
 void TestInstallFinalizer::FinalizeUpdate(
     const WebApplicationInfo& web_app_info,
+    content::WebContents* web_contents,
     InstallFinalizedCallback callback) {
   Finalize(web_app_info, InstallResultCode::kSuccessAlreadyInstalled,
            std::move(callback));

chrome/browser/web_applications/test/test_install_finalizer.h

@@ -33,6 +33,7 @@ class TestInstallFinalizer final : public InstallFinalizer {
   void FinalizeUninstallAfterSync(const AppId& app_id,
                                   UninstallWebAppCallback callback) override;
   void FinalizeUpdate(const WebApplicationInfo& web_app_info,
+                      content::WebContents* web_contents,
                       InstallFinalizedCallback callback) override;
   void UninstallExternalWebApp(const AppId& app_id,
                                ExternalInstallSource external_install_source,

chrome/browser/web_applications/test/test_os_integration_manager.cc

@@ -114,6 +114,7 @@ void TestOsIntegrationManager::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) {}
 
 void TestOsIntegrationManager::SetFileHandlerManager(

chrome/browser/web_applications/test/test_os_integration_manager.h

@@ -41,6 +41,7 @@ class TestOsIntegrationManager : public OsIntegrationManager {
   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) override;
 
   size_t num_create_shortcuts_calls() const {