[Extensions] Ensure ordering of extension [un]loaded, activated messages

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 <stddef.h>
 
 #include <memory>
 #include <utility>
@@ skipping 979 matching lines @@
   script_context_set_->ForEach(
       std::string(), nullptr,
       base::Bind(&ScriptContextSet::Remove,
                  base::Unretained(script_context_set_.get())));
 }
 
 void Dispatcher::OnActivateExtension(const std::string& extension_id) {
   const Extension* extension =
       RendererExtensionRegistry::Get()->GetByID(extension_id);
   if (!extension) {
+    NOTREACHED();
     // Extension was activated but was never loaded. This probably means that
     // the renderer failed to load it (or the browser failed to tell us when it
     // did). Failures shouldn't happen, but instead of crashing there (which
     // 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),
@@ skipping 75 matching lines @@
         RendererExtensionRegistry::Get();
     // TODO(kalman): This test is deliberately not a CHECK (though I wish it
     // could be) and 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.
+    // TODO(devlin): This may be fixed by crbug.com/528026. Monitor, and
+    // consider making this a release CHECK.
+    DCHECK(!extension_registry->Contains(extension->id()));

[lazyboy, 2016/07/22 00:29:31]

Change these to:
if (!extension_registry->Insert(extension)) {
  // TODO.. This may be ...
  NOTREACHED();
}

[Devlin, 2016/07/26 03:45:09]

Done.

     if (!extension_registry->Contains(extension->id()))
       extension_registry->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("");
 }
 
@@ skipping 48 matching lines @@
   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) {
   // See comment in OnLoaded for why it would be nice, but perhaps incorrect,
   // to CHECK here rather than guarding.
-  if (!RendererExtensionRegistry::Get()->Remove(id))
+  // TODO(devlin): This may be fixed by crbug.com/528026. Monitor, and
+  // consider making this a release CHECK.
+  if (!RendererExtensionRegistry::Get()->Remove(id)) {
+    NOTREACHED();
     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);
 
@@ skipping 491 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