Show alert failure for reloading unpacked extensions with bad manifest

chrome/browser/extensions/extension_service.cc

 // Copyright (c) 2013 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 <iterator>
 #include <set>
 
@@ skipping 680 matching lines @@
   scoped_ptr<ExtensionInfo> installed_extension(
       extension_prefs_->GetInstalledExtensionInfo(extension_id));
   if (installed_extension.get() &&
       installed_extension->extension_manifest.get()) {
     extensions::InstalledLoader(this).Load(*installed_extension, false);
   } else {
     // Otherwise, the extension is unpacked (location LOAD).
     // We should always be able to remember the extension's path. If it's not in
     // the map, someone failed to update |unloaded_extension_paths_|.
     CHECK(!path.empty());
-    extensions::UnpackedInstaller::Create(this)->Load(path);
+    scoped_refptr<extensions::UnpackedInstaller> unpacked_installer =
+        extensions::UnpackedInstaller::Create(this);
+    unpacked_installer->set_be_noisy_on_failure(false);

[gpdavis, 2014/06/30 19:42:52]

Still need to figure out how to address this problem.  Are we only displaying
the new UI for explicit extension loads, reloads and chrome://extensions page
refreshes?

[Devlin, 2014/06/30 21:06:41]

Not even (as there can be explicit extension loads from an external source, like
CAEDT).  Basically, we only show the new UI for an extension load that was
directly instigated by chrome:extensions.

Ideally, we'll want to make the new UI present in external sources like CAEDT,
too, but that's another project (possibly the next one). :)

[gpdavis, 2014/06/30 21:41:19]

Where exactly is the relevant code that triggers the reloads of all the
extensions when chrome://extensions is refreshed?  I'm not sure where to send
these signals from to indicate that the UI ought to be triggered.

[Devlin, 2014/06/30 22:32:36]

cs.chromium.org is pretty awesome.

So, you know that the function to reload an extension is in ExtensionService,
and is called ReloadExtension().  So we go there.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

Then we look to see where it's called.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

It looks like it's called from two places in WebUI, which sounds promising,
since chrome:extensions is all WebUI.  Let's check those two out.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

The first one doesn't seem very useful - it's just one extension.  Let's look at
the other.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

Bingo!  Here we see it looping over all unpacked extensions and reloading them -
that seems to be what we're looking for.  So where's this one called from?
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

Looks like it's called when the observed web contents has a provisional load. 
This makes sense with what we were thinking.

Doing this kind of hunt through code is amazingly useful, and is good to
practice. :)

[gpdavis, 2014/06/30 23:35:11]

I am fairly well acquainted with code search (I certainly wouldn't have made it
this far without it!), but I was going down that rabbit hole a little while back
and got lost rather quickly.  I think now that I'm more familiar with the
relevant code, I probably could have found that.  Thanks for the pointer in the
right direction, though!

So we need some way to set the noisy flag in the installer to false.  How about
adding a parameter to ReloadExtension, as I suggested earlier?  Seems like a
reasonable thing to require for such a function call.  Or maybe overloading it
so that any calls without the boolean flag will be redirected with a default
flag value?  Something like this:

ReloadExtension(const std::string& transient_extension_id, bool be_noisy) {
//current reload code, implementing boolean
}

ReloadExtension(const std::string& transient_extension_id) {
ReloadExtension(transient_extension_id, true);
}

[Devlin, 2014/07/01 17:02:55]

Yeah, adding a boolean be_noisy to ExtensionService is fine.  IIRC, your
previous suggestion was to have ExtensionService call the WebUI on failure,
which is incorrect (as it shouldn't have any knowledge of it).  If possible,
prefer only one Reload method (as opposed to overloading).

[gpdavis, 2014/07/01 20:58:55]

Sweet.  I went ahead and added that argument and updated all calls to the
function instead of overloading.

+    unpacked_installer->Load(path);
   }
   // When reloading is done, mark this extension as done reloading.
   SetBeingReloaded(extension_id, false);
 #endif  // defined(ENABLE_EXTENSIONS)
 }
 
 bool ExtensionService::UninstallExtension(
     // "transient" because the process of uninstalling may cause the reference
     // to become invalid. Instead, use |extenson->id()|.
     const std::string& transient_extension_id,
@@ skipping 1775 matching lines @@
 }
 
 void ExtensionService::OnProfileDestructionStarted() {
   ExtensionIdSet ids_to_unload = registry_->enabled_extensions().GetIDs();
   for (ExtensionIdSet::iterator it = ids_to_unload.begin();
        it != ids_to_unload.end();
        ++it) {
     UnloadExtension(*it, UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN);
   }
 }