Skip to content

Commit

Permalink
chrome_paths: refactor and sanitize cache directory handling
Browse files Browse the repository at this point in the history
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@67191 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
evan@chromium.org committed Nov 24, 2010
1 parent 54c8336 commit 8c61914
Show file tree
Hide file tree
Showing 13 changed files with 123 additions and 66 deletions.
9 changes: 5 additions & 4 deletions base/base_paths.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion base/base_paths_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
2 changes: 1 addition & 1 deletion base/base_paths_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 0 additions & 8 deletions base/path_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions base/path_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions base/path_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ 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
// If chromium has never been started on this account, the cache path may not
// exist.
if (dir_type == base::DIR_USER_CACHE)
if (dir_type == base::DIR_CACHE)
return result && !path.value().empty();
#endif
return result && !path.value().empty() && file_util::PathExists(path);
Expand Down
51 changes: 6 additions & 45 deletions chrome/browser/profile_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions chrome/chrome_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,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',
Expand Down
9 changes: 9 additions & 0 deletions chrome/common/chrome_paths_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
27 changes: 27 additions & 0 deletions chrome/common/chrome_paths_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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(
Expand Down
22 changes: 22 additions & 0 deletions chrome/common/chrome_paths_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
44 changes: 44 additions & 0 deletions chrome/common/chrome_paths_unittest.cc
Original file line number Diff line number Diff line change
@@ -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());
}
5 changes: 5 additions & 0 deletions chrome/common/chrome_paths_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 8c61914

Please sign in to comment.