Reland "Include profile name in AppUserModelId for default profile."

This reverts commit 8963402b.

Reason for revert: <reverting did not fix crbug.com/999467>

Original change's description:
> Revert "Include profile name in AppUserModelId for default profile."
> 
> This reverts commit 7f9c2bff.
> 
> Reason for revert: <crbug.com/999467>
> 
> Original change's description:
> > Include profile name in AppUserModelId for default profile.
> > 
> > Migrate pinned PWA shortcuts as well.
> > 
> > This makes pinned icons for the default profile on Windows have
> > the profile name in the AppUserModelId, which prevents weird behaviors
> > when other profiles are added/removed.
> > 
> > If shortcuts are migrated, shortcuts without a profile will be migrated
> > to point to the default profile.
> > 
> > Originally submitted as 1658991, which was reverted because of an issue
> > with pinned PWA shortcuts.
> > 
> > Bug: 593414, 328738
> > Change-Id: I68bc7083e233cf6dd3cfd56359245df59fd0e4a1
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1686222
> > Reviewed-by: Gabriel Charette <gab@chromium.org>
> > Reviewed-by: Alan Cutter <alancutter@chromium.org>
> > Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#677305}
> 
> TBR=gab@chromium.org,alancutter@chromium.org,davidbienvenu@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 593414, 328738
> Change-Id: I1414fe5915c0f8b78e7ae58f447274c42331f60c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787902
> Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
> Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#693955}

TBR=gab@chromium.org,alancutter@chromium.org,davidbienvenu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 593414, 328738
Change-Id: I1d1959632502114321847671361807be50dfcfb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1797698Reviewed-by: David Bienvenu <davidbienvenu@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695693}

chrome/browser/shell_integration_win.cc

@@ -43,6 +43,7 @@
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/components/web_app_shortcut_win.h"
 #include "chrome/browser/win/settings_app_monitor.h"
 #include "chrome/browser/win/util_win_service.h"
 #include "chrome/common/chrome_constants.h"
@@ -68,16 +69,6 @@ base::string16 GetProfileIdFromPath(const base::FilePath& profile_path) {
   if (profile_path.empty())
     return base::string16();
 
-  base::FilePath default_user_data_dir;
-  // Return empty string if profile_path is in default user data
-  // dir and is the default profile.
-  if (chrome::GetDefaultUserDataDirectory(&default_user_data_dir) &&
-      profile_path.DirName() == default_user_data_dir &&
-      profile_path.BaseName().value() ==
-          base::ASCIIToUTF16(chrome::kInitialProfile)) {
-    return base::string16();
-  }
-
   // Get joined basenames of user data dir and profile.
   base::string16 basenames = profile_path.DirName().BaseName().value() +
       L"." + profile_path.BaseName().value();
@@ -144,21 +135,6 @@ base::string16 GetExpectedAppId(const base::CommandLine& command_line,
   return win::GetAppModelIdForProfile(app_name, profile_path);
 }
 
-void MigrateTaskbarPinsCallback() {
-  // Get full path of chrome.
-  base::FilePath chrome_exe;
-  if (!base::PathService::Get(base::FILE_EXE, &chrome_exe))
-    return;
-
-  base::FilePath pins_path;
-  if (!base::PathService::Get(base::DIR_TASKBAR_PINS, &pins_path)) {
-    NOTREACHED();
-    return;
-  }
-
-  win::MigrateShortcutsInPathInternal(chrome_exe, pins_path);
-}
-
 // Windows treats a given scheme as an Internet scheme only if its registry
 // entry has a "URL Protocol" key. Check this, otherwise we allow ProgIDs to be
 // used as custom protocols which leads to security bugs.
@@ -724,9 +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 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));
+      ->PostTask(FROM_HERE,
+                 base::BindOnce(&MigrateTaskbarPinsCallback, pins_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;
+
+  win::MigrateShortcutsInPathInternal(chrome_exe, pins_path);
+
+  // Migrate any pinned PWA shortcuts.
+  win::MigrateShortcutsInPathInternal(web_app::GetChromeProxyPath(), pins_path);
 }
 
 void GetIsPinnedToTaskbarState(
@@ -785,6 +780,8 @@ int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,
     // |updated_properties|.
     base::win::ShortcutProperties updated_properties;
 
+    base::string16 current_app_id;
+
     // Validate the existing app id for the shortcut.
     Microsoft::WRL::ComPtr<IPropertyStore> property_store;
     propvariant.Reset();
@@ -802,7 +799,8 @@ int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,
             updated_properties.set_app_id(expected_app_id);
           break;
         case VT_LPWSTR:
-          if (expected_app_id != base::string16(propvariant.get().pwszVal))
+          current_app_id = base::string16(propvariant.get().pwszVal);
+          if (expected_app_id != current_app_id)
             updated_properties.set_app_id(expected_app_id);
           break;
         default:
@@ -816,7 +814,7 @@ int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,
     // |default_chromium_model_id|).
     base::string16 default_chromium_model_id(
         ShellUtil::GetBrowserModelId(is_per_user_install));
-    if (expected_app_id == default_chromium_model_id) {
+    if (current_app_id == default_chromium_model_id) {
       propvariant.Reset();
       if (property_store->GetValue(PKEY_AppUserModel_IsDualMode,
                                    propvariant.Receive()) != S_OK) {

chrome/browser/shell_integration_win.h

@@ -70,9 +70,12 @@ void GetIsPinnedToTaskbarState(
     const IsPinnedToTaskbarCallback& result_callback);
 
 // Migrates existing chrome taskbar pins by tagging them with correct app id.
-// see http://crbug.com/28104
+// see http://crbug.com/28104. Migrates taskbar pins via a task.
 void MigrateTaskbarPins();
 
+// Callback for MigrateTaskbarPins(). Exposed for testing.
+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
 // property from shortcuts with the default chrome app id.

chrome/browser/shell_integration_win_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/test/test_shortcut_win.h"
 #include "base/win/scoped_com_initializer.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/components/web_app_shortcut_win.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/install_static/install_util.h"
@@ -65,6 +66,9 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     non_default_user_data_dir_and_profile_chrome_app_id_ =
         GetChromiumModelIdForProfile(
             non_default_user_data_dir_.Append(non_default_profile_));
+    default_profile_ = L"Default";
+    default_profile_chrome_app_id_ = GetChromiumModelIdForProfile(
+        default_user_data_dir.Append(default_profile_));
 
     extension_id_ = L"chromiumexampleappidforunittests";
     base::string16 app_name =
@@ -73,8 +77,6 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     extension_app_id_ = GetAppModelIdForProfile(app_name, default_profile_path);
     non_default_profile_extension_app_id_ = GetAppModelIdForProfile(
         app_name, default_user_data_dir.Append(non_default_profile_));
-
-    CreateShortcuts();
   }
 
   // Creates a test shortcut corresponding to |shortcut_properties| and resets
@@ -116,13 +118,13 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     // 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(chrome_app_id_);
+    temp_properties.set_app_id(default_profile_chrome_app_id_);
     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.
-    base::string16 chrome_app_id_is_prefix(chrome_app_id_);
+    base::string16 chrome_app_id_is_prefix(default_profile_chrome_app_id_);
     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);
@@ -131,7 +133,8 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
 
     // Shortcut 4 is like shortcut 1, but it's appid is of the same size as the
     // expected appid.
-    base::string16 same_size_as_chrome_app_id(chrome_app_id_.size(), L'1');
+    base::string16 same_size_as_chrome_app_id(
+        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(
@@ -191,9 +194,9 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
     ASSERT_NO_FATAL_FAILURE(
         AddTestShortcutAndResetProperties(&temp_properties));
 
-    // Shortcut 11 points to chrome.exe, already has the right appid, and has
-    // dual_mode set and thus should only be migrated if dual_mode is being
-    // cleared.
+    // Shortcut 11 points to chrome.exe, has the chrome appid, and has
+    // dual_mode set and thus should be migrated to the
+    // default_profile_chrome_app_id, and dual_mode should be cleared.
     temp_properties.set_target(chrome_exe_);
     temp_properties.set_app_id(chrome_app_id_);
     temp_properties.set_dual_mode(true);
@@ -224,6 +227,12 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
   // Chrome's AppUserModelId.
   base::string16 chrome_app_id_;
 
+  // Chrome's AppUserModelId for the default profile.
+  base::string16 default_profile_chrome_app_id_;
+
+  // The Default profile.
+  base::string16 default_profile_;
+
   // A profile that isn't the Default profile.
   base::string16 non_default_profile_;
 
@@ -256,22 +265,26 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
 }  // namespace
 
 TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualModeAndAdjustAppIds) {
-  // 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,
+  CreateShortcuts();
+  // 11 shortcuts should have their app id updated below including shortcut 11,
+  // which should also be migrated away from dual_mode.
+  EXPECT_EQ(11,
             MigrateShortcutsInPathInternal(chrome_exe_, temp_dir_.GetPath()));
 
-  // 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_);
-  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_);
+  // Shortcut 1, 3, 4, 5, 6, 7, 8, 9, 10, 11 and 12 should have had their app_id
+  // fixed.
+  shortcuts_[1].properties.set_app_id(default_profile_chrome_app_id_);
+  shortcuts_[3].properties.set_app_id(default_profile_chrome_app_id_);
+  shortcuts_[4].properties.set_app_id(default_profile_chrome_app_id_);
+  shortcuts_[5].properties.set_app_id(default_profile_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_);
+  shortcuts_[11].properties.set_app_id(default_profile_chrome_app_id_);
+  shortcuts_[12].properties.set_app_id(default_profile_chrome_app_id_);
 
   // No shortcut should still have the dual_mode property.
   for (size_t i = 0; i < shortcuts_.size(); ++i)
@@ -287,6 +300,24 @@ TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualModeAndAdjustAppIds) {
             MigrateShortcutsInPathInternal(chrome_exe_, temp_dir_.GetPath()));
 }
 
+// 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
+  // profile name. "chrome_proxy.exe" won't exist, but it appears that
+  // creating the shortcut succeeds.
+  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_properties));
+
+  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);
+}
+
 TEST(ShellIntegrationWinTest, GetAppModelIdForProfileTest) {
   const base::string16 base_app_id(install_static::GetBaseAppId());
 
@@ -294,12 +325,13 @@ TEST(ShellIntegrationWinTest, GetAppModelIdForProfileTest) {
   base::FilePath empty_path;
   EXPECT_EQ(base_app_id, GetAppModelIdForProfile(base_app_id, empty_path));
 
-  // Default profile path should get chrome::kBrowserAppID
+  // Default profile path should get chrome::kBrowserAppID joined with
+  // profile info.
   base::FilePath default_user_data_dir;
   chrome::GetDefaultUserDataDirectory(&default_user_data_dir);
   base::FilePath default_profile_path =
       default_user_data_dir.AppendASCII(chrome::kInitialProfile);
-  EXPECT_EQ(base_app_id,
+  EXPECT_EQ(base_app_id + L".UserData.Default",
             GetAppModelIdForProfile(base_app_id, default_profile_path));
 
   // Non-default profile path should get chrome::kBrowserAppID joined with

chrome/browser/web_applications/components/web_app_shortcut_win.cc

@@ -44,12 +44,6 @@ constexpr base::FilePath::CharType kIconChecksumFileExt[] =
 constexpr base::FilePath::CharType kChromeProxyExecutable[] =
     FILE_PATH_LITERAL("chrome_proxy.exe");
 
-base::FilePath GetChromeProxyPath() {
-  base::FilePath chrome_dir;
-  CHECK(base::PathService::Get(base::DIR_EXE, &chrome_dir));
-  return chrome_dir.Append(kChromeProxyExecutable);
-}
-
 // Calculates checksum of an icon family using MD5.
 // The checksum is derived from all of the icons in the family.
 void GetImageCheckSum(const gfx::ImageFamily& image, base::MD5Digest* digest) {
@@ -195,7 +189,7 @@ bool CreateShortcutsInPaths(const base::FilePath& web_app_path,
     return false;
   }
 
-  base::FilePath chrome_proxy_path = GetChromeProxyPath();
+  base::FilePath chrome_proxy_path = web_app::GetChromeProxyPath();
 
   // Working directory.
   base::FilePath working_dir(chrome_proxy_path.DirName());
@@ -342,7 +336,7 @@ void CreateIconAndSetRelaunchDetails(
                                                     shortcut_info.extension_id,
                                                     shortcut_info.profile_path);
 
-  command_line.SetProgram(GetChromeProxyPath());
+  command_line.SetProgram(web_app::GetChromeProxyPath());
   ui::win::SetRelaunchDetailsForWindow(command_line.GetCommandLineString(),
                                        shortcut_info.title, hwnd);
 
@@ -357,6 +351,12 @@ void CreateIconAndSetRelaunchDetails(
 
 namespace web_app {
 
+base::FilePath GetChromeProxyPath() {
+  base::FilePath chrome_dir;
+  CHECK(base::PathService::Get(base::DIR_EXE, &chrome_dir));
+  return chrome_dir.Append(kChromeProxyExecutable);
+}
+
 namespace internals {
 
 void OnShortcutInfoLoadedForSetRelaunchDetails(

chrome/browser/web_applications/components/web_app_shortcut_win.h

@@ -11,6 +11,8 @@
 
 namespace web_app {
 
+base::FilePath GetChromeProxyPath();
+
 namespace internals {
 
 // Returns the Windows user-level shortcut paths that are specified in