Reduce MigrateChromiumShortcuts to only handling taskbar pins.

Start Menu and Desktop is already handled by the installer.

And the Start Menu pins folder doesn't exist after Win7 (and Chrome has
been doing this for long enough that all Win7 Start Menu pins have sure
been migrated by now -- and even if not I don't think there is a failure
mode as the app ids that matter at runtime are the taskbar one and the
chrome window's, it was probably always useless to migrate start menu
pins...).

BUG=577697
TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/1588733005

Cr-Commit-Position: refs/heads/master@{#369674}

4 files changed, 26 insertions, 100 deletions

diff --git a/chrome/browser/shell_integration.h b/chrome/browser/shell_integration.h
index 7073b88..f2c629a 100644
--- a/chrome/browser/shell_integration.h
+++ b/chrome/browser/shell_integration.h
@@ -155,20 +155,19 @@ class ShellIntegration {
   static base::string16 GetAppListAppModelIdForProfile(
       const base::FilePath& profile_path);
 
-  // Migrates existing chrome shortcuts by tagging them with correct app id.
+  // Migrates existing chrome taskbar pins by tagging them with correct app id.
   // see http://crbug.com/28104
-  static void MigrateChromiumShortcuts();
+  static void MigrateTaskbarPins();
 
   // Migrates all shortcuts in |path| which point to |chrome_exe| such that they
-  // have the appropriate AppUserModelId. Also clears the dual_mode property
-  // from shortcuts that previously had it if requested by |clear_dual_mode|.
+  // have the appropriate AppUserModelId. Also clears the legacy dual_mode
+  // property from shortcuts with the default chrome app id.
   // Returns the number of shortcuts migrated.
   // This method should not be called prior to Windows 7.
   // This method is only public for the sake of tests and shouldn't be called
   // externally otherwise.
   static int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,
-                                            const base::FilePath& path,
-                                            bool clear_dual_mode);
+                                            const base::FilePath& path);
 
   // Returns the path to the Start Menu shortcut for the given Chrome.
   static base::FilePath GetStartMenuShortcut(const base::FilePath& chrome_exe);
diff --git a/chrome/browser/shell_integration_win.cc b/chrome/browser/shell_integration_win.cc
index 246766f..c43b35e 100644
--- a/chrome/browser/shell_integration_win.cc
+++ b/chrome/browser/shell_integration_win.cc
@@ -154,7 +154,7 @@ base::string16 GetExpectedAppId(const base::CommandLine& command_line,
   return ShellIntegration::GetAppModelIdForProfile(app_name, profile_path);
 }
 
-void MigrateChromiumShortcutsCallback() {
+void MigrateTaskbarPinsCallback() {
   // This should run on the file thread.
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
@@ -163,42 +163,13 @@ void MigrateChromiumShortcutsCallback() {
   if (!PathService::Get(base::FILE_EXE, &chrome_exe))
     return;
 
-  // Locations to check for shortcuts migration.
-  static const struct {
-    int location_id;
-    const wchar_t* sub_dir;
-  } kLocations[] = {
-    {
-      base::DIR_TASKBAR_PINS,
-      NULL
-    }, {
-      base::DIR_USER_DESKTOP,
-      NULL
-    }, {
-      base::DIR_START_MENU,
-      NULL
-    }, {
-      base::DIR_APP_DATA,
-      L"Microsoft\\Internet Explorer\\Quick Launch\\User Pinned\\StartMenu"
-    }
-  };
-
-  for (size_t i = 0; i < arraysize(kLocations); ++i) {
-    base::FilePath path;
-    if (!PathService::Get(kLocations[i].location_id, &path)) {
-      NOTREACHED();
-      continue;
-    }
-
-    if (kLocations[i].sub_dir)
-      path = path.Append(kLocations[i].sub_dir);
-
-    // Clear |dual_mode| property from taskbar pins as those are user-level
-    // shortcuts which aren't handled by the installer.
-    bool clear_dual_mode = kLocations[i].location_id == base::DIR_TASKBAR_PINS;
-    ShellIntegration::MigrateShortcutsInPathInternal(chrome_exe, path,
-                                                     clear_dual_mode);
+  base::FilePath pins_path;
+  if (!PathService::Get(base::DIR_TASKBAR_PINS, &pins_path)) {
+    NOTREACHED();
+    return;
   }
+
+  ShellIntegration::MigrateShortcutsInPathInternal(chrome_exe, pins_path);
 }
 
 // Windows 8 introduced a new protocol->executable binding system which cannot
@@ -509,24 +480,23 @@ base::string16 ShellIntegration::GetAppListAppModelIdForProfile(
   return GetAppModelIdForProfile(GetAppListAppName(), profile_path);
 }
 
-void ShellIntegration::MigrateChromiumShortcuts() {
+void ShellIntegration::MigrateTaskbarPins() {
   if (base::win::GetVersion() < base::win::VERSION_WIN7)
     return;
 
   // This needs to happen eventually (e.g. so that the appid is fixed and the
   // run-time Chrome icon is merged with the taskbar shortcut), but this is not
   // urgent and shouldn't delay Chrome startup.
-  static const int64_t kMigrateChromiumShortcutsDelaySeconds = 15;
+  static const int64_t kMigrateTaskbarPinsDelaySeconds = 15;
   BrowserThread::PostDelayedTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&MigrateChromiumShortcutsCallback),
-      base::TimeDelta::FromSeconds(kMigrateChromiumShortcutsDelaySeconds));
+      base::Bind(&MigrateTaskbarPinsCallback),
+      base::TimeDelta::FromSeconds(kMigrateTaskbarPinsDelaySeconds));
 }
 
 int ShellIntegration::MigrateShortcutsInPathInternal(
     const base::FilePath& chrome_exe,
-    const base::FilePath& path,
-    bool clear_dual_mode) {
+    const base::FilePath& path) {
   DCHECK(base::win::GetVersion() >= base::win::VERSION_WIN7);
 
   // Enumerate all pinned shortcuts in the given path directly.
@@ -599,12 +569,13 @@ int ShellIntegration::MigrateShortcutsInPathInternal(
       }
     }
 
-    // Clear |dual_mode| property from any shortcuts that previously had it (as
-    // requested by caller).
+    // Clear dual_mode property from any shortcuts that previously had it (it
+    // was only ever installed on shortcuts with the
+    // |default_chromium_model_id|).
     BrowserDistribution* dist = BrowserDistribution::GetDistribution();
     base::string16 default_chromium_model_id(
         ShellUtil::GetBrowserModelId(dist, is_per_user_install));
-    if (clear_dual_mode && expected_app_id == default_chromium_model_id) {
+    if (expected_app_id == default_chromium_model_id) {
       propvariant.Reset();
       if (property_store->GetValue(PKEY_AppUserModel_IsDualMode,
                                    propvariant.Receive()) != S_OK) {
diff --git a/chrome/browser/shell_integration_win_unittest.cc b/chrome/browser/shell_integration_win_unittest.cc
index 65f38bb..0fc0975 100644
--- a/chrome/browser/shell_integration_win_unittest.cc
+++ b/chrome/browser/shell_integration_win_unittest.cc
@@ -268,57 +268,14 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
 
 }  // namespace
 
-// Test migration when not clearing dual mode.
-TEST_F(ShellIntegrationWinMigrateShortcutTest, DontClearDualMode) {
-  if (base::win::GetVersion() < base::win::VERSION_WIN7)
-    return;
-
-  EXPECT_EQ(9,
-            ShellIntegration::MigrateShortcutsInPathInternal(
-                chrome_exe_, temp_dir_.path(), false));
-
-  // Only shortcut 1, 3, 4, 5, 6, 7, 8, 9, and 10 should have been migrated.
-  shortcuts_[1].properties.set_app_id(chrome_app_id_);
-  shortcuts_[3].properties.set_app_id(chrome_app_id_);
-  shortcuts_[4].properties.set_app_id(chrome_app_id_);
-  shortcuts_[5].properties.set_app_id(chrome_app_id_);
-  shortcuts_[6].properties.set_app_id(non_default_profile_chrome_app_id_);
-  shortcuts_[7].properties.set_app_id(non_default_user_data_dir_chrome_app_id_);
-  shortcuts_[8].properties.set_app_id(
-      non_default_user_data_dir_and_profile_chrome_app_id_);
-  shortcuts_[9].properties.set_app_id(extension_app_id_);
-  shortcuts_[10].properties.set_app_id(non_default_profile_extension_app_id_);
-
-  // Explicitly set the dual_mode expectations on all shortcuts that didn't have
-  // it to ensure ValidateShortcut verifies it. Those that had it should remain
-  // unchanged per |clear_dual_mode| being false above.
-  for (size_t i = 0; i < shortcuts_.size(); ++i) {
-    if (!(shortcuts_[i].properties.options &
-          base::win::ShortcutProperties::PROPERTIES_DUAL_MODE)) {
-      shortcuts_[i].properties.set_dual_mode(false);
-    }
-  }
-
-  for (size_t i = 0; i < shortcuts_.size(); ++i) {
-    SCOPED_TRACE(i);
-    base::win::ValidateShortcut(shortcuts_[i].path, shortcuts_[i].properties);
-  }
-
-  // Make sure shortcuts are not re-migrated.
-  EXPECT_EQ(0,
-            ShellIntegration::MigrateShortcutsInPathInternal(
-                chrome_exe_, temp_dir_.path(), false));
-}
-
-// Test migration when also clearing dual mode.
-TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualMode) {
+TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualModeAndAdjustAppIds) {
   if (base::win::GetVersion() < base::win::VERSION_WIN7)
     return;
 
   // 9 shortcuts should have their app id updated below and shortcut 11 should
   // be migrated away from dual_mode for a total of 10 shortcuts migrated.
   EXPECT_EQ(10, ShellIntegration::MigrateShortcutsInPathInternal(
-                    chrome_exe_, temp_dir_.path(), true));
+                    chrome_exe_, temp_dir_.path()));
 
   // Shortcut 1, 3, 4, 5, 6, 7, 8, 9, and 10 should have had their app_id fixed.
   shortcuts_[1].properties.set_app_id(chrome_app_id_);
@@ -342,9 +299,8 @@ TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualMode) {
   }
 
   // Make sure shortcuts are not re-migrated.
-  EXPECT_EQ(0,
-            ShellIntegration::MigrateShortcutsInPathInternal(
-                chrome_exe_, temp_dir_.path(), false));
+  EXPECT_EQ(0, ShellIntegration::MigrateShortcutsInPathInternal(
+                   chrome_exe_, temp_dir_.path()));
 }
 
 TEST(ShellIntegrationWinTest, GetAppModelIdForProfileTest) {
diff --git a/chrome/browser/ui/startup/startup_browser_creator_impl.cc b/chrome/browser/ui/startup/startup_browser_creator_impl.cc
index d56a7ff..f59c681 100644
--- a/chrome/browser/ui/startup/startup_browser_creator_impl.cc
+++ b/chrome/browser/ui/startup/startup_browser_creator_impl.cc
@@ -386,7 +386,7 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
   // Active Setup versioning and on OS upgrades) instead of every startup.
   // http://crbug.com/577697
   if (process_startup)
-    ShellIntegration::MigrateChromiumShortcuts();
+    ShellIntegration::MigrateTaskbarPins();
 #endif  // defined(OS_WIN)
 
   return true;