Oilpan: move 2D Canvas and WebGL objects to the heap.

Source/core/html/canvas/WebGLObject.cpp

Index: Source/core/html/canvas/WebGLObject.cpp
diff --git a/Source/core/html/canvas/WebGLObject.cpp b/Source/core/html/canvas/WebGLObject.cpp
index 14dd8ba5ee8d78eeea2c29b7f2dfc08ece0c7a8f..2be33db2d12e501310419c594d1ebb6c7d8007bf 100644
--- a/Source/core/html/canvas/WebGLObject.cpp
+++ b/Source/core/html/canvas/WebGLObject.cpp
@@ -27,10 +27,15 @@
 
 #include "core/html/canvas/WebGLObject.h"
 
+#include "core/html/canvas/WebGLRenderingContextBase.h"
+
 namespace WebCore {
 
-WebGLObject::WebGLObject(WebGLRenderingContextBase*)
+WebGLObject::WebGLObject(WebGLRenderingContextBase* base)
     : m_object(0)
+#if ENABLE(OILPAN)
+    , m_context3d(base->webContext())

[haraken, 2014/07/11 01:55:02]

I basically like the idea of caching the webContext into WebGLObjects.

How is it guaranteed that the webContext cached here is still valid when
WebGLObject::deleteObject() gets called?

For example, WebGLContextObject is expecting to return 0 for
WebGLContextObject::getAWebGraphicsContext3D() after
WebGLContextObject::detachContext() is called. See
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[sof, 2014/07/11 06:51:35]

It's a very good question.

All remaining WebGLSharedObjects attached to a WebGLContextGroup will be
explicitly detached (=> delete their resources) when the final
WebGLRenderingContextBase removes itself. So no problem there, the resources
will be deleted and when those WebGLSharedObjects instances are finalized, the
object deletion will be a no-op.

But that leaves the WebGLContextObjects (the other WebGLObject-derived class)
attached to WebGLRenderingContextBase. When the WebGLRenderingContextBase
finalizer runs, it will in the end destruct the DrawingBuffer and its 3d context
at the end of that. But there might be WebGLContextObject finalizers running
after that, which will attempt to use the 3d context..

Hmm, I wonder if we can salvage that situation by having
WebGLRenderingContextBase keep an off-heap hash set of WebGLContextObjects.

[haraken, 2014/07/11 07:01:33]

Or probably can we create Context3DObserver or something like that, and let the
observer clear m_context3d when the context3d is destructed?

[Mads Ager (chromium), 2014/07/11 09:15:01]

We should attempt to avoid that. We are attempting to get rid of all tracing of
off-heap objects. In order to make GCs completely safe while we are in the
middle of object creation (nested allocations in constructors) we need to be
able to conservatively scan objects. We can't do that if a correct trace relies
on the trace method being called to trace object that are not on the oilpan
heap.

As haraken suggests, maybe we can use LifecycleObserver and make WebGLObjects
Context3DLifecycleObservers? That would add a set of observers to context3d but
then this pointer would be cleared in a safe way if the context3d dies before
the WebGLObject.

[sof, 2014/07/11 09:42:02]

That would be a tidy way to handle registration and lifetime mechanics, but it
would still require keeping the observers in a collection, I think. Which cannot
be on the heap.

To address this issue, I've done the following:

 - WebGLContextObjects (i.e., WebGLFramebuffer and WebGLVertexArrayObjectOES
only atm) now share a RefPtr<>'ed DrawingBuffer with their
WebGLRenderingContextBase.
 - when all those objects and the 'parent' WebGLRenderingContextBase object have
been finalized, the DrawingBuffer and its platform context object
(WebGraphicsContext3D) is destructed.
 - WebGLSharedObject-derived objects get their WebGraphicsContext3D from their
context object instead of keeping a separate object. As they're explicitly
detached when the WebGLRenderingContextBase removes itself from the
WebGLContextGroup, this is fine.
 - WebGLObject no longer keeps a copy of a WebGraphicsContext3D object.

Have a look; the concern with WebGLObject growing an extra field falls away with
this approach.

[Mads Ager (chromium), 2014/07/11 10:18:52]

I guess we could use PersistentHeapHashSet<Member<Observer> >? But it seems to
me that your new solution is cleaner. :)
This looks cleaner. It is more explicit and easier to wrap your head around.
What do you think Haraken?

+#endif
     , m_attachmentCount(0)
     , m_deleted(false)
 {
@@ -38,6 +43,10 @@ WebGLObject::WebGLObject(WebGLRenderingContextBase*)
 
 WebGLObject::~WebGLObject()
 {
+#if ENABLE(OILPAN)
+    // Verify that derived classes have released their platform objects.
+    ASSERT(m_deleted);
+#endif
 }
 
 void WebGLObject::setObject(Platform3DObject object)
@@ -57,8 +66,13 @@ void WebGLObject::deleteObject(blink::WebGraphicsContext3D* context3d)
         return;
 
     if (!m_attachmentCount) {
-        if (!context3d)
+        if (!context3d) {
+#if ENABLE(OILPAN)
+            context3d = m_context3d;
+#else
             context3d = getAWebGraphicsContext3D();
+#endif
+        }
 
         if (context3d)
             deleteObjectImpl(context3d, m_object);
@@ -70,8 +84,7 @@ void WebGLObject::deleteObject(blink::WebGraphicsContext3D* context3d)
 void WebGLObject::detach()
 {
     m_attachmentCount = 0; // Make sure OpenGL resource is deleted.
-    }
-
+}
 
 void WebGLObject::onDetached(blink::WebGraphicsContext3D* context3d)
 {