Fix UAF in closures posted from InProcessWorkerObjectProxy

Tasks posted by InProcessWorkerObjectProxy can be arbitrarily delayed
so it's not safe to post a closure with an crossThreadUnretained pointer
to the messaging proxy because we can't guarantee it's still there when
the closure eventually gets run.

We fix this with a WeakPtr.

BUG=660427

Review-Url: https://codereview.chromium.org/2478113002
Cr-Commit-Position: refs/heads/master@{#430251}

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

@@ -87,11 +87,14 @@ InProcessWorkerMessagingProxy::InProcessWorkerMessagingProxy(
     InProcessWorkerBase* workerObject,
     WorkerClients* workerClients)
     : ThreadedMessagingProxyBase(executionContext),
-      m_workerObjectProxy(InProcessWorkerObjectProxy::create(this)),
       m_workerObject(workerObject),
       m_workerClients(workerClients),
       m_unconfirmedMessageCount(0),
-      m_workerGlobalScopeMayHavePendingActivity(false) {}
+      m_workerGlobalScopeMayHavePendingActivity(false),
+      m_weakPtrFactory(this) {
+  m_workerObjectProxy =
+      InProcessWorkerObjectProxy::create(m_weakPtrFactory.createWeakPtr());
+}
 
 InProcessWorkerMessagingProxy::~InProcessWorkerMessagingProxy() {
   DCHECK(!m_workerObject);

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

@@ -104,6 +104,8 @@ class CORE_EXPORT InProcessWorkerMessagingProxy
   unsigned m_unconfirmedMessageCount;
 
   bool m_workerGlobalScopeMayHavePendingActivity;
+
+  WeakPtrFactory<InProcessWorkerMessagingProxy> m_weakPtrFactory;
 };
 
 }  // namespace blink

third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.cpp

@@ -53,7 +53,7 @@ const double kDefaultIntervalInSec = 1;
 const double kMaxIntervalInSec = 30;
 
 std::unique_ptr<InProcessWorkerObjectProxy> InProcessWorkerObjectProxy::create(
-    InProcessWorkerMessagingProxy* messagingProxy) {
+    const WeakPtr<InProcessWorkerMessagingProxy>& messagingProxy) {
   DCHECK(messagingProxy);
   return wrapUnique(new InProcessWorkerObjectProxy(messagingProxy));
 }
@@ -68,8 +68,8 @@ void InProcessWorkerObjectProxy::postMessageToWorkerObject(
       ->postTask(BLINK_FROM_HERE,
                  crossThreadBind(
                      &InProcessWorkerMessagingProxy::postMessageToWorkerObject,
-                     crossThreadUnretained(m_messagingProxy),
-                     std::move(message), passed(std::move(channels))));
+                     m_messagingProxyWeakPtr, std::move(message),
+                     passed(std::move(channels))));
 }
 
 void InProcessWorkerObjectProxy::postTaskToMainExecutionContext(
@@ -86,7 +86,7 @@ void InProcessWorkerObjectProxy::confirmMessageFromWorkerObject() {
           BLINK_FROM_HERE,
           crossThreadBind(
               &InProcessWorkerMessagingProxy::confirmMessageFromWorkerObject,
-              crossThreadUnretained(m_messagingProxy)));
+              m_messagingProxyWeakPtr));
 }
 
 void InProcessWorkerObjectProxy::startPendingActivityTimer() {
@@ -110,7 +110,7 @@ void InProcessWorkerObjectProxy::reportException(
       ->postTask(
           BLINK_FROM_HERE,
           crossThreadBind(&InProcessWorkerMessagingProxy::dispatchErrorEvent,
-                          crossThreadUnretained(m_messagingProxy), errorMessage,
+                          m_messagingProxyWeakPtr, errorMessage,
                           passed(location->clone()), exceptionId));
 }
 
@@ -124,8 +124,8 @@ void InProcessWorkerObjectProxy::reportConsoleMessage(
       ->postTask(
           BLINK_FROM_HERE,
           crossThreadBind(&InProcessWorkerMessagingProxy::reportConsoleMessage,
-                          crossThreadUnretained(m_messagingProxy), source,
-                          level, message, passed(location->clone())));
+                          m_messagingProxyWeakPtr, source, level, message,
+                          passed(location->clone())));
 }
 
 void InProcessWorkerObjectProxy::postMessageToPageInspector(
@@ -138,7 +138,7 @@ void InProcessWorkerObjectProxy::postMessageToPageInspector(
         BLINK_FROM_HERE,
         createCrossThreadTask(
             &InProcessWorkerMessagingProxy::postMessageToPageInspector,
-            crossThreadUnretained(m_messagingProxy), message));
+            m_messagingProxyWeakPtr, message));
   }
 }
 
@@ -160,7 +160,7 @@ void InProcessWorkerObjectProxy::didCloseWorkerGlobalScope() {
       ->postTask(
           BLINK_FROM_HERE,
           crossThreadBind(&InProcessWorkerMessagingProxy::terminateGlobalScope,
-                          crossThreadUnretained(m_messagingProxy)));
+                          m_messagingProxyWeakPtr));
 }
 
 void InProcessWorkerObjectProxy::willDestroyWorkerGlobalScope() {
@@ -175,12 +175,13 @@ void InProcessWorkerObjectProxy::didTerminateWorkerThread() {
       ->postTask(BLINK_FROM_HERE,
                  crossThreadBind(
                      &InProcessWorkerMessagingProxy::workerThreadTerminated,
-                     crossThreadUnretained(m_messagingProxy)));
+                     m_messagingProxyWeakPtr));
 }
 
 InProcessWorkerObjectProxy::InProcessWorkerObjectProxy(
-    InProcessWorkerMessagingProxy* messagingProxy)
-    : m_messagingProxy(messagingProxy),
+    const WeakPtr<InProcessWorkerMessagingProxy>& messagingProxy)
+    : m_messagingProxy(messagingProxy.get()),
+      m_messagingProxyWeakPtr(messagingProxy),
       m_defaultIntervalInSec(kDefaultIntervalInSec),
       m_nextIntervalInSec(kDefaultIntervalInSec),
       m_maxIntervalInSec(kMaxIntervalInSec) {}
@@ -206,7 +207,7 @@ void InProcessWorkerObjectProxy::checkPendingActivity(TimerBase*) {
         ->postTask(BLINK_FROM_HERE,
                    crossThreadBind(
                        &InProcessWorkerMessagingProxy::pendingActivityFinished,
-                       crossThreadUnretained(m_messagingProxy)));
+                       m_messagingProxyWeakPtr));
 
     // Don't schedule a timer. It will be started again when a message event
     // is dispatched.

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

@@ -64,7 +64,7 @@ class CORE_EXPORT InProcessWorkerObjectProxy : public WorkerReportingProxy {
 
  public:
   static std::unique_ptr<InProcessWorkerObjectProxy> create(
-      InProcessWorkerMessagingProxy*);
+      const WeakPtr<InProcessWorkerMessagingProxy>&);
   ~InProcessWorkerObjectProxy() override;
 
   void postMessageToWorkerObject(PassRefPtr<SerializedScriptValue>,
@@ -89,7 +89,7 @@ class CORE_EXPORT InProcessWorkerObjectProxy : public WorkerReportingProxy {
   void didTerminateWorkerThread() override;
 
  protected:
-  InProcessWorkerObjectProxy(InProcessWorkerMessagingProxy*);
+  InProcessWorkerObjectProxy(const WeakPtr<InProcessWorkerMessagingProxy>&);
   virtual ExecutionContext* getExecutionContext();
 
  private:
@@ -103,6 +103,11 @@ class CORE_EXPORT InProcessWorkerObjectProxy : public WorkerReportingProxy {
   // This object always outlives this proxy.
   InProcessWorkerMessagingProxy* m_messagingProxy;
 
+  // No guarantees about the lifetimes of tasks posted by this proxy wrt the
+  // InProcessWorkerMessagingProxy so a weak pointer must be used when posting
+  // the tasks.
+  WeakPtr<InProcessWorkerMessagingProxy> m_messagingProxyWeakPtr;
+
   // Used for checking pending activities on the worker global scope. This is
   // cancelled when the worker global scope is destroyed.
   std::unique_ptr<Timer<InProcessWorkerObjectProxy>> m_timer;