[Extensions Bindings] Call InitializeBindingsSystem for worker bindings

Ensure that we call DispatcherDelegate::InitializeBindingsSystem for
the bindings system created for worker-thread bindings in order to
allow adding custom bindings.

Bug: 653596
Change-Id: I4e5a71d13d27738098daf281ff49400f533b716d
Reviewed-on: https://chromium-review.googlesource.com/759720
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521108}

extensions/renderer/dispatcher.cc

@@ -201,19 +201,8 @@ Dispatcher::Dispatcher(std::unique_ptr<DispatcherDelegate> delegate)
   const base::CommandLine& command_line =
       *(base::CommandLine::ForCurrentProcess());
 
-  std::unique_ptr<IPCMessageSender> ipc_message_sender =
-      IPCMessageSender::CreateMainThreadIPCMessageSender();
-  if (base::FeatureList::IsEnabled(features::kNativeCrxBindings)) {
-    // This Unretained is safe because the IPCMessageSender is guaranteed to
-    // outlive the bindings system.
-    auto system = std::make_unique<NativeExtensionBindingsSystem>(
-        std::move(ipc_message_sender));
-    delegate_->InitializeBindingsSystem(this, system->api_system());
-    bindings_system_ = std::move(system);
-  } else {
-    bindings_system_ = std::make_unique<JsExtensionBindingsSystem>(
-        &source_map_, std::move(ipc_message_sender));
-  }
+  bindings_system_ = CreateBindingsSystem(
+      IPCMessageSender::CreateMainThreadIPCMessageSender());
 
   set_idle_notifications_ =
       command_line.HasSwitch(switches::kExtensionProcess) ||
@@ -417,8 +406,13 @@ void Dispatcher::DidInitializeServiceWorkerContextOnWorkerThread(
   context->set_service_worker_scope(service_worker_scope);
 
   if (ExtensionsClient::Get()->ExtensionAPIEnabledInExtensionServiceWorkers()) {
-    WorkerThreadDispatcher::Get()->AddWorkerData(service_worker_version_id,
-                                                 context, &source_map_);
+    WorkerThreadDispatcher* worker_dispatcher = WorkerThreadDispatcher::Get();
+    std::unique_ptr<IPCMessageSender> ipc_sender =
+        IPCMessageSender::CreateWorkerThreadIPCMessageSender(
+            worker_dispatcher, service_worker_version_id);
+    worker_dispatcher->AddWorkerData(
+        service_worker_version_id, context,
+        CreateBindingsSystem(std::move(ipc_sender)));
 
     // TODO(lazyboy): Make sure accessing |source_map_| in worker thread is
     // safe.
@@ -1449,4 +1443,19 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) {
   }
 }
 
+std::unique_ptr<ExtensionBindingsSystem> Dispatcher::CreateBindingsSystem(
+    std::unique_ptr<IPCMessageSender> ipc_sender) {
+  std::unique_ptr<ExtensionBindingsSystem> bindings_system;
+  if (base::FeatureList::IsEnabled(features::kNativeCrxBindings)) {
+    auto system =
+        std::make_unique<NativeExtensionBindingsSystem>(std::move(ipc_sender));
+    delegate_->InitializeBindingsSystem(this, system->api_system());
+    bindings_system = std::move(system);
+  } else {
+    bindings_system = std::make_unique<JsExtensionBindingsSystem>(
+        &source_map_, std::move(ipc_sender));
+  }
+  return bindings_system;
+}
+
 }  // namespace extensions

extensions/renderer/dispatcher.h

@@ -55,6 +55,7 @@ namespace extensions {
 class ContentWatcher;
 class DispatcherDelegate;
 class ExtensionBindingsSystem;
+class IPCMessageSender;
 class ScriptContext;
 class ScriptInjectionManager;
 struct EventFilteringInfo;
@@ -246,6 +247,12 @@ class Dispatcher : public content::RenderThreadObserver,
   // |context|.
   void RequireGuestViewModules(ScriptContext* context);
 
+  // Creates the ExtensionBindingsSystem. Note: this may be called on any
+  // thread, and thus cannot mutate any state or rely on state which can be
+  // mutated in Dispatcher.
+  std::unique_ptr<ExtensionBindingsSystem> CreateBindingsSystem(
+      std::unique_ptr<IPCMessageSender> ipc_sender);
+
   // The delegate for this dispatcher to handle embedder-specific logic.
   std::unique_ptr<DispatcherDelegate> delegate_;
 

extensions/renderer/worker_thread_dispatcher.cc

@@ -17,7 +17,7 @@
 #include "extensions/common/extension_messages.h"
 #include "extensions/renderer/dispatcher.h"
 #include "extensions/renderer/extension_bindings_system.h"
-#include "extensions/renderer/ipc_message_sender.h"
+#include "extensions/renderer/extensions_renderer_client.h"
 #include "extensions/renderer/js_extension_bindings_system.h"
 #include "extensions/renderer/native_extension_bindings_system.h"
 #include "extensions/renderer/service_worker_data.h"
@@ -148,26 +148,9 @@ void WorkerThreadDispatcher::OnDispatchEvent(
 void WorkerThreadDispatcher::AddWorkerData(
     int64_t service_worker_version_id,
     ScriptContext* context,
-    ResourceBundleSourceMap* source_map) {
+    std::unique_ptr<ExtensionBindingsSystem> bindings_system) {
   ServiceWorkerData* data = g_data_tls.Pointer()->Get();
   if (!data) {
-    std::unique_ptr<ExtensionBindingsSystem> bindings_system;
-    // QUESTION(lazyboy): Why is passing the WorkerThreadDispatcher to the
-    // IPCMessageSender (previously the ServiceWorkerRequestSender) safe? The
-    // WorkerThreadDispatcher is a process-wide singleton, but the
-    // IPCMessageSender is per-context (thus potentially many per process).
-    std::unique_ptr<IPCMessageSender> ipc_message_sender =
-        IPCMessageSender::CreateWorkerThreadIPCMessageSender(
-            this, service_worker_version_id);
-    if (base::FeatureList::IsEnabled(features::kNativeCrxBindings)) {
-      // The Unretained below is safe since the IPC message sender outlives the
-      // bindings system.
-      bindings_system = std::make_unique<NativeExtensionBindingsSystem>(
-          std::move(ipc_message_sender));
-    } else {
-      bindings_system = std::make_unique<JsExtensionBindingsSystem>(
-          source_map, std::move(ipc_message_sender));
-    }
     ServiceWorkerData* new_data = new ServiceWorkerData(
         service_worker_version_id, context, std::move(bindings_system));
     g_data_tls.Pointer()->Set(new_data);

extensions/renderer/worker_thread_dispatcher.h

@@ -23,7 +23,6 @@ class RenderThread;
 struct ExtensionMsg_DispatchEvent_Params;
 namespace extensions {
 class ExtensionBindingsSystem;
-class ResourceBundleSourceMap;
 class ScriptContext;
 class V8SchemaRegistry;
 
@@ -52,7 +51,7 @@ class WorkerThreadDispatcher : public content::RenderThreadObserver,
 
   void AddWorkerData(int64_t service_worker_version_id,
                      ScriptContext* context,
-                     ResourceBundleSourceMap* source_map);
+                     std::unique_ptr<ExtensionBindingsSystem> bindings_system);
   void RemoveWorkerData(int64_t service_worker_version_id);
 
   EventBookkeeper* event_bookkeeper() { return &event_bookkeeper_; }