[WebAppProvider] Fixing not enabling file handlers and other issues

This fixes an issue that I caused by my previous patch where file
handlers are not enabled by default, which is incorrect. It also
addresses a few issue that loyso@ found in my previous CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2505685

R=phillis@chromium.org

Bug: 1149981
Change-Id: I70221ff76ef576fb6f09ab89d3db0d90ffe30a60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2544843
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Phillis Tang <phillis@chromium.org>
Auto-Submit: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828348}

chrome/browser/sync/test/integration/two_client_web_apps_bmo_sync_test.cc

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <memory>
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ -51,8 +50,7 @@ class TwoClientWebAppsBMOSyncTest : public SyncTest {
   TwoClientWebAppsBMOSyncTest()
       : SyncTest(TWO_CLIENT),
         test_web_app_provider_creator_(
-            std::make_unique<TestWebAppProviderCreator>(
-                base::BindRepeating(&CreateTestWebAppProvider))) {}
+            base::BindRepeating(&CreateTestWebAppProvider)) {}
   ~TwoClientWebAppsBMOSyncTest() override = default;
 
   bool SetupClients() override {
@@ -160,7 +158,7 @@ class TwoClientWebAppsBMOSyncTest : public SyncTest {
     return *web_app_registrar;
   }
 
-  TestOsIntegrationManager& GetOSIntegrationManager(Profile* profile) {
+  TestOsIntegrationManager& GetOsIntegrationManager(Profile* profile) {
     return reinterpret_cast<TestOsIntegrationManager&>(
         WebAppProvider::Get(profile)->os_integration_manager());
   }
@@ -184,7 +182,7 @@ class TwoClientWebAppsBMOSyncTest : public SyncTest {
   }
 
  private:
-  std::unique_ptr<TestWebAppProviderCreator> test_web_app_provider_creator_;
+  TestWebAppProviderCreator test_web_app_provider_creator_;
 
   DISALLOW_COPY_AND_ASSIGN(TwoClientWebAppsBMOSyncTest);
 };
@@ -616,10 +614,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientWebAppsBMOSyncTest, NoShortcutsCreatedOnSync) {
     EXPECT_TRUE(AllProfilesHaveSameWebAppIds());
   }
   EXPECT_EQ(
-      1u, GetOSIntegrationManager(GetProfile(0)).num_create_shortcuts_calls());
+      1u, GetOsIntegrationManager(GetProfile(0)).num_create_shortcuts_calls());
 #if defined(OS_CHROMEOS)
   auto last_options =
-      GetOSIntegrationManager(GetProfile(1)).get_last_install_options();
+      GetOsIntegrationManager(GetProfile(1)).get_last_install_options();
   EXPECT_TRUE(last_options.has_value());
   OsHooksResults expected_os_hook_requests;
   expected_os_hook_requests[OsHookType::kShortcuts] = true;
@@ -631,9 +629,9 @@ IN_PROC_BROWSER_TEST_F(TwoClientWebAppsBMOSyncTest, NoShortcutsCreatedOnSync) {
   EXPECT_TRUE(last_options->add_to_desktop);
   EXPECT_FALSE(last_options->add_to_quick_launch_bar);
   EXPECT_EQ(
-      1u, GetOSIntegrationManager(GetProfile(1)).num_create_shortcuts_calls());
+      1u, GetOsIntegrationManager(GetProfile(1)).num_create_shortcuts_calls());
 #else
-  EXPECT_FALSE(GetOSIntegrationManager(GetProfile(1))
+  EXPECT_FALSE(GetOsIntegrationManager(GetProfile(1))
                    .get_last_install_options()
                    .has_value());
 #endif

chrome/browser/web_applications/components/os_integration_manager.cc

@@ -348,6 +348,10 @@ void OsIntegrationManager::SuppressOsManagersForTesting() {
   suppress_os_managers_for_testing_ = true;
 }
 
+TestOsIntegrationManager* OsIntegrationManager::AsTestOsIntegrationManager() {
+  return nullptr;
+}
+
 void OsIntegrationManager::CreateShortcuts(const AppId& app_id,
                                            bool add_to_desktop,
                                            CreateShortcutsCallback callback) {

chrome/browser/web_applications/components/os_integration_manager.h

@@ -30,6 +30,7 @@ class WebContents;
 namespace web_app {
 
 class AppIconManager;
+class TestOsIntegrationManager;
 class WebAppUiManager;
 
 // OsHooksResults contains the result of all Os hook deployments
@@ -134,6 +135,8 @@ class OsIntegrationManager {
   // Suppress calling individual OS managers for testing.
   void SuppressOsManagersForTesting();
 
+  virtual TestOsIntegrationManager* AsTestOsIntegrationManager();
+
  protected:
   AppShortcutManager* shortcut_manager() { return shortcut_manager_.get(); }
   FileHandlerManager* file_handler_manager() {

chrome/browser/web_applications/test/test_os_integration_manager.cc

@@ -99,6 +99,11 @@ void TestOsIntegrationManager::SetFileHandlerManager(
   set_file_handler_manager(std::move(file_handler_manager));
 }
 
+TestOsIntegrationManager*
+TestOsIntegrationManager::AsTestOsIntegrationManager() {
+  return this;
+}
+
 TestShortcutManager::TestShortcutManager(Profile* profile)
     : AppShortcutManager(profile) {}
 
@@ -114,4 +119,5 @@ void TestShortcutManager::GetShortcutInfoForApp(
     GetShortcutInfoCallback callback) {
   std::move(callback).Run(nullptr);
 }
+
 }  // namespace web_app

chrome/browser/web_applications/test/test_os_integration_manager.h

@@ -67,6 +67,8 @@ class TestOsIntegrationManager : public OsIntegrationManager {
   void SetFileHandlerManager(
       std::unique_ptr<FileHandlerManager> file_handler_manager);
 
+  TestOsIntegrationManager* AsTestOsIntegrationManager() override;
+
  private:
   size_t num_create_shortcuts_calls_ = 0;
   size_t num_register_run_on_os_login_calls_ = 0;

chrome/browser/web_applications/web_app_install_task.cc

@@ -799,6 +799,10 @@ void WebAppInstallTask::OnInstallFinalizedCreateShortcuts(
   options.add_to_desktop = true;
   options.add_to_quick_launch_bar = kAddAppsToQuickLaunchBarByDefault;
   options.os_hooks[OsHookType::kRunOnOsLogin] = web_app_info->run_on_os_login;
+  // TODO(crbug.com/1087219): Determine if file handlers should be
+  // configured from somewhere else rather than always true.
+  options.os_hooks[OsHookType::kFileHandlers] = true;
+  options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true;
 
   if (install_source_ == WebappInstallSource::SYNC)
     options.add_to_quick_launch_bar = false;
@@ -813,14 +817,12 @@ void WebAppInstallTask::OnInstallFinalizedCreateShortcuts(
         install_params_->run_on_os_login;
     options.os_hooks[OsHookType::kShortcutsMenu] =
         install_params_->add_to_applications_menu;
-    // TODO(crbug.com/1087219): Determine if file handlers should be
-    // configured from somewhere else rather than always true.
-    options.os_hooks[OsHookType::kFileHandlers] = true;
   }
-  options.os_hooks[OsHookType::kUninstallationViaOsSettings] = true;
+
   auto hooks_created_callback =
       base::BindOnce(&WebAppInstallTask::OnOsHooksCreated, GetWeakPtr(),
                      web_app_info->open_as_window, app_id);
+
   os_integration_manager_->InstallOsHooks(app_id,
                                           std::move(hooks_created_callback),
                                           std::move(web_app_info), options);