[Oilpan] Migrate classes under module/webgl onto oilpan heap

Source/modules/webgl/WebGLVertexArrayObjectBase.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 
 #include "modules/webgl/WebGLVertexArrayObjectBase.h"
 
 #include "modules/webgl/WebGLRenderingContextBase.h"
 
 namespace blink {
 
 WebGLVertexArrayObjectBase::WebGLVertexArrayObjectBase(WebGLRenderingContextBase* ctx, VaoType type)
     : WebGLContextObject(ctx)
     , m_object(0)
     , m_type(type)
     , m_hasEverBeenBound(false)
-#if ENABLE(OILPAN)
     , m_destructionInProgress(false)
-#endif
     , m_boundElementArrayBuffer(nullptr)
 {
     m_vertexAttribState.reserveCapacity(ctx->maxVertexAttribs());
 
     switch (m_type) {
     case VaoTypeDefault:
         break;
     default:
         m_object = context()->webContext()->createVertexArrayOES();
         break;
     }
 }
 
 WebGLVertexArrayObjectBase::~WebGLVertexArrayObjectBase()
 {
-#if ENABLE(OILPAN)
     m_destructionInProgress = true;

[haraken, 2015/08/04 00:06:52]

Instead of introducing another flag like m_destructionInProgress, I'd prefer
just clearing out m_boundElementArrayBuffer and m_vertexAttribState here.

c.f., see what WebGLRenderbuffer::~WebGLRenderbuffer() is doing.

[peria, 2015/08/04 09:10:26]

will do in another CL.
http://crbug.com/516613

-#endif
 
     // Delete the platform framebuffer resource. Explicit detachment
     // is for the benefit of Oilpan, where this vertex array object
     // isn't detached when it and the WebGLRenderingContextBase object
     // it is registered with are both finalized. Without Oilpan, the
     // object will have been detached.
     //
     // To keep the code regular, the trivial detach()ment is always
     // performed.
     detachAndDeleteObject();
 }
 
 void WebGLVertexArrayObjectBase::dispatchDetached(WebGraphicsContext3D* context3d)
 {
     if (m_boundElementArrayBuffer)

[haraken, 2015/08/04 00:06:52]

If you apply the above change, you'll need to add:

  if (!m_boundElementArrayBuffer)
    return;
  if (!m_vertexAttribState)
    return;

[peria, 2015/08/04 09:10:26]

Acknowledged.

         m_boundElementArrayBuffer->onDetached(context3d);
 
     for (size_t i = 0; i < m_vertexAttribState.size(); ++i) {
         VertexAttribState* state = m_vertexAttribState[i].get();
         if (state->bufferBinding)
             state->bufferBinding->onDetached(context3d);
     }
 }
 
 void WebGLVertexArrayObjectBase::deleteObjectImpl(WebGraphicsContext3D* context3d)
 {
     switch (m_type) {
     case VaoTypeDefault:
         break;
     default:
         context3d->deleteVertexArrayOES(m_object);
         m_object = 0;
         break;
     }
 
-#if ENABLE(OILPAN)
     // These m_boundElementArrayBuffer and m_vertexAttribState heap
     // objects must not be accessed during destruction in the oilpan
     // build. They could have been already finalized. The finalizers
     // of these objects (and their elements) will themselves handle
     // detachment.
     if (!m_destructionInProgress)
         dispatchDetached(context3d);

[haraken, 2015/08/04 00:06:52]

Sigbjorn: Do we still need to call dispatchDetached here? In oilpan, the
lifetime of a WebGL object is explicitly managed by the WebGL object itself
calling WebGL::detachAndDeleteObject(). So I'm wondering if we can completely
remove WebGLObject::m_attachmentCount. If we can remove it, we can remove
WebGLObject::onDetached(). Then we can remove dispatchDetached().

(Either way, we can handle this in a follow-up CL.)

[Ken Russell (switch to Gerrit), 2015/08/04 00:34:11]

One note about this. In order to maintain uniform behavior across many vendors'
OpenGL drivers, it has been found necessary in the past to defer destruction of
certain OpenGL objects. For example, if a shader is attached to a program and
the user calls deleteShader against it, we need to delay the destruction of the
shader until the program is deleted. Same for renderbuffers and textures that
are attached to framebuffer objects.

It's possible that we could reevaluate the need to do this work at the Blink
layer -- it may be well handled by Chromium's command buffer at this point. We
should discuss this more. However, in the spirit of minimizing the amount of
changes in this already-large CL, yes, this should be considered for a follow-on
CL.

[haraken, 2015/08/04 00:47:05]

Thanks for the context. I now understand why the onDetached calls are needed.

(Another option might be to remove the onDetached calls and keep the underlying
platform object alive until the WebGLObject gets destructed (the underlying
platform object is destructed in WebGLObject's destructor). That said, it will
prolong the platform object lifetime too much.)

[sof, 2015/08/04 06:20:45]

Good point, I think we can remove the Oilpan specific part (i.e., remove
m_destructionInProgress) due to the partial ordering of finalizers that we've
now got here.

[peria, 2015/08/04 09:10:26]

Acknowledged.
http://crbug.com/516613

-#else
-    dispatchDetached(context3d);
-#endif
 }
 
-void WebGLVertexArrayObjectBase::setElementArrayBuffer(PassRefPtrWillBeRawPtr<WebGLBuffer> buffer)
+void WebGLVertexArrayObjectBase::setElementArrayBuffer(WebGLBuffer* buffer)
 {
     if (buffer)
         buffer->onAttached();
     if (m_boundElementArrayBuffer)
         m_boundElementArrayBuffer->onDetached(context()->webContext());
     m_boundElementArrayBuffer = buffer;
 }
 
 WebGLVertexArrayObjectBase::VertexAttribState* WebGLVertexArrayObjectBase::getVertexAttribState(size_t index)
 {
     ASSERT(index < context()->maxVertexAttribs());
     // Lazily create the vertex attribute states.
     for (size_t i = m_vertexAttribState.size(); i <= index; i++)
-        m_vertexAttribState.append(adoptPtrWillBeNoop(new VertexAttribState()));
+        m_vertexAttribState.append(new VertexAttribState);
     return m_vertexAttribState[index].get();
 }
 
 void WebGLVertexArrayObjectBase::setVertexAttribState(
-    GLuint index, GLsizei bytesPerElement, GLint size, GLenum type, GLboolean normalized, GLsizei stride, GLintptr offset, PassRefPtrWillBeRawPtr<WebGLBuffer> buffer)
+    GLuint index, GLsizei bytesPerElement, GLint size, GLenum type, GLboolean normalized, GLsizei stride, GLintptr offset, WebGLBuffer* buffer)
 {
     GLsizei validatedStride = stride ? stride : bytesPerElement;
     VertexAttribState* state = getVertexAttribState(index);
 
     if (buffer)
         buffer->onAttached();
     if (state->bufferBinding)
         state->bufferBinding->onDetached(context()->webContext());
 
     state->bufferBinding = buffer;
     state->bytesPerElement = bytesPerElement;
     state->size = size;
     state->type = type;
     state->normalized = normalized;
     state->stride = validatedStride;
     state->originalStride = stride;
     state->offset = offset;
 }
 
-void WebGLVertexArrayObjectBase::unbindBuffer(PassRefPtrWillBeRawPtr<WebGLBuffer> buffer)
+void WebGLVertexArrayObjectBase::unbindBuffer(WebGLBuffer* buffer)
 {
     if (m_boundElementArrayBuffer == buffer) {
         m_boundElementArrayBuffer->onDetached(context()->webContext());
         m_boundElementArrayBuffer = nullptr;
     }
 
     for (size_t i = 0; i < m_vertexAttribState.size(); ++i) {
-        VertexAttribState* state = m_vertexAttribState[i].get();
+        VertexAttribState* state = m_vertexAttribState[i];
         if (state->bufferBinding == buffer) {
             buffer->onDetached(context()->webContext());
             state->bufferBinding = nullptr;
         }
     }
 }
 
 void WebGLVertexArrayObjectBase::setVertexAttribDivisor(GLuint index, GLuint divisor)
 {
     VertexAttribState* state = getVertexAttribState(index);
     state->divisor = divisor;
 }
 
 DEFINE_TRACE(WebGLVertexArrayObjectBase::VertexAttribState)
 {
     visitor->trace(bufferBinding);
 }
 
 DEFINE_TRACE(WebGLVertexArrayObjectBase)
 {
     visitor->trace(m_boundElementArrayBuffer);
     visitor->trace(m_vertexAttribState);
     WebGLContextObject::trace(visitor);
 }
 
 } // namespace blink