Cleaning Up Extensions When Local Content Removed

This fixes an issue where removing an extension's local content, e.g. deleting
the user-data-dir/Default/Extensions/<id> directory, would result in a broken
extension.  Chrome will now uninstall/remove any extensions where the path to
the extension cannot be resolved.

BUG=31910
TEST=ExtensionServiceTest.CleanupInternalExtensionsMissingLocalContent;
     ExtensionServiceTest.CleanupUnpackedExtensionsMissingLocalContent;
     test by hand by loading an extension, deleting the local directory, and
     restarting chrome


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

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

12 files changed, 410 insertions, 158 deletions

diff --git a/chrome/browser/extensions/extension_garbage_collector.cc b/chrome/browser/extensions/extension_garbage_collector.cc
new file mode 100644
index 0000000..9434771
--- /dev/null
+++ b/chrome/browser/extensions/extension_garbage_collector.cc
@@ -0,0 +1,187 @@
+// Copyright (c) 2012 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/extensions/extension_garbage_collector.h"
+
+#include <map>
+#include <string>
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/common/extensions/extension_file_util.h"
+#include "content/public/browser/browser_thread.h"
+
+ExtensionGarbageCollector::ExtensionGarbageCollector(
+    ExtensionService* extension_service)
+    : extension_service_(extension_service) {
+}
+
+ExtensionGarbageCollector::~ExtensionGarbageCollector() {
+}
+
+void ExtensionGarbageCollector::GarbageCollectExtensions() {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  FilePath install_directory = extension_service_->install_directory();
+
+  // NOTE: It's important to use the file thread here because that is the thread
+  // ExtensionService uses, and there are ordering dependencies between all of
+  // the extension-file-management tasks.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(
+           &ExtensionGarbageCollector::
+               CheckExtensionDirectoriesOnBackgroundThread,
+           this,
+           install_directory));
+}
+
+void ExtensionGarbageCollector::CheckExtensionDirectoriesOnBackgroundThread(
+    const FilePath& install_directory) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+
+  DVLOG(1) << "Garbage collecting extensions...";
+
+  if (!file_util::DirectoryExists(install_directory))
+    return;
+
+  std::set<FilePath> extension_paths;
+
+  file_util::FileEnumerator enumerator(install_directory,
+                                       false,  // Not recursive.
+                                       file_util::FileEnumerator::DIRECTORIES);
+
+  for (FilePath extension_path = enumerator.Next(); !extension_path.empty();
+       extension_path = enumerator.Next()) {
+    std::string extension_id = extension_path.BaseName().MaybeAsASCII();
+    if (!Extension::IdIsValid(extension_id))
+      extension_id.clear();
+
+    // Delete directories that aren't valid IDs.
+    if (extension_id.empty()) {
+      DLOG(WARNING) << "Invalid extension ID encountered in extensions "
+                       "directory: " << extension_path.BaseName().value();
+      DVLOG(1) << "Deleting invalid extension directory "
+               << extension_path.value() << ".";
+      file_util::Delete(extension_path, true);  // Recursive.
+      continue;
+    }
+
+    extension_paths.insert(extension_path);
+  }
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(
+           &ExtensionGarbageCollector::CheckExtensionPreferences,
+           this,
+           extension_paths));
+}
+
+void ExtensionGarbageCollector::CheckExtensionPreferences(
+    const std::set<FilePath>& extension_paths) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  ExtensionPrefs* extension_prefs = extension_service_->extension_prefs();
+
+  if (extension_prefs->pref_service()->ReadOnly())
+    return;
+
+  scoped_ptr<ExtensionPrefs::ExtensionsInfo> info(
+      extension_prefs->GetInstalledExtensionsInfo());
+
+  std::map<std::string, FilePath> extension_pref_paths;
+  for (size_t i = 0; i < info->size(); ++i) {
+    extension_pref_paths[info->at(i)->extension_id] =
+        info->at(i)->extension_path;
+  }
+
+  std::set<std::string> extension_ids;
+
+  // Check each directory for a reference in the preferences.
+  for (std::set<FilePath>::const_iterator path = extension_paths.begin();
+       path != extension_paths.end();
+       ++path) {
+    std::string extension_id;
+    extension_id = path->BaseName().MaybeAsASCII();
+    if (extension_id.empty())
+      continue;
+
+    extension_ids.insert(extension_id);
+
+    std::map<std::string, FilePath>::const_iterator iter =
+        extension_pref_paths.find(extension_id);
+
+    if (iter == extension_pref_paths.end()) {
+      DVLOG(1) << "Deleting unreferenced install for directory "
+               << path->LossyDisplayName() << ".";
+      content::BrowserThread::PostTask(
+          content::BrowserThread::FILE, FROM_HERE,
+          base::Bind(
+              &extension_file_util::DeleteFile,
+              *path,
+              true));  // recursive.
+      continue;
+    }
+
+    // Clean up old version directories.
+    content::BrowserThread::PostTask(
+        content::BrowserThread::FILE, FROM_HERE,
+        base::Bind(
+             &ExtensionGarbageCollector::CleanupOldVersionsOnBackgroundThread,
+             this, *path, iter->second.BaseName()));
+  }
+
+  // Check each entry in the preferences for an existing path.
+  for (std::map<std::string, FilePath>::const_iterator extension =
+           extension_pref_paths.begin();
+       extension != extension_pref_paths.end();
+       ++extension) {
+    std::set<std::string>::const_iterator iter = extension_ids.find(
+        extension->first);
+
+    if (iter != extension_ids.end())
+      continue;
+
+    std::string extension_id = extension->first;
+
+    DVLOG(1) << "Could not access local content for extension with id "
+             << extension_id << "; uninstalling extension.";
+
+    // If the extension failed to load fully (e.g. the user deleted an unpacked
+    // extension's manifest or the manifest points to the wrong path), we cannot
+    // use UninstallExtension, which relies on a valid Extension object.
+    if (!extension_service_->GetInstalledExtension(extension_id)) {
+      scoped_ptr<ExtensionInfo> info(extension_service_->extension_prefs()
+          ->GetInstalledExtensionInfo(extension_id));
+      extension_service_->extension_prefs()->OnExtensionUninstalled(
+          extension_id, info->extension_location, false);
+    } else {
+      extension_service_->UninstallExtension(extension_id, false, NULL);
+    }
+  }
+}
+
+void ExtensionGarbageCollector::CleanupOldVersionsOnBackgroundThread(
+    const FilePath& path,
+    const FilePath& current_version) {
+  CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  file_util::FileEnumerator versions_enumerator(
+      path,
+      false,  // Not recursive.
+      file_util::FileEnumerator::DIRECTORIES);
+  for (FilePath version_dir = versions_enumerator.Next();
+       !version_dir.value().empty();
+       version_dir = versions_enumerator.Next()) {
+    if (version_dir.BaseName() != current_version) {
+      DVLOG(1) << "Deleting old version for directory "
+               << version_dir.LossyDisplayName() << ".";
+      if (!file_util::Delete(version_dir, true))  // Recursive.
+        NOTREACHED();
+    }
+  }
+}
diff --git a/chrome/browser/extensions/extension_garbage_collector.h b/chrome/browser/extensions/extension_garbage_collector.h
new file mode 100644
index 0000000..b568e83
--- /dev/null
+++ b/chrome/browser/extensions/extension_garbage_collector.h
@@ -0,0 +1,51 @@
+// Copyright (c) 2012 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_GARBAGE_COLLECTOR_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_GARBAGE_COLLECTOR_H_
+#pragma once
+
+#include <set>
+
+#include "base/file_path.h"
+#include "base/memory/ref_counted.h"
+
+class ExtensionService;
+
+// Garbage collection for extensions. This will delete any directories in the
+// installation directory of |extension_service_| that aren't valid, as well
+// as removing any references in the preferences to extensions that are
+// missing local content.
+class ExtensionGarbageCollector
+    : public base::RefCountedThreadSafe<ExtensionGarbageCollector> {
+ public:
+  explicit ExtensionGarbageCollector(ExtensionService* extension_service);
+  virtual ~ExtensionGarbageCollector();
+
+  // Begin garbage collection; fetch the installed extensions' ids.
+  void GarbageCollectExtensions();
+
+ private:
+  // Check the installation directory for directories with invalid extension
+  // ids, deleting them if found. Call CheckExtensionPreferences with a list of
+  // present directories.
+  void CheckExtensionDirectoriesOnBackgroundThread(
+      const FilePath& installation_directory);
+
+  // Check for any extension directories which:
+  //   - Are an old version of a current extension;
+  //   - Are present in the preferences but not on the file system;
+  //   - Are present on the file system, but not in the preferences
+  // and delete them if found.
+  void CheckExtensionPreferences(const std::set<FilePath>& extension_paths);
+
+  // Helper function to cleanup any old versions of an extension in the
+  // extension's base directory.
+  void CleanupOldVersionsOnBackgroundThread(
+      const FilePath& path, const FilePath& current_version);
+
+  ExtensionService* extension_service_;
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_GARBAGE_COLLECTOR_H_
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 6ea06da..f2b8112 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -117,7 +117,6 @@
 #include "chrome/browser/chromeos/extensions/input_method_event_router.h"
 #include "chrome/browser/chromeos/extensions/media_player_event_router.h"
 #include "chrome/browser/chromeos/input_method/input_method_manager.h"
-#include "chrome/browser/extensions/extension_input_ime_api.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_mount_point_provider.h"
 #endif
@@ -384,6 +383,7 @@ ExtensionService::ExtensionService(Profile* profile,
     : profile_(profile),
       system_(ExtensionSystem::Get(profile)),
       extension_prefs_(extension_prefs),
+      extension_garbage_collector_(new ExtensionGarbageCollector(this)),
       settings_frontend_(extensions::SettingsFrontend::Create(profile)),
       pending_extension_manager_(*ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       install_directory_(install_directory),
@@ -1898,7 +1898,7 @@ void ExtensionService::UnloadExtension(
   // Clean up runtime data.
   extension_runtime_data_.erase(extension_id);
 
-if (disabled_extensions_.Contains(extension->id())) {
+  if (disabled_extensions_.Contains(extension->id())) {
     UnloadedExtensionInfo details(extension, reason);
     details.already_disabled = true;
     disabled_extensions_.Remove(extension->id());
@@ -1940,23 +1940,7 @@ void ExtensionService::ReloadExtensions() {
 }
 
 void ExtensionService::GarbageCollectExtensions() {
-  if (extension_prefs_->pref_service()->ReadOnly())
-    return;
-
-  scoped_ptr<ExtensionPrefs::ExtensionsInfo> info(
-      extension_prefs_->GetInstalledExtensionsInfo());
-
-  std::map<std::string, FilePath> extension_paths;
-  for (size_t i = 0; i < info->size(); ++i)
-    extension_paths[info->at(i)->extension_id] = info->at(i)->extension_path;
-
-  if (!BrowserThread::PostTask(
-          BrowserThread::FILE, FROM_HERE,
-          base::Bind(
-              &extension_file_util::GarbageCollectExtensions,
-              install_directory_,
-              extension_paths)))
-    NOTREACHED();
+  extension_garbage_collector_->GarbageCollectExtensions();
 
   // Also garbage-collect themes.  We check |profile_| to be
   // defensive; in the future, we may call GarbageCollectExtensions()
@@ -2126,7 +2110,7 @@ void ExtensionService::InitializePermissions(const Extension* extension) {
     // Other than for unpacked extensions, CrxInstaller should have guaranteed
     // that we aren't downgrading.
     if (extension->location() != Extension::LOAD)
-      CHECK(extension->version()->CompareTo(*(old->version())) >= 0);
+      CHECK_GE(extension->version()->CompareTo(*(old->version())), 0);
 
     // Extensions get upgraded if the privileges are allowed to increase or
     // the privileges haven't increased.
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 7ddbca4..8596b6b 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -25,6 +25,7 @@
 #include "chrome/browser/extensions/app_shortcut_manager.h"
 #include "chrome/browser/extensions/app_sync_bundle.h"
 #include "chrome/browser/extensions/apps_promo.h"
+#include "chrome/browser/extensions/extension_garbage_collector.h"
 #include "chrome/browser/extensions/extension_icon_manager.h"
 #include "chrome/browser/extensions/extension_menu_manager.h"
 #include "chrome/browser/extensions/extension_prefs.h"
@@ -706,6 +707,10 @@ class ExtensionService
   // Preferences for the owning profile (weak reference).
   ExtensionPrefs* extension_prefs_;
 
+  // The ExtensionGarbageCollector associated with this service; this is
+  // responsible for cleaning up old or partially deleted extensions.
+  scoped_refptr<ExtensionGarbageCollector> extension_garbage_collector_;
+
   // Settings for the owning profile.
   scoped_ptr<extensions::SettingsFrontend> settings_frontend_;
 
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 00a7c70..e2f4bd2 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -252,7 +252,6 @@ class MockExtensionProvider : public ExternalExtensionProviderInterface {
 class MockProviderVisitor
     : public ExternalExtensionProviderInterface::VisitorInterface {
  public:
-
   // The provider will return |fake_base_path| from
   // GetBaseCrxFilePath().  User can test the behavior with
   // and without an empty path using this parameter.
@@ -1008,7 +1007,6 @@ void PackExtensionTestClient::OnPackFailure(const std::string& error_message,
      FAIL() << "Packing should not fail.";
   else
      FAIL() << "Existing CRX should have been overwritten.";
-
 }
 
 // Test loading good extensions from the profile directory.
@@ -1152,9 +1150,46 @@ TEST_F(ExtensionServiceTest, LoadAllExtensionsFromDirectoryFail) {
       UTF16ToUTF8(GetErrors()[3]);
 };
 
-// Test that partially deleted extensions are cleaned up during startup
+// Test that old versions of extensions are deleted during garbage collection.
+TEST_F(ExtensionServiceTest, GarbageCollectOldVersions) {
+  PluginService::GetInstance()->Init();
+
+  FilePath source_install_dir = data_dir_
+      .AppendASCII("garbage_collection")
+      .AppendASCII("Extensions");
+
+  FilePath pref_path = source_install_dir
+      .DirName()
+      .AppendASCII("Preferences");
+
+  InitializeInstalledExtensionService(pref_path, source_install_dir);
+
+  // Verify that there are two versions present initially.
+  ASSERT_TRUE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.0_0")));
+  ASSERT_TRUE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.1_0")));
+
+  ValidatePrefKeyCount(1u);
+
+  service_->Init();
+  loop_.RunAllPending();
+
+  // Garbage collection should have deleted the old version.
+  ASSERT_FALSE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.0_0")));
+  ASSERT_TRUE(file_util::PathExists(
+      extensions_install_dir_.AppendASCII("fdoajpacpdeapbmhbblepcnilfkpmkff")
+                             .AppendASCII("1.1_0")));
+}
+
+// Test that extensions which were deleted from the preferences are cleaned up
+// during startup.
 // Test loading bad extensions from the profile directory.
-TEST_F(ExtensionServiceTest, CleanupOnStartup) {
+TEST_F(ExtensionServiceTest, GarbageCollectOnStartup) {
   PluginService::GetInstance()->Init();
 
   FilePath source_install_dir = data_dir_
@@ -1166,11 +1201,13 @@ TEST_F(ExtensionServiceTest, CleanupOnStartup) {
 
   InitializeInstalledExtensionService(pref_path, source_install_dir);
 
+  ValidatePrefKeyCount(3u);
+
   // Simulate that one of them got partially deleted by clearing its pref.
   {
     DictionaryPrefUpdate update(profile_->GetPrefs(), "extensions.settings");
     DictionaryValue* dict = update.Get();
-    ASSERT_TRUE(dict != NULL);
+    ASSERT_TRUE(dict);
     dict->Remove("behllobkkfkfnphdnhnkndlbkcpglgmj", NULL);
   }
 
@@ -1193,6 +1230,93 @@ TEST_F(ExtensionServiceTest, CleanupOnStartup) {
   ASSERT_FALSE(file_util::PathExists(extension_dir));
 }
 
+// Test that internal extensions which are referenced in the preferences but
+// had their content deleted are cleaned up during startup.
+TEST_F(ExtensionServiceTest, GarbageCollectInternalExtensionsMissingContent) {
+  PluginService::GetInstance()->Init();
+
+  FilePath source_dir = data_dir_.AppendASCII("good").AppendASCII("Extensions");
+  FilePath pref_path = source_dir.DirName().AppendASCII("Preferences");
+
+  InitializeInstalledExtensionService(pref_path, source_dir);
+
+  ValidatePrefKeyCount(3u);
+
+  // Delete the extension's directory.
+  FilePath extension_dir = extensions_install_dir_
+      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj");
+  ASSERT_TRUE(file_util::Delete(extension_dir, true));
+
+  // Run GarbageCollectExtensions.
+  service_->Init();
+  loop_.RunAllPending();
+
+  file_util::FileEnumerator dirs(extensions_install_dir_, false,
+                                 file_util::FileEnumerator::DIRECTORIES);
+  size_t count = 0;
+  while (!dirs.Next().empty())
+    count++;
+
+  // We should have only gotten two extensions now.
+  EXPECT_EQ(2u, count);
+
+  const DictionaryValue* dictionary =
+      profile_->GetPrefs()->GetDictionary("extensions.settings");
+  ASSERT_TRUE(dictionary);
+  ASSERT_FALSE(dictionary->HasKey("behllobkkfkfnphdnhnkndlbkcpglgmj"));
+}
+
+// Test that unpacked extensions which are referenced in the preferences but
+// had their content deleted are cleaned up during startup.
+TEST_F(ExtensionServiceTest, GarbageCollectUnpackedExtensionsMissingContent) {
+  InitializeEmptyExtensionService();
+
+  ScopedTempDir temp;
+  ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+  // Write the manifest dynamically, since we have to delete the file anyway.
+  FilePath extension_path = temp.path();
+  FilePath manifest_path = extension_path.Append(Extension::kManifestFilename);
+  ASSERT_FALSE(file_util::PathExists(manifest_path));
+
+  // Construct a simple manifest and install it.
+  DictionaryValue manifest;
+  manifest.SetString("version", "1.0");
+  manifest.SetString("name", "Cleanup Unpacked Extensions Missing Content");
+  manifest.SetInteger("manifest_version", 2);
+
+  JSONFileValueSerializer serializer(manifest_path);
+  ASSERT_TRUE(serializer.Serialize(manifest));
+
+  extensions::UnpackedInstaller::Create(service_)->Load(extension_path);
+  loop_.RunAllPending();
+
+  // Make sure it installed correctly.
+  EXPECT_EQ(0u, GetErrors().size());
+  ASSERT_EQ(1u, loaded_.size());
+  EXPECT_EQ(Extension::LOAD, loaded_[0]->location());
+  EXPECT_EQ(1u, service_->extensions()->size());
+
+  std::string extension_id = loaded_[0]->id();
+
+  // Make sure it is in the preferences at this point.
+  const DictionaryValue* initial_dictionary =
+      profile_->GetPrefs()->GetDictionary("extensions.settings");
+  ASSERT_TRUE(initial_dictionary);
+  ASSERT_TRUE(initial_dictionary->HasKey(extension_id));
+
+  // Delete local content, GarbageCollectExtensions, and test whether the key
+  // is still in the preferences.
+  ASSERT_TRUE(file_util::Delete(extension_path, true));
+  service_->GarbageCollectExtensions();
+  loop_.RunAllPending();
+
+  const DictionaryValue* final_dictionary =
+      profile_->GetPrefs()->GetDictionary("extensions.settings");
+  ASSERT_TRUE(final_dictionary);
+  ASSERT_FALSE(final_dictionary->HasKey(extension_id));
+}
+
 // Test installing extensions. This test tries to install few extensions using
 // crx files. If you need to change those crx files, feel free to repackage
 // them, throw away the key used and change the id's above.
diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi
index 672a88b..6b874a3 100644
--- a/chrome/chrome_browser_extensions.gypi
+++ b/chrome/chrome_browser_extensions.gypi
@@ -247,6 +247,8 @@
         'browser/extensions/extension_function_dispatcher.h',
         'browser/extensions/extension_function_registry.cc',
         'browser/extensions/extension_function_registry.h',
+        'browser/extensions/extension_garbage_collector.cc',
+        'browser/extensions/extension_garbage_collector.h',
         'browser/extensions/extension_global_error.cc',
         'browser/extensions/extension_global_error.h',
         'browser/extensions/extension_global_error_badge.cc',
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index 3f9a9b9..79e80e0 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -366,69 +366,6 @@ bool ValidateExtension(const Extension* extension, std::string* error) {
   return true;
 }
 
-void GarbageCollectExtensions(
-    const FilePath& install_directory,
-    const std::map<std::string, FilePath>& extension_paths) {
-  // Nothing to clean up if it doesn't exist.
-  if (!file_util::DirectoryExists(install_directory))
-    return;
-
-  DVLOG(1) << "Garbage collecting extensions...";
-  file_util::FileEnumerator enumerator(install_directory,
-                                       false,  // Not recursive.
-                                       file_util::FileEnumerator::DIRECTORIES);
-  FilePath extension_path;
-  for (extension_path = enumerator.Next(); !extension_path.value().empty();
-       extension_path = enumerator.Next()) {
-    std::string extension_id;
-
-    FilePath basename = extension_path.BaseName();
-    if (IsStringASCII(basename.value())) {
-      extension_id = UTF16ToASCII(basename.LossyDisplayName());
-      if (!Extension::IdIsValid(extension_id))
-        extension_id.clear();
-    }
-
-    // Delete directories that aren't valid IDs.
-    if (extension_id.empty()) {
-      DLOG(WARNING) << "Invalid extension ID encountered in extensions "
-                       "directory: " << basename.value();
-      DVLOG(1) << "Deleting invalid extension directory "
-               << extension_path.value() << ".";
-      file_util::Delete(extension_path, true);  // Recursive.
-      continue;
-    }
-
-    std::map<std::string, FilePath>::const_iterator iter =
-        extension_paths.find(extension_id);
-
-    // If there is no entry in the prefs file, just delete the directory and
-    // move on. This can legitimately happen when an uninstall does not
-    // complete, for example, when a plugin is in use at uninstall time.
-    if (iter == extension_paths.end()) {
-      DVLOG(1) << "Deleting unreferenced install for directory "
-               << extension_path.LossyDisplayName() << ".";
-      file_util::Delete(extension_path, true);  // Recursive.
-      continue;
-    }
-
-    // Clean up old version directories.
-    file_util::FileEnumerator versions_enumerator(
-        extension_path,
-        false,  // Not recursive.
-        file_util::FileEnumerator::DIRECTORIES);
-    for (FilePath version_dir = versions_enumerator.Next();
-         !version_dir.value().empty();
-         version_dir = versions_enumerator.Next()) {
-      if (version_dir.BaseName() != iter->second.BaseName()) {
-        DVLOG(1) << "Deleting old version for directory "
-                 << version_dir.LossyDisplayName() << ".";
-        file_util::Delete(version_dir, true);  // Recursive.
-      }
-    }
-  }
-}
-
 ExtensionMessageBundle* LoadExtensionMessageBundle(
     const FilePath& extension_path,
     const std::string& default_locale,
diff --git a/chrome/common/extensions/extension_file_util.h b/chrome/common/extensions/extension_file_util.h
index 1ab3f07..4771fc4 100644
--- a/chrome/common/extensions/extension_file_util.h
+++ b/chrome/common/extensions/extension_file_util.h
@@ -62,19 +62,6 @@ base::DictionaryValue* LoadManifest(const FilePath& extension_root,
 // Otherwise, a description of the error is returned in |error|.
 bool ValidateExtension(const Extension* extension, std::string* error);
 
-// Cleans up the extension install directory. It can end up with garbage in it
-// if extensions can't initially be removed when they are uninstalled (eg if a
-// file is in use).
-//
-// |extensions_dir| is the install directory to look in. |extension_paths| is a
-// map from extension id to full installation path.
-//
-// Obsolete version directories are removed, as are directories that aren't
-// found in |extension_paths|.
-void GarbageCollectExtensions(
-    const FilePath& extensions_dir,
-    const std::map<std::string, FilePath>& extension_paths);
-
 // Loads extension message catalogs and returns message bundle.
 // Returns NULL on error, or if extension is not localized.
 ExtensionMessageBundle* LoadExtensionMessageBundle(
diff --git a/chrome/common/extensions/extension_file_util_unittest.cc b/chrome/common/extensions/extension_file_util_unittest.cc
index e2d1066..69a2138 100644
--- a/chrome/common/extensions/extension_file_util_unittest.cc
+++ b/chrome/common/extensions/extension_file_util_unittest.cc
@@ -19,63 +19,6 @@
 
 namespace keys = extension_manifest_keys;
 
-#if defined(OS_WIN)
-// http://crbug.com/106381
-#define InstallUninstallGarbageCollect DISABLED_InstallUninstallGarbageCollect
-#endif
-TEST(ExtensionFileUtil, InstallUninstallGarbageCollect) {
-  ScopedTempDir temp;
-  ASSERT_TRUE(temp.CreateUniqueTempDir());
-
-  // Create a source extension.
-  std::string extension_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
-  std::string version("1.0");
-  FilePath src = temp.path().AppendASCII(extension_id);
-  ASSERT_TRUE(file_util::CreateDirectory(src));
-
-  // Create a extensions tree.
-  FilePath all_extensions = temp.path().AppendASCII("extensions");
-  ASSERT_TRUE(file_util::CreateDirectory(all_extensions));
-
-  // Install in empty directory. Should create parent directories as needed.
-  FilePath version_1 = extension_file_util::InstallExtension(src,
-                                                             extension_id,
-                                                             version,
-                                                             all_extensions);
-  ASSERT_EQ(version_1.value(),
-            all_extensions.AppendASCII(extension_id).AppendASCII("1.0_0")
-                .value());
-  ASSERT_TRUE(file_util::DirectoryExists(version_1));
-
-  // Should have moved the source.
-  ASSERT_FALSE(file_util::DirectoryExists(src));
-
-  // Install again. Should create a new one with different name.
-  ASSERT_TRUE(file_util::CreateDirectory(src));
-  FilePath version_2 = extension_file_util::InstallExtension(src,
-                                                             extension_id,
-                                                             version,
-                                                             all_extensions);
-  ASSERT_EQ(version_2.value(),
-            all_extensions.AppendASCII(extension_id).AppendASCII("1.0_1")
-                .value());
-  ASSERT_TRUE(file_util::DirectoryExists(version_2));
-
-  // Collect garbage. Should remove first one.
-  std::map<std::string, FilePath> extension_paths;
-  extension_paths[extension_id] =
-      FilePath().AppendASCII(extension_id).Append(version_2.BaseName());
-  extension_file_util::GarbageCollectExtensions(all_extensions,
-                                                extension_paths);
-  ASSERT_FALSE(file_util::DirectoryExists(version_1));
-  ASSERT_TRUE(file_util::DirectoryExists(version_2));
-
-  // Uninstall. Should remove entire extension subtree.
-  extension_file_util::UninstallExtension(all_extensions, extension_id);
-  ASSERT_FALSE(file_util::DirectoryExists(version_2.DirName()));
-  ASSERT_TRUE(file_util::DirectoryExists(all_extensions));
-}
-
 TEST(ExtensionFileUtil, LoadExtensionWithValidLocales) {
   FilePath install_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
diff --git a/chrome/test/data/extensions/garbage_collection/Extensions/fdoajpacpdeapbmhbblepcnilfkpmkff/1.0_0/manifest.json b/chrome/test/data/extensions/garbage_collection/Extensions/fdoajpacpdeapbmhbblepcnilfkpmkff/1.0_0/manifest.json
new file mode 100644
index 0000000..e69d36b
--- /dev/null
+++ b/chrome/test/data/extensions/garbage_collection/Extensions/fdoajpacpdeapbmhbblepcnilfkpmkff/1.0_0/manifest.json
@@ -0,0 +1,6 @@
+{
+   "description": "An extension to test garbage collection of old versions.",
+   "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC3Yox7PDjjZl7upms+MLnOuswzlX63YH5jVvzug1WoAHVnq/djfHsViG0WgTrxxPiMhuTNqOh2OzdqHRko+xbkBBgpSjV+X1lIvUgZGOx9ImH2pz4kntyRv1z9abuN+rQh+gjglPvLJQuWU6VvjH0zypTaXZiQ4GCojRqD/dWacQIDAQAB",
+   "name": "GarbageCollectOldVersions Extension",
+   "version": "1.0"
+}
diff --git a/chrome/test/data/extensions/garbage_collection/Extensions/fdoajpacpdeapbmhbblepcnilfkpmkff/1.1_0/manifest.json b/chrome/test/data/extensions/garbage_collection/Extensions/fdoajpacpdeapbmhbblepcnilfkpmkff/1.1_0/manifest.json
new file mode 100644
index 0000000..20a4b3b
--- /dev/null
+++ b/chrome/test/data/extensions/garbage_collection/Extensions/fdoajpacpdeapbmhbblepcnilfkpmkff/1.1_0/manifest.json
@@ -0,0 +1,6 @@
+{
+   "description": "An extension to test garbage collection of old versions.",
+   "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC3Yox7PDjjZl7upms+MLnOuswzlX63YH5jVvzug1WoAHVnq/djfHsViG0WgTrxxPiMhuTNqOh2OzdqHRko+xbkBBgpSjV+X1lIvUgZGOx9ImH2pz4kntyRv1z9abuN+rQh+gjglPvLJQuWU6VvjH0zypTaXZiQ4GCojRqD/dWacQIDAQAB",
+   "name": "GarbageCollectOldVersions Extension",
+   "version": "1.1"
+}
diff --git a/chrome/test/data/extensions/garbage_collection/Preferences b/chrome/test/data/extensions/garbage_collection/Preferences
new file mode 100644
index 0000000..92e3b6e
--- /dev/null
+++ b/chrome/test/data/extensions/garbage_collection/Preferences
@@ -0,0 +1,20 @@
+{
+   "extensions": {
+      "settings": {
+         "fdoajpacpdeapbmhbblepcnilfkpmkff": {
+            "from_bookmark": false,
+            "from_webstore": false,
+            "install_time": "12977681506510306",
+            "location": 1,
+            "manifest": {
+               "description": "An extension to test garbage collection of old versions.",
+               "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC3Yox7PDjjZl7upms+MLnOuswzlX63YH5jVvzug1WoAHVnq/djfHsViG0WgTrxxPiMhuTNqOh2OzdqHRko+xbkBBgpSjV+X1lIvUgZGOx9ImH2pz4kntyRv1z9abuN+rQh+gjglPvLJQuWU6VvjH0zypTaXZiQ4GCojRqD/dWacQIDAQAB",
+               "name": "GarbageCollectOldVersions Extension",
+               "version": "1.1"
+            },
+            "path": "fdoajpacpdeapbmhbblepcnilfkpmkff/1.1_0",
+            "state": 1
+         }
+      }
+   }
+}