Move pinned taskbar migration to PostBrowserStart().

Currently when we do taskbar migration, we do it for every profile
launched. After the first profile, this should be a noop, but we still
iterate over the shortcuts. Moving migration to
ChromeBrowserMainPartsWin::PostBrowserStart() will make us do it
only once.

Bug: 1132578
Change-Id: I07cb8ea74145407f978efcd6067523d7d79ca1df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433427
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813774}

chrome/browser/chrome_browser_main_win.cc

@@ -52,6 +52,7 @@
 #include "chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_win.h"
 #include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h"
 #include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_util_win.h"
+#include "chrome/browser/shell_integration_win.h"
 #include "chrome/browser/ui/simple_message_box.h"
 #include "chrome/browser/ui/uninstall_browser_prompt.h"
 #include "chrome/browser/web_applications/chrome_pwa_launcher/last_browser_file_util.h"
@@ -75,6 +76,7 @@
 #include "chrome/common/conflicts/module_watcher_win.h"
 #include "chrome/common/crash_keys.h"
 #include "chrome/common/env_vars.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/install_static/install_details.h"
@@ -87,7 +89,9 @@
 #include "components/crash/core/app/dump_hung_process_with_ptype.h"
 #include "components/crash/core/common/crash_key.h"
 #include "components/os_crypt/os_crypt.h"
+#include "components/prefs/pref_service.h"
 #include "components/version_info/channel.h"
+#include "components/version_info/version_info.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
@@ -488,6 +492,28 @@ void UpdatePwaLaunchersForProfile(const base::FilePath& profile_dir) {
                      std::move(pwa_launcher_paths)));
 }
 
+void MigratePinnedTaskBarShortcutsIfNeeded() {
+  // Update this number when users should go through a taskbar shortcut
+  // migration again. The last reason to do this was crrev.com/798174. @
+  // 86.0.4231.0.
+  //
+  // Note: If shortcut updates need to be done once after a future OS upgrade,
+  // that should be done by re-versioning Active Setup (see //chrome/installer
+  // and https://crbug.com/577697 for details).
+  const base::Version kLastVersionNeedingMigration({86, 0, 4231, 0});
+
+  PrefService* local_state = g_browser_process->local_state();
+  if (local_state) {
+    const base::Version last_version_migrated(
+        local_state->GetString(prefs::kShortcutMigrationVersion));
+    if (!last_version_migrated.IsValid() ||
+        last_version_migrated < kLastVersionNeedingMigration) {
+      shell_integration::win::MigrateTaskbarPins(base::BindOnce(
+          &PrefService::SetString, base::Unretained(local_state),
+          prefs::kShortcutMigrationVersion, version_info::GetVersionNumber()));
+    }
+  }
+}
 // This error message is not localized because we failed to load the
 // localization data files.
 const char kMissingLocaleDataTitle[] = "Missing File Error";
@@ -707,6 +733,12 @@ void ChromeBrowserMainPartsWin::PostBrowserStart() {
       FROM_HERE, {base::TaskPriority::BEST_EFFORT, base::MayBlock()},
       base::BindOnce(&web_app::RecordPwaLauncherResult));
 
+  // Possibly migrate pinned taskbar shortcuts.
+  content::GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT,
+                                  base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})
+      ->PostTask(FROM_HERE,
+                 base::BindOnce(&MigratePinnedTaskBarShortcutsIfNeeded));
+
   base::ImportantFileWriterCleaner::GetInstance().Start();
 }
 

chrome/browser/chrome_browser_main_win_browsertest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chrome_browser_main_win.h"
+
+#include "chrome/browser/browser_process.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/prefs/pref_service.h"
+#include "components/version_info/version_info.h"
+#include "content/public/test/browser_test.h"
+#include "content/public/test/test_utils.h"
+
+using ChromeBrowserMainWinTest = InProcessBrowserTest;
+
+IN_PROC_BROWSER_TEST_F(ChromeBrowserMainWinTest, PRE_ShortcutsAreMigratedOnce) {
+  // Wait for all startup tasks to run.
+  content::RunAllTasksUntilIdle();
+
+  // Confirm that shortcuts were migrated.
+  const std::string last_version_migrated =
+      g_browser_process->local_state()->GetString(
+          prefs::kShortcutMigrationVersion);
+  EXPECT_EQ(last_version_migrated, version_info::GetVersionNumber());
+
+  // Set the version back as far as kLastVersionNeedingMigration and ensure it's
+  // not migrated again.
+  g_browser_process->local_state()->SetString(prefs::kShortcutMigrationVersion,
+                                              "86.0.4231.0");
+}
+
+IN_PROC_BROWSER_TEST_F(ChromeBrowserMainWinTest, ShortcutsAreMigratedOnce) {
+  content::RunAllTasksUntilIdle();
+
+  // Confirm that shortcuts weren't migrated when marked as having last been
+  // migrated in kLastVersionNeedingMigration+.
+  const std::string last_version_migrated =
+      g_browser_process->local_state()->GetString(
+          prefs::kShortcutMigrationVersion);
+  EXPECT_EQ(last_version_migrated, "86.0.4231.0");
+}

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

@@ -67,7 +67,6 @@
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_service.h"
-#include "components/version_info/version_info.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test.h"
@@ -1189,34 +1188,6 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
 
 #endif  // !defined(OS_CHROMEOS)
 
-#if defined(OS_WIN)
-IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
-                       PRE_ShortcutsAreMigratedOnce) {
-  content::RunAllTasksUntilIdle();
-
-  // Confirm that shortcuts were migrated.
-  const std::string last_version_migrated =
-      g_browser_process->local_state()->GetString(
-          prefs::kShortcutMigrationVersion);
-  EXPECT_EQ(last_version_migrated, version_info::GetVersionNumber());
-
-  // Set the version back as far as kLastVersionNeedingMigration and ensure it's
-  // not migrated again.
-  g_browser_process->local_state()->SetString(prefs::kShortcutMigrationVersion,
-                                              "86.0.4231.0");
-}
-IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, ShortcutsAreMigratedOnce) {
-  content::RunAllTasksUntilIdle();
-
-  // Confirm that shortcuts weren't migrated when marked as having last been
-  // migrated in kLastVersionNeedingMigration+.
-  const std::string last_version_migrated =
-      g_browser_process->local_state()->GetString(
-          prefs::kShortcutMigrationVersion);
-  EXPECT_EQ(last_version_migrated, "86.0.4231.0");
-}
-#endif  // defined(OS_WIN)
-
 class StartupBrowserCreatorExtensionsCheckupExperimentTest
     : public extensions::ExtensionBrowserTest {
  public:

chrome/browser/ui/startup/startup_browser_creator_impl.cc

@@ -15,7 +15,6 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/values.h"
-#include "base/version.h"
 #include "build/branding_buildflags.h"
 #include "build/build_config.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
@@ -59,7 +58,6 @@
 #include "chrome/common/url_constants.h"
 #include "components/prefs/pref_service.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
-#include "components/version_info/version_info.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/storage_partition.h"
@@ -79,7 +77,6 @@
 #include "chrome/browser/win/conflicts/incompatible_applications_updater.h"
 #endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
 #include "chrome/browser/notifications/notification_platform_bridge_win.h"
-#include "chrome/browser/shell_integration_win.h"
 #include "chrome/browser/ui/startup/credential_provider_signin_dialog_win.h"
 #include "chrome/credential_provider/common/gcp_strings.h"
 #endif  // defined(OS_WIN)
@@ -297,32 +294,6 @@ bool StartupBrowserCreatorImpl::Launch(
       MaybeToggleFullscreen(browser);
   }
 
-#if defined(OS_WIN)
-  if (process_startup) {
-    // Update this number when users should go through a taskbar shortcut
-    // migration again. The last reason to do this was crrev.com/2285399 @
-    // 86.0.4231.0.
-    //
-    // Note: If shortcut updates need to be done once after a future OS upgrade,
-    // that should be done by re-versioning Active Setup (see //chrome/installer
-    // and http://crbug.com/577697 for details).
-    const base::Version kLastVersionNeedingMigration({86U, 0U, 4231U, 0U});
-
-    PrefService* local_state = g_browser_process->local_state();
-    if (local_state) {
-      const base::Version last_version_migrated(
-          local_state->GetString(prefs::kShortcutMigrationVersion));
-      if (!last_version_migrated.IsValid() ||
-          last_version_migrated < kLastVersionNeedingMigration) {
-        shell_integration::win::MigrateTaskbarPins(base::BindOnce(
-            &PrefService::SetString, base::Unretained(local_state),
-            prefs::kShortcutMigrationVersion,
-            version_info::GetVersionNumber()));
-      }
-    }
-  }
-#endif  // defined(OS_WIN)
-
   return true;
 }
 

chrome/test/BUILD.gn

@@ -2881,6 +2881,7 @@ if (!is_android) {
     }
     if (is_win) {
       sources += [
+        "../browser/chrome_browser_main_win_browsertest.cc",
         "../browser/printing/pdf_to_emf_converter_browsertest.cc",
         "../browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc",
         "../browser/ui/startup/credential_provider_signin_dialog_win_browsertest.cc",