Revert 56738 - Fix CheckFalseTest.CheckFails on Linux after my change to ui_test.

This makes PathService clear its cache after overriding a path.
We have many paths depending on each other, so this is necessary
to avoid inconsistencies.

TEST=ui_tests in Release mode
BUG=49838

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

TBR=phajdan.jr@chromium.org
Review URL: http://codereview.chromium.org/3169031

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

5 files changed, 27 insertions, 51 deletions

diff --git a/base/path_service.cc b/base/path_service.cc
index ee94c16..57cbc33 100644
--- a/base/path_service.cc
+++ b/base/path_service.cc
@@ -31,6 +31,7 @@ namespace base {
 namespace {
 
 typedef base::hash_map<int, FilePath> PathMap;
+typedef base::hash_set<int> PathSet;
 
 // We keep a linked list of providers.  In a debug build we ensure that no two
 // providers claim overlapping keys.
@@ -93,8 +94,8 @@ static Provider base_provider_posix = {
 
 struct PathData {
   Lock      lock;
-  PathMap   cache;      // Cache mappings from path key to path value.
-  PathMap   overrides;  // Track path overrides.
+  PathMap   cache;      // Track mappings from path key to path value.
+  PathSet   overrides;  // Track whether a path has been overridden.
   Provider* providers;  // Linked list of path service providers.
 
   PathData() {
@@ -140,20 +141,6 @@ bool PathService::GetFromCache(int key, FilePath* result) {
 }
 
 // static
-bool PathService::GetFromOverrides(int key, FilePath* result) {
-  PathData* path_data = GetPathData();
-  AutoLock scoped_lock(path_data->lock);
-
-  // check for an overriden version.
-  PathMap::const_iterator it = path_data->overrides.find(key);
-  if (it != path_data->overrides.end()) {
-    *result = it->second;
-    return true;
-  }
-  return false;
-}
-
-// static
 void PathService::AddToCache(int key, const FilePath& path) {
   PathData* path_data = GetPathData();
   AutoLock scoped_lock(path_data->lock);
@@ -175,15 +162,8 @@ bool PathService::Get(int key, FilePath* result) {
   if (key == base::DIR_CURRENT)
     return file_util::GetCurrentDirectory(result);
 
-  if (GetFromCache(key, result)) {
-    LOG(ERROR) << "Key: " << key << " returning " << result->value() << " from cache.";
+  if (GetFromCache(key, result))
     return true;
-  }
-
-  if (GetFromOverrides(key, result)) {
-    LOG(ERROR) << "Key: " << key << " returning " << result->value() << " from overrides.";
-    return true;
-  }
 
   FilePath path;
 
@@ -204,7 +184,6 @@ bool PathService::Get(int key, FilePath* result) {
   AddToCache(key, path);
 
   *result = path;
-  LOG(ERROR) << "Key: " << key << " returning " << result->value() << ".";
   return true;
 }
 
@@ -220,6 +199,14 @@ bool PathService::Get(int key, std::wstring* result) {
 }
 #endif
 
+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);
@@ -241,14 +228,8 @@ bool PathService::Override(int key, const FilePath& path) {
     return false;
 
   AutoLock scoped_lock(path_data->lock);
-
-  // Clear the cache now. Some of its entries could have depended
-  // on the value we are overriding, and are now out of sync with reality.
-  path_data->cache.clear();
-
   path_data->cache[key] = file_path;
-  path_data->overrides[key] = file_path;
-
+  path_data->overrides.insert(key);
   return true;
 }
 
diff --git a/base/path_service.h b/base/path_service.h
index 4d99cdc..2b59247 100644
--- a/base/path_service.h
+++ b/base/path_service.h
@@ -45,6 +45,9 @@ 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.
+  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.
@@ -62,7 +65,6 @@ class PathService {
                                int key_end);
  private:
   static bool GetFromCache(int key, FilePath* path);
-  static bool GetFromOverrides(int key, FilePath* path);
   static void AddToCache(int key, const FilePath& path);
 };
 
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index 10ef0e7..84a2b17 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -305,15 +305,17 @@ ProfileImpl::ProfileImpl(const FilePath& path)
   // 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 (!PathService::IsOverridden(chrome::DIR_USER_DATA)) {
 #if defined(GOOGLE_CHROME_BUILD)
-  const char kCacheDir[] = "google-chrome";
+    const char kCacheDir[] = "google-chrome";
 #else
-  const char kCacheDir[] = "chromium";
+    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_);
+    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_;
diff --git a/chrome/common/logging_chrome_uitest.cc b/chrome/common/logging_chrome_uitest.cc
index 216f156..328a5a49 100644
--- a/chrome/common/logging_chrome_uitest.cc
+++ b/chrome/common/logging_chrome_uitest.cc
@@ -130,6 +130,9 @@ class CheckFalseTest : public UITest {
 #elif defined(OS_MACOSX)
 // Crash service doesn't exist for the Mac yet: http://crbug.com/45243
 #define CheckFails DISABLED_CheckFails
+#elif defined(OS_LINUX)
+// TODO(phajdan) Fix this - http://crbug.com/49838
+#define CheckFails FAILS_CheckFails
 #endif
 // Launch the app in assertion test mode, then close the app.
 TEST_F(CheckFalseTest, CheckFails) {
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 2ca1690..b321ebf 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -15,7 +15,6 @@
 #include "app/sql/connection.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
-#include "base/environment.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
@@ -386,17 +385,6 @@ void UITestBase::LaunchBrowser(const CommandLine& arguments,
     // side. Using PathService seems to be the most reliable, consistent way
     // to do that.
     ASSERT_TRUE(PathService::Override(chrome::DIR_USER_DATA, user_data_dir()));
-
-#if defined(OS_LINUX)
-    // Make sure the cache directory is inside our clear profile. Otherwise
-    // the cache may contain data from earlier tests that could break the
-    // current test.
-    //
-    // Note: we use an environment variable here, because we have to pass the
-    // value to the child process. This is the simplest way to do it.
-    scoped_ptr<base::Environment> env(base::Environment::Create());
-    env->SetVar("XDG_CACHE_HOME", user_data_dir().value());
-#endif
   }
 
   if (!template_user_data_.empty()) {