chrome_paths: refactor and sanitize cache directory handling

chrome/browser/profile_impl.cc

 // 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/browser/profile_impl.h"
 
 #include "app/resource_bundle.h"
 #include "base/command_line.h"
 #include "base/environment.h"
 #include "base/file_path.h"
@@ skipping 60 matching lines @@
 #include "chrome/browser/tabs/pinned_tab_service.h"
 #include "chrome/browser/themes/browser_theme_provider.h"
 #include "chrome/browser/transport_security_persister.h"
 #include "chrome/browser/user_style_sheet_watcher.h"
 #include "chrome/browser/visitedlink_event_listener.h"
 #include "chrome/browser/visitedlink_master.h"
 #include "chrome/browser/web_resource/web_resource_service.h"
 #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"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "grit/browser_resources.h"
 #include "grit/locale_settings.h"
 #include "net/base/transport_security_state.h"
 #include "webkit/database/database_tracker.h"
 
@@ skipping 71 matching lines @@
 }
 
 FilePath GetCachePath(const FilePath& base) {
   return base.Append(chrome::kCacheDirname);
 }
 
 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:
   explicit ProfileSizeTask(const FilePath& path) : path_(path) {}
   virtual ~ProfileSizeTask() {}
 
   virtual void Run();
  private:
   FilePath path_;
 };
@@ skipping 83 matching lines @@
 
   PrefService* prefs = GetPrefs();
   pref_change_registrar_.Init(prefs);
   pref_change_registrar_.Add(prefs::kSpellCheckDictionary, this);
   pref_change_registrar_.Add(prefs::kEnableSpellCheck, this);
   pref_change_registrar_.Add(prefs::kEnableAutoSpellCorrect, this);
 
   // 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())
+  // 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.
+  if (!chrome::GetUserCacheDirectory(path_, &base_cache_path_))

[akalin, 2010/11/22 23:22:44]

All implementations of GetUserCacheDirectory() already set the output dir to the
input profile dir and return true for the default case; why not make
GetUserCacheDirectory return void, or change the implementations to return
'false' as the default case?

     base_cache_path_ = path_;
+  file_util::CreateDirectory(base_cache_path_);
 
   // Listen for theme installations from our original profile.
   registrar_.Add(this, NotificationType::THEME_INSTALLED,
                  Source<Profile>(GetOriginalProfile()));
 
   // Listen for bookmark model load, to bootstrap the sync service.
   registrar_.Add(this, NotificationType::BOOKMARK_MODEL_LOADED,
                  Source<Profile>(this));
 
   ssl_config_service_manager_.reset(
@@ skipping 1006 matching lines @@
 #if defined(OS_CHROMEOS)
 chromeos::ProxyConfigServiceImpl*
     ProfileImpl::GetChromeOSProxyConfigServiceImpl() {
   if (!chromeos_proxy_config_service_impl_) {
     chromeos_proxy_config_service_impl_ =
         new chromeos::ProxyConfigServiceImpl();
   }
   return chromeos_proxy_config_service_impl_;
 }
 #endif  // defined(OS_CHROMEOS)