[dPWA] Move uninstall APIs from WebAppInstallFinalizer to WebAppComma…

…ndScheduler

This CL cleans up WebAppInstallFinalizer's interface and removes
redundancy with WebAppCommandScheduler for uninstalling web apps
and removing install sources/URLs.

This is a pure clean up with no changes in behaviour other than
the removal of the IsExternalInstallSource() DCHECK.

Bug: 1427340
Change-Id: Ia70e287ad2c6f877d32cb84ad93aa3b760533210
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4592631
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1170285}

chrome/browser/ash/android_sms/android_sms_app_setup_controller_impl.cc

@@ -78,7 +78,7 @@ void AndroidSmsAppSetupControllerImpl::PwaDelegate::RemovePwa(
     return;
   }
 
-  provider->install_finalizer().UninstallExternalWebApp(
+  provider->scheduler().RemoveInstallSource(
       app_id, web_app::WebAppManagement::kDefault,
       webapps::WebappUninstallSource::kInternalPreinstalled,
       base::BindOnce(

chrome/browser/ash/apps/apk_web_app_installer_browsertest.cc

@@ -454,7 +454,7 @@ IN_PROC_BROWSER_TEST_F(ApkWebAppInstallerDelayedArcStartBrowserTest,
 
   for (const auto& id : uninstall_ids) {
     base::RunLoop run_loop;
-    provider_->install_finalizer().UninstallWebApp(
+    provider_->scheduler().UninstallWebApp(
         id, webapps::WebappUninstallSource::kShelf,
         base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
           EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);

chrome/browser/ash/apps/apk_web_app_service.cc

@@ -329,7 +329,7 @@ void ApkWebAppService::MaybeUninstallWebApp(const web_app::AppId& web_app_id) {
 
   auto* provider = web_app::WebAppProvider::GetForWebApps(profile_);
   DCHECK(provider);
-  provider->install_finalizer().UninstallExternalWebApp(
+  provider->scheduler().RemoveInstallSource(
       web_app_id, web_app::WebAppManagement::kWebAppStore,
       webapps::WebappUninstallSource::kArc, base::DoNothing());
 }

chrome/browser/ash/child_accounts/time_limits/app_service_wrapper_unittest.cc

@@ -179,16 +179,13 @@ class AppServiceWrapperTest : public ::testing::Test {
 
     if (app_id.app_type() == apps::AppType::kWeb) {
       base::RunLoop run_loop;
-      WebAppProvider::GetForTest(&profile_)
-          ->install_finalizer()
-          .UninstallExternalWebApp(
-              app_id.app_id(), web_app::WebAppManagement::kDefault,
-              webapps::WebappUninstallSource::kExternalPreinstalled,
-              base::BindLambdaForTesting(
-                  [&](webapps::UninstallResultCode code) {
-                    EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-                    run_loop.Quit();
-                  }));
+      WebAppProvider::GetForTest(&profile_)->scheduler().RemoveInstallSource(
+          app_id.app_id(), web_app::WebAppManagement::kDefault,
+          webapps::WebappUninstallSource::kExternalPreinstalled,
+          base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
+            EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
+            run_loop.Quit();
+          }));
       run_loop.Run();
       task_environment_.RunUntilIdle();
       return;

chrome/browser/ash/policy/status_collector/app_info_generator_unittest.cc

@@ -17,7 +17,6 @@
 #include "chrome/browser/apps/app_service/publisher_host.h"
 #include "chrome/browser/ash/login/users/chrome_user_manager.h"
 #include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
-#include "chrome/browser/web_applications/test/fake_install_finalizer.h"
 #include "chrome/browser/web_applications/test/fake_web_app_provider.h"
 #include "chrome/browser/web_applications/test/web_app_test_utils.h"
 #include "chrome/browser/web_applications/web_app.h"

chrome/browser/lacros/web_app_provider_bridge_lacros.cc

@@ -146,7 +146,7 @@ void WebAppProviderBridgeLacros::WebAppUninstalledInArcImpl(
     Profile* profile) {
   DCHECK(profile);
   auto* provider = web_app::WebAppProvider::GetForWebApps(profile);
-  provider->install_finalizer().UninstallExternalWebApp(
+  provider->scheduler().RemoveInstallSource(
       app_id, web_app::WebAppManagement::kWebAppStore,
       webapps::WebappUninstallSource::kArc, std::move(callback));
 }

chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc

@@ -53,7 +53,7 @@
 #include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
 #include "chrome/browser/web_applications/os_integration/os_integration_manager.h"
 #include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
-#include "chrome/browser/web_applications/web_app_install_finalizer.h"
+#include "chrome/browser/web_applications/web_app_command_scheduler.h"
 #include "chrome/browser/web_applications/web_app_install_info.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_features.h"
@@ -955,7 +955,7 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
     base::RunLoop run_loop;
 
     ASSERT_TRUE(provider->registrar_unsafe().CanUserUninstallWebApp(app_id));
-    provider->install_finalizer().UninstallWebApp(
+    provider->scheduler().UninstallWebApp(
         app_id, webapps::WebappUninstallSource::kAppMenu,
         base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
           EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);

chrome/browser/ui/views/page_action/pwa_install_view_browsertest.cc

@@ -261,11 +261,11 @@ class PwaInstallViewBrowserTest : public extensions::ExtensionBrowserTest {
   void UninstallWebApp(const web_app::AppId& app_id) {
     base::RunLoop run_loop;
     web_app::WebAppProvider::GetForTest(browser()->profile())
-        ->install_finalizer()
+        ->scheduler()
         .UninstallWebApp(
             app_id, webapps::WebappUninstallSource::kAppMenu,
             base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-              EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
+              EXPECT_TRUE(UninstallSucceeded(code));
               run_loop.Quit();
             }));
     run_loop.Run();

chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc

@@ -874,14 +874,11 @@ void WebAppIntegrationTestDriver::TearDownOnMainThread() {
       if (provider->registrar_unsafe().IsInstalled(app_id)) {
         DCHECK(app->CanUserUninstallWebApp());
         UninstallCompleteWaiter uninstall_waiter(profile, app_id);
-        base::RunLoop run_loop;
-        provider->install_finalizer().UninstallWebApp(
+        base::test::TestFuture<webapps::UninstallResultCode> future;
+        provider->scheduler().UninstallWebApp(
             app_id, webapps::WebappUninstallSource::kAppsPage,
-            base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-              EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-              run_loop.Quit();
-            }));
-        run_loop.Run();
+            future.GetCallback());
+        EXPECT_TRUE(UninstallSucceeded(future.Get()));
         uninstall_waiter.Wait();
       }
       LOG(INFO) << "TearDownOnMainThread: Uninstall complete.";

chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc

@@ -14,8 +14,8 @@
 #include "chrome/browser/ui/views/chrome_typography.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/web_apps/web_app_info_image_source.h"
+#include "chrome/browser/web_applications/web_app_command_scheduler.h"
 #include "chrome/browser/web_applications/web_app_icon_manager.h"
-#include "chrome/browser/web_applications/web_app_install_finalizer.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
@@ -176,8 +176,8 @@ void WebAppUninstallDialogDelegateView::Uninstall() {
   // WebAppUninstallDialogDelegateView lifetime is controlled by Widget and it
   // is terminiated as soon as dialog is closed regardless of web app
   // uninstallation.
-  provider->install_finalizer().UninstallWebApp(app_id_, uninstall_source_,
-                                                dialog_->UninstallStarted());
+  provider->scheduler().UninstallWebApp(app_id_, uninstall_source_,
+                                        dialog_->UninstallStarted());
   // We successfully call Web App Uninstall routine, then
   // WebAppUninstallDialogDelegateView can be terminated, but can't call the
   // callback of the dialog caller.

chrome/browser/ui/web_applications/sub_apps_service_impl.cc

@@ -407,7 +407,7 @@ void SubAppsServiceImpl::RemoveSubApp(
         manifest_id_path, SubAppsServiceResultCode::kFailure));
   }
 
-  provider->install_finalizer().UninstallExternalWebApp(
+  provider->scheduler().RemoveInstallSource(
       sub_app_id, WebAppManagement::Type::kSubApp,
       webapps::WebappUninstallSource::kSubApp,
       base::BindOnce(

chrome/browser/ui/web_applications/sub_apps_service_impl_browsertest.cc

@@ -17,9 +17,9 @@
 #include "chrome/browser/ui/web_applications/web_app_controller_browsertest.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_command_scheduler.h"
 #include "chrome/browser/web_applications/web_app_constants.h"
 #include "chrome/browser/web_applications/web_app_helpers.h"
-#include "chrome/browser/web_applications/web_app_install_finalizer.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/browser/web_applications/web_app_registry_update.h"
@@ -115,7 +115,7 @@ class SubAppsServiceImplBrowserTest : public WebAppControllerBrowserTest {
 
   void UninstallParentAppBySource(WebAppManagement::Type source) {
     base::test::TestFuture<void> uninstall_future;
-    provider().install_finalizer().UninstallExternalWebApp(
+    provider().scheduler().RemoveInstallSource(
         parent_app_id_, source, webapps::WebappUninstallSource::kAppsPage,
         base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
           EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);

chrome/browser/ui/web_applications/test/web_app_navigation_browsertest.cc

@@ -246,7 +246,7 @@ void WebAppNavigationBrowserTest::TearDownOnMainThread() {
     AppReadinessWaiter app_readiness_waiter(
         profile(), app_id, apps::Readiness::kUninstalledByUser);
     base::RunLoop run_loop;
-    provider->install_finalizer().UninstallWebApp(
+    provider->scheduler().UninstallWebApp(
         app_id, webapps::WebappUninstallSource::kAppsPage,
         base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
           EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);

chrome/browser/ui/web_applications/web_app_browsertest.cc

@@ -1385,14 +1385,10 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, ShortcutIconCorrectColor) {
       << "Actual color (RGB) is: "
       << color_utils::SkColorToRgbString(icon_pixel_color.value());
 
-  base::RunLoop run_loop_uninstall;
-  provider->install_finalizer().UninstallWebApp(
-      app_id, webapps::WebappUninstallSource::kAppMenu,
-      base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-        EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-        run_loop_uninstall.Quit();
-      }));
-  run_loop_uninstall.Run();
+  base::test::TestFuture<webapps::UninstallResultCode> future;
+  provider->scheduler().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppMenu, future.GetCallback());
+  EXPECT_TRUE(UninstallSucceeded(future.Get()));
 }
 #endif
 
@@ -1479,17 +1475,12 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest_ShortcutMenu, ShortcutsMenuSuccess) {
           .GetSwitchValueASCII(switches::kAppLaunchUrlForShortcutsMenuItem)
           .find("/banners/launch_url2"));
 
-  base::RunLoop run_loop_uninstall;
-  WebAppProvider::GetForTest(profile())->install_finalizer().UninstallWebApp(
-      app_id, webapps::WebappUninstallSource::kAppMenu,
-      base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-        EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-        EXPECT_THAT(
-            tester.GetAllSamples("WebApp.ShortcutsMenuUnregistered.Result"),
-            BucketsAre(base::Bucket(true, 1)));
-        run_loop_uninstall.Quit();
-      }));
-  run_loop_uninstall.Run();
+  base::test::TestFuture<webapps::UninstallResultCode> future;
+  provider().scheduler().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppMenu, future.GetCallback());
+  EXPECT_TRUE(UninstallSucceeded(future.Get()));
+  EXPECT_THAT(tester.GetAllSamples("WebApp.ShortcutsMenuUnregistered.Result"),
+              BucketsAre(base::Bucket(true, 1)));
 }
 
 IN_PROC_BROWSER_TEST_F(WebAppBrowserTest_ShortcutMenu,
@@ -1542,28 +1533,22 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest_ShortcutMenu,
   // No shortcuts should be read.
   EXPECT_TRUE(shortcuts_menu_items.empty());
 
-  base::RunLoop run_loop_uninstall;
   bool sub_manager_execute_enabled = AreSubManagersExecuteEnabled();
-  WebAppProvider::GetForTest(profile())->install_finalizer().UninstallWebApp(
-      app_id, webapps::WebappUninstallSource::kAppMenu,
-      base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-        EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-        if (sub_manager_execute_enabled) {
-          // TODO(crbug.com/1401125): Sub manager code smartly knows that there
-          // aren't any shortcuts menu data, so doesn't do anything. The old OS
-          // integration code does not read current OS states, so it triggers
-          // the histogram. Clean up once sub managers are released.
-          EXPECT_THAT(
-              tester.GetAllSamples("WebApp.ShortcutsMenuUnregistered.Result"),
-              BucketsAre(base::Bucket(true, 0), base::Bucket(false, 0)));
-        } else {
-          EXPECT_THAT(
-              tester.GetAllSamples("WebApp.ShortcutsMenuUnregistered.Result"),
-              BucketsAre(base::Bucket(true, 1)));
-        }
-        run_loop_uninstall.Quit();
-      }));
-  run_loop_uninstall.Run();
+  base::test::TestFuture<webapps::UninstallResultCode> future;
+  provider().scheduler().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppMenu, future.GetCallback());
+  EXPECT_TRUE(UninstallSucceeded(future.Get()));
+  if (sub_manager_execute_enabled) {
+    // TODO(crbug.com/1401125): Sub manager code smartly knows that there
+    // aren't any shortcuts menu data, so doesn't do anything. The old OS
+    // integration code does not read current OS states, so it triggers
+    // the histogram. Clean up once sub managers are released.
+    EXPECT_THAT(tester.GetAllSamples("WebApp.ShortcutsMenuUnregistered.Result"),
+                BucketsAre(base::Bucket(true, 0), base::Bucket(false, 0)));
+  } else {
+    EXPECT_THAT(tester.GetAllSamples("WebApp.ShortcutsMenuUnregistered.Result"),
+                BucketsAre(base::Bucket(true, 1)));
+  }
 }
 
 #endif
@@ -1598,15 +1583,11 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, WebAppCreateAndDeleteShortcut) {
   EXPECT_TRUE(registration->test_override->IsShortcutCreated(
       profile(), app_id, provider->registrar_unsafe().GetAppShortName(app_id)));
 
-  // Unistall the web app
-  base::RunLoop run_loop_uninstall;
-  provider->install_finalizer().UninstallWebApp(
-      app_id, webapps::WebappUninstallSource::kAppMenu,
-      base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-        EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-        run_loop_uninstall.Quit();
-      }));
-  run_loop_uninstall.Run();
+  // Uninstall the web app
+  base::test::TestFuture<webapps::UninstallResultCode> future;
+  provider->scheduler().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppMenu, future.GetCallback());
+  EXPECT_TRUE(UninstallSucceeded(future.Get()));
 
 #if BUILDFLAG(IS_WIN)
   base::FilePath desktop_shortcut_path =
@@ -2352,19 +2333,13 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest_FileHandler,
   ASSERT_TRUE(registration->test_override->DeleteChromeAppsDir());
 #endif
 
-  // Unistall the web app
-  NavigateToURLAndWait(browser(), GURL(chrome::kChromeUIAppsURL));
-  base::RunLoop run_loop_uninstall;
-  WebAppProvider::GetForTest(profile())->install_finalizer().UninstallWebApp(
-      app_id, webapps::WebappUninstallSource::kAppsPage,
-      base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-        EXPECT_THAT(
-            tester.GetAllSamples("WebApp.FileHandlersUnregistration.Result"),
-            BucketsAre(base::Bucket(true, 1)));
-        EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-        run_loop_uninstall.Quit();
-      }));
-  run_loop_uninstall.Run();
+  // Uninstall the web app
+  base::test::TestFuture<webapps::UninstallResultCode> future;
+  provider().scheduler().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppsPage, future.GetCallback());
+  EXPECT_TRUE(UninstallSucceeded(future.Get()));
+  EXPECT_THAT(tester.GetAllSamples("WebApp.FileHandlersUnregistration.Result"),
+              BucketsAre(base::Bucket(true, 1)));
 
 #if BUILDFLAG(IS_WIN)
   // Check file associations after the web app is uninstalled.
@@ -2431,16 +2406,11 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest_FileHandler,
   }
   ASSERT_TRUE(registration->test_override->DeleteChromeAppsDir());
 
-  // Unistall the web app
-  NavigateToURLAndWait(browser(), GURL(chrome::kChromeUIAppsURL));
-  base::RunLoop run_loop_uninstall;
-  WebAppProvider::GetForTest(profile())->install_finalizer().UninstallWebApp(
-      app_id, webapps::WebappUninstallSource::kAppsPage,
-      base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-        EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-        run_loop_uninstall.Quit();
-      }));
-  run_loop_uninstall.Run();
+  // Uninstall the web app
+  base::test::TestFuture<webapps::UninstallResultCode> future;
+  provider().scheduler().UninstallWebApp(
+      app_id, webapps::WebappUninstallSource::kAppsPage, future.GetCallback());
+  EXPECT_TRUE(UninstallSucceeded(future.Get()));
 }
 #endif  // BUILDFLAG(IS_MAC)
 #endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)

chrome/browser/ui/web_applications/web_app_uninstall_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include "base/run_loop.h"
 #include "base/test/bind.h"
+#include "base/test/test_future.h"
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
@@ -20,8 +21,8 @@
 #include "chrome/browser/web_applications/test/web_app_test_observers.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_command_manager.h"
+#include "chrome/browser/web_applications/web_app_command_scheduler.h"
 #include "chrome/browser/web_applications/web_app_id.h"
-#include "chrome/browser/web_applications/web_app_install_finalizer.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "components/services/app_service/public/cpp/app_launch_util.h"
@@ -44,17 +45,13 @@ class WebAppUninstallBrowserTest : public WebAppControllerBrowserTest {
 
   void UninstallWebApp(const AppId& app_id) {
     WebAppProvider* const provider = WebAppProvider::GetForTest(profile());
-    base::RunLoop run_loop;
 
+    base::test::TestFuture<webapps::UninstallResultCode> future;
     DCHECK(provider->registrar_unsafe().CanUserUninstallWebApp(app_id));
-    provider->install_finalizer().UninstallWebApp(
-        app_id, webapps::WebappUninstallSource::kAppMenu,
-        base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
-          EXPECT_EQ(code, webapps::UninstallResultCode::kSuccess);
-          run_loop.Quit();
-        }));
+    provider->scheduler().UninstallWebApp(
+        app_id, webapps::WebappUninstallSource::kAppMenu, future.GetCallback());
+    EXPECT_EQ(future.Get(), webapps::UninstallResultCode::kSuccess);
 
-    run_loop.Run();
     base::RunLoop().RunUntilIdle();
   }
 };
@@ -160,7 +157,7 @@ IN_PROC_BROWSER_TEST_F(WebAppUninstallBrowserTest, TwoUninstallCalls) {
   WebAppProvider* const provider = WebAppProvider::GetForTest(profile());
   EXPECT_TRUE(provider->registrar_unsafe().IsInstalled(app_id));
   DCHECK(provider->registrar_unsafe().CanUserUninstallWebApp(app_id));
-  provider->install_finalizer().UninstallWebApp(
+  provider->scheduler().UninstallWebApp(
       app_id, webapps::WebappUninstallSource::kAppMenu,
       base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
         if (quit_run_loop)
@@ -171,7 +168,7 @@ IN_PROC_BROWSER_TEST_F(WebAppUninstallBrowserTest, TwoUninstallCalls) {
   EXPECT_EQ(1u, provider->command_manager().GetCommandCountForTesting());
 
   // Trigger second uninstall call and wait for result.
-  provider->install_finalizer().UninstallWebApp(
+  provider->scheduler().UninstallWebApp(
       app_id, webapps::WebappUninstallSource::kAppMenu,
       base::BindLambdaForTesting([&](webapps::UninstallResultCode code) {
         if (quit_run_loop)