Add new PathService paths for Windows' All Users Desktop and Quick Launch folders.

Re-commit, reverted in http://crrev.com/157680 (original commit in http://crrev.com/157667).

This allows usage of PathService to cache the shortcut install paths and more importantly to mock them in shortcut tests!

Also move chrome::DIR_USER_DESKTOP to base::DIR_USER_DESKTOP; this is really where it belongs. In fact it is only in chrome_paths.h because it used to be called DIR_DEFAULT_DOWNLOAD and cpu@ renamed it to DIR_USER_DESKTOP in http://crrev.com/1753 (early days!) after that it started to be used all over the place as the Desktop path. Finally bringing it to base_paths.h, beside DIR_START_MENU and friends, is the right thing to do imo.

TBR=brettw@chromium.org
BUG=148539
TEST=Quick Launch shortcut installed in the right place on XP (both Default and current user)
Desktop shortcuts installed in the right place (both All Users and per-user installs).

installer_util_unittests.exe --gtest_filter=ShellUtilShortcutTest*
unit_tests.exe --gtest_filter=ProfileShortcutManagerTest*
base_unittests --gtest_filter=PathServiceTest*

Review URL: https://chromiumcodereview.appspot.com/10964007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@157779 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 48 insertions, 96 deletions

diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index 3dd5840..d18d876 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -138,7 +138,7 @@ bool DownloadPathIsDangerous(const FilePath& download_path) {
   return false;
 #else
   FilePath desktop_dir;
-  if (!PathService::Get(chrome::DIR_USER_DESKTOP, &desktop_dir)) {
+  if (!PathService::Get(base::DIR_USER_DESKTOP, &desktop_dir)) {
     NOTREACHED();
     return false;
   }
diff --git a/chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc b/chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc
index 6625964..81be184 100644
--- a/chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc
+++ b/chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc
@@ -61,7 +61,12 @@ class ProfileShortcutManagerTest : public testing::Test {
         file_thread_(BrowserThread::FILE, &message_loop_) {
   }
 
-  virtual void SetUp() {
+  virtual void SetUp() OVERRIDE {
+    // Mock the user's Desktop into a temp directory.
+    ASSERT_TRUE(fake_user_desktop_.CreateUniqueTempDir());
+    ASSERT_TRUE(PathService::Override(base::DIR_USER_DESKTOP,
+                                      fake_user_desktop_.path()));
+
     TestingBrowserProcess* browser_process =
         static_cast<TestingBrowserProcess*>(g_browser_process);
     profile_manager_.reset(new TestingProfileManager(browser_process));
@@ -80,7 +85,7 @@ class ProfileShortcutManagerTest : public testing::Test {
     second_profile_name_ = ASCIIToUTF16("My profile 2");
   }
 
-  virtual void TearDown() {
+  virtual void TearDown() OVERRIDE {
     message_loop_.RunAllPending();
 
     int num_profiles =
@@ -148,6 +153,7 @@ class ProfileShortcutManagerTest : public testing::Test {
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
   scoped_ptr<TestingProfileManager> profile_manager_;
+  ScopedTempDir fake_user_desktop_;
   FilePath dest_path_;
   string16 profile_name_;
   FilePath second_dest_path_;
diff --git a/chrome/browser/shell_integration_linux.cc b/chrome/browser/shell_integration_linux.cc
index b4f7942..e478893 100644
--- a/chrome/browser/shell_integration_linux.cc
+++ b/chrome/browser/shell_integration_linux.cc
@@ -32,7 +32,6 @@
 #include "build/build_config.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "ui/gfx/codec/png_codec.h"
@@ -119,7 +118,7 @@ bool CreateShortcutOnDesktop(const FilePath& shortcut_filename,
   DCHECK_EQ(shortcut_filename.BaseName().value(), shortcut_filename.value());
 
   FilePath desktop_path;
-  if (!PathService::Get(chrome::DIR_USER_DESKTOP, &desktop_path))
+  if (!PathService::Get(base::DIR_USER_DESKTOP, &desktop_path))
     return false;
 
   int desktop_fd = open(desktop_path.value().c_str(), O_RDONLY | O_DIRECTORY);
@@ -156,7 +155,7 @@ bool CreateShortcutOnDesktop(const FilePath& shortcut_filename,
 
 void DeleteShortcutOnDesktop(const FilePath& shortcut_filename) {
   FilePath desktop_path;
-  if (PathService::Get(chrome::DIR_USER_DESKTOP, &desktop_path))
+  if (PathService::Get(base::DIR_USER_DESKTOP, &desktop_path))
     file_util::Delete(desktop_path.Append(shortcut_filename), false);
 }
 
@@ -484,7 +483,7 @@ FilePath GetWebShortcutFilename(const GURL& url) {
   file_util::ReplaceIllegalCharactersInPath(&filename, '_');
 
   FilePath desktop_path;
-  if (!PathService::Get(chrome::DIR_USER_DESKTOP, &desktop_path))
+  if (!PathService::Get(base::DIR_USER_DESKTOP, &desktop_path))
     return FilePath();
 
   FilePath filepath = desktop_path.Append(filename);
diff --git a/chrome/browser/shell_integration_win.cc b/chrome/browser/shell_integration_win.cc
index 08b9885..4782a3c 100644
--- a/chrome/browser/shell_integration_win.cc
+++ b/chrome/browser/shell_integration_win.cc
@@ -25,7 +25,6 @@
 #include "base/win/windows_version.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/setup/setup_util.h"
@@ -364,7 +363,7 @@ void MigrateChromiumShortcutsCallback() {
       base::DIR_APP_DATA,
       L"Microsoft\\Internet Explorer\\Quick Launch\\User Pinned\\TaskBar"
     }, {
-      chrome::DIR_USER_DESKTOP,
+      base::DIR_USER_DESKTOP,
       NULL
     }, {
       base::DIR_START_MENU,
diff --git a/chrome/browser/ui/views/test/ui_test_utils_win.cc b/chrome/browser/ui/views/test/ui_test_utils_win.cc
index 81460a4..cad080b 100644
--- a/chrome/browser/ui/views/test/ui_test_utils_win.cc
+++ b/chrome/browser/ui/views/test/ui_test_utils_win.cc
@@ -15,7 +15,6 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/window_snapshot/window_snapshot.h"
-#include "chrome/common/chrome_paths.h"
 #include "ui/base/win/foreground_helper.h"
 #include "ui/ui_controls/ui_controls.h"
 #include "ui/views/focus/focus_manager.h"
@@ -144,7 +143,7 @@ bool SaveScreenSnapshotToDirectory(const FilePath& directory,
 bool SaveScreenSnapshotToDesktop(FilePath* screenshot_path) {
   FilePath desktop;
 
-  return PathService::Get(chrome::DIR_USER_DESKTOP, &desktop) &&
+  return PathService::Get(base::DIR_USER_DESKTOP, &desktop) &&
       SaveScreenSnapshotToDirectory(desktop, screenshot_path);
 }
 
diff --git a/chrome/browser/ui/web_applications/web_app_ui.cc b/chrome/browser/ui/web_applications/web_app_ui.cc
index 48ab162..6b764be 100644
--- a/chrome/browser/ui/web_applications/web_app_ui.cc
+++ b/chrome/browser/ui/web_applications/web_app_ui.cc
@@ -16,7 +16,6 @@
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_registrar.h"
@@ -191,7 +190,7 @@ void UpdateShortcutWorker::CheckExistingShortcuts() {
   } locations[] = {
     {
       shortcut_info_.create_on_desktop,
-      chrome::DIR_USER_DESKTOP,
+      base::DIR_USER_DESKTOP,
       NULL
     }, {
       shortcut_info_.create_in_applications_menu,
diff --git a/chrome/browser/web_applications/web_app_win.cc b/chrome/browser/web_applications/web_app_win.cc
index cfc68e1..d1d5634 100644
--- a/chrome/browser/web_applications/web_app_win.cc
+++ b/chrome/browser/web_applications/web_app_win.cc
@@ -15,7 +15,6 @@
 #include "base/utf_string_conversions.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
-#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/gfx/icon_util.h"
@@ -82,7 +81,7 @@ std::vector<FilePath> GetShortcutPaths(
   } locations[] = {
     {
       shortcut_info.create_on_desktop,
-      chrome::DIR_USER_DESKTOP,
+      base::DIR_USER_DESKTOP,
       NULL
     }, {
       shortcut_info.create_in_applications_menu,
diff --git a/chrome/common/chrome_paths.cc b/chrome/common/chrome_paths.cc
index 62c5b37..dd098ab 100644
--- a/chrome/common/chrome_paths.cc
+++ b/chrome/common/chrome_paths.cc
@@ -208,10 +208,6 @@ bool PathProvider(int key, FilePath* result) {
       cur = cur.Append(FILE_PATH_LITERAL("Crash Reports"));
       create_dir = true;
       break;
-    case chrome::DIR_USER_DESKTOP:
-      if (!GetUserDesktop(&cur))
-        return false;
-      break;
     case chrome::DIR_RESOURCES:
 #if defined(OS_MACOSX)
       cur = base::mac::FrameworkBundlePath();
diff --git a/chrome/common/chrome_paths.h b/chrome/common/chrome_paths.h
index ef8e95d..64813a7 100644
--- a/chrome/common/chrome_paths.h
+++ b/chrome/common/chrome_paths.h
@@ -23,7 +23,6 @@ enum {
                                 // (the one that isn't in use).
 #endif
   DIR_CRASH_DUMPS,              // Directory where crash dumps are written.
-  DIR_USER_DESKTOP,             // Directory that correspond to the desktop.
   DIR_RESOURCES,                // Directory containing separate file resources
                                 // used by Chrome at runtime.
   DIR_INSPECTOR,                // Directory where web inspector is located.
diff --git a/chrome/common/chrome_paths_android.cc b/chrome/common/chrome_paths_android.cc
index d647806..6a7d9ea 100644
--- a/chrome/common/chrome_paths_android.cc
+++ b/chrome/common/chrome_paths_android.cc
@@ -48,11 +48,6 @@ bool GetUserVideosDirectory(FilePath* result) {
   return false;
 }
 
-bool GetUserDesktop(FilePath* result) {
-  NOTIMPLEMENTED() << "Android doesn't support GetUserDesktop";
-  return false;
-}
-
 bool ProcessNeedsProfileDir(const std::string& process_type) {
   return true;
 }
diff --git a/chrome/common/chrome_paths_internal.h b/chrome/common/chrome_paths_internal.h
index 40e9742..60fab11 100644
--- a/chrome/common/chrome_paths_internal.h
+++ b/chrome/common/chrome_paths_internal.h
@@ -65,9 +65,6 @@ bool GetUserPicturesDirectory(FilePath* result);
 // Gets the path to the user's videos directory.
 bool GetUserVideosDirectory(FilePath* result);
 
-// The path to the user's desktop.
-bool GetUserDesktop(FilePath* result);
-
 #if defined(OS_MACOSX)
 // The "versioned directory" is a directory in the browser .app bundle.  It
 // contains the bulk of the application, except for the things that the system
diff --git a/chrome/common/chrome_paths_linux.cc b/chrome/common/chrome_paths_linux.cc
index 321032f..babb501 100644
--- a/chrome/common/chrome_paths_linux.cc
+++ b/chrome/common/chrome_paths_linux.cc
@@ -37,7 +37,8 @@ bool GetUserMediaDirectory(const std::string& xdg_name,
   FilePath home = file_util::GetHomeDir();
   if (*result != home) {
     FilePath desktop;
-    GetUserDesktop(&desktop);
+    if (!PathService::Get(base::DIR_USER_DESKTOP, &desktop))
+      return false;
     if (*result != desktop) {
       return true;
     }
@@ -142,11 +143,6 @@ bool GetUserVideosDirectory(FilePath* result) {
   return GetUserMediaDirectory("VIDEOS", kVideosDir, result);
 }
 
-bool GetUserDesktop(FilePath* result) {
-  *result = GetXDGUserDirectory("DESKTOP", "Desktop");
-  return true;
-}
-
 bool ProcessNeedsProfileDir(const std::string& process_type) {
   // For now we have no reason to forbid this on Linux as we don't
   // have the roaming profile troubles there. Moreover the Linux breakpad needs
diff --git a/chrome/common/chrome_paths_mac.mm b/chrome/common/chrome_paths_mac.mm
index f933796..5b63f47 100644
--- a/chrome/common/chrome_paths_mac.mm
+++ b/chrome/common/chrome_paths_mac.mm
@@ -147,10 +147,6 @@ bool GetUserVideosDirectory(FilePath* result) {
   return base::mac::GetUserDirectory(NSMoviesDirectory, result);
 }
 
-bool GetUserDesktop(FilePath* result) {
-  return base::mac::GetUserDirectory(NSDesktopDirectory, result);
-}
-
 FilePath GetVersionedDirectory() {
   if (g_override_versioned_directory)
     return *g_override_versioned_directory;
diff --git a/chrome/common/chrome_paths_win.cc b/chrome/common/chrome_paths_win.cc
index f0319a2..bb4036d 100644
--- a/chrome/common/chrome_paths_win.cc
+++ b/chrome/common/chrome_paths_win.cc
@@ -131,10 +131,6 @@ bool GetUserVideosDirectory(FilePath* result) {
   return GetUserDirectory(CSIDL_MYVIDEO, result);
 }
 
-bool GetUserDesktop(FilePath* result) {
-  return GetUserDirectory(CSIDL_DESKTOPDIRECTORY, result);
-}
-
 bool ProcessNeedsProfileDir(const std::string& process_type) {
   // On windows we don't want subprocesses other than the browser process and
   // service processes to be able to use the profile directory because if it
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index f6e696c..3e2d19a 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -1098,43 +1098,15 @@ bool ShellUtil::GetChromeShortcutName(BrowserDistribution* dist,
 }
 
 bool ShellUtil::GetDesktopPath(bool system_level, FilePath* path) {
-  wchar_t desktop[MAX_PATH];
-  int dir = system_level ? CSIDL_COMMON_DESKTOPDIRECTORY :
-                           CSIDL_DESKTOPDIRECTORY;
-  if (FAILED(SHGetFolderPath(NULL, dir, NULL, SHGFP_TYPE_CURRENT, desktop)))
-    return false;
-  *path = FilePath(desktop);
-  return true;
+  int dir_key = system_level ? base::DIR_COMMON_DESKTOP :
+                               base::DIR_USER_DESKTOP;
+  return PathService::Get(dir_key, path);
 }
 
 bool ShellUtil::GetQuickLaunchPath(bool system_level, FilePath* path) {
-  if (system_level) {
-    wchar_t qlaunch[MAX_PATH];
-    // We are accessing GetDefaultUserProfileDirectory this way so that we do
-    // not have to declare dependency to Userenv.lib for chrome.exe
-    typedef BOOL (WINAPI *PROFILE_FUNC)(LPWSTR, LPDWORD);
-    HMODULE module = LoadLibrary(L"Userenv.dll");
-    PROFILE_FUNC p = reinterpret_cast<PROFILE_FUNC>(GetProcAddress(module,
-        "GetDefaultUserProfileDirectoryW"));
-    DWORD size = _countof(qlaunch);
-    if ((p == NULL) || ((p)(qlaunch, &size) != TRUE))
-      return false;
-    *path = FilePath(qlaunch);
-    if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
-      *path = path->AppendASCII("AppData");
-      *path = path->AppendASCII("Roaming");
-    } else {
-      *path = path->AppendASCII("Application Data");
-    }
-  } else {
-    if (!PathService::Get(base::DIR_APP_DATA, path)) {
-      return false;
-    }
-  }
-  *path = path->AppendASCII("Microsoft");
-  *path = path->AppendASCII("Internet Explorer");
-  *path = path->AppendASCII("Quick Launch");
-  return true;
+  int dir_key = system_level ? base::DIR_DEFAULT_USER_QUICK_LAUNCH :
+                               base::DIR_USER_QUICK_LAUNCH;
+  return PathService::Get(dir_key, path);
 }
 
 void ShellUtil::GetRegisteredBrowsers(
diff --git a/chrome/installer/util/shell_util_unittest.cc b/chrome/installer/util/shell_util_unittest.cc
index 298a9a4..1bbe661 100644
--- a/chrome/installer/util/shell_util_unittest.cc
+++ b/chrome/installer/util/shell_util_unittest.cc
@@ -2,14 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <shlobj.h>
-
 #include <fstream>
 #include <vector>
 
 #include "base/file_util.h"
-#include "base/path_service.h"
 #include "base/md5.h"
+#include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string16.h"
 #include "base/string_util.h"
@@ -24,17 +22,27 @@
 
 namespace {
 
-class ShellUtilTestWithDirAndDist : public testing::Test {
+class ShellUtilShortcutTest : public testing::Test {
  protected:
-  virtual void SetUp() {
-    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+  virtual void SetUp() OVERRIDE {
     dist_ = BrowserDistribution::GetDistribution();
     ASSERT_TRUE(dist_ != NULL);
+
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    ASSERT_TRUE(fake_user_desktop_.CreateUniqueTempDir());
+    ASSERT_TRUE(fake_common_desktop_.CreateUniqueTempDir());
+    ASSERT_TRUE(PathService::Override(base::DIR_USER_DESKTOP,
+                                      fake_user_desktop_.path()));
+    ASSERT_TRUE(PathService::Override(base::DIR_COMMON_DESKTOP,
+                                      fake_common_desktop_.path()));
   }
 
   BrowserDistribution* dist_;
 
   ScopedTempDir temp_dir_;
+
+  ScopedTempDir fake_user_desktop_;
+  ScopedTempDir fake_common_desktop_;
 };
 
 // Returns the status of a call to base::win::VerifyShorcut for the properties
@@ -58,7 +66,7 @@ base::win::VerifyShortcutStatus VerifyChromeShortcut(
 }
 
 // Test that we can open archives successfully.
-TEST_F(ShellUtilTestWithDirAndDist, UpdateChromeShortcutTest) {
+TEST_F(ShellUtilShortcutTest, UpdateChromeShortcut) {
   // Create an executable in test path by copying ourself to it.
   wchar_t exe_full_path_str[MAX_PATH];
   EXPECT_FALSE(::GetModuleFileName(NULL, exe_full_path_str, MAX_PATH) == 0);
@@ -122,13 +130,7 @@ TEST_F(ShellUtilTestWithDirAndDist, UpdateChromeShortcutTest) {
             VerifyChromeShortcut(exe_path, shortcut_path, description2, 1));
 }
 
-TEST_F(ShellUtilTestWithDirAndDist, CreateChromeDesktopShortcutTest) {
-  // Run this test on Vista+ only if we are running elevated.
-  if (base::win::GetVersion() > base::win::VERSION_XP && !IsUserAnAdmin()) {
-    LOG(ERROR) << "Must be admin to run this test on Vista+";
-    return;
-  }
-
+TEST_F(ShellUtilShortcutTest, CreateChromeDesktopShortcut) {
   // Create an executable in test path by copying ourself to it.
   wchar_t exe_full_path_str[MAX_PATH];
   EXPECT_FALSE(::GetModuleFileName(NULL, exe_full_path_str, MAX_PATH) == 0);
@@ -306,16 +308,18 @@ TEST_F(ShellUtilTestWithDirAndDist, CreateChromeDesktopShortcutTest) {
       profile_names));
 }
 
-TEST_F(ShellUtilTestWithDirAndDist, BuildAppModelIdBasic) {
+TEST(ShellUtilTest, BuildAppModelIdBasic) {
   std::vector<string16> components;
-  const string16 base_app_id(dist_->GetBaseAppId());
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  const string16 base_app_id(dist->GetBaseAppId());
   components.push_back(base_app_id);
   ASSERT_EQ(base_app_id, ShellUtil::BuildAppModelId(components));
 }
 
-TEST_F(ShellUtilTestWithDirAndDist, BuildAppModelIdManySmall) {
+TEST(ShellUtilTest, BuildAppModelIdManySmall) {
   std::vector<string16> components;
-  const string16 suffixed_app_id(dist_->GetBaseAppId().append(L".gab"));
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  const string16 suffixed_app_id(dist->GetBaseAppId().append(L".gab"));
   components.push_back(suffixed_app_id);
   components.push_back(L"Default");
   components.push_back(L"Test");
@@ -323,7 +327,7 @@ TEST_F(ShellUtilTestWithDirAndDist, BuildAppModelIdManySmall) {
             ShellUtil::BuildAppModelId(components));
 }
 
-TEST_F(ShellUtilTestWithDirAndDist, BuildAppModelIdLongUsernameNormalProfile) {
+TEST(ShellUtilTest, BuildAppModelIdLongUsernameNormalProfile) {
   std::vector<string16> components;
   const string16 long_appname(
       L"Chrome.a_user_who_has_a_crazy_long_name_with_some_weird@symbols_in_it_"
@@ -334,7 +338,7 @@ TEST_F(ShellUtilTestWithDirAndDist, BuildAppModelIdLongUsernameNormalProfile) {
             ShellUtil::BuildAppModelId(components));
 }
 
-TEST_F(ShellUtilTestWithDirAndDist, BuildAppModelIdLongEverything) {
+TEST(ShellUtilTest, BuildAppModelIdLongEverything) {
   std::vector<string16> components;
   const string16 long_appname(
       L"Chrome.a_user_who_has_a_crazy_long_name_with_some_weird@symbols_in_it_"