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..b1de7b892d1af0c9fc4f6713853d1c20f7c3ec35 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"
@@ -72,8 +73,10 @@ public:
 
     virtual void didStartRunLoop();
     virtual void didStopRunLoop();
+    virtual void initializeBackingThread();
+    virtual void shutdownBackingThread();

[kinuko, 2015/05/28 16:21:59]

Are they called outside WorkerThread or subclass?  Can they be placed next to
initialize/destroyIsolate()?

[sadrul, 2015/05/28 16:51:56]

These are called only from inside WorkerThread, except for didStopRunLoop(),
which is called by the WorkerScriptController().

 
-    v8::Isolate* isolate() const { return m_isolate; }
+    v8::Isolate* isolate() const { return m_isolate ? m_isolate->isolate() : nullptr; }
 
     // 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,7 +125,7 @@ protected:
 
     virtual void postInitialize() { }
 
-    virtual v8::Isolate* initializeIsolate();
+    virtual PassOwnPtr<WorkerV8Isolate> initializeIsolate();
     virtual void willDestroyIsolate();
     virtual void destroyIsolate();
     virtual void terminateV8Execution();
@@ -155,13 +158,12 @@ 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|, |m_terminated|, and |m_isolate|.
     Mutex m_threadStateMutex;
 
     RefPtrWillBePersistent<WorkerGlobalScope> m_workerGlobalScope;
 
-    v8::Isolate* m_isolate;
-    OwnPtr<V8IsolateInterruptor> m_interruptor;
+    OwnPtr<WorkerV8Isolate> m_isolate;

[kinuko, 2015/05/28 16:21:59]

thread and isolate are used in a very similar way, but how they are tied to
WorkerThread are very different.  Why are they?

[sadrul, 2015/05/28 16:51:56]

They don't actually have to be. It needs a little more code duplication for the
various subclasses, and I wanted to avoid that. But I can go back to having each
subclass be responsible for managing the isolate instead.

[sadrul, 2015/05/28 18:00:47]

I have gone ahead and made this change. PTAL.

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

Well, was just asking because I vaguely remember you were planning to bundle
thread and isolate before (weren't you?).

I don't think each subclass should be necessarily responsible for managing it
either.

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

I was, yes! (I was going to put it in WebThreadSupportingGC, which the consumer
would optionally initialize) But since there was discussion about allowing
multiple isolates in the same thread at some point, I didn't want to make a
change here that would make experimenting with that harder.
I tend to agree. That was the approach I had in patchset6, which allows the
subclass to optionally override the behaviour, ridding all the other subclasses
to have to similar code. (although in terms of API, I think patchset7 is better,
and I can move the ownership of the WorkerV8Isolate to WorkerThread, have a
default implementation of workerIsolate(), and override it from
CompositorWorkerThread).

[kinuko, 2015/05/31 15:18:24]

WebThreadSupportingGC is used not only for JS-controlled threads but also for
background threads that don't require v8 isolate, so bundling isolate in it
would have been probably a bit undesirable anyway.
Ok. Can we rename workerIsolate() to createWorkerIsolate() if that's the case?
'initialize' (as in PS6) doesn't really mean creation, and 'workerIsolate'
accessor (as in PS7) makes the ownership and creation semantics unclear.

 
     // Used to signal thread shutdown.
     OwnPtr<WebWaitableEvent> m_shutdownEvent;