Cleaning Up Extensions When Local Content Removed

chrome/browser/extensions/extension_garbage_collector.cc

+// 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 "base/file_util.h"
+#include "base/logging.h"
+#include "base/string_util.h"
+#include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/browser/extensions/extension_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();
+
+  if (!content::BrowserThread::PostTask(

[Aaron Boodman, 2012/04/05 21:53:14]

I think the current guidance to fire-and-forget PostTask. No need to check
return value.

[Devlin, 2012/04/07 17:38:21]

There are a few instances where checking the value is used, such as
extension_service and crx_installer, but it looks like you're right - the vast
majority are fire-and-forget.  Changed to match.

+          content::BrowserThread::FILE, FROM_HERE,

[Aaron Boodman, 2012/04/05 21:53:14]

Add a comment saying something like:

// NOTE: It's important to use the file thread in particular
// because that is what the rest of the extension service 
// does and there are ordering dependencies between all the
// extension-file-management tasks.

[Devlin, 2012/04/07 17:38:21]

Done.

+          base::Bind(
+               &ExtensionGarbageCollector::CheckExtensionDirectories,
+               this,
+               install_directory)))
+    NOTREACHED();
+}
+
+void ExtensionGarbageCollector::CheckExtensionDirectories(
+    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.value().empty();

[Aaron Boodman, 2012/04/05 21:53:14]

Can this part fit on the previous line?

[Devlin, 2012/04/07 17:38:21]

Done.

+       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;
+    }
+
+    extension_paths.insert(extension_path);
+  }
+
+  if (!content::BrowserThread::PostTask(
+          content::BrowserThread::UI, FROM_HERE,
+          base::Bind(
+               &ExtensionGarbageCollector::CheckExtensionPreferences,
+               this,
+               extension_paths)))
+    NOTREACHED();
+}
+
+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;
+    FilePath basename = path->BaseName();
+    if (IsStringASCII(basename.value()))
+      extension_id = UTF16ToASCII(basename.LossyDisplayName());

[Aaron Boodman, 2012/04/05 21:53:14]

basename.MaybeAsASCII would simplify this a bit.

[Devlin, 2012/04/07 17:38:21]

Done.

+    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() << ".";
+      if (!content::BrowserThread::PostTask(
+              content::BrowserThread::FILE, FROM_HERE,
+              base::Bind(
+                  &extension_file_util::DeleteFile,
+                  *path,
+                  true)))  // recursive
+        NOTREACHED();
+      continue;
+    }
+
+    // Clean up old version directories.
+    file_util::FileEnumerator versions_enumerator(

[Aaron Boodman, 2012/04/05 21:53:14]

You are on the UI thread here. No filesystem access on the UI thread (why isn't
this DCHECKing?). You could create a new helper method to do this part of the
work and PostTask to it here on the file thread.

[Devlin, 2012/04/07 17:38:21]

Done.

+        *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() << ".";
+        if (!content::BrowserThread::PostTask(
+                content::BrowserThread::FILE, FROM_HERE,
+                base::Bind(
+                    &extension_file_util::DeleteFile,
+                    version_dir,
+                    true)))  // Recursive.
+          NOTREACHED();
+      }
+    }
+  }
+
+  // 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);
+    }
+  }
+}