Make runtime.reload() work with component extensions.

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 68ae5822feb842789917b7326b115b45becdc26e..6d74c10646a17c9896e9527e58b491d222f3a1f6 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -824,7 +824,17 @@ void ExtensionService::EnableExtension(const std::string& extension_id) {
     return;
   }
 
-  extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
+  if (extension && extension->manifest() &&
+      extension->manifest()->location() == Manifest::COMPONENT) {
+    // Component extensions need to be put into the "ENABLED_COMPONENT",

[benwells, 2014/08/12 04:17:14]

Um. Hmm. ENABLED_COMPONENT feels pretty strange. Do you think we could kill it
totally (i.e. just use ENABLED regardless of it being a component app or not)
and fix the crashing at its source?

[Anand Mistry (off Chromium), 2014/08/12 05:06:42]

Adding Trent since he introduced this in
https://src.chromium.org/viewvc/chrome?view=revision&revision=170137

[tapted, 2014/08/12 05:39:06]

Woww so long ago. That was one of my first chrome patches!

I think when I wrote that, the absence of "state" in the extension prefs was a
signal for that extension info sub-map in $profile/Preferences being for a
component extension, and so some extra validation on keys in that sub-map being
unnecessary/unwanted [since component extensions get exceptions to some manifest
requirements].

But the absence of "state" was achieved by skipping some install tasks that were
actually necessary.

Doing the missing install task required a 'state' to be written, but it had to
be a new state otherwise it would opt all currently installed component
extensions into the more stringent manifest requirement checks, and at the time
of reading the manifest info.


I can't remember if there was an explicit reason for leaning on `state` over,
say, refactoring GetInstalledExtensionInfo and relying instead on `location`,
and doing the same thing there. If that works, I agree with Ben, and we should
be able to ditch ENABLED_COMPONENT.


There might still be a risk.. at the time it was a royal mess because a
first_run would start up two Chrome processes both writing to
$profile/Preferences -- the other-browser-import process would partially load
component extensions and then write out garbage prefs for them that the "real"
browser process would read back and have to handle. That extra process is now
gone (IPC is used instead of writing to disk directly), but the prefs reading
still needs to be robust to handle any existing crap that may have been written
for component extensions :/. That is, it might be possible that mess was writing
prefs for component extensions without a valid `location`.

[Anand Mistry (off Chromium), 2014/08/12 07:32:10]

Removed the ENABLED_COMPONENT state, but not sure if I got it right. I guess the
trybots will tell us.

+    // otherwise, other things break later on
+    // (*cough* ExtensionPrefs::GetInstalledExtensionInfo()) that rely on
+    // component extensions being in that state.
+    extension_prefs_->SetExtensionState(extension_id,
+                                        Extension::ENABLED_COMPONENT);
+  } else {
+    extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
+  }
   extension_prefs_->ClearDisableReasons(extension_id);
 
   // This can happen if sync enables an extension that is not