DPWA: fix default apps migration rollback behavior

MarkAppAsMigratedToWebApp that are successfully installed by
ExternalWebAppManager and have default apps to replace, even if the apps
to replace are not originally installed, so that if the migration flag
is on from the start, and rolled back, the apps will be re-installed
by default_apps.

(cherry picked from commit 354b9f4)

Bug: 1182928
Change-Id: Ifb0a41749c25b200a351cc32ddb7668abe4a7dc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2729736
Commit-Queue: Phillis Tang <phillis@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#859607}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2739309
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Auto-Submit: Phillis Tang <phillis@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/4430@{#164}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}

chrome/browser/extensions/default_apps_browsertest.cc

@@ -230,6 +230,25 @@ class DefaultAppsMigrationEnabledBrowserTest
   bool ShouldEnableWebAppMigration() override { return true; }
 };
 
+class DefaultAppsMigrationEnabledThenRolledBackBrowserTest
+    : public DefaultAppsMigrationBrowserTest {
+ public:
+  bool ShouldEnableWebAppMigration() override {
+    // Simulate the switch going back and forth between states.
+    // Step 0 (pre=1): Enabled (web app is installed).
+    // Step 1 (pre=0): Disabled (extension app is re-installed).
+    switch (GetTestPreCount()) {
+      case 1:
+        return true;
+      case 0:
+        return false;
+      default:
+        NOTREACHED();
+        return false;
+    }
+  }
+};
+
 // Default apps are handled differently on ChromeOS.
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
 
@@ -341,7 +360,6 @@ IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationBrowserTest,
 IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationBrowserTest,
                        TestExtensionWasAlreadyUninstalled) {
   // Web app should stay uninstalled on second launch.
-  TestExtensionRegistryObserver observer(registry(), kDefaultInstalledId);
   WaitForSystemReady();
   EXPECT_FALSE(IsWebAppCurrentlyInstalled());
 
@@ -351,9 +369,8 @@ IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationBrowserTest,
 IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationEnabledBrowserTest,
                        PRE_TestAppInstalled) {
   // Migration feature enabled on first launch. Web app should be installed
-  TestExtensionRegistryObserver observer(registry(), kDefaultInstalledId);
   WaitForSystemReady();
-  EXPECT_FALSE(WasMigratedToWebApp());
+  EXPECT_TRUE(WasMigratedToWebApp());
   EXPECT_TRUE(WasWebAppInstalledInThisRun());
   EXPECT_TRUE(IsWebAppCurrentlyInstalled());
 
@@ -363,12 +380,31 @@ IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationEnabledBrowserTest,
 IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationEnabledBrowserTest,
                        TestAppInstalled) {
   // Web app should stay installed on second launch.
-  TestExtensionRegistryObserver observer(registry(), kDefaultInstalledId);
   WaitForSystemReady();
   EXPECT_TRUE(IsWebAppCurrentlyInstalled());
 
   EXPECT_FALSE(registry()->enabled_extensions().GetByID(kDefaultInstalledId));
 }
+
+IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationEnabledThenRolledBackBrowserTest,
+                       PRE_TestAppInstalled) {
+  // Migration feature enabled on first launch. Web app should be installed
+  WaitForSystemReady();
+  EXPECT_TRUE(WasMigratedToWebApp());
+  EXPECT_TRUE(WasWebAppInstalledInThisRun());
+  EXPECT_TRUE(IsWebAppCurrentlyInstalled());
+
+  EXPECT_FALSE(registry()->enabled_extensions().GetByID(kDefaultInstalledId));
+}
+
+IN_PROC_BROWSER_TEST_F(DefaultAppsMigrationEnabledThenRolledBackBrowserTest,
+                       TestAppInstalled) {
+  // Extension app should be installed on second launch.
+  WaitForSystemReady();
+  EXPECT_FALSE(IsWebAppCurrentlyInstalled());
+
+  EXPECT_TRUE(registry()->enabled_extensions().GetByID(kDefaultInstalledId));
+}
 #endif  // !BUILDFLAG(IS_CHROMEOS_ASH)
 
 }  // namespace extensions

chrome/browser/web_applications/external_web_app_manager.cc

@@ -497,10 +497,16 @@ void ExternalWebAppManager::OnExternalWebAppsSynchronized(
                               url_and_result.second.code);
     if (url_and_result.second.did_uninstall_and_replace) {
       ++uninstall_and_replace_count;
+    }
+    // We mark the app as migrated to a web app as long as the installation
+    // was successful, even if the previous app was not installed. This ensures
+    // we properly re-install apps if the migration feature is rolled back.
+    if (IsSuccess(url_and_result.second.code)) {
       auto iter = desired_uninstalls.find(url_and_result.first);
-      DCHECK(iter != desired_uninstalls.end());
-      for (const auto& uninstalled_id : iter->second) {
-        MarkAppAsMigratedToWebApp(profile_, uninstalled_id, true);
+      if (iter != desired_uninstalls.end()) {
+        for (const auto& uninstalled_id : iter->second) {
+          MarkAppAsMigratedToWebApp(profile_, uninstalled_id, true);
+        }
       }
     }
   }