Make runtime.reload() work with component extensions.

extensions/browser/extension_prefs.cc

Index: extensions/browser/extension_prefs.cc
diff --git a/extensions/browser/extension_prefs.cc b/extensions/browser/extension_prefs.cc
index 502816ca2d1df7b4d8da46ba272c9fbad7ac4ee6..f12dceae6ae4135a40ff4e42ce3d682a733c50fa 100644
--- a/extensions/browser/extension_prefs.cc
+++ b/extensions/browser/extension_prefs.cc
@@ -1344,18 +1344,18 @@ scoped_ptr<ExtensionInfo> ExtensionPrefs::GetInstalledInfoHelper(
   // Make path absolute. Unpacked extensions will already have absolute paths,
   // otherwise make it so.
   Manifest::Location location = static_cast<Manifest::Location>(location_value);
-#if !defined(OS_CHROMEOS)
-  if (!Manifest::IsUnpackedLocation(location)) {
-    DCHECK(location == Manifest::COMPONENT ||
-           !base::FilePath(path).IsAbsolute());
-#else
-  // On Chrome OS some extensions can be installed to shared location and
-  // thus use absolute paths in prefs.
+  // Most (but not all) extension types don't have absolute paths.
   if (!base::FilePath(path).IsAbsolute()) {

[tapted, 2014/08/13 00:25:07]

nit: no curlies

[Anand Mistry (off Chromium), 2014/08/13 01:53:15]

Yuk! I prefer braces.

-#endif  // !defined(OS_CHROMEOS)
     path = install_directory_.Append(path).value();
   }
 
+  if (location == Manifest::COMPONENT) {
+    // Component extensions are ignored. Replicates the state before this
+    // change.
+    // TODO(amistry): Is this what we want?

[tapted, 2014/08/13 00:25:07]

yep! But I think the sentiment we want is: Component extensions may have data
saved in preferences, but they are already loaded at this point (by
ComponentLoader) and shouldn't be populated into the result of
GetInstalledExtensionsInfo, otherwise InstalledLoader would also want to load
them. Exiting here also skips the path validation, which doesn't make sense for
component extensions since they are loaded from the resource bundle.

(.. maybe a shorter version of that)

[Anand Mistry (off Chromium), 2014/08/13 01:53:15]

Done.

+    return scoped_ptr<ExtensionInfo>();
+  }
+
   // Only the following extension types have data saved in the preferences.
   if (location != Manifest::INTERNAL &&
       !Manifest::IsUnpackedLocation(location) &&
@@ -1384,8 +1384,7 @@ scoped_ptr<ExtensionInfo> ExtensionPrefs::GetInstalledExtensionInfo(
       !extensions->GetDictionaryWithoutPathExpansion(extension_id, &ext))
     return scoped_ptr<ExtensionInfo>();
   int state_value;
-  if (!ext->GetInteger(kPrefState, &state_value) ||
-      state_value == Extension::ENABLED_COMPONENT) {
+  if (!ext->GetInteger(kPrefState, &state_value)) {

[tapted, 2014/08/13 00:25:07]

I'd probably combine this with the statement below, and drop the comment about
component extensions. i.e.

if (ext->GetInteger(kPrefState, &state_value) && state_value ==
..EXTERNAL_EXTENSION_UNINSTALLED) {
  LOG(..);
  return;
}

i.e. rely just on the check in GetInstalledInfoHelper, which is more robust

[Anand Mistry (off Chromium), 2014/08/13 01:53:15]

Done.

     // Old preferences files may not have kPrefState for component extensions.
     return scoped_ptr<ExtensionInfo>();
   }
@@ -2061,7 +2060,7 @@ void ExtensionPrefs::PopulateExtensionInfoPrefs(
   // Leave the state blank for component extensions so that old chrome versions
   // loading new profiles do not fail in GetInstalledExtensionInfo. Older
   // Chrome versions would only check for an omitted state.
-  if (initial_state != Extension::ENABLED_COMPONENT)
+  if (extension->manifest()->location() != Manifest::COMPONENT)

[tapted, 2014/08/13 00:25:07]

This extra check can probably be removed (i.e. write the pref unconditionally).
_really_ old versions of Chrome will still crash if this is encountered, but for
~20 months they will have had that early exit case. And we don't usually cater
for old versions of chome this way - but when this first went in crash on
startup was guaranteed otherwise if you try to go e.g. Beta->Stable with the
same profile. Should be safer now.

[Anand Mistry (off Chromium), 2014/08/13 01:53:15]

Done.

[tapted, 2014/08/13 03:06:55]

so yeah - looks like this can't tickle a crash out of stable, so should be fine.
cool.

     extension_dict->Set(kPrefState, new base::FundamentalValue(initial_state));
 
   extension_dict->Set(kPrefLocation,