Worker: Fix broken GC logic on Dedicated Worker while DOMTimer is set

third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h

Index: third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
diff --git a/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h b/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
index f1aa6e3c6e5f6254446fe494fe01baffb0ad3f84..de0c2fd69e2ad4dd326d95dc938f1d078631b1dd 100644
--- a/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
+++ b/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
@@ -70,9 +70,9 @@ public:
     void reportException(const String& errorMessage, std::unique_ptr<SourceLocation>, int exceptionId);
     void reportConsoleMessage(MessageSource, MessageLevel, const String& message, std::unique_ptr<SourceLocation>);
     void postMessageToPageInspector(const String&);
-    void confirmMessageFromWorkerObject(bool hasPendingActivity);
-    void reportPendingActivity(bool hasPendingActivity);
-    void workerThreadTerminated();
+    void confirmMessageFromWorkerObject();
+    virtual void reportPendingActivity(bool hasPendingActivity);
+    virtual void workerThreadTerminated();
     void workerThreadCreated();
 
     ExecutionContext* getExecutionContext() const { return m_executionContext.get(); }
@@ -81,7 +81,7 @@ public:
     static int proxyCount();
 
 protected:
-    InProcessWorkerMessagingProxy(InProcessWorkerBase*, WorkerClients*);
+    InProcessWorkerMessagingProxy(ExecutionContext*, InProcessWorkerBase*, WorkerClients*);

[kinuko, 2016/08/05 14:55:29]

Does this start to take ExecutionContext* separately only for testing? 
Alternatively we could add a test-only private constructor or override
getExecutionContext() instead?

[nhiroki, 2016/08/08 09:19:13]

Yes, this change is only for testing. Some tests create a messaging proxy with
null InProcessWorkerBase* and that results in a crash on the ctor
(|workerObject->getExecutionContext()|), so I made this change.

Instead of the approach, I added a private ctor that can accept a null
WorkerObjectBase, and then made the proper ctor and tests call it.

     ~InProcessWorkerMessagingProxy() override;
 
     virtual std::unique_ptr<WorkerThread> createWorkerThread(double originTime) = 0;
@@ -89,6 +89,12 @@ protected:
     PassRefPtr<WorkerLoaderProxy> loaderProxy() { return m_loaderProxy; }
     InProcessWorkerObjectProxy& workerObjectProxy() { return *m_workerObjectProxy.get(); }
 
+    std::unique_ptr<WorkerThread> m_workerThread;
+
+    // The latest confirmation from worker thread reported that it was still
+    // active.
+    bool m_workerThreadHadPendingActivity = false;

[kinuko, 2016/08/05 14:55:29]

nit: not a huge fan of exposing field members to subclasses, esp. if it's only
for testing class

[nhiroki, 2016/08/08 09:19:14]

Moved these fields back to the private section.

+
 private:
     void workerObjectDestroyedInternal();
 
@@ -104,17 +110,12 @@ private:
     Persistent<ExecutionContext> m_executionContext;
     std::unique_ptr<InProcessWorkerObjectProxy> m_workerObjectProxy;
     WeakPersistent<InProcessWorkerBase> m_workerObject;
-    bool m_mayBeDestroyed;
-    std::unique_ptr<WorkerThread> m_workerThread;
+    bool m_mayBeDestroyed = false;
 
     // Unconfirmed messages from the parent context thread to the worker thread.
     unsigned m_unconfirmedMessageCount;
 
-    // The latest confirmation from worker thread reported that it was still
-    // active.
-    bool m_workerThreadHadPendingActivity;
-
-    bool m_askedToTerminate;
+    bool m_askedToTerminate = false;
 
     // Tasks are queued here until there's a thread object created.
     Vector<std::unique_ptr<ExecutionContextTask>> m_queuedEarlyTasks;