chrome_paths: refactor and sanitize cache directory handling

Previously we had a bunch of logic in profile_impl.cc that computed
where the cache directory lives.  This change relocates it to
chrome_paths.cc.

Some background on cache directories.
There are two possible places to store cache files:
1) in the user data directory
2) in an OS-specific user cache directory

On Windows, we always pick (1).  On Mac and Linux, we currently use
(2) in some circumstances.  This patch changes both Mac and Linux to
have the same behavior with respect to (2).

The Mac/Linux shared behavior is that if the profile directory
is in the standard location for profiles, we put the cache files
into a matching directory name in the standard system cache directory
(e.g. on Linux if you're using ~/.config/google-chrome, your cache
ends up in ~/.cache/google-chrome; on Mac, the directories are
~/Library/Application Support versus ~/Library/Caches).  If your user
data directory is not in the standard location, we use behavior (1).

The semantic changes of this patch should be:
- On Mac, previously we checked whether the (2) directory had some
  particular subdirectories already when picking which one to use.
  This was removed; which directory is used is solely a question of
  whether the profile directory is in the standard location.
  I think the previous behavior was unpredictable.
- On Linux, previously we only used behavior (2) if you hadn't changed
  your user-data-directory at all.  Now, to match Mac, as long as
  your user-data-dir is in the standard place, you use the system
  cache dir.  So e.g. using ~/.config/foobar puts your cache in
  ~/.cache/foobar.
- On Linux, previously the default cache would end up as directories
  under ~/.cache/google-chrome/; now it ends up as directories under
  ~/.cache/google-chrome/Default/.

(In all instances above, on Linux we continue to obey $XDG_CACHE_HOME.)

BUG=59824
TEST=New test ChromePaths.UserCacheDir

Review URL: http://codereview.chromium.org/5123004

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

13 files changed, 121 insertions, 70 deletions

diff --git a/base/base_paths.h b/base/base_paths.h
index 0f9be04..72cff15 100644
--- a/base/base_paths.h
+++ b/base/base_paths.h
@@ -33,10 +33,11 @@ enum {
                     // for tests that need to locate various resources.  It
                     // should not be used outside of test code.
 #if defined(OS_POSIX)
-  DIR_USER_CACHE,   // Directory where user cache data resides. The Chromium
-                    // browser cache can be a subdirectory of DIR_USER_CACHE.
-                    // This is $XDG_CACHE_HOME on Linux and ~/Library/Caches
-                    // on Mac.
+  DIR_CACHE,    // Directory where to put cache data.  Note this is
+                // *not* where the browser cache lives, but the
+                // browser cache can be a subdirectory.
+                // This is $XDG_CACHE_HOME on Linux and
+                // ~/Library/Caches on Mac.
 #endif
 
   PATH_END
diff --git a/base/base_paths_linux.cc b/base/base_paths_linux.cc
index 6d837f2..701b983 100644
--- a/base/base_paths_linux.cc
+++ b/base/base_paths_linux.cc
@@ -105,7 +105,7 @@ bool PathProviderPosix(int key, FilePath* result) {
                  << "Try running from your chromium/src directory.";
       return false;
     }
-    case base::DIR_USER_CACHE:
+    case base::DIR_CACHE:
       scoped_ptr<base::Environment> env(base::Environment::Create());
       FilePath cache_dir(base::nix::GetXDGDirectory(env.get(), "XDG_CACHE_HOME",
                                                     ".cache"));
diff --git a/base/base_paths_mac.mm b/base/base_paths_mac.mm
index b9225d8..97413ba 100644
--- a/base/base_paths_mac.mm
+++ b/base/base_paths_mac.mm
@@ -47,7 +47,7 @@ bool PathProviderMac(int key, FilePath* result) {
     case base::FILE_MODULE: {
       return GetNSExecutablePath(result);
     }
-    case base::DIR_USER_CACHE:
+    case base::DIR_CACHE:
       return mac_util::GetUserDirectory(NSCachesDirectory, result);
     case base::DIR_APP_DATA:
       return mac_util::GetUserDirectory(NSApplicationSupportDirectory, result);
diff --git a/base/path_service.cc b/base/path_service.cc
index 9eed56d..8660c42 100644
--- a/base/path_service.cc
+++ b/base/path_service.cc
@@ -215,14 +215,6 @@ bool PathService::Get(int key, std::wstring* result) {
 }
 #endif
 
-// TODO(evan): remove me -- see comments in header.
-bool PathService::IsOverridden(int key) {
-  PathData* path_data = GetPathData();
-  DCHECK(path_data);
-  AutoLock scoped_lock(path_data->lock);
-  return path_data->overrides.find(key) != path_data->overrides.end();
-}
-
 bool PathService::Override(int key, const FilePath& path) {
   PathData* path_data = GetPathData();
   DCHECK(path_data);
diff --git a/base/path_service.h b/base/path_service.h
index 6873e1f..4d99cdc 100644
--- a/base/path_service.h
+++ b/base/path_service.h
@@ -45,11 +45,6 @@ class PathService {
   // over the lifetime of the app, so this method should be used with caution.
   static bool Override(int key, const FilePath& path);
 
-  // Return whether a path was overridden.
-  // TODO(evan): temporarily restoring this to quick-fix a regression.
-  // Remove me again once it's fixed properly.
-  static bool IsOverridden(int key);
-
   // To extend the set of supported keys, you can register a path provider,
   // which is just a function mirroring PathService::Get.  The ProviderFunc
   // returns false if it cannot provide a non-empty path for the given key.
diff --git a/base/path_service_unittest.cc b/base/path_service_unittest.cc
index c65f7a5..4c41aed 100644
--- a/base/path_service_unittest.cc
+++ b/base/path_service_unittest.cc
@@ -21,12 +21,6 @@ namespace {
 bool ReturnsValidPath(int dir_type) {
   FilePath path;
   bool result = PathService::Get(dir_type, &path);
-#if defined(OS_POSIX)
-  // If chromium has never been started on this account, the cache path will not
-  // exist.
-  if (dir_type == base::DIR_USER_CACHE)
-    return result && !path.value().empty();
-#endif
   return result && !path.value().empty() && file_util::PathExists(path);
 }
 
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index 464c45c..c66428d 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -81,6 +81,7 @@
 #include "chrome/browser/webdata/web_data_service.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/common/json_pref_store.h"
 #include "chrome/common/notification_service.h"
@@ -172,11 +173,6 @@ FilePath GetMediaCachePath(const FilePath& base) {
   return base.Append(chrome::kMediaCacheDirname);
 }
 
-bool HasACacheSubdir(const FilePath &dir) {
-  return file_util::PathExists(GetCachePath(dir)) ||
-         file_util::PathExists(GetMediaCachePath(dir));
-}
-
 // Simple task to log the size of the current profile.
 class ProfileSizeTask : public Task {
  public:
@@ -280,46 +276,11 @@ ProfileImpl::ProfileImpl(const FilePath& path)
   // Convert active labs into switches. Modifies the current command line.
   about_flags::ConvertFlagsToSwitches(prefs, CommandLine::ForCurrentProcess());
 
-#if defined(OS_MACOSX)
-  // If the profile directory doesn't already have a cache directory and it
-  // is under ~/Library/Application Support, use a suitable cache directory
-  // under ~/Library/Caches.  For example, a profile directory of
-  // ~/Library/Application Support/Google/Chrome/MyProfileName that doesn't
-  // have a "Cache" or "MediaCache" subdirectory would use the cache directory
-  // ~/Library/Caches/Google/Chrome/MyProfileName.
-  //
-  // TODO(akalin): Come up with unit tests for this.
-  if (!HasACacheSubdir(path_)) {
-    FilePath app_data_path, user_cache_path;
-    if (PathService::Get(base::DIR_APP_DATA, &app_data_path) &&
-        PathService::Get(base::DIR_USER_CACHE, &user_cache_path) &&
-        app_data_path.AppendRelativePath(path_, &user_cache_path)) {
-      base_cache_path_ = user_cache_path;
-    }
-  }
-#elif defined(OS_POSIX)  // Posix minus Mac.
-  // See http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
-  // for a spec on where cache files go.  The net effect for most systems is we
-  // use ~/.cache/chromium/ for Chromium and ~/.cache/google-chrome/ for
-  // official builds.
-  // If we're using a different user-data-dir, though, fall through
-  // to the "normal" cache directory (a subdirectory of that).
-  // TODO(evan): all of this logic belongs in path_service; refactor and remove
-  // this IsOverriden business.
-  if (!PathService::IsOverridden(chrome::DIR_USER_DATA)) {
-#if defined(GOOGLE_CHROME_BUILD)
-    const char kCacheDir[] = "google-chrome";
-#else
-    const char kCacheDir[] = "chromium";
-#endif
-    PathService::Get(base::DIR_USER_CACHE, &base_cache_path_);
-    base_cache_path_ = base_cache_path_.Append(kCacheDir);
-    if (!file_util::PathExists(base_cache_path_))
-      file_util::CreateDirectory(base_cache_path_);
-  }
-#endif
-  if (base_cache_path_.empty())
-    base_cache_path_ = path_;
+  // It would be nice to use PathService for fetching this directory, but
+  // the cache directory depends on the profile directory, which isn't available
+  // to PathService.
+  chrome::GetUserCacheDirectory(path_, &base_cache_path_);
+  file_util::CreateDirectory(base_cache_path_);
 
   // Listen for theme installations from our original profile.
   registrar_.Add(this, NotificationType::THEME_INSTALLED,
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 045a4c0..74055cd 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1550,6 +1550,7 @@
         'browser/wrench_menu_model_unittest.cc',
         'common/bzip2_unittest.cc',
         'common/child_process_logging_mac_unittest.mm',
+        'common/chrome_paths_unittest.cc',
         'common/common_param_traits_unittest.cc',
         'common/content_settings_helper_unittest.cc',
         'common/deprecated/event_sys_unittest.cc',
diff --git a/chrome/common/chrome_paths_internal.h b/chrome/common/chrome_paths_internal.h
index 6671e25..dfe927d 100644
--- a/chrome/common/chrome_paths_internal.h
+++ b/chrome/common/chrome_paths_internal.h
@@ -21,6 +21,15 @@ bool GetDefaultUserDataDirectory(FilePath* result);
 // CF and Google Chrome want to share the same binaries.
 bool GetChromeFrameUserDataDirectory(FilePath* result);
 
+// Get the path to the user's cache directory.  This is normally the
+// same as the profile directory, but on Linux it can also be
+// $XDG_CACHE_HOME and on Mac it can be under ~/Library/Caches.
+// Note that the Chrome cache directories are actually subdirectories
+// of this directory, with names like "Cache" and "Media Cache".
+// This will always fill in |result| with a directory, sometimes
+// just |profile_dir|.
+void GetUserCacheDirectory(const FilePath& profile_dir, FilePath* result);
+
 // Get the path to the user's documents directory.
 bool GetUserDocumentsDirectory(FilePath* result);
 
diff --git a/chrome/common/chrome_paths_linux.cc b/chrome/common/chrome_paths_linux.cc
index 42c6ae7..a7fdb07 100644
--- a/chrome/common/chrome_paths_linux.cc
+++ b/chrome/common/chrome_paths_linux.cc
@@ -6,6 +6,7 @@
 
 #include "base/environment.h"
 #include "base/file_util.h"
+#include "base/path_service.h"
 #include "base/scoped_ptr.h"
 #include "base/nix/xdg_util.h"
 
@@ -28,6 +29,32 @@ bool GetDefaultUserDataDirectory(FilePath* result) {
   return true;
 }
 
+void GetUserCacheDirectory(const FilePath& profile_dir, FilePath* result) {
+  // See http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
+  // for a spec on where cache files go.  Our rule is:
+  // - if the user-data-dir in the standard place,
+  //     use same subdirectory of the cache directory.
+  //     (this maps ~/.config/google-chrome to ~/.cache/google-chrome as well
+  //      as the same thing for ~/.config/chromium)
+  // - otherwise, use the profile dir directly.
+
+  // Default value in cases where any of the following fails.
+  *result = profile_dir;
+
+  scoped_ptr<base::Environment> env(base::Environment::Create());
+
+  FilePath cache_dir;
+  if (!PathService::Get(base::DIR_CACHE, &cache_dir))
+    return;
+  FilePath config_dir(
+      base::nix::GetXDGDirectory(env.get(), "XDG_CONFIG_HOME", ".config"));
+
+  if (!config_dir.AppendRelativePath(profile_dir, &cache_dir))
+    return;
+
+  *result = cache_dir;
+}
+
 bool GetChromeFrameUserDataDirectory(FilePath* result) {
   scoped_ptr<base::Environment> env(base::Environment::Create());
   FilePath config_dir(
diff --git a/chrome/common/chrome_paths_mac.mm b/chrome/common/chrome_paths_mac.mm
index b87b397..57ed3bd 100644
--- a/chrome/common/chrome_paths_mac.mm
+++ b/chrome/common/chrome_paths_mac.mm
@@ -48,6 +48,28 @@ bool GetUserDocumentsDirectory(FilePath* result) {
   return mac_util::GetUserDirectory(NSDocumentDirectory, result);
 }
 
+void GetUserCacheDirectory(const FilePath& profile_dir, FilePath* result) {
+  // If the profile directory is under ~/Library/Application Support,
+  // use a suitable cache directory under ~/Library/Caches.  For
+  // example, a profile directory of ~/Library/Application
+  // Support/Google/Chrome/MyProfileName would use the cache directory
+  // ~/Library/Caches/Google/Chrome/MyProfileName.
+
+  // Default value in cases where any of the following fails.
+  *result = profile_dir;
+
+  FilePath app_data_dir;
+  if (!PathService::Get(base::DIR_APP_DATA, &app_data_dir))
+    return;
+  FilePath cache_dir;
+  if (!PathService::Get(base::DIR_CACHE, &cache_dir))
+    return;
+  if (!app_data_dir.AppendRelativePath(profile_dir, &cache_dir))
+    return;
+
+  *result = cache_dir;
+}
+
 bool GetUserDownloadsDirectory(FilePath* result) {
   return mac_util::GetUserDirectory(NSDownloadsDirectory, result);
 }
diff --git a/chrome/common/chrome_paths_unittest.cc b/chrome/common/chrome_paths_unittest.cc
new file mode 100644
index 0000000..23bc7d4
--- /dev/null
+++ b/chrome/common/chrome_paths_unittest.cc
@@ -0,0 +1,44 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/chrome_paths_internal.h"
+
+#include <stdlib.h>
+
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// Test the behavior of chrome::GetUserCacheDirectory.
+// See that function's comments for discussion of the subtleties.
+TEST(ChromePaths, UserCacheDir) {
+  FilePath test_profile_dir, cache_dir;
+#if defined(OS_MACOSX)
+  ASSERT_TRUE(PathService::Get(base::DIR_APP_DATA, &test_profile_dir));
+  test_profile_dir = test_profile_dir.Append("foobar");
+  FilePath expected_cache_dir;
+  ASSERT_TRUE(PathService::Get(base::DIR_CACHE, &expected_cache_dir));
+  expected_cache_dir = expected_cache_dir.Append("foobar");
+#elif(OS_POSIX)
+  FilePath homedir = file_util::GetHomeDir();
+  // Note: we assume XDG_CACHE_HOME/XDG_CONFIG_HOME are at their
+  // default settings.
+  test_profile_dir = homedir.Append(".config/foobar");
+  FilePath expected_cache_dir = homedir.Append(".cache/foobar");
+#endif
+
+  // Verify that a profile in the special platform-specific source
+  // location ends up in the special target location.
+#if !defined(OS_WIN)  // No special behavior on Windows.
+  chrome::GetUserCacheDirectory(test_profile_dir, &cache_dir);
+  EXPECT_EQ(expected_cache_dir.value(), cache_dir.value());
+#endif
+
+  // Verify that a profile in some other random directory doesn't use
+  // the special cache dir.
+  test_profile_dir = FilePath(FILE_PATH_LITERAL("/some/other/path"));
+  chrome::GetUserCacheDirectory(test_profile_dir, &cache_dir);
+  EXPECT_EQ(test_profile_dir.value(), cache_dir.value());
+}
diff --git a/chrome/common/chrome_paths_win.cc b/chrome/common/chrome_paths_win.cc
index c9fee0f..74a730d 100644
--- a/chrome/common/chrome_paths_win.cc
+++ b/chrome/common/chrome_paths_win.cc
@@ -38,6 +38,11 @@ bool GetChromeFrameUserDataDirectory(FilePath* result) {
   return true;
 }
 
+void GetUserCacheDirectory(const FilePath& profile_dir, FilePath* result) {
+  // This function does more complicated things on Mac/Linux.
+  *result = profile_dir;
+}
+
 bool GetUserDocumentsDirectory(FilePath* result) {
   wchar_t path_buf[MAX_PATH];
   if (FAILED(SHGetFolderPath(NULL, CSIDL_MYDOCUMENTS, NULL,