[compositorworker] Register compositor proxies with proxy client

third_party/WebKit/Source/core/dom/CompositorProxy.cpp

Index: third_party/WebKit/Source/core/dom/CompositorProxy.cpp
diff --git a/third_party/WebKit/Source/core/dom/CompositorProxy.cpp b/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
index fd5ae040a01d31bbdd2c5275fa63c9f58ea9ad29..6f029c19b8262b0b22bdc0f88eb2dc3fba5d48b8 100644
--- a/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
+++ b/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
@@ -9,6 +9,8 @@
 #include "core/dom/DOMNodeIds.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
+#include "core/workers/WorkerClients.h"
+#include "core/workers/WorkerGlobalScope.h"
 #include "platform/ThreadSafeFunctional.h"
 #include "platform/graphics/CompositorMutableProperties.h"
 #include "public/platform/Platform.h"
@@ -112,14 +114,18 @@ CompositorProxy* CompositorProxy::create(ExecutionContext* context, Element* ele
     return new CompositorProxy(*element, attributeArray);
 }
 
-CompositorProxy* CompositorProxy::create(uint64_t elementId, uint32_t compositorMutableProperties)
+CompositorProxy* CompositorProxy::create(ExecutionContext* context, uint64_t elementId, uint32_t compositorMutableProperties)
 {
-    return new CompositorProxy(elementId, compositorMutableProperties);
+    WorkerClients* clients = toWorkerGlobalScope(context)->clients();

[jbroman, 2016/06/08 14:45:34]

Won't this crash if you postMessage a CompositorProxy back to the main thread?
Why is it safe to assume the execution context is a WorkerGlobalScope?

[majidvp, 2016/06/09 20:34:21]

Fixed. It is not safe and crashes!
Modified one of existing tests to cover this.

+    DCHECK(clients);
+    CompositorProxyClient* client = CompositorProxyClient::from(clients);
+    return new CompositorProxy(elementId, compositorMutableProperties, client);
 }
 
 CompositorProxy::CompositorProxy(Element& element, const Vector<String>& attributeArray)
     : m_elementId(DOMNodeIds::idForNode(&element))
     , m_compositorMutableProperties(compositorMutablePropertiesFromNames(attributeArray))
+    , m_client(nullptr)
 {
     DCHECK(isMainThread());
     DCHECK(m_compositorMutableProperties);
@@ -130,21 +136,32 @@ CompositorProxy::CompositorProxy(Element& element, const Vector<String>& attribu
     incrementCompositorProxiedPropertiesForElement(m_elementId, m_compositorMutableProperties);
 }
 
-CompositorProxy::CompositorProxy(uint64_t elementId, uint32_t compositorMutableProperties)
+CompositorProxy::CompositorProxy(uint64_t elementId, uint32_t compositorMutableProperties, CompositorProxyClient* client)
     : m_elementId(elementId)
     , m_compositorMutableProperties(compositorMutableProperties)
+    , m_client(client)
 {
     DCHECK(isControlThread());
 #if DCHECK_IS_ON()
     DCHECK(sanityCheckMutableProperties(m_compositorMutableProperties));
 #endif
     Platform::current()->mainThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&incrementCompositorProxiedPropertiesForElement, m_elementId, m_compositorMutableProperties));
+    m_client->registerCompositorProxy(this);
+    ThreadState::current()->registerPreFinalizer(this);
 }
 
 CompositorProxy::~CompositorProxy()
 {
-    if (m_connected)
+    if (isMainThread())
         disconnect();

[haraken, 2016/06/08 00:10:36]

You don't need to call disconnect() because disconnect() has already been called
in the pre-finalizer.

[majidvp, 2016/06/09 20:34:21]

Done.

+    DCHECK(!m_connected);
+}
+
+void CompositorProxy::dispose()
+{
+    // On compositor worker thread, disconnecting requires unregistering from
+    // CompositorProxyClient which needs to be done in pre-finalizer.
+    disconnect();
 }
 
 bool CompositorProxy::supports(const String& attributeName) const
@@ -239,11 +256,16 @@ bool CompositorProxy::raiseExceptionIfNotMutable(uint32_t property, ExceptionSta
 
 void CompositorProxy::disconnect()
 {
+    if (!m_connected)
+        return;
     m_connected = false;
-    if (isMainThread())
+    if (isMainThread()) {
         decrementCompositorProxiedPropertiesForElement(m_elementId, m_compositorMutableProperties);
-    else
+    } else {
         Platform::current()->mainThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&decrementCompositorProxiedPropertiesForElement, m_elementId, m_compositorMutableProperties));
+        if (m_client)
+            m_client->unregisterCompositorProxy(this);
+    }
 }
 
 } // namespace blink