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

Source/modules/webgl/WebGLFramebuffer.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 20 matching lines @@
 #include "modules/webgl/WebGLRenderingContextBase.h"
 #include "modules/webgl/WebGLTexture.h"
 #include "platform/NotImplemented.h"
 
 namespace blink {
 
 namespace {
 
 class WebGLRenderbufferAttachment final : public WebGLFramebuffer::WebGLAttachment {
 public:
-    static PassRefPtrWillBeRawPtr<WebGLFramebuffer::WebGLAttachment> create(WebGLRenderbuffer*);
+    static WebGLFramebuffer::WebGLAttachment* create(WebGLRenderbuffer*);
 
     DECLARE_VIRTUAL_TRACE();
 
 private:
     explicit WebGLRenderbufferAttachment(WebGLRenderbuffer*);
     WebGLRenderbufferAttachment() { }
 
     GLsizei width() const override;
     GLsizei height() const override;
     GLenum format() const override;
     GLenum type() const override;
     WebGLSharedObject* object() const override;
     bool isSharedObject(WebGLSharedObject*) const override;
     bool valid() const override;
     void onDetached(WebGraphicsContext3D*) override;
     void attach(WebGraphicsContext3D*, GLenum target, GLenum attachment) override;
     void unattach(WebGraphicsContext3D*, GLenum target, GLenum attachment) override;
 
-    RefPtrWillBeMember<WebGLRenderbuffer> m_renderbuffer;
+    Member<WebGLRenderbuffer> m_renderbuffer;
 };
 
-PassRefPtrWillBeRawPtr<WebGLFramebuffer::WebGLAttachment> WebGLRenderbufferAttachment::create(WebGLRenderbuffer* renderbuffer)
+WebGLFramebuffer::WebGLAttachment* WebGLRenderbufferAttachment::create(WebGLRenderbuffer* renderbuffer)
 {
-    return adoptRefWillBeNoop(new WebGLRenderbufferAttachment(renderbuffer));
+    return new WebGLRenderbufferAttachment(renderbuffer);
 }
 
 DEFINE_TRACE(WebGLRenderbufferAttachment)
 {
     visitor->trace(m_renderbuffer);
     WebGLFramebuffer::WebGLAttachment::trace(visitor);
 }
 
 WebGLRenderbufferAttachment::WebGLRenderbufferAttachment(WebGLRenderbuffer* renderbuffer)
     : m_renderbuffer(renderbuffer)
@@ skipping 57 matching lines @@
     if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
         context->framebufferRenderbuffer(target, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, 0);
         context->framebufferRenderbuffer(target, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER, 0);
     } else {
         context->framebufferRenderbuffer(target, attachment, GL_RENDERBUFFER, 0);
     }
 }
 
 GLenum WebGLRenderbufferAttachment::type() const
 {
+    notImplemented();
     return 0;
 }
 
 class WebGLTextureAttachment final : public WebGLFramebuffer::WebGLAttachment {
 public:
-    static PassRefPtrWillBeRawPtr<WebGLFramebuffer::WebGLAttachment> create(WebGLTexture*, GLenum target, GLint level);
+    static WebGLFramebuffer::WebGLAttachment* create(WebGLTexture*, GLenum target, GLint level);
 
     DECLARE_VIRTUAL_TRACE();
 
 private:
     WebGLTextureAttachment(WebGLTexture*, GLenum target, GLint level);
     WebGLTextureAttachment() { }
 
     GLsizei width() const override;
     GLsizei height() const override;
     GLenum format() const override;
     GLenum type() const override;
     WebGLSharedObject* object() const override;
     bool isSharedObject(WebGLSharedObject*) const override;
     bool valid() const override;
     void onDetached(WebGraphicsContext3D*) override;
     void attach(WebGraphicsContext3D*, GLenum target, GLenum attachment) override;
     void unattach(WebGraphicsContext3D*, GLenum target, GLenum attachment) override;
 
-    RefPtrWillBeMember<WebGLTexture> m_texture;
+    Member<WebGLTexture> m_texture;
     GLenum m_target;
     GLint m_level;
 };
 
-PassRefPtrWillBeRawPtr<WebGLFramebuffer::WebGLAttachment> WebGLTextureAttachment::create(WebGLTexture* texture, GLenum target, GLint level)
+WebGLFramebuffer::WebGLAttachment* WebGLTextureAttachment::create(WebGLTexture* texture, GLenum target, GLint level)
 {
-    return adoptRefWillBeNoop(new WebGLTextureAttachment(texture, target, level));
+    return new WebGLTextureAttachment(texture, target, level);
 }
 
 DEFINE_TRACE(WebGLTextureAttachment)
 {
     visitor->trace(m_texture);
     WebGLFramebuffer::WebGLAttachment::trace(visitor);
 }
 
 WebGLTextureAttachment::WebGLTextureAttachment(WebGLTexture* texture, GLenum target, GLint level)
     : m_texture(texture)
@@ skipping 132 matching lines @@
 } // anonymous namespace
 
 WebGLFramebuffer::WebGLAttachment::WebGLAttachment()
 {
 }
 
 WebGLFramebuffer::WebGLAttachment::~WebGLAttachment()
 {
 }
 
-PassRefPtrWillBeRawPtr<WebGLFramebuffer> WebGLFramebuffer::create(WebGLRenderingContextBase* ctx)
+WebGLFramebuffer* WebGLFramebuffer::create(WebGLRenderingContextBase* ctx)
 {
-    return adoptRefWillBeNoop(new WebGLFramebuffer(ctx));
+    return new WebGLFramebuffer(ctx);
 }
 
 WebGLFramebuffer::WebGLFramebuffer(WebGLRenderingContextBase* ctx)
     : WebGLContextObject(ctx)
     , m_object(ctx->webContext()->createFramebuffer())
     , m_hasEverBeenBound(false)
     , m_readBuffer(GL_COLOR_ATTACHMENT0)
 {
 }
 
 WebGLFramebuffer::~WebGLFramebuffer()
 {
-    // Delete the platform framebuffer resource. Explicit detachment
-    // is for the benefit of Oilpan, where the framebuffer 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.
+    // Attachments in |m_attachments| will be deleted from other places, so we
+    // clear it to avoid deleting those attachments in detachAndDeleteObject().
+    m_attachments.clear();

[sof, 2015/08/09 08:35:21]

I don't think is quite correct with Oilpan:

 - if the context is swept at the same time, it will take care of detaching
WebGLFramebuffer. Hence, this map will be empty & the dtor a no-op.
 - if the context isn't swept at the same time, we will clear m_attachments,
causing onDetached() not to be called for its elements.

So, there are no guarantees that onDetached() will always be called for its
WebGLAttachments with Oilpan on destruction. I think that implies that they need
to handle detachment on their own (I believe they do already.) And if they do,
the clearing here isn't needed and the iteration over them in deleteObjectImpl()
can be removed.

[haraken, 2015/08/09 14:16:45]

Yeah, you're right -- I think we can just remove the onDetach call (though I'd
prefer doing that in a follow-up CL since this CL is already complex).

[sof, 2015/08/09 15:07:37]

That's fine, I think. The Oilpan changes made here do no harm, but are not
needed.

[peria, 2015/08/10 03:15:23]

Thanks, I filed an issue to follow it up.
http://crbug.com/518706

+
+    // See the comment in WebGLObject::detachAndDeleteObject().
     detachAndDeleteObject();
 }
 
 void WebGLFramebuffer::setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, GLenum texTarget, WebGLTexture* texture, GLint level)
 {
     ASSERT(isBound(target));
     removeAttachmentFromBoundFramebuffer(target, attachment);
     if (!m_object)
         return;
     if (texture && texture->object()) {
@@ skipping 214 matching lines @@
 bool WebGLFramebuffer::hasStencilBuffer() const
 {
     WebGLAttachment* attachment = getAttachment(GL_STENCIL_ATTACHMENT);
     if (!attachment)
         attachment = getAttachment(GL_DEPTH_STENCIL_ATTACHMENT);
     return attachment && attachment->valid();
 }
 
 void WebGLFramebuffer::deleteObjectImpl(WebGraphicsContext3D* context3d)
 {
-#if !ENABLE(OILPAN)
-    // With Oilpan, both the AttachmentMap and its WebGLAttachment objects are
-    // GCed objects and cannot be accessed, as they may have been finalized
+    // Both the AttachmentMap and its WebGLAttachment objects are GCed

[sof, 2015/08/09 08:35:20]

The code and the comment is inconsistent now; do these really need to be
notified of onDetached() with Oilpan?

[peria, 2015/08/10 03:15:23]

Removed latter half comment.
I think it is not needed to call onDetached(), but I will work for it in a
follow-up CL.

+    // objects and cannot be accessed, as they may have been finalized
     // already during the same GC sweep.
     //
     // The WebGLAttachment-derived classes instead handle detachment
     // on their own when finalizing, so the explicit notification is
     // not needed.
-    for (const auto& attachment : m_attachments)
-        attachment.value->onDetached(context3d);
-#endif
+    if (!m_attachments.isEmpty()) {

[haraken, 2015/08/07 07:43:58]

This check won't be needed, since if the m_attachements is empty, the following
loop does nothing.

[peria, 2015/08/10 03:15:23]

Done.

+        for (const auto& attachment : m_attachments)
+            attachment.value->onDetached(context3d);
+    }
 
     context3d->deleteFramebuffer(m_object);
     m_object = 0;
 }
 
 bool WebGLFramebuffer::isBound(GLenum target) const
 {
     return (context()->getFramebufferBinding(target) == this);
 }
 
@@ skipping 51 matching lines @@
         return false;
     if (format)
         *format = image->format();
     if (type)
         *type = image->type();
     return true;
 }
 
 DEFINE_TRACE(WebGLFramebuffer)
 {
-#if ENABLE(OILPAN)
     visitor->trace(m_attachments);
-#endif
     WebGLContextObject::trace(visitor);
 }
 
 }