[Web Apps] Add uninstall_and_replace to PendingAppInstallTask for migrations.

This CL pulls the System Web App migration logic into PendingAppInstallTask
in preparation for External App migration. This also changes migration_source
to a list of apps that the Web App System will uninstall using the App Service
and then migrate OS data from.

This CL also adds the Settings SWA as a migration target for OS Settings.

Bug: 976578
Change-Id: I1d938690d7550101138563a3b266bc491fefd2df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1736129
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688403}

chrome/browser/ui/app_list/app_list_syncable_service.cc

@@ -1184,10 +1184,7 @@ void AppListSyncableService::SendSyncChange(
 
 AppListSyncableService::SyncItem* AppListSyncableService::FindSyncItem(
     const std::string& item_id) {
-  auto iter = sync_items_.find(item_id);
-  if (iter == sync_items_.end())
-    return NULL;
-  return iter->second.get();
+  return const_cast<SyncItem*>(GetSyncItem(item_id));
 }
 
 AppListSyncableService::SyncItem* AppListSyncableService::CreateSyncItem(

chrome/browser/ui/web_applications/app_browser_controller_browsertest.cc

@@ -46,7 +46,7 @@ class AppBrowserControllerBrowserTest
 
     auto* provider = WebAppProvider::Get(profile());
     provider->system_web_app_manager().SetSystemAppsForTesting(
-        {{SystemAppType::TERMINAL, {app_url, app_id}}});
+        {{SystemAppType::TERMINAL, SystemAppInfo(app_url)}});
     web_app::ExternallyInstalledWebAppPrefs(profile()->GetPrefs())
         .Insert(app_url, app_id,
                 web_app::ExternalInstallSource::kInternalDefault);

chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc

@@ -8,6 +8,8 @@
 
 #include "base/callback.h"
 #include "build/build_config.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -83,12 +85,28 @@ void WebAppUiManagerImpl::NotifyOnAllAppWindowsClosed(
   windows_closed_requests_map_[app_id].push_back(std::move(callback));
 }
 
-void WebAppUiManagerImpl::MigrateOSAttributes(const AppId& from,
-                                              const AppId& to) {
+void WebAppUiManagerImpl::UninstallAndReplace(
+    const std::vector<AppId>& from_apps,
+    const AppId& to_app) {
+  bool has_migrated = false;
+  for (const AppId& from_app : from_apps) {
+    if (!has_migrated) {
 #if defined(OS_CHROMEOS)
-  app_list::AppListSyncableServiceFactory::GetForProfile(profile_)
-      ->TransferItemAttributes(from, to);
+      auto* app_list_syncable_service =
+          app_list::AppListSyncableServiceFactory::GetForProfile(profile_);
+      if (app_list_syncable_service->GetSyncItem(from_app)) {
+        app_list_syncable_service->TransferItemAttributes(from_app, to_app);
+        has_migrated = true;
+      }
 #endif
+    }
+
+    if (apps::AppServiceProxyFactory::IsEnabled()) {
+      apps::AppServiceProxy* proxy =
+          apps::AppServiceProxyFactory::GetForProfile(profile_);
+      proxy->Uninstall(from_app);
+    }
+  }
 }
 
 bool WebAppUiManagerImpl::CanAddAppToQuickLaunchBar() const {

chrome/browser/ui/web_applications/web_app_ui_manager_impl.h

@@ -25,6 +25,8 @@ class WebAppProvider;
 class WebAppDialogManager;
 
 // This KeyedService is a UI counterpart for WebAppProvider.
+// TODO(calamity): Rename this to WebAppProviderDelegate to better reflect that
+// this class serves a wide range of Web Applications <-> Browser purposes.
 class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
  public:
   static WebAppUiManagerImpl* Get(Profile* profile);
@@ -39,7 +41,8 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
   size_t GetNumWindowsForApp(const AppId& app_id) override;
   void NotifyOnAllAppWindowsClosed(const AppId& app_id,
                                    base::OnceClosure callback) override;
-  void MigrateOSAttributes(const AppId& from, const AppId& to) override;
+  void UninstallAndReplace(const std::vector<AppId>& from_apps,
+                           const AppId& to_app) override;
   bool CanAddAppToQuickLaunchBar() const override;
   void AddAppToQuickLaunchBar(const AppId& app_id) override;
   bool IsInAppWindow(content::WebContents* web_contents) const override;

chrome/browser/ui/web_applications/web_app_ui_manager_impl_browsertest.cc

@@ -180,7 +180,7 @@ IN_PROC_BROWSER_TEST_F(WebAppUiManagerImplBrowserTest,
 }
 
 #if defined(OS_CHROMEOS)
-class WebAppUiServiceMigrationBrowserTest
+class WebAppUiManagerMigrationBrowserTest
     : public WebAppUiManagerImplBrowserTest {
  public:
   void SetUp() override {
@@ -204,7 +204,7 @@ class WebAppUiServiceMigrationBrowserTest
 
 // Tests that the Settings app migrates the launcher and app list details from
 // the Settings internal app.
-IN_PROC_BROWSER_TEST_F(WebAppUiServiceMigrationBrowserTest,
+IN_PROC_BROWSER_TEST_F(WebAppUiManagerMigrationBrowserTest,
                        SettingsSystemWebAppMigration) {
   base::test::ScopedFeatureList scoped_feature_list;
   scoped_feature_list.InitAndEnableFeature(features::kSystemWebApps);

chrome/browser/web_applications/components/external_install_options.cc

@@ -7,6 +7,7 @@
 #include <ostream>
 #include <tuple>
 
+#include "base/strings/string_util.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 
 namespace web_app {
@@ -37,14 +38,15 @@ bool ExternalInstallOptions::operator==(
                   add_to_quick_launch_bar, override_previous_user_uninstall,
                   bypass_service_worker_check, require_manifest,
                   force_reinstall, wait_for_windows_closed, install_placeholder,
-                  reinstall_placeholder) ==
+                  reinstall_placeholder, uninstall_and_replace) ==
          std::tie(other.url, other.launch_container, other.install_source,
                   other.add_to_applications_menu, other.add_to_desktop,
                   other.add_to_quick_launch_bar,
                   other.override_previous_user_uninstall,
                   other.bypass_service_worker_check, other.require_manifest,
                   other.force_reinstall, other.wait_for_windows_closed,
-                  other.install_placeholder, other.reinstall_placeholder);
+                  other.install_placeholder, other.reinstall_placeholder,
+                  other.uninstall_and_replace);
 }
 
 std::ostream& operator<<(std::ostream& out,
@@ -69,7 +71,9 @@ std::ostream& operator<<(std::ostream& out,
              << "\n install_placeholder: "
              << install_options.install_placeholder
              << "\n reinstall_placeholder: "
-             << install_options.reinstall_placeholder;
+             << install_options.reinstall_placeholder
+             << "\n uninstall_and_replace:\n  "
+             << base::JoinString(install_options.uninstall_and_replace, "\n  ");
 }
 
 InstallManager::InstallParams ConvertExternalInstallOptionsToParams(

chrome/browser/web_applications/components/external_install_options.h

@@ -8,6 +8,7 @@
 #include <iosfwd>
 
 #include "chrome/browser/web_applications/components/install_manager.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "url/gurl.h"
 
 namespace web_app {
@@ -78,6 +79,10 @@ struct ExternalInstallOptions {
   // Whether we should try to reinstall the app if there is a placeholder for
   // it.
   bool reinstall_placeholder = false;
+
+  // A list of app_ids that the Web App System should attempt to uninstall and
+  // replace with this app (e.g maintain shelf pins, app list positions).
+  std::vector<AppId> uninstall_and_replace;
 };
 
 std::ostream& operator<<(std::ostream& out,

chrome/browser/web_applications/components/web_app_ui_manager.h

@@ -35,9 +35,11 @@ class WebAppUiManager {
   virtual void NotifyOnAllAppWindowsClosed(const AppId& app_id,
                                            base::OnceClosure callback) = 0;
 
-  // Migrates an app's OS attributes (e.g pin position, app list
-  // folder/position, shortcuts).
-  virtual void MigrateOSAttributes(const AppId& from, const AppId& to) = 0;
+  // Uninstalls the the apps in |from_apps| and migrates an |to_app|'s OS
+  // attributes (e.g pin position, app list folder/position, shortcuts) to the
+  // first |from_app| found.
+  virtual void UninstallAndReplace(const std::vector<AppId>& from_apps,
+                                   const AppId& to_app) = 0;
 
   virtual bool CanAddAppToQuickLaunchBar() const = 0;
   virtual void AddAppToQuickLaunchBar(const AppId& app_id) = 0;

chrome/browser/web_applications/extensions/pending_app_install_task_unittest.cc

@@ -31,6 +31,7 @@
 #include "chrome/browser/web_applications/test/test_data_retriever.h"
 #include "chrome/browser/web_applications/test/test_install_finalizer.h"
 #include "chrome/browser/web_applications/test/test_web_app_provider.h"
+#include "chrome/browser/web_applications/test/test_web_app_ui_manager.h"
 #include "chrome/browser/web_applications/test/test_web_app_url_loader.h"
 #include "chrome/browser/web_applications/web_app_install_manager.h"
 #include "chrome/common/chrome_features.h"
@@ -286,21 +287,22 @@ class PendingAppInstallTaskTest : public ChromeRenderViewHostTestHarness {
         std::make_unique<TestPendingAppInstallFinalizer>(registrar.get());
     install_finalizer_ = install_finalizer.get();
 
-    auto data_retriever = std::make_unique<TestDataRetriever>();
-    data_retriever_ = data_retriever.get();
-
     auto install_manager = std::make_unique<WebAppInstallManager>(profile());
-    install_manager->SetDataRetrieverFactoryForTesting(
-        GetFactoryForRetriever(std::move(data_retriever)));
+    install_manager_ = install_manager.get();
+
+    auto ui_manager = std::make_unique<TestWebAppUiManager>();
+    ui_manager_ = ui_manager.get();
 
     provider->SetRegistrar(std::move(registrar));
     provider->SetInstallManager(std::move(install_manager));
     provider->SetInstallFinalizer(std::move(install_finalizer));
+    provider->SetWebAppUiManager(std::move(ui_manager));
 
     provider->Start();
   }
 
  protected:
+  TestWebAppUiManager* ui_manager() { return ui_manager_; }
   TestAppRegistrar* registrar() { return registrar_; }
   TestPendingAppInstallFinalizer* finalizer() { return install_finalizer_; }
 
@@ -318,6 +320,11 @@ class PendingAppInstallTaskTest : public ChromeRenderViewHostTestHarness {
 
   std::unique_ptr<PendingAppInstallTask> GetInstallationTaskWithTestMocks(
       ExternalInstallOptions options) {
+    auto data_retriever = std::make_unique<TestDataRetriever>();
+    data_retriever_ = data_retriever.get();
+
+    install_manager_->SetDataRetrieverFactoryForTesting(
+        GetFactoryForRetriever(std::move(data_retriever)));
     auto manifest = std::make_unique<blink::Manifest>();
     manifest->start_url = options.url;
 
@@ -335,16 +342,19 @@ class PendingAppInstallTaskTest : public ChromeRenderViewHostTestHarness {
         install_finalizer_->GetAppIdForUrl(options.url), true);
 
     auto task = std::make_unique<PendingAppInstallTask>(
-        profile(), registrar_, install_finalizer_, std::move(options));
+        profile(), registrar_, ui_manager_, install_finalizer_,
+        std::move(options));
     return task;
   }
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
 
+  WebAppInstallManager* install_manager_ = nullptr;
   TestAppRegistrar* registrar_ = nullptr;
   TestDataRetriever* data_retriever_ = nullptr;
   TestPendingAppInstallFinalizer* install_finalizer_ = nullptr;
+  TestWebAppUiManager* ui_manager_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(PendingAppInstallTaskTest);
 };
@@ -748,4 +758,51 @@ TEST_F(PendingAppInstallTaskTest, ReinstallPlaceholderFails) {
   run_loop.Run();
 }
 
+TEST_F(PendingAppInstallTaskTest, UninstallAndReplace) {
+  ExternalInstallOptions options = {kWebAppUrl, LaunchContainer::kDefault,
+                                    ExternalInstallSource::kInternalDefault};
+  AppId app_id;
+  {
+    // Migrate app1 and app2.
+    options.uninstall_and_replace = {"app1", "app2"};
+
+    base::RunLoop run_loop;
+    auto task = GetInstallationTaskWithTestMocks(options);
+    task->Install(
+        web_contents(), WebAppUrlLoader::Result::kUrlLoaded,
+        base::BindLambdaForTesting([&](PendingAppInstallTask::Result result) {
+          app_id = *result.app_id;
+
+          EXPECT_EQ(InstallResultCode::kSuccess, result.code);
+          EXPECT_EQ(app_id,
+                    *ExternallyInstalledWebAppPrefs(profile()->GetPrefs())
+                         .LookupAppId(kWebAppUrl));
+
+          EXPECT_TRUE(ui_manager()->DidUninstallAndReplace("app1", app_id));
+          EXPECT_TRUE(ui_manager()->DidUninstallAndReplace("app2", app_id));
+
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+  }
+  {
+    // Migration shouldn't run on subsequent installs of the same app.
+    options.uninstall_and_replace = {"app3"};
+
+    base::RunLoop run_loop;
+    auto task = GetInstallationTaskWithTestMocks(options);
+    task->Install(
+        web_contents(), WebAppUrlLoader::Result::kUrlLoaded,
+        base::BindLambdaForTesting([&](PendingAppInstallTask::Result result) {
+          EXPECT_EQ(InstallResultCode::kSuccess, result.code);
+          EXPECT_EQ(app_id, *result.app_id);
+
+          EXPECT_FALSE(ui_manager()->DidUninstallAndReplace("app3", app_id));
+
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+  }
+}
+
 }  // namespace web_app

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.cc

@@ -153,8 +153,8 @@ SystemWebAppManagerBrowserTest::CreateWebAppProvider(Profile* profile) {
   provider->SetSystemWebAppManager(std::move(test_system_web_app_manager));
 
   base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-  system_apps[SystemAppType::SETTINGS] = {
-      content::GetWebUIURL("test-system-app/pwa.html")};
+  system_apps[SystemAppType::SETTINGS] =
+      SystemAppInfo(content::GetWebUIURL("test-system-app/pwa.html"));
   test_system_web_app_manager_->SetSystemApps(std::move(system_apps));
 
   // Start registry and all dependent subsystems:

chrome/browser/web_applications/extensions/system_web_app_manager_unittest.cc

@@ -128,7 +128,8 @@ TEST_F(SystemWebAppManagerTest, Disabled) {
                                  ExternalInstallSource::kSystemInstalled);
 
   base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-  system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
+
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
 
   system_web_app_manager()->SetSystemApps(std::move(system_apps));
   system_web_app_manager()->Start();
@@ -146,8 +147,8 @@ TEST_F(SystemWebAppManagerTest, Disabled) {
 // Test that System Apps do install with the feature enabled.
 TEST_F(SystemWebAppManagerTest, Enabled) {
   base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-  system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
-  system_apps[SystemAppType::DISCOVER] = {kAppUrl2};
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
+  system_apps[SystemAppType::DISCOVER] = SystemAppInfo(kAppUrl2);
 
   system_web_app_manager()->SetSystemApps(std::move(system_apps));
   system_web_app_manager()->Start();
@@ -168,7 +169,7 @@ TEST_F(SystemWebAppManagerTest, UninstallAppInstalledInPreviousSession) {
   SimulatePreviouslyInstalledApp(kAppUrl3,
                                  ExternalInstallSource::kInternalDefault);
   base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-  system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
 
   system_web_app_manager()->SetSystemApps(std::move(system_apps));
   system_web_app_manager()->Start();
@@ -192,7 +193,7 @@ TEST_F(SystemWebAppManagerTest, AlwaysUpdate) {
       SystemWebAppManager::UpdatePolicy::kAlwaysUpdate);
 
   base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-  system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
   system_web_app_manager()->SetSystemApps(system_apps);
 
   system_web_app_manager()->set_current_version(base::Version("1.0.0.0"));
@@ -203,7 +204,7 @@ TEST_F(SystemWebAppManagerTest, AlwaysUpdate) {
 
   // Create another app. The version hasn't changed but the app should still
   // install.
-  system_apps[SystemAppType::DISCOVER] = {kAppUrl2};
+  system_apps[SystemAppType::DISCOVER] = SystemAppInfo(kAppUrl2);
   system_web_app_manager()->SetSystemApps(system_apps);
   system_web_app_manager()->Start();
 
@@ -236,7 +237,7 @@ TEST_F(SystemWebAppManagerTest, UpdateOnVersionChange) {
       SystemWebAppManager::UpdatePolicy::kOnVersionChange);
 
   base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-  system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
   system_web_app_manager()->SetSystemApps(system_apps);
 
   system_web_app_manager()->set_current_version(base::Version("1.0.0.0"));
@@ -250,7 +251,7 @@ TEST_F(SystemWebAppManagerTest, UpdateOnVersionChange) {
   // Create another app. The version hasn't changed, but we should immediately
   // install anyway, as if a user flipped a chrome://flag. The first app won't
   // force reinstall.
-  system_apps[SystemAppType::DISCOVER] = {kAppUrl2};
+  system_apps[SystemAppType::DISCOVER] = SystemAppInfo(kAppUrl2);
   system_web_app_manager()->SetSystemApps(system_apps);
   system_web_app_manager()->Start();
   base::RunLoop().RunUntilIdle();
@@ -298,7 +299,7 @@ TEST_F(SystemWebAppManagerTest, UpdateOnVersionChange) {
 
   // Changing the install URL of a system app propagates even without a version
   // change.
-  system_apps[SystemAppType::SETTINGS] = {kAppUrl3};
+  system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl3);
   system_web_app_manager()->SetSystemApps(system_apps);
   system_web_app_manager()->Start();
   base::RunLoop().RunUntilIdle();
@@ -315,7 +316,7 @@ TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
   base::HistogramTester histograms;
   {
     base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-    system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
+    system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
     system_web_app_manager()->SetSystemApps(system_apps);
 
     histograms.ExpectTotalCount(
@@ -331,8 +332,8 @@ TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
   }
   {
     base::flat_map<SystemAppType, SystemAppInfo> system_apps;
-    system_apps[SystemAppType::SETTINGS] = {kAppUrl1};
-    system_apps[SystemAppType::DISCOVER] = {kAppUrl2};
+    system_apps[SystemAppType::SETTINGS] = SystemAppInfo(kAppUrl1);
+    system_apps[SystemAppType::DISCOVER] = SystemAppInfo(kAppUrl2);
     system_web_app_manager()->SetSystemApps(system_apps);
     pending_app_manager()->SetInstallResultCode(
         InstallResultCode::kInstallManagerDestroyed);

chrome/browser/web_applications/pending_app_install_task.cc

@@ -20,6 +20,7 @@
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
+#include "chrome/browser/web_applications/components/web_app_ui_manager.h"
 #include "chrome/common/web_application_info.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -46,11 +47,13 @@ void PendingAppInstallTask::CreateTabHelpers(
 PendingAppInstallTask::PendingAppInstallTask(
     Profile* profile,
     AppRegistrar* registrar,
+    WebAppUiManager* ui_manager,
     InstallFinalizer* install_finalizer,
     ExternalInstallOptions install_options)
     : profile_(profile),
       registrar_(registrar),
       install_finalizer_(install_finalizer),
+      ui_manager_(ui_manager),
       externally_installed_app_prefs_(profile_->GetPrefs()),
       install_options_(std::move(install_options)) {}
 
@@ -188,6 +191,13 @@ void PendingAppInstallTask::OnWebAppInstalled(bool is_placeholder,
     return;
   }
 
+  // If this is the first time the app has been installed, run a migration. This
+  // will not happen again, even if the app is uninstalled and reinstalled.
+  if (!externally_installed_app_prefs_.LookupAppId(install_options_.url)) {
+    ui_manager_->UninstallAndReplace(install_options().uninstall_and_replace,
+                                     app_id);
+  }
+
   externally_installed_app_prefs_.Insert(install_options_.url, app_id,
                                          install_options_.install_source);
   externally_installed_app_prefs_.SetIsPlaceholder(install_options_.url,

chrome/browser/web_applications/pending_app_install_task.h

@@ -27,6 +27,7 @@ namespace web_app {
 
 enum class InstallResultCode;
 class InstallFinalizer;
+class WebAppUiManager;
 
 // Class to install WebApp from a WebContents. A queue of such tasks is owned by
 // PendingAppManager. Can only be called from the UI thread.
@@ -55,6 +56,7 @@ class PendingAppInstallTask {
   // policy, etc.
   explicit PendingAppInstallTask(Profile* profile,
                                  AppRegistrar* registrar,
+                                 WebAppUiManager* ui_manager,
                                  InstallFinalizer* install_finalizer,
                                  ExternalInstallOptions install_options);
 
@@ -87,6 +89,7 @@ class PendingAppInstallTask {
   Profile* profile_;
   AppRegistrar* registrar_;
   InstallFinalizer* install_finalizer_;
+  WebAppUiManager* ui_manager_;
 
   ExternallyInstalledWebAppPrefs externally_installed_app_prefs_;
 

chrome/browser/web_applications/pending_app_manager_impl.cc

@@ -88,8 +88,9 @@ void PendingAppManagerImpl::SetUrlLoaderForTesting(
 std::unique_ptr<PendingAppInstallTask>
 PendingAppManagerImpl::CreateInstallationTask(
     ExternalInstallOptions install_options) {
-  return std::make_unique<PendingAppInstallTask>(
-      profile_, registrar(), finalizer(), std::move(install_options));
+  return std::make_unique<PendingAppInstallTask>(profile_, registrar(),
+                                                 ui_manager(), finalizer(),
+                                                 std::move(install_options));
 }
 
 void PendingAppManagerImpl::MaybeStartNextInstallation() {

chrome/browser/web_applications/pending_app_manager_impl_unittest.cc

@@ -123,6 +123,7 @@ class TestPendingAppManagerImpl : public PendingAppManagerImpl {
         ExternalInstallOptions install_options)
         : PendingAppInstallTask(profile,
                                 pending_app_manager_impl->registrar(),
+                                pending_app_manager_impl->ui_manager(),
                                 pending_app_manager_impl->finalizer(),
                                 install_options),
           pending_app_manager_impl_(pending_app_manager_impl),

chrome/browser/web_applications/system_web_app_manager.cc

@@ -27,6 +27,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "ash/public/cpp/app_list/internal_app_id_constants.h"
+#include "chrome/browser/chromeos/extensions/default_web_app_ids.h"
 #include "chromeos/constants/chromeos_features.h"
 #endif  // defined(OS_CHROMEOS)
 
@@ -39,26 +40,31 @@ base::flat_map<SystemAppType, SystemAppInfo> CreateSystemWebApps() {
 // TODO(calamity): Split this into per-platform functions.
 #if defined(OS_CHROMEOS)
   if (SystemWebAppManager::IsAppEnabled(SystemAppType::DISCOVER))
-    infos[SystemAppType::DISCOVER] = {GURL(chrome::kChromeUIDiscoverURL)};
+    infos[SystemAppType::DISCOVER].install_url =
+        GURL(chrome::kChromeUIDiscoverURL);
 
   if (SystemWebAppManager::IsAppEnabled(SystemAppType::CAMERA)) {
     constexpr char kCameraAppPWAURL[] = "chrome://camera/pwa.html";
-    infos[SystemAppType::CAMERA] = {GURL(kCameraAppPWAURL),
-                                    app_list::kInternalAppIdCamera};
+    infos[SystemAppType::CAMERA].install_url = GURL(kCameraAppPWAURL);
+    infos[SystemAppType::CAMERA].uninstall_and_replace = {
+        app_list::kInternalAppIdCamera};
   }
 
   if (base::FeatureList::IsEnabled(chromeos::features::kSplitSettings)) {
     constexpr char kChromeSettingsPWAURL[] = "chrome://os-settings/pwa.html";
-    infos[SystemAppType::SETTINGS] = {GURL(kChromeSettingsPWAURL),
-                                      app_list::kInternalAppIdSettings};
+    infos[SystemAppType::SETTINGS].install_url = GURL(kChromeSettingsPWAURL);
+    infos[SystemAppType::SETTINGS].uninstall_and_replace = {
+        chromeos::default_web_apps::kSettingsAppId,
+        app_list::kInternalAppIdSettings};
   } else {
     constexpr char kChromeSettingsPWAURL[] = "chrome://settings/pwa.html";
-    infos[SystemAppType::SETTINGS] = {GURL(kChromeSettingsPWAURL),
-                                      app_list::kInternalAppIdSettings};
+    infos[SystemAppType::SETTINGS].install_url = GURL(kChromeSettingsPWAURL);
+    infos[SystemAppType::SETTINGS].uninstall_and_replace = {
+        app_list::kInternalAppIdSettings};
   }
   if (SystemWebAppManager::IsAppEnabled(SystemAppType::TERMINAL)) {
     constexpr char kChromeTerminalPWAURL[] = "chrome://terminal/pwa.html";
-    infos[SystemAppType::TERMINAL] = {GURL(kChromeTerminalPWAURL)};
+    infos[SystemAppType::TERMINAL].install_url = GURL(kChromeTerminalPWAURL);
   }
 #endif  // OS_CHROMEOS
 
@@ -78,11 +84,21 @@ ExternalInstallOptions CreateInstallOptionsForSystemApp(
   install_options.add_to_quick_launch_bar = false;
   install_options.bypass_service_worker_check = true;
   install_options.force_reinstall = force_update;
+  install_options.uninstall_and_replace = info.uninstall_and_replace;
   return install_options;
 }
 
 }  // namespace
 
+SystemAppInfo::SystemAppInfo() = default;
+
+SystemAppInfo::SystemAppInfo(const GURL& install_url)
+    : install_url(install_url) {}
+
+SystemAppInfo::SystemAppInfo(const SystemAppInfo& other) = default;
+
+SystemAppInfo::~SystemAppInfo() = default;
+
 // static
 const char SystemWebAppManager::kInstallResultHistogramName[];
 
@@ -136,20 +152,6 @@ void SystemWebAppManager::SetSubsystems(PendingAppManager* pending_app_manager,
 }
 
 void SystemWebAppManager::Start() {
-  std::map<AppId, GURL> installed_apps = registrar_->GetExternallyInstalledApps(
-      ExternalInstallSource::kSystemInstalled);
-
-  std::set<SystemAppType> installed_app_types;
-  for (const auto& type_and_info : system_app_infos_) {
-    const GURL& install_url = type_and_info.second.install_url;
-    if (std::find_if(installed_apps.begin(), installed_apps.end(),
-                     [install_url](const std::pair<AppId, GURL> id_and_url) {
-                       return id_and_url.second == install_url;
-                     }) != installed_apps.end()) {
-      installed_app_types.insert(type_and_info.first);
-    }
-  }
-
   std::vector<ExternalInstallOptions> install_options_list;
   if (IsEnabled()) {
     // Skipping this will uninstall all System Apps currently installed.
@@ -162,7 +164,7 @@ void SystemWebAppManager::Start() {
   pending_app_manager_->SynchronizeInstalledApps(
       std::move(install_options_list), ExternalInstallSource::kSystemInstalled,
       base::BindOnce(&SystemWebAppManager::OnAppsSynchronized,
-                     weak_ptr_factory_.GetWeakPtr(), installed_app_types));
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SystemWebAppManager::InstallSystemAppsForTesting() {
@@ -216,7 +218,6 @@ const base::Version& SystemWebAppManager::CurrentVersion() const {
 }
 
 void SystemWebAppManager::OnAppsSynchronized(
-    std::set<SystemAppType> already_installed,
     std::map<GURL, InstallResultCode> install_results,
     std::map<GURL, bool> uninstall_results) {
   if (IsEnabled()) {
@@ -227,8 +228,6 @@ void SystemWebAppManager::OnAppsSynchronized(
   RecordExternalAppInstallResultCode(kInstallResultHistogramName,
                                      install_results);
 
-  MigrateSystemWebApps(already_installed);
-
   // May be called more than once in tests.
   if (!on_apps_synchronized_->is_signaled())
     on_apps_synchronized_->Signal();
@@ -246,29 +245,4 @@ bool SystemWebAppManager::NeedsUpdate() const {
          last_update_version != CurrentVersion();
 }
 
-void SystemWebAppManager::MigrateSystemWebApps(
-    std::set<SystemAppType> already_installed) {
-  DCHECK(ui_manager_);
-
-  for (const auto& type_and_info : system_app_infos_) {
-    // Migrate if a migration source is specified and the app has been newly
-    // installed.
-    if (!type_and_info.second.migration_source.empty() &&
-        !base::Contains(already_installed, type_and_info.first)) {
-      base::Optional<AppId> system_app_id =
-          GetAppIdForSystemApp(type_and_info.first);
-      // TODO(crbug.com/977466): Replace with a DCHECK once we understand why
-      // this is happening.
-      if (!system_app_id) {
-        LOG(ERROR) << "System App Type "
-                   << static_cast<int>(type_and_info.first)
-                   << " could not be found when running migration.";
-        continue;
-      }
-      ui_manager_->MigrateOSAttributes(type_and_info.second.migration_source,
-                                       *system_app_id);
-    }
-  }
-}
-
 }  // namespace web_app

chrome/browser/web_applications/system_web_app_manager.h

@@ -44,12 +44,17 @@ enum class SystemAppType {
 
 // The configuration options for a System App.
 struct SystemAppInfo {
+  SystemAppInfo();
+  explicit SystemAppInfo(const GURL& install_url);
+  SystemAppInfo(const SystemAppInfo& other);
+  ~SystemAppInfo();
+
   // The URL that the System App will be installed from.
   GURL install_url;
 
-  // If specified, the app with AppId |migration_source| will have its data
+  // If specified, the apps in |uninstall_and_replace| will have their data
   // migrated to this System App.
-  AppId migration_source;
+  std::vector<AppId> uninstall_and_replace;
 };
 
 // Installs, uninstalls, and updates System Web Apps.
@@ -112,15 +117,10 @@ class SystemWebAppManager {
   virtual const base::Version& CurrentVersion() const;
 
  private:
-  void OnAppsSynchronized(std::set<SystemAppType> already_installed,
-                          std::map<GURL, InstallResultCode> install_results,
+  void OnAppsSynchronized(std::map<GURL, InstallResultCode> install_results,
                           std::map<GURL, bool> uninstall_results);
   bool NeedsUpdate() const;
 
-  // TODO(calamity): Move migration into the install task once the install task
-  // is able to distinguish between an update install and a fresh install.
-  void MigrateSystemWebApps(std::set<SystemAppType> already_installed);
-
   std::unique_ptr<base::OneShotEvent> on_apps_synchronized_;
 
   UpdatePolicy update_policy_;

chrome/browser/web_applications/test/test_web_app_ui_manager.cc

@@ -22,6 +22,11 @@ void TestWebAppUiManager::SetNumWindowsForApp(const AppId& app_id,
   app_id_to_num_windows_map_[app_id] = num_windows_for_app;
 }
 
+bool TestWebAppUiManager::DidUninstallAndReplace(const AppId& from_app,
+                                                 const AppId& to_app) {
+  return uninstall_and_replace_map_[from_app] == to_app;
+}
+
 WebAppUiManagerImpl* TestWebAppUiManager::AsImpl() {
   return nullptr;
 }
@@ -42,8 +47,13 @@ void TestWebAppUiManager::NotifyOnAllAppWindowsClosed(
                      }));
 }
 
-void TestWebAppUiManager::MigrateOSAttributes(const AppId& from,
-                                              const AppId& to) {}
+void TestWebAppUiManager::UninstallAndReplace(
+    const std::vector<AppId>& from_apps,
+    const AppId& to_app) {
+  for (const AppId& from_app : from_apps) {
+    uninstall_and_replace_map_[from_app] = to_app;
+  }
+}
 
 bool TestWebAppUiManager::CanAddAppToQuickLaunchBar() const {
   return false;

chrome/browser/web_applications/test/test_web_app_ui_manager.h

@@ -18,13 +18,15 @@ class TestWebAppUiManager : public WebAppUiManager {
   ~TestWebAppUiManager() override;
 
   void SetNumWindowsForApp(const AppId& app_id, size_t num_windows_for_app);
+  bool DidUninstallAndReplace(const AppId& from_app, const AppId& to_app);
 
   // WebAppUiManager:
   WebAppUiManagerImpl* AsImpl() override;
   size_t GetNumWindowsForApp(const AppId& app_id) override;
   void NotifyOnAllAppWindowsClosed(const AppId& app_id,
                                    base::OnceClosure callback) override;
-  void MigrateOSAttributes(const AppId& from, const AppId& to) override;
+  void UninstallAndReplace(const std::vector<AppId>& from_apps,
+                           const AppId& to_app) override;
   bool CanAddAppToQuickLaunchBar() const override;
   void AddAppToQuickLaunchBar(const AppId& app_id) override;
   bool IsInAppWindow(content::WebContents* web_contents) const override;
@@ -36,6 +38,7 @@ class TestWebAppUiManager : public WebAppUiManager {
 
  private:
   std::map<AppId, size_t> app_id_to_num_windows_map_;
+  std::map<AppId, AppId> uninstall_and_replace_map_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWebAppUiManager);
 };