Don't send unnecessary ExtensionMsg_Loaded IPCs.

extensions/renderer/dispatcher.cc

Index: extensions/renderer/dispatcher.cc
diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc
index f210e27e2b0f290c16be549822ba72fb78076561..9eedd9a088e24e7e7ea88f5463a54809c40c5273 100644
--- a/extensions/renderer/dispatcher.cc
+++ b/extensions/renderer/dispatcher.cc
@@ -684,6 +684,10 @@ void Dispatcher::RegisterNativeHandlers(ModuleSystem* module_system,
       "runtime", scoped_ptr<NativeHandler>(new RuntimeCustomBindings(context)));
 }
 
+void Dispatcher::LoadExtensionForTest(const Extension* extension) {
+  CHECK(extensions_.Insert(extension));
+}
+
 bool Dispatcher::OnControlMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(Dispatcher, message)
@@ -732,12 +736,9 @@ void Dispatcher::WebKitInitialized() {
 
   // Initialize host permissions for any extensions that were activated before
   // WebKit was initialized.
-  for (std::set<std::string>::iterator iter = active_extension_ids_.begin();
-       iter != active_extension_ids_.end();
-       ++iter) {
-    const Extension* extension = extensions_.GetByID(*iter);
+  for (const std::string& extension_id : active_extension_ids_) {
+    const Extension* extension = extensions_.GetByID(extension_id);
     CHECK(extension);
-
     InitOriginPermissions(extension);
   }
 
@@ -784,7 +785,7 @@ void Dispatcher::OnActivateExtension(const std::string& extension_id) {
                    "e::dispatcher:%s:%s",
                    extension_id.c_str(),
                    error.c_str());
-    CHECK(extension) << extension_id << " was never loaded: " << error;
+    LOG(FATAL) << extension_id << " was never loaded: " << error;
   }
 
   active_extension_ids_.insert(extension_id);
@@ -848,26 +849,36 @@ void Dispatcher::OnDispatchOnDisconnect(int port_id,
 
 void Dispatcher::OnLoaded(
     const std::vector<ExtensionMsg_Loaded_Params>& loaded_extensions) {
-  std::vector<ExtensionMsg_Loaded_Params>::const_iterator i;
-  for (i = loaded_extensions.begin(); i != loaded_extensions.end(); ++i) {
+  for (const auto& param : loaded_extensions) {
     std::string error;
-    scoped_refptr<const Extension> extension = i->ConvertToExtension(&error);
+    scoped_refptr<const Extension> extension = param.ConvertToExtension(&error);
     if (!extension.get()) {
-      extension_load_errors_[i->id] = error;
+      NOTREACHED() << error;
+      // Note: in tests |param.id| has been observed to be empty (see comment
+      // just below) so this isn't all that reliable.
+      extension_load_errors_[param.id] = error;
       continue;
     }
-    OnLoadedInternal(extension);
+
+    // This test is deliberately not a CHECK (though I wish it could be) and

[Devlin, 2015/06/03 20:53:37]

This kind of sounds like a TODO.  Wanna own it? :)

[not at google - send to devlin, 2015/06/03 21:53:03]

well I don't plan on doing it, but sure :-)

+    // uses extension->id() not params.id:
+    // 1. For some reason params.id can be empty. I've only seen it with
+    //    the webstore extension, in tests, and I've spent some time trying to
+    //    figure out why - but cost/benefit won.
+    // 2. The browser only sends this IPC to RenderProcessHosts once, but the
+    //    Dispatcher is attached to a RenderThread. Presumably there is a
+    //    mismatch there. In theory one would think it's possible for the
+    //    browser to figure this out itself - but again, cost/benefit.
+    if (!extensions_.Contains(extension->id()))
+      extensions_.Insert(extension);
   }
+
   // Update the available bindings for all contexts. These may have changed if
   // an externally_connectable extension was loaded that can connect to an
   // open webpage.
   UpdateBindings("");
 }
 
-void Dispatcher::OnLoadedInternal(scoped_refptr<const Extension> extension) {
-  extensions_.Insert(extension);
-}
-
 void Dispatcher::OnMessageInvoke(const std::string& extension_id,
                                  const std::string& module_name,
                                  const std::string& function_name,
@@ -919,7 +930,11 @@ void Dispatcher::OnTransferBlobs(const std::vector<std::string>& blob_uuids) {
 }
 
 void Dispatcher::OnUnloaded(const std::string& id) {
-  extensions_.Remove(id);
+  // See comment in OnLoaded for why it would be nice, but perhaps incorrect,
+  // to CHECK here rather than guarding.
+  if (!extensions_.Remove(id))
+    return;
+
   active_extension_ids_.erase(id);
 
   script_injection_manager_->OnExtensionUnloaded(id);