Add PathService::RemoveOverride to clear path overrides.

base/path_service.cc

Index: base/path_service.cc
diff --git a/base/path_service.cc b/base/path_service.cc
index a3b882c2a9e8f76f668ed71b8549706059aecf51..be495dc84d6a0402c4757db771f60df69c2b3072 100644
--- a/base/path_service.cc
+++ b/base/path_service.cc
@@ -140,14 +140,14 @@ static PathData* GetPathData() {
   return g_path_data.Pointer();
 }
 
-}  // namespace
-
-
-// static
-bool PathService::GetFromCache(int key, FilePath* result) {
-  PathData* path_data = GetPathData();
-  base::AutoLock scoped_lock(path_data->lock);
+// Adds a value to the cache map. |path_data| should be locked by the caller!
+void AddToCache(int key, const FilePath& path, PathData* path_data) {
+  // Save the computed path in our cache.
+  path_data->cache[key] = path;
+}
 
+// Tries to find |key| in the cache. |path_data| should be locked by the caller!
+bool GetFromCache(int key, const PathData* path_data, FilePath* result) {
   // check for a cached version
   PathMap::const_iterator it = path_data->cache.find(key);
   if (it != path_data->cache.end()) {
@@ -157,27 +157,20 @@ bool PathService::GetFromCache(int key, FilePath* result) {
   return false;
 }
 
-// static
-bool PathService::GetFromOverrides(int key, FilePath* result) {
-  PathData* path_data = GetPathData();
-  base::AutoLock scoped_lock(path_data->lock);
-
+// Tries to find |key| in the overrides map. |path_data| should be locked by the
+// caller!
+bool GetFromOverrides(int key, PathData* path_data, FilePath* result) {

[jar (doing other things), 2012/09/17 17:06:08]

A good readability pattern is to preface helper functions with the word "Locked"
if the lock was acquired before calling.  In this case, it would be:

LockedGetFromOverrides().

The same is true for:

LockedAddToCache();
and
LockedGetFromeCache();

etc.

[pastarmovj, 2012/09/18 14:27:13]

Done.

   // check for an overridden version.
   PathMap::const_iterator it = path_data->overrides.find(key);
   if (it != path_data->overrides.end()) {
+    AddToCache(key, it->second, path_data);
     *result = it->second;
     return true;
   }
   return false;
 }
 
-// static
-void PathService::AddToCache(int key, const FilePath& path) {
-  PathData* path_data = GetPathData();
-  base::AutoLock scoped_lock(path_data->lock);
-  // Save the computed path in our cache.
-  path_data->cache[key] = path;
-}
+}  // namespace
 
 // TODO(brettw): this function does not handle long paths (filename > MAX_PATH)
 // characters). This isn't supported very well by Windows right now, so it is
@@ -193,11 +186,14 @@ bool PathService::Get(int key, FilePath* result) {
   if (key == base::DIR_CURRENT)
     return file_util::GetCurrentDirectory(result);
 
-  if (GetFromCache(key, result))
-    return true;
+  {
+    base::AutoLock scoped_lock(path_data->lock);
+    if (GetFromCache(key, path_data, result))
+      return true;
 
-  if (GetFromOverrides(key, result))
-    return true;
+    if (GetFromOverrides(key, path_data, result))
+      return true;
+  }
 
   FilePath path;
 
@@ -215,17 +211,22 @@ bool PathService::Get(int key, FilePath* result) {
   if (path.empty())
     return false;
 
-  AddToCache(key, path);
+  {
+    base::AutoLock scoped_lock(path_data->lock);
+    AddToCache(key, path, path_data);
+  }
 
   *result = path;
   return true;
 }
 
+// static
 bool PathService::Override(int key, const FilePath& path) {
   // Just call the full function with true for the value of |create|.
   return OverrideAndCreateIfNeeded(key, path, true);
 }
 
+// static
 bool PathService::OverrideAndCreateIfNeeded(int key,
                                             const FilePath& path,
                                             bool create) {
@@ -258,38 +259,58 @@ bool PathService::OverrideAndCreateIfNeeded(int key,
   // 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;
 
   return true;
 }
 
+// static
+bool PathService::RemoveOverride(int key) {
+  PathData* path_data = GetPathData();
+  DCHECK(path_data);
+
+  base::AutoLock scoped_lock(path_data->lock);
+
+  if (path_data->overrides.find(key) == path_data->overrides.end())
+    return false;
+
+  // Clear the cache now. Some of its entries could have depended on the value
+  // we are going to remove, and are now out of sync.
+  path_data->cache.clear();
+
+  path_data->overrides.erase(key);
+
+  return true;
+}
+
+// static
 void PathService::RegisterProvider(ProviderFunc func, int key_start,
                                    int key_end) {
   PathData* path_data = GetPathData();
   DCHECK(path_data);
   DCHECK_GT(key_end, key_start);
 
-  base::AutoLock scoped_lock(path_data->lock);
-
   Provider* p;
 
-#ifndef NDEBUG
-  p = path_data->providers;
-  while (p) {
-    DCHECK(key_start >= p->key_end || key_end <= p->key_start) <<
-      "path provider collision";
-    p = p->next;
-  }
-#endif
-
   p = new Provider;
   p->is_static = false;
   p->func = func;
-  p->next = path_data->providers;
 #ifndef NDEBUG
   p->key_start = key_start;
   p->key_end = key_end;
 #endif
+
+  base::AutoLock scoped_lock(path_data->lock);
+
+#ifndef NDEBUG
+  Provider *iter = path_data->providers;
+  while (iter) {
+    DCHECK(key_start >= iter->key_end || key_end <= iter->key_start) <<
+      "path provider collision";
+    iter = iter->next;
+  }
+#endif
+
+  p->next = path_data->providers;
   path_data->providers = p;
 }