Revert "Migrate pinned icons in ImplicitApp subdirs."

This reverts commit 4105186f.

Reason for revert: <crbug.com/999467>

Original change's description:
> Migrate pinned icons in ImplicitApp subdirs.
>
> Chrome migrates pinned icons in UserPinned/Taskbar dir, but not the
> subdirectories of UserPinned/ImplicitAppShortcuts. It's unclear to me why
> Windows puts some pinned icons in the ImplicitAppShortcuts subdirectories,
> but we should handle them.
>
> Bug: 980239
> Change-Id: Ia41c3853b2a4bb78781a39733f388915f58c02eb
> TBR:
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1710722
> Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#681035}

TBR=msarda@chromium.org,grt@chromium.org,davidbienvenu@chromium.org

Bug: 980239
Change-Id: I2b19e825ccd0f1bd2975678d9acb1ad7ff90cc6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787560Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693898}

chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc

@@ -263,14 +263,14 @@ class ProfileShortcutManagerTest : public testing::Test {
   // Posts a task to call ShellUtil::CreateOrUpdateShortcut on the COM thread.
   void PostCreateOrUpdateShortcut(
       const base::Location& location,
-      ShellUtil::ShortcutLocation shortcut_location,
       const ShellUtil::ShortcutProperties& properties) {
     base::PostTaskAndReplyWithResult(
         base::CreateCOMSTATaskRunner({base::ThreadPool(), base::MayBlock()})
             .get(),
         location,
-        base::Bind(&ShellUtil::CreateOrUpdateShortcut, shortcut_location,
-                   properties, ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS),
+        base::Bind(&ShellUtil::CreateOrUpdateShortcut,
+                   ShellUtil::SHORTCUT_LOCATION_DESKTOP, properties,
+                   ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS),
         base::Bind([](bool succeeded) { EXPECT_TRUE(succeeded); }));
     task_environment_.RunUntilIdle();
   }
@@ -287,8 +287,7 @@ class ProfileShortcutManagerTest : public testing::Test {
     ShellUtil::ShortcutProperties properties(ShellUtil::CURRENT_USER);
     ShellUtil::AddDefaultShortcutProperties(GetExePath(), &properties);
     properties.set_shortcut_name(shortcut_name);
-    PostCreateOrUpdateShortcut(location, ShellUtil::SHORTCUT_LOCATION_DESKTOP,
-                               properties);
+    PostCreateOrUpdateShortcut(location, properties);
     EXPECT_TRUE(base::PathExists(shortcut_path)) << location.ToString();
 
     return shortcut_path;
@@ -298,8 +297,7 @@ class ProfileShortcutManagerTest : public testing::Test {
       const base::Location& location) {
     ShellUtil::ShortcutProperties properties(ShellUtil::SYSTEM_LEVEL);
     ShellUtil::AddDefaultShortcutProperties(GetExePath(), &properties);
-    PostCreateOrUpdateShortcut(location, ShellUtil::SHORTCUT_LOCATION_DESKTOP,
-                               properties);
+    PostCreateOrUpdateShortcut(location, properties);
     const base::FilePath system_level_shortcut_path =
         GetSystemShortcutsDirectory().Append(InstallUtil::GetShortcutName() +
                                              installer::kLnkExt);

chrome/browser/shell_integration_win.cc

@@ -492,16 +492,6 @@ void IsPinnedToTaskbarHelper::OnIsPinnedToTaskbarResult(
   delete this;
 }
 
-void MigrateChromeAndChromeProxyShortcuts(
-    const base::FilePath& chrome_exe,
-    const base::FilePath& chrome_proxy_path,
-    const base::FilePath& shortcut_path) {
-  win::MigrateShortcutsInPathInternal(chrome_exe, shortcut_path);
-
-  // Migrate any pinned PWA shortcuts in taskbar directory.
-  win::MigrateShortcutsInPathInternal(chrome_proxy_path, shortcut_path);
-}
-
 }  // namespace
 
 bool SetAsDefaultBrowser() {
@@ -710,44 +700,28 @@ void MigrateTaskbarPins() {
   // This needs to happen (e.g. so that the appid is fixed and the
   // run-time Chrome icon is merged with the taskbar shortcut), but it is not an
   // urgent task.
-  base::FilePath taskbar_path;
-  if (!base::PathService::Get(base::DIR_TASKBAR_PINS, &taskbar_path)) {
-    NOTREACHED();
-    return;
-  }
-
-  // Migrate any pinned shortcuts in ImplicitApps sub-directories.
-  base::FilePath implicit_apps_path;
-  if (!base::PathService::Get(base::DIR_IMPLICIT_APP_SHORTCUTS,
-                              &implicit_apps_path)) {
+  base::FilePath pins_path;
+  if (!base::PathService::Get(base::DIR_TASKBAR_PINS, &pins_path)) {
     NOTREACHED();
     return;
   }
 
   base::CreateCOMSTATaskRunner(
       {base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT})
-      ->PostTask(FROM_HERE, base::BindOnce(&MigrateTaskbarPinsCallback,
-                                           taskbar_path, implicit_apps_path));
+      ->PostTask(FROM_HERE,
+                 base::BindOnce(&MigrateTaskbarPinsCallback, pins_path));
 }
 
-void MigrateTaskbarPinsCallback(const base::FilePath& taskbar_path,
-                                const base::FilePath& implicit_apps_path) {
+void MigrateTaskbarPinsCallback(const base::FilePath& pins_path) {
   // Get full path of chrome.
   base::FilePath chrome_exe;
   if (!base::PathService::Get(base::FILE_EXE, &chrome_exe))
     return;
-  base::FilePath chrome_proxy_path(web_app::GetChromeProxyPath());
-
-  MigrateChromeAndChromeProxyShortcuts(chrome_exe, chrome_proxy_path,
-                                       taskbar_path);
-  base::FileEnumerator directory_enum(implicit_apps_path, /*recursive=*/false,
-                                      base::FileEnumerator::DIRECTORIES);
-  for (base::FilePath implicit_app_sub_directory = directory_enum.Next();
-       !implicit_app_sub_directory.empty();
-       implicit_app_sub_directory = directory_enum.Next()) {
-    MigrateChromeAndChromeProxyShortcuts(chrome_exe, chrome_proxy_path,
-                                         implicit_app_sub_directory);
-  }
+
+  win::MigrateShortcutsInPathInternal(chrome_exe, pins_path);
+
+  // Migrate any pinned PWA shortcuts.
+  win::MigrateShortcutsInPathInternal(web_app::GetChromeProxyPath(), pins_path);
 }
 
 void GetIsPinnedToTaskbarState(

chrome/browser/shell_integration_win.h

@@ -74,8 +74,7 @@ void GetIsPinnedToTaskbarState(
 void MigrateTaskbarPins();
 
 // Callback for MigrateTaskbarPins(). Exposed for testing.
-void MigrateTaskbarPinsCallback(const base::FilePath& pins_path,
-                                const base::FilePath& implicit_apps_path);
+void MigrateTaskbarPinsCallback(const base::FilePath& pins_path);
 
 // Migrates all shortcuts in |path| which point to |chrome_exe| such that they
 // have the appropriate AppUserModelId. Also clears the legacy dual_mode

chrome/browser/shell_integration_win_unittest.cc

@@ -43,8 +43,7 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
 
   void SetUp() override {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    ASSERT_TRUE(
-        temp_dir_sub_dir_.CreateUniqueTempDirUnderPath(temp_dir_.GetPath()));
+
     // A path to a random target.
     base::CreateTemporaryFileInDir(temp_dir_.GetPath(), &other_target_);
 
@@ -84,10 +83,9 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
   // |shortcut_properties| after copying it to an internal structure for later
   // verification.
   void AddTestShortcutAndResetProperties(
-      const base::FilePath& shortcut_dir,
       base::win::ShortcutProperties* shortcut_properties) {
     ShortcutTestObject shortcut_test_object;
-    base::FilePath shortcut_path = shortcut_dir.Append(
+    base::FilePath shortcut_path = temp_dir_.GetPath().Append(
         L"Shortcut " + base::NumberToString16(shortcuts_.size()) +
         installer::kLnkExt);
     shortcut_test_object.path = shortcut_path;
@@ -107,22 +105,22 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     // Shortcut 0 doesn't point to chrome.exe and thus should never be migrated.
     temp_properties.set_target(other_target_);
     temp_properties.set_app_id(L"Dumbo");
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 1 points to chrome.exe and thus should be migrated.
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(L"Dumbo");
     temp_properties.set_dual_mode(false);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 2 points to chrome.exe, but already has the right appid and thus
     // should only be migrated if dual_mode is desired.
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(default_profile_chrome_app_id_);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 3 is like shortcut 1, but it's appid is a prefix of the expected
     // appid instead of being totally different.
@@ -130,8 +128,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     chrome_app_id_is_prefix.push_back(L'1');
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(chrome_app_id_is_prefix);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 4 is like shortcut 1, but it's appid is of the same size as the
     // expected appid.
@@ -139,14 +137,14 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
         default_profile_chrome_app_id_.size(), L'1');
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(same_size_as_chrome_app_id);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 5 doesn't have an app_id, nor is dual_mode even set; they should
     // be set as expected upon migration.
     temp_properties.set_target(chrome_exe_);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 6 has a non-default profile directory and so should get a non-
     // default app id.
@@ -154,8 +152,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     temp_properties.set_app_id(L"Dumbo");
     temp_properties.set_arguments(
         L"--profile-directory=" + non_default_profile_);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 7 has a non-default user data directory and so should get a non-
     // default app id.
@@ -163,8 +161,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     temp_properties.set_app_id(L"Dumbo");
     temp_properties.set_arguments(
         L"--user-data-dir=\"" + non_default_user_data_dir_.value() + L"\"");
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 8 has a non-default user data directory as well as a non-default
     // profile directory and so should get a non-default app id.
@@ -173,8 +171,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     temp_properties.set_arguments(
         L"--user-data-dir=\"" + non_default_user_data_dir_.value() + L"\" " +
         L"--profile-directory=" + non_default_profile_);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 9 is a shortcut to an app and should get an app id for that app
     // rather than the chrome app id.
@@ -182,8 +180,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     temp_properties.set_app_id(L"Dumbo");
     temp_properties.set_arguments(
         L"--app-id=" + extension_id_);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 10 is a shortcut to an app with a non-default profile and should
     // get an app id for that app with a non-default app id rather than the
@@ -193,8 +191,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     temp_properties.set_arguments(
         L"--app-id=" + extension_id_ +
         L" --profile-directory=" + non_default_profile_);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 11 points to chrome.exe, has the chrome appid, and has
     // dual_mode set and thus should be migrated to the
@@ -202,24 +200,21 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(chrome_app_id_);
     temp_properties.set_dual_mode(true);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
 
     // Shortcut 12 is similar to 11 but with dual_mode explicitly set to false.
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(chrome_app_id_);
     temp_properties.set_dual_mode(false);
-    ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-        temp_dir_.GetPath(), &temp_properties));
+    ASSERT_NO_FATAL_FAILURE(
+        AddTestShortcutAndResetProperties(&temp_properties));
   }
 
   base::win::ScopedCOMInitializer com_initializer_;
 
   base::ScopedTempDir temp_dir_;
 
-  // Used to test migration of shortcuts in ImplicitApps sub-directories.
-  base::ScopedTempDir temp_dir_sub_dir_;
-
   // Test shortcuts.
   std::vector<ShortcutTestObject> shortcuts_;
 
@@ -305,9 +300,8 @@ TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualModeAndAdjustAppIds) {
             MigrateShortcutsInPathInternal(chrome_exe_, temp_dir_.GetPath()));
 }
 
-// Test that chrome_proxy.exe shortcuts (PWA) have their app_id migrated
-// to include the default profile name. This tests both shortcuts in the
-// DIR_TASKBAR_PINS and sub-directories of DIR_IMPLICIT_APP_SHORTCUTS.
+// Test that a chrome_proxy.exe shortcut (PWA) has its app_id migrated
+// to include the default profile name.
 TEST_F(ShellIntegrationWinMigrateShortcutTest, MigrateChromeProxyTest) {
   // Create shortcut to chrome_proxy_exe in executable directory,
   // using the default profile, with the AppModelId not containing the
@@ -316,22 +310,12 @@ TEST_F(ShellIntegrationWinMigrateShortcutTest, MigrateChromeProxyTest) {
   base::win::ShortcutProperties temp_properties;
   temp_properties.set_target(web_app::GetChromeProxyPath());
   temp_properties.set_app_id(L"Dumbo");
-  ASSERT_NO_FATAL_FAILURE(
-      AddTestShortcutAndResetProperties(temp_dir_.GetPath(), &temp_properties));
-  temp_properties.set_target(web_app::GetChromeProxyPath());
-  temp_properties.set_app_id(L"Dumbo2");
-  ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
-      temp_dir_sub_dir_.GetPath(), &temp_properties));
+  ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(&temp_properties));
 
-  MigrateTaskbarPinsCallback(temp_dir_.GetPath(), temp_dir_.GetPath());
-  // Verify that the migrated shortcut in temp_dir_ now contains the default
-  // profile name.
+  MigrateTaskbarPinsCallback(temp_dir_.GetPath());
+  // Verify that the migrated shortcut now contains the default profile name.
   shortcuts_[0].properties.set_app_id(default_profile_chrome_app_id_);
   base::win::ValidateShortcut(shortcuts_[0].path, shortcuts_[0].properties);
-  // Verify that the migrated shortcut in temp_dir_sub now contains the default
-  // profile name.
-  shortcuts_[1].properties.set_app_id(default_profile_chrome_app_id_);
-  base::win::ValidateShortcut(shortcuts_[1].path, shortcuts_[1].properties);
 }
 
 TEST(ShellIntegrationWinTest, GetAppModelIdForProfileTest) {