Skip to content

GitLab

T
tangled
Commit 9b4a9378 authored by David Bienvenu's avatar David Bienvenu Committed by Commit Bot
Browse files
Options

Revert app id containing default profile. 

This is a partial revert of crrev.com/c/1686222, which changed app model
ids to include the default profile name. That caused issues with duplicate
taskbar icons, which were worse than the original bug being fixed.

Bug: 1007911
Change-Id: I28da312817b4223b7be842acf71e2d14f39711c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1934525
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719141}
parent 646ba862
Show whitespace changes
Inline Side-by-side
Showing with 46 additions and 42 deletions
chrome/browser/shell_integration_win.cc
View file @ 9b4a9378
......@@ -69,6 +69,16 @@ 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();
......@@ -820,8 +830,6 @@ 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();
......@@ -839,8 +847,7 @@ int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,
updated_properties.set_app_id(expected_app_id);
break;
case VT_LPWSTR:
current_app_id = base::string16(propvariant.get().pwszVal);
if (expected_app_id != current_app_id)
if (expected_app_id != base::string16(propvariant.get().pwszVal))
updated_properties.set_app_id(expected_app_id);
break;
default:
......@@ -854,7 +861,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 (current_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) {
......
chrome/browser/shell_integration_win_unittest.cc
View file @ 9b4a9378
......@@ -67,9 +67,6 @@ 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 =
......@@ -120,13 +117,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(default_profile_chrome_app_id_);
temp_properties.set_app_id(chrome_app_id_);
ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
temp_dir_.GetPath(), &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(default_profile_chrome_app_id_);
base::string16 chrome_app_id_is_prefix(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);
......@@ -135,8 +132,7 @@ 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(
default_profile_chrome_app_id_.size(), L'1');
base::string16 same_size_as_chrome_app_id(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(
......@@ -196,9 +192,9 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
temp_dir_.GetPath(), &temp_properties));
// 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.
// 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.
temp_properties.set_target(chrome_exe_);
temp_properties.set_app_id(chrome_app_id_);
temp_properties.set_dual_mode(true);
......@@ -211,6 +207,15 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
temp_properties.set_dual_mode(false);
ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
temp_dir_.GetPath(), &temp_properties));
// Shortcut 13 is like shortcut 1, but it's appid explicitly includes the
// default profile.
base::string16 chrome_app_id_with_default_profile =
chrome_app_id_ + L".Default";
temp_properties.set_target(chrome_exe_);
temp_properties.set_app_id(chrome_app_id_with_default_profile);
ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
temp_dir_.GetPath(), &temp_properties));
}
base::win::ScopedCOMInitializer com_initializer_;
......@@ -232,12 +237,6 @@ 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_;
......@@ -271,25 +270,24 @@ class ShellIntegrationWinMigrateShortcutTest : public testing::Test {
TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualModeAndAdjustAppIds) {
CreateShortcuts();
// 11 shortcuts should have their app id updated below including shortcut 11,
// which should also be migrated away from dual_mode.
// 10 shortcuts should have their app id updated below and shortcut 11 should
// be migrated away from dual_mode for a total of 11 shortcuts migrated.
EXPECT_EQ(11,
MigrateShortcutsInPathInternal(chrome_exe_, temp_dir_.GetPath()));
// Shortcut 1, 3, 4, 5, 6, 7, 8, 9, 10, 11 and 12 should have had their app_id
// Shortcut 1, 3, 4, 5, 6, 7, 8, 9, 10, and 13 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_[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_);
shortcuts_[11].properties.set_app_id(default_profile_chrome_app_id_);
shortcuts_[12].properties.set_app_id(default_profile_chrome_app_id_);
shortcuts_[13].properties.set_app_id(chrome_app_id_);
// No shortcut should still have the dual_mode property.
for (size_t i = 0; i < shortcuts_.size(); ++i)
......@@ -306,7 +304,7 @@ TEST_F(ShellIntegrationWinMigrateShortcutTest, ClearDualModeAndAdjustAppIds) {
}
// Test that chrome_proxy.exe shortcuts (PWA) have their app_id migrated
// to include the default profile name. This tests both shortcuts in the
// to not include the default profile name. This tests both shortcuts in the
// DIR_TASKBAR_PINS and sub-directories of DIR_IMPLICIT_APP_SHORTCUTS.
TEST_F(ShellIntegrationWinMigrateShortcutTest, MigrateChromeProxyTest) {
// Create shortcut to chrome_proxy_exe in executable directory,
......@@ -315,22 +313,22 @@ TEST_F(ShellIntegrationWinMigrateShortcutTest, MigrateChromeProxyTest) {
// creating the shortcut succeeds.
base::win::ShortcutProperties temp_properties;
temp_properties.set_target(web_app::GetChromeProxyPath());
temp_properties.set_app_id(L"Dumbo");
temp_properties.set_app_id(L"Dumbo.Default");
ASSERT_NO_FATAL_FAILURE(
AddTestShortcutAndResetProperties(temp_dir_.GetPath(), &temp_properties));
temp_properties.set_target(web_app::GetChromeProxyPath());
temp_properties.set_app_id(L"Dumbo2");
temp_properties.set_app_id(L"Dumbo2.Default");
ASSERT_NO_FATAL_FAILURE(AddTestShortcutAndResetProperties(
temp_dir_sub_dir_.GetPath(), &temp_properties));
MigrateTaskbarPinsCallback(temp_dir_.GetPath(), temp_dir_.GetPath());
// Verify that the migrated shortcut in temp_dir_ now contains the default
// Verify that the migrated shortcut in temp_dir_ does not contain the default
// profile name.
shortcuts_[0].properties.set_app_id(default_profile_chrome_app_id_);
shortcuts_[0].properties.set_app_id(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_);
// Verify that the migrated shortcut in temp_dir_sub does not contain the
// default profile name.
shortcuts_[1].properties.set_app_id(chrome_app_id_);
base::win::ValidateShortcut(shortcuts_[1].path, shortcuts_[1].properties);
}
......@@ -341,13 +339,12 @@ 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 joined with
// profile info.
// Default profile path should get chrome::kBrowserAppID
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 + L".UserData.Default",
EXPECT_EQ(base_app_id,
GetAppModelIdForProfile(base_app_id, default_profile_path));
// Non-default profile path should get chrome::kBrowserAppID joined with
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment