webgl: Replace query task observer with a posted task

third_party/WebKit/Source/modules/webgl/WebGLQuery.cpp

Index: third_party/WebKit/Source/modules/webgl/WebGLQuery.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLQuery.cpp b/third_party/WebKit/Source/modules/webgl/WebGLQuery.cpp
index 853dc0118882f0adfd286c2ea1dc569d26011d4e..490dff1d127a9fb8ad0d384eba9484d9b0895877 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLQuery.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLQuery.cpp
@@ -4,6 +4,7 @@
 
 #include "modules/webgl/WebGLQuery.h"
 
+#include "core/dom/TaskRunnerHelper.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "modules/webgl/WebGL2RenderingContextBase.h"
 #include "public/platform/Platform.h"
@@ -17,8 +18,6 @@ WebGLQuery* WebGLQuery::create(WebGL2RenderingContextBase* ctx)
 
 WebGLQuery::~WebGLQuery()
 {
-    unregisterTaskObserver();
-
     // See the comment in WebGLObject::detachAndDeleteObject().
     detachAndDeleteObject();
 }
@@ -26,10 +25,11 @@ WebGLQuery::~WebGLQuery()
 WebGLQuery::WebGLQuery(WebGL2RenderingContextBase* ctx)
     : WebGLSharedPlatform3DObject(ctx)
     , m_target(0)
-    , m_taskObserverRegistered(false)
     , m_canUpdateAvailability(false)
     , m_queryResultAvailable(false)
     , m_queryResult(0)
+    , m_taskRunner(TaskRunnerHelper::get(TaskType::Unthrottled, &ctx->canvas()->document())->clone())
+    , m_weakFactory(this)

[Ken Russell (switch to Gerrit), 2016/09/16 23:12:39]

As vmiura@ points out, I think the use of WeakPtrs pointing to this Oilpan-GC'd
type introduces the same basic problem as before -- that there is a
non-Oilpan-aware pointer to this GC'd object. (base::WeakPtrFactory and
consequently WTF::WeakPtrFactory doesn't know about Oilpan's GC.) There was an
attempt in the earlier code to unregister the task observer in the destructor,
but as we discovered, that violated some of Oilpan's invariants, since the
destructor wasn't called before poisoning the page the object lived on.

I think the safe way to handle this would be to have a separate, C-heap
allocated object which contains a WeakMember pointing back to this one, and use
that object in the postTask call. I'm not sure how the lifetime of that object
should be managed though. What do you think?

Alternatively, as Victor points out, we could cause these objects to be eagerly
finalized, but apparently that adds overhead so perhaps we should find another
solution.

 {
     GLuint query;
     ctx->contextGL()->GenQueriesEXT(1, &query);
@@ -57,7 +57,7 @@ void WebGLQuery::resetCachedResult()
     // When this is called, the implication is that we should start
     // keeping track of whether we can update the cached availability
     // and result.
-    registerTaskObserver();
+    scheduleAllowAvailabilityUpdate();
 }
 
 void WebGLQuery::updateCachedResult(gpu::gles2::GLES2Interface* gl)
@@ -80,7 +80,9 @@ void WebGLQuery::updateCachedResult(gpu::gles2::GLES2Interface* gl)
         GLuint result = 0;
         gl->GetQueryObjectuivEXT(object(), GL_QUERY_RESULT_EXT, &result);
         m_queryResult = result;
-        unregisterTaskObserver();
+        m_weakFactory.revokeAll();
+    } else {
+        scheduleAllowAvailabilityUpdate();
     }
 }
 
@@ -94,23 +96,14 @@ GLuint WebGLQuery::getQueryResult()
     return m_queryResult;
 }
 
-void WebGLQuery::registerTaskObserver()
+void WebGLQuery::scheduleAllowAvailabilityUpdate()
 {
-    if (!m_taskObserverRegistered) {
-        m_taskObserverRegistered = true;
-        Platform::current()->currentThread()->addTaskObserver(this);
-    }
-}
-
-void WebGLQuery::unregisterTaskObserver()
-{
-    if (m_taskObserverRegistered) {
-        m_taskObserverRegistered = false;
-        Platform::current()->currentThread()->removeTaskObserver(this);
-    }
+    if (m_weakFactory.hasWeakPtrs())
+        return;
+    m_taskRunner->postTask(BLINK_FROM_HERE, WTF::bind(&WebGLQuery::allowAvailabilityUpdate, m_weakFactory.createWeakPtr()));
 }
 
-void WebGLQuery::didProcessTask()
+void WebGLQuery::allowAvailabilityUpdate()
 {
     m_canUpdateAvailability = true;
 }