diff options
author | gab@chromium.org <gab@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-08 18:17:53 +0000 |
---|---|---|
committer | gab@chromium.org <gab@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-08 18:19:12 +0000 |
commit | da4d4fb02cb4849277880d95db38361de09b3885 (patch) | |
tree | 9e3e438569c43c861dd69073b03ce3df45885ca9 | |
parent | 33d76bb0f87cd9e3ad5b1589bb29872924e4e5e3 (diff) | |
download | chromium_src-da4d4fb02cb4849277880d95db38361de09b3885.zip chromium_src-da4d4fb02cb4849277880d95db38361de09b3885.tar.gz chromium_src-da4d4fb02cb4849277880d95db38361de09b3885.tar.bz2 |
Fix shortcut tests and remove legacy shortcut code for the Default-user
Quick Launch shortcut.
InstallShortcutTest.CreateAllShortcutsSystemLevel has been broken since http://crrev.com/164849 ...
The test expectations had not been adjusted in this CL as setup_unittests.exe
doesn't run on the waterfall yet (issue #153829) and the breakage had gone
unnoticed.
Expectations have now been adjusted such that we no longer expect a system-level
(Default-user) Quick Launch shortcut, but a per-user Quick Launch shortcut for
the admin running the install as the product code's logic has been doing for over
a year...
Also remove the code cleaning up the legacy Default-user Quick Launch shortcut as
this cleanup code has been running for 21 months now and leaving this shortcut
behind for a few users is a no-op (i.e. the Default-user shortcut will get copied
to new Windows profiles and Active Setup will kick in a few seconds later to try
to install the same shortcut which will already exist... the UX result will be
the same: a single Chrome Quick Launch shortcut will be installed).
BUG=329239, 153829
Review URL: https://codereview.chromium.org/432273005
Cr-Commit-Position: refs/heads/master@{#288393}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288393 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/base_paths_win.cc | 49 | ||||
-rw-r--r-- | base/base_paths_win.h | 2 | ||||
-rw-r--r-- | base/path_service_unittest.cc | 16 | ||||
-rw-r--r-- | chrome/installer/setup/install.cc | 13 | ||||
-rw-r--r-- | chrome/installer/setup/install_unittest.cc | 25 | ||||
-rw-r--r-- | chrome/installer/util/shell_util.cc | 19 | ||||
-rw-r--r-- | chrome/installer/util/shell_util_unittest.cc | 24 |
7 files changed, 48 insertions, 100 deletions
diff --git a/base/base_paths_win.cc b/base/base_paths_win.cc index 509d5fd..a9b31c7b 100644 --- a/base/base_paths_win.cc +++ b/base/base_paths_win.cc @@ -16,38 +16,6 @@ extern "C" IMAGE_DOS_HEADER __ImageBase; using base::FilePath; -namespace { - -bool GetQuickLaunchPath(bool default_user, FilePath* result) { - if (default_user) { - wchar_t system_buffer[MAX_PATH]; - system_buffer[0] = 0; - // As per MSDN, passing -1 for |hToken| indicates the Default user: - // http://msdn.microsoft.com/library/windows/desktop/bb762181.aspx - if (FAILED(SHGetFolderPath(NULL, CSIDL_APPDATA, - reinterpret_cast<HANDLE>(-1), SHGFP_TYPE_CURRENT, - system_buffer))) { - return false; - } - *result = FilePath(system_buffer); - } else if (!PathService::Get(base::DIR_APP_DATA, result)) { - // For the current user, grab the APPDATA directory directly from the - // PathService cache. - return false; - } - // According to various sources, appending - // "Microsoft\Internet Explorer\Quick Launch" to %appdata% is the only - // reliable way to get the quick launch folder across all versions of Windows. - // http://stackoverflow.com/questions/76080/how-do-you-reliably-get-the-quick- - // http://www.microsoft.com/technet/scriptcenter/resources/qanda/sept05/hey0901.mspx - *result = result->AppendASCII("Microsoft"); - *result = result->AppendASCII("Internet Explorer"); - *result = result->AppendASCII("Quick Launch"); - return true; -} - -} // namespace - namespace base { bool PathProviderWin(int key, FilePath* result) { @@ -178,12 +146,17 @@ bool PathProviderWin(int key, FilePath* result) { cur = FilePath(system_buffer); break; case base::DIR_USER_QUICK_LAUNCH: - if (!GetQuickLaunchPath(false, &cur)) - return false; - break; - case base::DIR_DEFAULT_USER_QUICK_LAUNCH: - if (!GetQuickLaunchPath(true, &cur)) - return false; + if (!PathService::Get(base::DIR_APP_DATA, &cur)) + return false; + // According to various sources, appending + // "Microsoft\Internet Explorer\Quick Launch" to %appdata% is the only + // reliable way to get the quick launch folder across all versions of + // Windows. + // http://stackoverflow.com/questions/76080/how-do-you-reliably-get-the-quick- + // http://www.microsoft.com/technet/scriptcenter/resources/qanda/sept05/hey0901.mspx + cur = cur.AppendASCII("Microsoft") + .AppendASCII("Internet Explorer") + .AppendASCII("Quick Launch"); break; case base::DIR_TASKBAR_PINS: if (!PathService::Get(base::DIR_USER_QUICK_LAUNCH, &cur)) diff --git a/base/base_paths_win.h b/base/base_paths_win.h index b042d08..4620171 100644 --- a/base/base_paths_win.h +++ b/base/base_paths_win.h @@ -37,8 +37,6 @@ enum { DIR_COMMON_DESKTOP, // Directory for the common desktop (visible // on all user's Desktop). DIR_USER_QUICK_LAUNCH, // Directory for the quick launch shortcuts. - DIR_DEFAULT_USER_QUICK_LAUNCH, // Directory for the quick launch shortcuts - // of the Default user. DIR_TASKBAR_PINS, // Directory for the shortcuts pinned to taskbar via // base::win::TaskbarPinShortcutLink(). DIR_WINDOWS_FONTS, // Usually C:\Windows\Fonts. diff --git a/base/path_service_unittest.cc b/base/path_service_unittest.cc index 7a70a89..a345262 100644 --- a/base/path_service_unittest.cc +++ b/base/path_service_unittest.cc @@ -15,10 +15,7 @@ #include "testing/platform_test.h" #if defined(OS_WIN) -#include <userenv.h> #include "base/win/windows_version.h" -// userenv.dll is required for GetDefaultUserProfileDirectory(). -#pragma comment(lib, "userenv.lib") #endif namespace { @@ -45,18 +42,7 @@ bool ReturnsValidPath(int dir_type) { check_path_exists = false; #endif #if defined(OS_WIN) - if (dir_type == base::DIR_DEFAULT_USER_QUICK_LAUNCH) { - // On Windows XP, the Quick Launch folder for the "Default User" doesn't - // exist by default. At least confirm that the path returned begins with the - // Default User's profile path. - if (base::win::GetVersion() < base::win::VERSION_VISTA) { - wchar_t default_profile_path[MAX_PATH]; - DWORD size = arraysize(default_profile_path); - return (result && - ::GetDefaultUserProfileDirectory(default_profile_path, &size) && - StartsWith(path.value(), default_profile_path, false)); - } - } else if (dir_type == base::DIR_TASKBAR_PINS) { + if (dir_type == base::DIR_TASKBAR_PINS) { // There is no pinned-to-taskbar shortcuts prior to Win7. if (base::win::GetVersion() < base::win::VERSION_WIN7) check_path_exists = false; diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc index 5ba2608..f1ab38d 100644 --- a/chrome/installer/setup/install.cc +++ b/chrome/installer/setup/install.cc @@ -244,11 +244,8 @@ installer::InstallStatus InstallNewVersion( return installer::INSTALL_FAILED; } -// Deletes the old "Uninstall Google Chrome" shortcut in the Start menu and, if -// this is a system-level install, also deletes the old Default user Quick -// Launch shortcut. Both of these were created prior to Chrome 24; in Chrome 24, -// the uninstall shortcut was removed and the Default user Quick Launch shortcut -// was replaced by per-user shortcuts created via Active Setup. +// Deletes the old "Uninstall Google Chrome" shortcut in the Start menu which +// was installed prior to Chrome 24. void CleanupLegacyShortcuts(const installer::InstallerState& installer_state, BrowserDistribution* dist, const base::FilePath& chrome_exe) { @@ -260,12 +257,6 @@ void CleanupLegacyShortcuts(const installer::InstallerState& installer_state, uninstall_shortcut_path = uninstall_shortcut_path.Append( dist->GetUninstallLinkName() + installer::kLnkExt); base::DeleteFile(uninstall_shortcut_path, false); - - if (installer_state.system_install()) { - ShellUtil::RemoveShortcuts( - ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, dist, - ShellUtil::SYSTEM_LEVEL, chrome_exe); - } } // Returns the appropriate shortcut operations for App Launcher, diff --git a/chrome/installer/setup/install_unittest.cc b/chrome/installer/setup/install_unittest.cc index ed2fd97..54b3629 100644 --- a/chrome/installer/setup/install_unittest.cc +++ b/chrome/installer/setup/install_unittest.cc @@ -94,7 +94,6 @@ class InstallShortcutTest : public testing::Test { ASSERT_TRUE(fake_user_desktop_.CreateUniqueTempDir()); ASSERT_TRUE(fake_common_desktop_.CreateUniqueTempDir()); ASSERT_TRUE(fake_user_quick_launch_.CreateUniqueTempDir()); - ASSERT_TRUE(fake_default_user_quick_launch_.CreateUniqueTempDir()); ASSERT_TRUE(fake_start_menu_.CreateUniqueTempDir()); ASSERT_TRUE(fake_common_start_menu_.CreateUniqueTempDir()); user_desktop_override_.reset( @@ -106,9 +105,6 @@ class InstallShortcutTest : public testing::Test { user_quick_launch_override_.reset( new base::ScopedPathOverride(base::DIR_USER_QUICK_LAUNCH, fake_user_quick_launch_.path())); - default_user_quick_launch_override_.reset( - new base::ScopedPathOverride(base::DIR_DEFAULT_USER_QUICK_LAUNCH, - fake_default_user_quick_launch_.path())); start_menu_override_.reset( new base::ScopedPathOverride(base::DIR_START_MENU, fake_start_menu_.path())); @@ -134,8 +130,6 @@ class InstallShortcutTest : public testing::Test { .Append(shortcut_name); system_desktop_shortcut_ = fake_common_desktop_.path().Append(shortcut_name); - system_quick_launch_shortcut_ = - fake_default_user_quick_launch_.path().Append(shortcut_name); system_start_menu_shortcut_ = fake_common_start_menu_.path().Append( dist_->GetStartMenuShortcutSubfolder( @@ -195,13 +189,11 @@ class InstallShortcutTest : public testing::Test { base::ScopedTempDir fake_user_desktop_; base::ScopedTempDir fake_common_desktop_; base::ScopedTempDir fake_user_quick_launch_; - base::ScopedTempDir fake_default_user_quick_launch_; base::ScopedTempDir fake_start_menu_; base::ScopedTempDir fake_common_start_menu_; scoped_ptr<base::ScopedPathOverride> user_desktop_override_; scoped_ptr<base::ScopedPathOverride> common_desktop_override_; scoped_ptr<base::ScopedPathOverride> user_quick_launch_override_; - scoped_ptr<base::ScopedPathOverride> default_user_quick_launch_override_; scoped_ptr<base::ScopedPathOverride> start_menu_override_; scoped_ptr<base::ScopedPathOverride> common_start_menu_override_; @@ -209,7 +201,6 @@ class InstallShortcutTest : public testing::Test { base::FilePath user_quick_launch_shortcut_; base::FilePath user_start_menu_shortcut_; base::FilePath system_desktop_shortcut_; - base::FilePath system_quick_launch_shortcut_; base::FilePath system_start_menu_shortcut_; base::FilePath user_alternate_desktop_shortcut_; }; @@ -264,16 +255,17 @@ TEST_F(InstallShortcutTest, CreateAllShortcuts) { expected_start_menu_properties_); } -// Disabled failing test; http://crbug.com/329239. -TEST_F(InstallShortcutTest, DISABLED_CreateAllShortcutsSystemLevel) { +TEST_F(InstallShortcutTest, CreateAllShortcutsSystemLevel) { installer::CreateOrUpdateShortcuts( chrome_exe_, *product_, *prefs_, installer::ALL_USERS, installer::INSTALL_SHORTCUT_CREATE_ALL); base::win::ValidateShortcut(system_desktop_shortcut_, expected_properties_); - base::win::ValidateShortcut(system_quick_launch_shortcut_, - expected_properties_); base::win::ValidateShortcut(system_start_menu_shortcut_, expected_start_menu_properties_); + // The quick launch shortcut is always created per-user for the admin running + // the install (other users will get it via Active Setup). + base::win::ValidateShortcut(user_quick_launch_shortcut_, + expected_properties_); } TEST_F(InstallShortcutTest, CreateAllShortcutsAlternateDesktopName) { @@ -376,9 +368,6 @@ TEST_F(InstallShortcutTest, CreateIfNoSystemLevelAllSystemShortcutsExist) { ASSERT_TRUE(base::win::CreateOrUpdateShortcutLink( system_desktop_shortcut_, dummy_properties, base::win::SHORTCUT_CREATE_ALWAYS)); - ASSERT_TRUE(base::win::CreateOrUpdateShortcutLink( - system_quick_launch_shortcut_, dummy_properties, - base::win::SHORTCUT_CREATE_ALWAYS)); ASSERT_TRUE(base::CreateDirectory( system_start_menu_shortcut_.DirName())); ASSERT_TRUE(base::win::CreateOrUpdateShortcutLink( @@ -389,8 +378,10 @@ TEST_F(InstallShortcutTest, CreateIfNoSystemLevelAllSystemShortcutsExist) { chrome_exe_, *product_, *prefs_, installer::CURRENT_USER, installer::INSTALL_SHORTCUT_CREATE_EACH_IF_NO_SYSTEM_LEVEL); ASSERT_FALSE(base::PathExists(user_desktop_shortcut_)); - ASSERT_FALSE(base::PathExists(user_quick_launch_shortcut_)); ASSERT_FALSE(base::PathExists(user_start_menu_shortcut_)); + // There is no system-level quick launch shortcut, so creating the user-level + // one should always succeed. + ASSERT_TRUE(base::PathExists(user_quick_launch_shortcut_)); } TEST_F(InstallShortcutTest, CreateIfNoSystemLevelNoSystemShortcutsExist) { diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc index 34406a6..2893f75 100644 --- a/chrome/installer/util/shell_util.cc +++ b/chrome/installer/util/shell_util.cc @@ -1335,6 +1335,13 @@ bool BatchShortcutAction( ShellUtil::ShellChange level, const scoped_refptr<ShellUtil::SharedCancellationFlag>& cancel) { DCHECK(!shortcut_operation.is_null()); + + // There is no system-level Quick Launch shortcut folder. + if (level == ShellUtil::SYSTEM_LEVEL && + location == ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH) { + return true; + } + base::FilePath shortcut_folder; if (!ShellUtil::GetShortcutPath(location, dist, level, &shortcut_folder)) { LOG(WARNING) << "Cannot find path at location " << location; @@ -1474,8 +1481,9 @@ bool ShellUtil::GetShortcutPath(ShellUtil::ShortcutLocation location, base::DIR_COMMON_DESKTOP; break; case SHORTCUT_LOCATION_QUICK_LAUNCH: - dir_key = (level == CURRENT_USER) ? base::DIR_USER_QUICK_LAUNCH : - base::DIR_DEFAULT_USER_QUICK_LAUNCH; + // There is no support for a system-level Quick Launch shortcut. + DCHECK_EQ(level, CURRENT_USER); + dir_key = base::DIR_USER_QUICK_LAUNCH; break; case SHORTCUT_LOCATION_START_MENU_ROOT: dir_key = (level == CURRENT_USER) ? base::DIR_START_MENU : @@ -1539,7 +1547,11 @@ bool ShellUtil::CreateOrUpdateShortcut( base::FilePath user_shortcut_path; base::FilePath system_shortcut_path; - if (!GetShortcutPath(location, dist, SYSTEM_LEVEL, &system_shortcut_path)) { + if (location == SHORTCUT_LOCATION_QUICK_LAUNCH) { + // There is no system-level shortcut for Quick Launch. + DCHECK_EQ(properties.level, CURRENT_USER); + } else if (!GetShortcutPath( + location, dist, SYSTEM_LEVEL, &system_shortcut_path)) { NOTREACHED(); return false; } @@ -1554,6 +1566,7 @@ bool ShellUtil::CreateOrUpdateShortcut( // Install the system-level shortcut if requested. chosen_path = &system_shortcut_path; } else if (operation != SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL || + system_shortcut_path.empty() || !base::PathExists(system_shortcut_path)) { // Otherwise install the user-level shortcut, unless the system-level // variant of this shortcut is present on the machine and |operation| states diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc index 134b31c..9cca44a 100644 --- a/chrome/installer/util/shell_util_unittest.cc +++ b/chrome/installer/util/shell_util_unittest.cc @@ -71,9 +71,6 @@ class ShellUtilShortcutTest : public testing::Test { user_quick_launch_override_.reset( new base::ScopedPathOverride(base::DIR_USER_QUICK_LAUNCH, fake_user_quick_launch_.path())); - default_user_quick_launch_override_.reset( - new base::ScopedPathOverride(base::DIR_DEFAULT_USER_QUICK_LAUNCH, - fake_default_user_quick_launch_.path())); start_menu_override_.reset( new base::ScopedPathOverride(base::DIR_START_MENU, fake_start_menu_.path())); @@ -104,9 +101,7 @@ class ShellUtilShortcutTest : public testing::Test { fake_user_desktop_.path() : fake_common_desktop_.path(); break; case ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH: - expected_path = (properties.level == ShellUtil::CURRENT_USER) ? - fake_user_quick_launch_.path() : - fake_default_user_quick_launch_.path(); + expected_path = fake_user_quick_launch_.path(); break; case ShellUtil::SHORTCUT_LOCATION_START_MENU_CHROME_DIR: expected_path = (properties.level == ShellUtil::CURRENT_USER) ? @@ -195,7 +190,6 @@ class ShellUtilShortcutTest : public testing::Test { scoped_ptr<base::ScopedPathOverride> user_desktop_override_; scoped_ptr<base::ScopedPathOverride> common_desktop_override_; scoped_ptr<base::ScopedPathOverride> user_quick_launch_override_; - scoped_ptr<base::ScopedPathOverride> default_user_quick_launch_override_; scoped_ptr<base::ScopedPathOverride> start_menu_override_; scoped_ptr<base::ScopedPathOverride> common_start_menu_override_; @@ -209,18 +203,19 @@ class ShellUtilShortcutTest : public testing::Test { TEST_F(ShellUtilShortcutTest, GetShortcutPath) { base::FilePath path; + ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::CURRENT_USER, &path); EXPECT_EQ(fake_user_desktop_.path(), path); + ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, ShellUtil::SYSTEM_LEVEL, &path); EXPECT_EQ(fake_common_desktop_.path(), path); + ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, dist_, ShellUtil::CURRENT_USER, &path); EXPECT_EQ(fake_user_quick_launch_.path(), path); - ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, dist_, - ShellUtil::SYSTEM_LEVEL, &path); - EXPECT_EQ(fake_default_user_quick_launch_.path(), path); + base::string16 start_menu_subfolder = dist_->GetStartMenuShortcutSubfolder( BrowserDistribution::SUBFOLDER_CHROME); @@ -228,6 +223,7 @@ TEST_F(ShellUtilShortcutTest, GetShortcutPath) { dist_, ShellUtil::CURRENT_USER, &path); EXPECT_EQ(fake_start_menu_.path().Append(start_menu_subfolder), path); + ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_START_MENU_CHROME_DIR, dist_, ShellUtil::SYSTEM_LEVEL, &path); EXPECT_EQ(fake_common_start_menu_.path().Append(start_menu_subfolder), @@ -255,10 +251,10 @@ TEST_F(ShellUtilShortcutTest, CreateStartMenuShortcutWithAllProperties) { dist_, test_properties_); } -TEST_F(ShellUtilShortcutTest, ReplaceSystemLevelQuickLaunchShortcut) { +TEST_F(ShellUtilShortcutTest, ReplaceSystemLevelDesktopShortcut) { test_properties_.level = ShellUtil::SYSTEM_LEVEL; ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut( - ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, + ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, test_properties_, ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS)); @@ -267,7 +263,7 @@ TEST_F(ShellUtilShortcutTest, ReplaceSystemLevelQuickLaunchShortcut) { new_properties.set_description(L"New description"); new_properties.set_arguments(L"--new-arguments"); ASSERT_TRUE(ShellUtil::CreateOrUpdateShortcut( - ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, + ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, new_properties, ShellUtil::SHELL_SHORTCUT_REPLACE_EXISTING)); @@ -277,7 +273,7 @@ TEST_F(ShellUtilShortcutTest, ReplaceSystemLevelQuickLaunchShortcut) { ShellUtil::ShortcutProperties expected_properties(new_properties); expected_properties.set_dual_mode(false); - ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_QUICK_LAUNCH, dist_, + ValidateChromeShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP, dist_, expected_properties); } |