compositor-worker: Share a thread and an isolate for compositor workers.

Source/core/workers/WorkerThread.h

Index: Source/core/workers/WorkerThread.h
diff --git a/Source/core/workers/WorkerThread.h b/Source/core/workers/WorkerThread.h
index 2405c86493da3a2d74d818d58207bc8e2333576c..62abd398b6676f9c6f5e97a5fc54db5e64fbaa35 100644
--- a/Source/core/workers/WorkerThread.h
+++ b/Source/core/workers/WorkerThread.h
@@ -32,6 +32,7 @@
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/workers/WorkerGlobalScope.h"
 #include "core/workers/WorkerLoaderProxy.h"
+#include "core/workers/WorkerV8Isolate.h"
 #include "platform/WebThreadSupportingGC.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "wtf/Forward.h"
@@ -69,11 +70,9 @@ public:
     // some implementations can use an existing thread that is already being
     // used by other workers (e.g.  compositor workers).
     virtual WebThreadSupportingGC& backingThread() = 0;
-
-    virtual void didStartRunLoop();
     virtual void didStopRunLoop();
 
-    v8::Isolate* isolate() const { return m_isolate; }
+    v8::Isolate* isolate();
 
     // Can be used to wait for this worker thread to shut down.
     // (This is signalled on the main thread, so it's assumed to be waited on the worker context thread)
@@ -122,11 +121,13 @@ protected:
 
     virtual void postInitialize() { }
 
-    virtual v8::Isolate* initializeIsolate();
-    virtual void willDestroyIsolate();
-    virtual void destroyIsolate();
+    virtual OwnPtr<WorkerV8Isolate>& workerIsolate() = 0;

[kinuko, 2015/05/29 17:45:18]

Returning OwnPtr but not for the caller to take ownership looks a bit uncommon
to me... hmm.

I think the functionality wise this change (or previous change) looks good to
me, but in general I'm feeling that the WorkerThread's interface is getting less
and less organized.  I can't come up with a good plan for now though.

[sadrul, 2015/05/29 18:26:32]

With this change:

. For managing the WebThread: WorkerThread now has initializeBackingThread(),
shutdownBackingThread(), and backingThread(). (I am not counting
didStartRunLoop() and didStopRunLoop(), since these are more about setting
things up in chrome-land than managing the thread)

. For managing the v8::Isolate: WorkerThread now has workerIsolate(),
terminateV8Execution(), and isolate() (since this CL is also removing some
now-unnecessary isolate-managing functions).

Both of the sets are fairly small, and pretty self-explanatory. Is there
anything specific you think looks ugly/untidy that I could address?

     virtual void terminateV8Execution();
 
+    virtual void didStartRunLoop();

[kinuko, 2015/05/29 17:45:18]

nit: I think this can be public as far as didStartRunLoop() is public, mainly
for symmetricity and readability.

[sadrul, 2015/05/29 18:26:32]

Acknowledged. Will make this change.

[sadrul, 2015/06/01 05:24:11]

Done.

+    virtual void initializeBackingThread();
+    virtual void shutdownBackingThread();
+
     // This is protected virtual for testing.
     virtual bool doIdleGc(double deadlineSeconds);
 
@@ -155,14 +156,11 @@ private:
     RefPtrWillBePersistent<WorkerInspectorController> m_workerInspectorController;
     Mutex m_workerInspectorControllerMutex;
 
-    // This lock protects |m_workerGlobalScope|, |m_terminated|, |m_isolate| and |m_microtaskRunner|.
+    // This lock protects |m_workerGlobalScope|, and |m_terminated|.
     Mutex m_threadStateMutex;
 
     RefPtrWillBePersistent<WorkerGlobalScope> m_workerGlobalScope;
 
-    v8::Isolate* m_isolate;
-    OwnPtr<V8IsolateInterruptor> m_interruptor;
-
     // Used to signal thread shutdown.
     OwnPtr<WebWaitableEvent> m_shutdownEvent;