Don't send unnecessary ExtensionMsg_Loaded IPCs.

extensions/renderer/dispatcher.cc

 // Copyright 2014 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 "extensions/renderer/dispatcher.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
@@ skipping 666 matching lines @@
           new GuestViewInternalCustomBindings(context)));
   module_system->RegisterNativeHandler(
       "i18n", scoped_ptr<NativeHandler>(new I18NCustomBindings(context)));
   module_system->RegisterNativeHandler(
       "id_generator",
       scoped_ptr<NativeHandler>(new IdGeneratorCustomBindings(context)));
   module_system->RegisterNativeHandler(
       "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)
   IPC_MESSAGE_HANDLER(ExtensionMsg_ActivateExtension, OnActivateExtension)
   IPC_MESSAGE_HANDLER(ExtensionMsg_CancelSuspend, OnCancelSuspend)
   IPC_MESSAGE_HANDLER(ExtensionMsg_DeliverMessage, OnDeliverMessage)
   IPC_MESSAGE_HANDLER(ExtensionMsg_DispatchOnConnect, OnDispatchOnConnect)
   IPC_MESSAGE_HANDLER(ExtensionMsg_DispatchOnDisconnect, OnDispatchOnDisconnect)
   IPC_MESSAGE_HANDLER(ExtensionMsg_Loaded, OnLoaded)
   IPC_MESSAGE_HANDLER(ExtensionMsg_MessageInvoke, OnMessageInvoke)
@@ skipping 28 matching lines @@
     forced_idle_timer_.reset(new base::RepeatingTimer<content::RenderThread>);
     forced_idle_timer_->Start(
         FROM_HERE,
         base::TimeDelta::FromMilliseconds(kMaxExtensionIdleHandlerDelayMs),
         RenderThread::Get(),
         &RenderThread::IdleHandler);
   }
 
   // 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);
   }
 
   EnableCustomElementWhiteList();
 
   is_webkit_initialized_ = true;
 }
 
 void Dispatcher::IdleNotification() {
   if (set_idle_notifications_ && forced_idle_timer_) {
@@ skipping 26 matching lines @@
     // executes on all renderers) be conservative and only crash in the renderer
     // of the extension which failed to load; this one.
     std::string& error = extension_load_errors_[extension_id];
     char minidump[256];
     base::debug::Alias(&minidump);
     base::snprintf(minidump,
                    arraysize(minidump),
                    "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);
 
   // This is called when starting a new extension page, so start the idle
   // handler ticking.
   RenderThread::Get()->ScheduleIdleHandler(kInitialExtensionIdleHandlerDelayMs);
 
   if (is_webkit_initialized_) {
     DOMActivityLogger::AttachToWorld(
@@ skipping 43 matching lines @@
 
 void Dispatcher::OnDispatchOnDisconnect(int port_id,
                                         const std::string& error_message) {
   MessagingBindings::DispatchOnDisconnect(*script_context_set_, port_id,
                                           error_message,
                                           NULL);  // All render frames.
 }
 
 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,
                                  const base::ListValue& args,
                                  bool user_gesture) {
   InvokeModuleSystemMethod(
       NULL, extension_id, module_name, function_name, args, user_gesture);
 }
 
 void Dispatcher::OnSetChannel(int channel) {
@@ skipping 31 matching lines @@
   // event creates.
   DispatchEvent(extension_id, kOnSuspendEvent);
   RenderThread::Get()->Send(new ExtensionHostMsg_SuspendAck(extension_id));
 }
 
 void Dispatcher::OnTransferBlobs(const std::vector<std::string>& blob_uuids) {
   RenderThread::Get()->Send(new ExtensionHostMsg_TransferBlobsAck(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);
 
   // If the extension is later reloaded with a different set of permissions,
   // we'd like it to get a new isolated world ID, so that it can pick up the
   // changed origin whitelist.
   ScriptInjection::RemoveIsolatedWorld(id);
 
   // Invalidate all of the contexts that were removed.
@@ skipping 477 matching lines @@
   // The "guestViewDeny" module must always be loaded last. It registers
   // error-providing custom elements for the GuestView types that are not
   // available, and thus all of those types must have been checked and loaded
   // (or not loaded) beforehand.
   if (context_type == Feature::BLESSED_EXTENSION_CONTEXT) {
     module_system->Require("guestViewDeny");
   }
 }
 
 }  // namespace extensions