[ServiceWorker] Add EmbeddedWorkerInstanceHost Interface.

content/browser/service_worker/embedded_worker_instance.h

Index: content/browser/service_worker/embedded_worker_instance.h
diff --git a/content/browser/service_worker/embedded_worker_instance.h b/content/browser/service_worker/embedded_worker_instance.h
index ca5d522cd3500b0e1719f730a61c9c5a1667bc83..546d368edacc97303d9599432e1d96f1b911fdab 100644
--- a/content/browser/service_worker/embedded_worker_instance.h
+++ b/content/browser/service_worker/embedded_worker_instance.h
@@ -26,6 +26,7 @@
 #include "content/common/service_worker/embedded_worker.mojom.h"
 #include "content/common/service_worker/service_worker_event_dispatcher.mojom.h"
 #include "content/common/service_worker/service_worker_status_code.h"
+#include "mojo/public/cpp/bindings/associated_binding.h"
 #include "url/gurl.h"
 
 // Windows headers will redefine SendMessage.
@@ -46,7 +47,8 @@ class ServiceWorkerContextCore;
 // This gives an interface to control one EmbeddedWorker instance, which
 // may be 'in-waiting' or running in one of the child processes added by
 // AddProcessReference().
-class CONTENT_EXPORT EmbeddedWorkerInstance {
+class CONTENT_EXPORT EmbeddedWorkerInstance
+    : NON_EXPORTED_BASE(public mojom::EmbeddedWorkerInstanceHost) {
  public:
   typedef base::Callback<void(ServiceWorkerStatusCode)> StatusCallback;
 
@@ -99,7 +101,7 @@ class CONTENT_EXPORT EmbeddedWorkerInstance {
     CONTENT_EXPORT virtual bool OnMessageReceived(const IPC::Message& message);
   };
 
-  ~EmbeddedWorkerInstance();
+  ~EmbeddedWorkerInstance() override;
 
   // Starts the worker. It is invalid to call this when the worker is not in
   // STOPPED status. |callback| is invoked after the worker script has been
@@ -219,37 +221,43 @@ class CONTENT_EXPORT EmbeddedWorkerInstance {
   // Called back from StartTask after a start worker message is sent.
   void OnStartWorkerMessageSent();
 
-  // Called back from Registry when the worker instance has ack'ed that
+  // Implements mojom::EmbeddedWorkerInstanceHost.
+  // Be called when the worker instance has ack'ed that

[kinuko, 2017/04/11 14:03:17]

nit: 'Called' seems to be more common than 'Be called' in SW code base?

Also: we prefer descriptive comments rather than imperative ones for function
declarations, so if we change this 'Gets called' is probably preferable

https://google.github.io/styleguide/cppguide.html#Comments

[falken, 2017/04/11 14:10:05]

I'm thinking we should move all these comments to the mojom file. Generally, the
mojo interface should be a well-documented interface that the C++ class
implements. Then almost all that's needed here is
"mojom::EmbeddedWorkerInstanceHost implementation". We also would have less
duplicated comments.

What would belong here are implementation details only like:
"This will change the internal status from STARTING to RUNNING."

[kinuko, 2017/04/12 00:49:44]

Yeah I agree, SGTM

[leonhsl(Using Gerrit), 2017/04/12 03:34:03]

I see, and agree that we should document mojom file well and add comments about
impl details in C++ header file. Will update.

[leonhsl(Using Gerrit), 2017/04/12 12:32:12]

Done.

   // it is ready for inspection.
-  void OnReadyForInspection();
-
-  // Called back from Registry when the worker instance has ack'ed that
+  void OnReadyForInspection() override;
+  // Be called when the worker instance has ack'ed that
   // it finished loading the script.
-  void OnScriptLoaded();
-
-  // Called back from Registry when the worker instance has ack'ed that
+  void OnScriptLoaded() override;
+  // Be called when the worker instance has ack'ed that
   // it has started a worker thread.
-  void OnThreadStarted(int thread_id);
-
-  // Called back from Registry when the worker instance has ack'ed that
+  void OnThreadStarted(int thread_id, int provider_id) override;
+  // Be called when the worker instance has ack'ed that
   // it failed to load the script.
-  void OnScriptLoadFailed();
-
-  // Called back from Registry when the worker instance has ack'ed that
+  void OnScriptLoadFailed() override;
+  // Be called when the worker instance has ack'ed that
   // it finished evaluating the script. This is called before OnStarted.
-  void OnScriptEvaluated(bool success);
-
-  // Called back from Registry when the worker instance has ack'ed that its
+  void OnScriptEvaluated(bool success) override;
+  // Be called when the worker instance has ack'ed that
   // WorkerGlobalScope has actually started and evaluated the script. This is
   // called after OnScriptEvaluated.
   // This will change the internal status from STARTING to RUNNING.
-  void OnStarted();
-
-  // Called back from Registry when the worker instance has ack'ed that
+  void OnStarted() override;
+  // Be called when the worker instance has ack'ed that
   // its WorkerGlobalScope is actually stopped in the child process.
   // This will change the internal status from STARTING or RUNNING to
   // STOPPED.
-  void OnStopped();
+  void OnStopped() override;
+  // Be called when the worker instance reports the exception.
+  void OnReportException(const base::string16& error_message,
+                         int line_number,
+                         int column_number,
+                         const GURL& source_url) override;
+  // Be called when the worker instance reports to the console.
+  void OnReportConsoleMessage(int source_identifier,
+                              int message_level,
+                              const base::string16& message,
+                              int line_number,
+                              const GURL& source_url) override;
 
   // Called when ServiceWorkerDispatcherHost for the worker died while it was
   // running.
@@ -260,19 +268,6 @@ class CONTENT_EXPORT EmbeddedWorkerInstance {
   // Returns false if the message is not handled.
   bool OnMessageReceived(const IPC::Message& message);
 
-  // Called back from Registry when the worker instance reports the exception.
-  void OnReportException(const base::string16& error_message,
-                         int line_number,
-                         int column_number,
-                         const GURL& source_url);
-
-  // Called back from Registry when the worker instance reports to the console.
-  void OnReportConsoleMessage(int source_identifier,
-                              int message_level,
-                              const base::string16& message,
-                              int line_number,
-                              const GURL& source_url);
-
   // Resets all running state. After this function is called, |status_| is
   // STOPPED.
   void ReleaseProcess();
@@ -303,6 +298,9 @@ class CONTENT_EXPORT EmbeddedWorkerInstance {
   // |client_| is used to send messages to the renderer process.
   mojom::EmbeddedWorkerInstanceClientPtr client_;
 
+  // Binding for EmbeddedWorkerInstanceHost, runs on IO thread.
+  mojo::AssociatedBinding<EmbeddedWorkerInstanceHost> instance_host_binding_;
+
   // TODO(shimazu): Remove this after EmbeddedWorkerStartParams is changed to
   // a mojo struct.
   mojom::ServiceWorkerEventDispatcherRequest pending_dispatcher_request_;