Garbage Collect the Storage directory on next profile start after an extension uninstall.

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 09f50f253a29320825c3dfb34e653ef8bcd97e62..da29c2288395c9de5dfdab865f7469c86b1c3511 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -109,6 +109,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/common/pepper_plugin_info.h"
 #include "extensions/common/error_utils.h"
 #include "googleurl/src/gurl.h"
@@ -204,6 +205,20 @@ ExtensionService::NaClModuleInfo::NaClModuleInfo() {
 ExtensionService::NaClModuleInfo::~NaClModuleInfo() {
 }
 
+ExtensionService::PendingInstall::PendingInstall(
+    const extensions::Extension* extension,
+    const syncer::StringOrdinal& page_ordinal,
+    bool has_requirement_errors,
+    bool wait_for_idle)
+    : extension(extension),
+      page_ordinal(page_ordinal),
+      has_requirement_errors(has_requirement_errors),
+      wait_for_idle(wait_for_idle) {
+}
+
+ExtensionService::PendingInstall::~PendingInstall() {
+}
+
 // ExtensionService.
 
 const char ExtensionService::kInstallDirectoryName[] = "Extensions";
@@ -374,6 +389,7 @@ ExtensionService::ExtensionService(Profile* profile,
       event_routers_initialized_(false),
       update_once_all_providers_are_ready_(false),
       browser_terminating_(false),
+      delay_all_installs_(false),
       app_sync_bundle_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       extension_sync_bundle_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       app_shortcut_manager_(profile) {
@@ -618,6 +634,11 @@ void ExtensionService::Init() {
       GarbageCollectExtensions();
     }
   }
+
+  if (extension_prefs_->NeedsStorageGarbageCollection()) {
+    GarbageCollectIsolatedStorage();
+    extension_prefs_->SetNeedsStorageGarbageCollection(false);
+  }
 }
 
 bool ExtensionService::UpdateExtension(const std::string& id,
@@ -849,14 +870,24 @@ bool ExtensionService::UninstallExtension(
   launch_web_url_origin = launch_web_url_origin.GetOrigin();
   bool is_storage_isolated = extension->is_storage_isolated();
 
-  if (extension->is_hosted_app() &&
-      !profile_->GetExtensionSpecialStoragePolicy()->
-          IsStorageProtected(launch_web_url_origin)) {
-    extensions::DataDeleter::StartDeleting(
-        profile_, extension_id, launch_web_url_origin, is_storage_isolated);
+  if (is_storage_isolated) {
+    // TODO(ajwong): At teast abstract this into function.
+    const GURL& site = Extension::GetBaseURLFromExtensionId(extension_id);
+    BrowserContext::AsyncObliterateStoragePartition(
+        profile_, site,
+        // TODO(ajwong): We must bounce to UI thread and do correct cancelation.

[Charlie Reis, 2012/12/05 02:25:40]

Not quite sure what this means.  Also, long line below.

[awong, 2012/12/08 01:45:23]

Resolved it. I hadn't fixed the API to make sure the callback was run back on
the UI thread, and to cancel if the ExtensionService gets deleted.

+        base::Bind(&extensions::ExtensionPrefs::SetNeedsStorageGarbageCollection,
+                   base::Unretained(extension_prefs_), true));
+  } else {
+    if (extension->is_hosted_app() &&
+        !profile_->GetExtensionSpecialStoragePolicy()->
+            IsStorageProtected(launch_web_url_origin)) {
+      extensions::DataDeleter::StartDeleting(
+          profile_, extension_id, launch_web_url_origin);
+    }
+    extensions::DataDeleter::StartDeleting(profile_, extension_id,
+                                           extension->url());
   }
-  extensions::DataDeleter::StartDeleting(
-      profile_, extension_id, extension->url(), is_storage_isolated);
 
   UntrackTerminatedExtension(extension_id);
 
@@ -2279,6 +2310,14 @@ void ExtensionService::OnExtensionInstalled(
     bool wait_for_idle) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // If installs are delayed, then disable.

[Charlie Reis, 2012/12/05 02:25:40]

I guess I'm a bit confused by the names here.  "OnExtensionInstalled" sounds
past tense, so perhaps we're just postponing the enabling?

[awong, 2012/12/08 01:45:23]

Yeah, it confused me too.  OnExtensionInstalled() isn't really enabling. 
There's another state for that. This is where things get inserted into the
actual recording keeping structures (prefs, internal lists, etc) so until this
is finished the extension really isn't "installed."  But given Matt's comments
below, I had to change this logic anyways.

+  if (delay_all_installs()) {
+    pending_extension_installs_.push_back(

[Matt Perry, 2012/12/05 19:55:33]

Can you explain why reusing the FinishDelayedInstallation bits doesn't work?
It's going to be very confusing that we have delayed installs and pending
installs, and both features are disjoint.

[awong, 2012/12/05 21:01:57]

I probably misunderstood the code.  What I saw was
pending_extension_updates_.Insert() is gated by ShouldDelayExtensionUpdate(id,
wait_for_idle).

If I were to modify the ShouldDelayExtensionUpdate() call so it returned true
and added the extension id to the pending_extension_updates_ when
delay_all_installs_ is set, I run into these issues:

  (1) I don't know which elements in pending_extension_updates_ needs to passed
to FinishDelayedInstallation when the GC is finished.
  (2) I've lost the wait_for_idle flag information so I can't just blindly retry
them all.
  (3) The code calling extension_prefs_->OnExtensionInstalled() and
extension_prefs_->SetAllowFileAccess() is skipped.


The crux is FinishDelayedInstall() didn't look like a delayed execution of
OnExtensionInstalled()'s code.  Instead, it looks like something handling
extension updates with a variety of attributes I couldn't understand...

[Matt Perry, 2012/12/05 21:22:08]

I think the main question is, exactly what do you need to defer? Currently
you're deferring all of OnExtensionInstalled. Maybe you only need to defer the
final call to FinishInstallation at the end of this method.

If it's the latter, then you should be able to share some code with
pending_extension_updates_. Of course, you'll need to change it a bit since that
code assumes it's only dealing with installs delayed while waiting for extension
idle.

[awong, 2012/12/08 01:45:23]

I've redone this a bit. Let me know what you think.

+        PendingInstall(extension, page_ordinal, has_requirement_errors,
+                       wait_for_idle));
+    return;
+  }
+
   const std::string& id = extension->id();
   bool initial_enable = ShouldEnableOnInstall(extension);
   const extensions::PendingExtensionInfo* pending_extension_info = NULL;
@@ -2982,6 +3021,39 @@ bool ExtensionService::ShouldDelayExtensionUpdate(
   }
 }
 
+void ExtensionService::GarbageCollectIsolatedStorage() {
+  base::hash_set<FilePath> active_paths;
+  for (ExtensionSet::const_iterator it = extensions_.begin();
+       it != extensions_.end(); ++it) {
+    if ((*it)->is_storage_isolated()) {
+      // TODO(ajwong): This should be passed through effective url or something.
+      active_paths.insert(
+          BrowserContext::GetStoragePartitionForSite(
+              profile_,
+              Extension::GetBaseURLFromExtensionId((*it)->id()))->GetPath());
+    }
+  }
+
+  DCHECK(!delay_all_installs());
+  set_delay_all_installs(true);
+  BrowserContext::GarbageCollectStoragePartitions(
+      profile_, &active_paths,
+      base::Bind(&ExtensionService::OnGarbageCollectIsolatedStorageFinished,
+                 AsWeakPtr()));
+}
+
+void ExtensionService::OnGarbageCollectIsolatedStorageFinished() {
+  set_delay_all_installs(false);
+  for (std::vector<PendingInstall>::const_iterator it =
+           pending_extension_installs_.begin();
+       it != pending_extension_installs_.end();
+       ++it) {
+    OnExtensionInstalled(it->extension, it->page_ordinal,
+                         it->has_requirement_errors, it->wait_for_idle);
+  }
+  pending_extension_installs_.clear();
+}
+
 void ExtensionService::OnBlacklistUpdated() {
   CheckManagementPolicy();
 }