Cleaning Up Extensions When Local Content Removed

chrome/browser/extensions/extension_service.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_service.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 92 matching lines @@
 #include "net/base/registry_controlled_domain.h"
 #include "webkit/database/database_tracker.h"
 #include "webkit/database/database_util.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/extensions/file_browser_event_router.h"
 #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
 
 #if defined(OS_CHROMEOS) && defined(USE_VIRTUAL_KEYBOARD)
 #include "chrome/browser/extensions/extension_input_ui_api.h"
 #endif
 
 using base::Time;
 using content::BrowserContext;
@@ skipping 1754 matching lines @@
   // Keep information about the extension so that we can reload it later
   // even if it's not permanently installed.
   unloaded_extension_paths_[extension->id()] = extension->path();
 
   // Clean up if the extension is meant to be enabled after a reload.
   disabled_extension_paths_.erase(extension->id());
 
   // 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());
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_EXTENSION_UNLOADED,
         content::Source<Profile>(profile_),
         content::Details<UnloadedExtensionInfo>(&details));
     // Make sure the profile cleans up its RequestContexts when an already
     // disabled extension is unloaded (since they are also tracking the disabled
     // extensions).
@@ skipping 28 matching lines @@
 }
 
 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;
+  for (size_t i = 0; i < info->size(); ++i) {
+    // Uninstall the extension if the path does not exist. If the extension
+    // failed to load fully (e.g. the user deleted an external extension's
+    // manifest or the manifest points to the wrong path), we cannot use
+    // UninstallExtension, which relies on a valid Extension object.
+    if (!file_util::PathExists(info->at(i)->extension_path)) {

[Aaron Boodman, 2012/03/21 21:00:47]

You cannot use file_util::PathExists here (or indeed anything in file_util)
because it touches disk. In debug builds this will DCHECK.

You need to post a message to the file thread to collect a list of extension IDs
that are known there, and then compare it to the list that is known in
ExtensionService.

Stepping back slightly, I would like to pull GarbageCollection out into its own
little helper class (I hate how big ExtensionService is).

It would look a little similar to something like UnpackedInstaller. It would
start out on the UI thread, gather a list of known extension IDs, bounce to the
file thread, delete any unknown directories as well as collect a list of known
ids there, then bounce back to the UI thread where it would uninstall any
extensions with no directory.

Do you think this is something you could do?

[Devlin, 2012/03/21 21:48:38]

Not entirely sure why file_util works...but it does. (Is this something that
should be looked into separately?)
http://imgur.com/zJlhO

Otherwise, I see no problem with separating out GarbageCollection; I should be
able to do that.

+      LOG(WARNING) << "Could not access local content for extension with id " <<
+          info->at(i)->extension_id << "; uninstalling extension.";
+      LOG(WARNING) << "Path: " << info->at(i)->extension_path.value();
+      if (!GetInstalledExtension(info->at(i)->extension_id)) {
+        UnloadExtension(info->at(i)->extension_id,
+                        extension_misc::UNLOAD_REASON_UNINSTALL);
+        extension_prefs_->OnExtensionUninstalled(
+            info->at(i)->extension_id, info->at(i)->extension_location, false);
+      } else {
+        UninstallExtension(info->at(i)->extension_id, false, NULL);
+      }
+    } else {
+      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();
 
   // Also garbage-collect themes.  We check |profile_| to be
@@ skipping 167 matching lines @@
     // to a version that requires additional privileges.
     is_privilege_increase =
         granted_permissions->HasLessPrivilegesThan(
             extension->GetActivePermissions());
   }
 
   if (is_extension_upgrade) {
     // 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.
     if (!is_privilege_increase) {
       SetBeingUpgraded(old, true);
       SetBeingUpgraded(extension, true);
     }
 
     // To upgrade an extension in place, unload the old one and
     // then load the new one.
@@ skipping 496 matching lines @@
   return api_resource_controller_;
 }
 
 extensions::RulesRegistryService* ExtensionService::GetRulesRegistryService() {
   if (!rules_registry_service_.get()) {
     rules_registry_service_.reset(
         new extensions::RulesRegistryService(profile_));
   }
   return rules_registry_service_.get();
 }