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

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))

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.

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;

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,

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) {

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

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);
 }