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

extensions/browser/renderer_startup_helper.cc

Index: extensions/browser/renderer_startup_helper.cc
diff --git a/extensions/browser/renderer_startup_helper.cc b/extensions/browser/renderer_startup_helper.cc
index 9f3c171d9deaad4410cf87fe3c024ca1fc7db34f..d42bc8833d6742177c21943346b5c1118a1f5e05 100644
--- a/extensions/browser/renderer_startup_helper.cc
+++ b/extensions/browser/renderer_startup_helper.cc
@@ -28,6 +28,8 @@ RendererStartupHelper::RendererStartupHelper(BrowserContext* browser_context)
   DCHECK(browser_context);
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+                 content::NotificationService::AllBrowserContextsAndSources());
 }
 
 RendererStartupHelper::~RendererStartupHelper() {}
@@ -36,9 +38,21 @@ void RendererStartupHelper::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  DCHECK_EQ(content::NOTIFICATION_RENDERER_PROCESS_CREATED, type);
-  content::RenderProcessHost* process =
-      content::Source<content::RenderProcessHost>(source).ptr();
+  switch (type) {
+    case content::NOTIFICATION_RENDERER_PROCESS_CREATED:
+      InitializeProcess(
+          content::Source<content::RenderProcessHost>(source).ptr());
+      break;
+    case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED:
+      UntrackProcess(content::Source<content::RenderProcessHost>(source).ptr());
+      break;
+    default:
+      NOTREACHED() << "Unexpected notification: " << type;
+  }
+}
+
+void RendererStartupHelper::InitializeProcess(
+    content::RenderProcessHost* process) {
   ExtensionsBrowserClient* client = ExtensionsBrowserClient::Get();
   if (!client->IsSameContext(browser_context_, process->GetBrowserContext()))
     return;
@@ -91,6 +105,50 @@ void RendererStartupHelper::Observe(
     }
   }
   process->Send(new ExtensionMsg_Loaded(loaded_extensions));
+  auto iter = pending_active_extensions_.find(process);
+  if (iter != pending_active_extensions_.end()) {
+    for (const ExtensionId& id : iter->second)
+      process->Send(new ExtensionMsg_ActivateExtension(id));

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

DCHECK(extensions.Contains(id)) or does that make sense here?

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

Sure, done.

+  }
+
+  initialized_processes_.insert(process);
+}
+
+void RendererStartupHelper::UntrackProcess(
+    content::RenderProcessHost* process) {
+  initialized_processes_.erase(process);

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

Though not necessary, I think bailing out of this method early when process's
context doesn't match |browser_context_| is a good idea.
For one that is the correct thing to do. The other would be to avoid unnecessary
erase calls below.

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

Done.

+  pending_active_extensions_.erase(process);
+}
+
+void RendererStartupHelper::ActivateExtensionInProcess(
+    const ExtensionId& id,
+    content::RenderProcessHost* process) {
+  if (initialized_processes_.count(process))
+    process->Send(new ExtensionMsg_ActivateExtension(id));
+  else
+    pending_active_extensions_[process].insert(id);
+}
+
+void RendererStartupHelper::OnExtensionLoaded(const Extension& extension) {
+  if (extension.is_theme())

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

Retain the old comment:
// Renderers don't need to know about themes.

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

Done.

+    return;
+
+  // We don't need to include tab permisisons here, since the extension
+  // was just loaded.
+  // Uninitialized renderers will be informed of the extension load during the
+  // first batch of messages.
+  std::vector<ExtensionMsg_Loaded_Params> params(
+      1,
+      ExtensionMsg_Loaded_Params(&extension, false /* no tab permissions */));
+  for (content::RenderProcessHost* process : initialized_processes_)
+    process->Send(new ExtensionMsg_Loaded(params));
+}
+
+void RendererStartupHelper::OnExtensionUnloaded(const ExtensionId& id) {
+  for (content::RenderProcessHost* process : initialized_processes_)
+    process->Send(new ExtensionMsg_Unloaded(id));
+  for (auto& kv : pending_active_extensions_)

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

s/kv/extensions_in_process or sth.

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

replaced with process_extensions_pair (it's not extensions_in_process, since it
contains the key as well).  kv is my go to for map types for key-value, but I
suppose this is more legible.

+    kv.second.erase(id);
 }
 
 //////////////////////////////////////////////////////////////////////////////