Begin to enable extension APIs in Extension Service Worker.

extensions/browser/extension_function_dispatcher.cc

Index: extensions/browser/extension_function_dispatcher.cc
diff --git a/extensions/browser/extension_function_dispatcher.cc b/extensions/browser/extension_function_dispatcher.cc
index 1ad91613b234814753d71e9abd69060c82270bc8..c0097dc95324fd656858d377e12351f54e607768 100644
--- a/extensions/browser/extension_function_dispatcher.cc
+++ b/extensions/browser/extension_function_dispatcher.cc
@@ -21,6 +21,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
@@ -208,6 +209,89 @@ class ExtensionFunctionDispatcher::UIThreadResponseCallbackWrapper
   DISALLOW_COPY_AND_ASSIGN(UIThreadResponseCallbackWrapper);
 };
 
+class ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper
+    : public content::RenderProcessHostObserver {
+ public:
+  UIThreadWorkerResponseCallbackWrapper(
+      const base::WeakPtr<ExtensionFunctionDispatcher>& dispatcher,
+      int render_process_id,
+      int worker_thread_id)
+      : dispatcher_(dispatcher),
+        render_process_id_(render_process_id),
+        worker_thread_id_(worker_thread_id),
+        weak_ptr_factory_(this) {
+    content::RenderProcessHost::FromID(render_process_id_)->AddObserver(this);
+  }
+
+  ~UIThreadWorkerResponseCallbackWrapper() override {}
+
+  // content::RenderProcessHostObserver override.
+  void RenderProcessExited(content::RenderProcessHost* process_host,
+                           base::TerminationStatus status,
+                           int exit_code) override {
+    process_host->RemoveObserver(this);
+    Cleanup(process_host->GetID());
+  }
+
+  // content::RenderProcessHostObserver override.
+  void RenderProcessHostDestroyed(
+      content::RenderProcessHost* process_host) override {
+    process_host->RemoveObserver(this);
+    Cleanup(process_host->GetID());
+  }
+
+  ExtensionFunction::ResponseCallback CreateCallback(int request_id) {
+    return base::Bind(
+        &UIThreadWorkerResponseCallbackWrapper::OnExtensionFunctionCompleted,
+        weak_ptr_factory_.GetWeakPtr(), request_id);
+  }
+
+ private:
+  void Cleanup(int render_process_id) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    if (render_process_id != render_process_id_)
+      return;
+
+    if (dispatcher_.get()) {
+      UIThreadWorkerResponseCallbackWrapperMap& map =

[Devlin, 2016/04/13 19:46:30]

This could potentially remove multiple CallbackWrappers, right?  If that's the
case, don't we need to remove ourselves as an observer in the dtor as well, if
we're still observing?

[lazyboy, 2016/04/14 02:07:52]

Ah, RenderProcessHost would still keep a reference to this.
Fixed.

+          dispatcher_->ui_thread_response_callback_wrappers_for_worker_;
+      for (UIThreadWorkerResponseCallbackWrapperMap::iterator it = map.begin();
+           it != map.end();) {
+        if (it->first.first == render_process_id) {
+          it = map.erase(it);
+          continue;
+        }
+        ++it;
+      }
+    }
+    // Note: we might be deleted here!

[Devlin, 2016/04/13 19:46:30]

Technically, this object might even be deleted *before* this point, anywhere in
the for-loop.  For that reason, I think I'd prefer to make a
RemoveWorkerCallbacksForProcess() function on the function dispatcher.

[lazyboy, 2016/04/14 02:07:52]

Done.

+  }
+
+  void OnExtensionFunctionCompleted(int request_id,
+                                    ExtensionFunction::ResponseType type,
+                                    const base::ListValue& results,
+                                    const std::string& error,
+                                    functions::HistogramValue histogram_value) {
+    if (type == ExtensionFunction::BAD_MESSAGE) {
+      // TODO(lazyboy): Kill the offending process.
+      return;
+    }
+    content::RenderProcessHost* sender =
+        content::RenderProcessHost::FromID(render_process_id_);
+    DCHECK(sender);
+    sender->Send(new ExtensionMsg_ResponseWorker(
+        worker_thread_id_, request_id, type == ExtensionFunction::SUCCEEDED,
+        results, error));
+  }
+
+  base::WeakPtr<ExtensionFunctionDispatcher> dispatcher_;
+  const int render_process_id_;
+  const int worker_thread_id_;
+  base::WeakPtrFactory<UIThreadWorkerResponseCallbackWrapper> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(UIThreadWorkerResponseCallbackWrapper);
+};
+
 WindowController*
 ExtensionFunctionDispatcher::Delegate::GetExtensionWindowController() const {
   return nullptr;
@@ -300,36 +384,59 @@ void ExtensionFunctionDispatcher::DispatchOnIOThread(
 
 ExtensionFunctionDispatcher::ExtensionFunctionDispatcher(
     content::BrowserContext* browser_context)
-    : browser_context_(browser_context) {
-}
+    : browser_context_(browser_context), delegate_(nullptr) {}
 
 ExtensionFunctionDispatcher::~ExtensionFunctionDispatcher() {
 }
 
 void ExtensionFunctionDispatcher::Dispatch(
     const ExtensionHostMsg_Request_Params& params,
-    content::RenderFrameHost* render_frame_host) {
-  UIThreadResponseCallbackWrapperMap::const_iterator
-      iter = ui_thread_response_callback_wrappers_.find(render_frame_host);
-  UIThreadResponseCallbackWrapper* callback_wrapper = nullptr;
-  if (iter == ui_thread_response_callback_wrappers_.end()) {
-    callback_wrapper = new UIThreadResponseCallbackWrapper(AsWeakPtr(),
-                                                           render_frame_host);
-    ui_thread_response_callback_wrappers_[render_frame_host] = callback_wrapper;
+    content::RenderFrameHost* render_frame_host,
+    int render_process_id) {
+  if (render_frame_host) {
+    UIThreadResponseCallbackWrapperMap::const_iterator iter =
+        ui_thread_response_callback_wrappers_.find(render_frame_host);
+    UIThreadResponseCallbackWrapper* callback_wrapper = nullptr;
+    if (iter == ui_thread_response_callback_wrappers_.end()) {
+      callback_wrapper =
+          new UIThreadResponseCallbackWrapper(AsWeakPtr(), render_frame_host);
+      ui_thread_response_callback_wrappers_[render_frame_host] =
+          callback_wrapper;
+    } else {
+      callback_wrapper = iter->second;
+    }
+    DispatchWithCallbackInternal(
+        params, render_frame_host, render_process_id,
+        callback_wrapper->CreateCallback(params.request_id));
   } else {
-    callback_wrapper = iter->second;
+    // Extension API from Service Worker.

[Devlin, 2016/04/13 19:46:31]

Add an analogous comment in the other if clause on line 397.

[lazyboy, 2016/04/14 02:07:52]

Done.

+    DCHECK(params.embedded_worker_id >= 0);

[Devlin, 2016/04/13 19:46:31]

DCHECK_GE?

[lazyboy, 2016/04/14 02:07:52]

Done.

+    const int& id = params.embedded_worker_id;

[Devlin, 2016/04/13 19:46:31]

no need to make this a const & - it's no cheaper than copying the int.

[lazyboy, 2016/04/14 02:07:52]

Done.

+    UIThreadWorkerResponseCallbackWrapperMapKey key =
+        std::make_pair(render_process_id, id);
+    UIThreadWorkerResponseCallbackWrapperMap::const_iterator iter =
+        ui_thread_response_callback_wrappers_for_worker_.find(key);
+    UIThreadWorkerResponseCallbackWrapper* callback_wrapper = nullptr;
+    if (iter == ui_thread_response_callback_wrappers_for_worker_.end()) {
+      callback_wrapper = new UIThreadWorkerResponseCallbackWrapper(
+          AsWeakPtr(), render_process_id, params.worker_thread_id);
+      ui_thread_response_callback_wrappers_for_worker_[key] =
+          std::unique_ptr<UIThreadWorkerResponseCallbackWrapper>(
+              callback_wrapper);
+    } else {
+      callback_wrapper = iter->second.get();
+    }
+    DispatchWithCallbackInternal(

[Devlin, 2016/04/13 19:46:31]

We can lump these (this and line 408) together if we just have a callback object
that we set in the two different if clauses.

I suspect we can also simplify some of the other duplicate logic here, but it's
a little messy to do so... I'll think on it a bit.

[lazyboy, 2016/04/14 02:07:52]

I had that in the beginning, but later the code diverged to two unrelated
callback classes instead of one:
UIThreadResponseCallbackWrapper, UIThreadWorkerResponseCallbackWrapper.

+        params, nullptr, render_process_id,
+        callback_wrapper->CreateCallback(params.request_id));
   }
-
-  DispatchWithCallbackInternal(
-      params, render_frame_host,
-      callback_wrapper->CreateCallback(params.request_id));
 }
 
 void ExtensionFunctionDispatcher::DispatchWithCallbackInternal(
     const ExtensionHostMsg_Request_Params& params,
     content::RenderFrameHost* render_frame_host,
+    int render_process_id,
     const ExtensionFunction::ResponseCallback& callback) {
-  DCHECK(render_frame_host);
   // TODO(yzshen): There is some shared logic between this method and
   // DispatchOnIOThread(). It is nice to deduplicate.
   ProcessMap* process_map = ProcessMap::Get(browser_context_);
@@ -344,15 +451,12 @@ void ExtensionFunctionDispatcher::DispatchWithCallbackInternal(
         registry->enabled_extensions().GetHostedAppByURL(params.source_url);
   }
 
-  int process_id = render_frame_host->GetProcess()->GetID();
-  scoped_refptr<ExtensionFunction> function(
-      CreateExtensionFunction(params,
-                              extension,
-                              process_id,
-                              *process_map,
-                              ExtensionAPI::GetSharedInstance(),
-                              browser_context_,
-                              callback));
+  if (render_frame_host)
+    DCHECK_EQ(render_process_id, render_frame_host->GetProcess()->GetID());
+
+  scoped_refptr<ExtensionFunction> function(CreateExtensionFunction(
+      params, extension, render_process_id, *process_map,
+      ExtensionAPI::GetSharedInstance(), browser_context_, callback));
   if (!function.get())
     return;
 
@@ -419,11 +523,15 @@ void ExtensionFunctionDispatcher::DispatchWithCallbackInternal(
   // now, largely for simplicity's sake. This is OK because currently, only
   // the webRequest API uses IOThreadExtensionFunction, and that API is not
   // compatible with lazy background pages.
+  // TODO(lazyboy): API functions from extension Service Worker will incorrectly
+  // change keepalive count below.
   process_manager->IncrementLazyKeepaliveCount(extension);
 }
 
 void ExtensionFunctionDispatcher::OnExtensionFunctionCompleted(
     const Extension* extension) {
+  // TODO(lazyboy): API functions from extension Service Worker will incorrectly
+  // change keepalive count below.
   if (extension) {
     ProcessManager::Get(browser_context_)
         ->DecrementLazyKeepaliveCount(extension);