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

Source/core/html/canvas/WebGLRenderingContextBase.h

Index: Source/core/html/canvas/WebGLRenderingContextBase.h
diff --git a/Source/core/html/canvas/WebGLRenderingContextBase.h b/Source/core/html/canvas/WebGLRenderingContextBase.h
index afa9619774cb74963830b03384933c9c9343b893..2ba56612669a6f259dbac91461c3ea78f8d74e2e 100644
--- a/Source/core/html/canvas/WebGLRenderingContextBase.h
+++ b/Source/core/html/canvas/WebGLRenderingContextBase.h
@@ -146,12 +146,12 @@ public:
     void copyTexImage2D(GLenum target, GLint level, GLenum internalformat, GLint x, GLint y, GLsizei width, GLsizei height, GLint border);
     void copyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height);
 
-    PassRefPtr<WebGLBuffer> createBuffer();
-    PassRefPtr<WebGLFramebuffer> createFramebuffer();
-    PassRefPtr<WebGLProgram> createProgram();
-    PassRefPtr<WebGLRenderbuffer> createRenderbuffer();
-    PassRefPtr<WebGLShader> createShader(GLenum type);
-    PassRefPtr<WebGLTexture> createTexture();
+    PassRefPtrWillBeRawPtr<WebGLBuffer> createBuffer();
+    PassRefPtrWillBeRawPtr<WebGLFramebuffer> createFramebuffer();
+    PassRefPtrWillBeRawPtr<WebGLProgram> createProgram();
+    PassRefPtrWillBeRawPtr<WebGLRenderbuffer> createRenderbuffer();
+    PassRefPtrWillBeRawPtr<WebGLShader> createShader(GLenum type);
+    PassRefPtrWillBeRawPtr<WebGLTexture> createTexture();
 
     void cullFace(GLenum mode);
 
@@ -183,14 +183,14 @@ public:
     void frontFace(GLenum mode);
     void generateMipmap(GLenum target);
 
-    PassRefPtr<WebGLActiveInfo> getActiveAttrib(WebGLProgram*, GLuint index);
-    PassRefPtr<WebGLActiveInfo> getActiveUniform(WebGLProgram*, GLuint index);
-    bool getAttachedShaders(WebGLProgram*, Vector<RefPtr<WebGLShader> >&);
+    PassRefPtrWillBeRawPtr<WebGLActiveInfo> getActiveAttrib(WebGLProgram*, GLuint index);
+    PassRefPtrWillBeRawPtr<WebGLActiveInfo> getActiveUniform(WebGLProgram*, GLuint index);
+    bool getAttachedShaders(WebGLProgram*, WillBeHeapVector<RefPtrWillBeMember<WebGLShader> >&);
     GLint getAttribLocation(WebGLProgram*, const String& name);
     WebGLGetInfo getBufferParameter(GLenum target, GLenum pname);
-    PassRefPtr<WebGLContextAttributes> getContextAttributes();
+    PassRefPtrWillBeRawPtr<WebGLContextAttributes> getContextAttributes();
     GLenum getError();
-    PassRefPtr<WebGLExtension> getExtension(const String& name);
+    PassRefPtrWillBeRawPtr<WebGLExtension> getExtension(const String& name);
     WebGLGetInfo getFramebufferAttachmentParameter(GLenum target, GLenum attachment, GLenum pname);
     WebGLGetInfo getParameter(GLenum pname);
     WebGLGetInfo getProgramParameter(WebGLProgram*, GLenum pname);
@@ -198,12 +198,12 @@ public:
     WebGLGetInfo getRenderbufferParameter(GLenum target, GLenum pname);
     WebGLGetInfo getShaderParameter(WebGLShader*, GLenum pname);
     String getShaderInfoLog(WebGLShader*);
-    PassRefPtr<WebGLShaderPrecisionFormat> getShaderPrecisionFormat(GLenum shaderType, GLenum precisionType);
+    PassRefPtrWillBeRawPtr<WebGLShaderPrecisionFormat> getShaderPrecisionFormat(GLenum shaderType, GLenum precisionType);
     String getShaderSource(WebGLShader*);
     Vector<String> getSupportedExtensions();
     WebGLGetInfo getTexParameter(GLenum target, GLenum pname);
     WebGLGetInfo getUniform(WebGLProgram*, const WebGLUniformLocation*);
-    PassRefPtr<WebGLUniformLocation> getUniformLocation(WebGLProgram*, const String&);
+    PassRefPtrWillBeRawPtr<WebGLUniformLocation> getUniformLocation(WebGLProgram*, const String&);
     WebGLGetInfo getVertexAttrib(GLuint index, GLenum pname);
     long long getVertexAttribOffset(GLuint index, GLenum pname);
 
@@ -349,6 +349,18 @@ public:
     virtual void stop() OVERRIDE;
 
     void setSavingImage(bool isSaving) { m_savingImage = isSaving; }
+
+    virtual void trace(Visitor*) OVERRIDE;
+
+    class TextureUnitState {
+        ALLOW_ONLY_INLINE_ALLOCATION();
+    public:
+        RefPtrWillBeMember<WebGLTexture> m_texture2DBinding;
+        RefPtrWillBeMember<WebGLTexture> m_textureCubeMapBinding;
+
+        void trace(Visitor*);
+    };
+
 protected:
     friend class WebGLDrawBuffers;
     friend class WebGLFramebuffer;
@@ -415,17 +427,17 @@ protected:
 
     bool m_needsUpdate;
     bool m_markedCanvasDirty;
-    HashSet<WebGLContextObject*> m_contextObjects;
+    WillBeHeapHashSet<RawPtrWillBeWeakMember<WebGLContextObject> > m_contextObjects;
 
-    OwnPtr<WebGLRenderingContextLostCallback> m_contextLostCallbackAdapter;
-    OwnPtr<WebGLRenderingContextErrorMessageCallback> m_errorMessageCallbackAdapter;
+    OwnPtrWillBeMember<WebGLRenderingContextLostCallback> m_contextLostCallbackAdapter;
+    OwnPtrWillBeMember<WebGLRenderingContextErrorMessageCallback> m_errorMessageCallbackAdapter;
 
     // List of bound VBO's. Used to maintain info about sizes for ARRAY_BUFFER and stored values for ELEMENT_ARRAY_BUFFER
-    RefPtr<WebGLBuffer> m_boundArrayBuffer;
+    RefPtrWillBeMember<WebGLBuffer> m_boundArrayBuffer;
 
-    RefPtr<WebGLVertexArrayObjectOES> m_defaultVertexArrayObject;
-    RefPtr<WebGLVertexArrayObjectOES> m_boundVertexArrayObject;
-    void setBoundVertexArrayObject(PassRefPtr<WebGLVertexArrayObjectOES> arrayObject)
+    RefPtrWillBeMember<WebGLVertexArrayObjectOES> m_defaultVertexArrayObject;
+    RefPtrWillBeMember<WebGLVertexArrayObjectOES> m_boundVertexArrayObject;
+    void setBoundVertexArrayObject(PassRefPtrWillBeRawPtr<WebGLVertexArrayObjectOES> arrayObject)
     {
         if (arrayObject)
             m_boundVertexArrayObject = arrayObject;
@@ -452,25 +464,21 @@ protected:
     };
     Vector<VertexAttribValue> m_vertexAttribValue;
     unsigned m_maxVertexAttribs;
-    RefPtr<WebGLBuffer> m_vertexAttrib0Buffer;
+    RefPtrWillBeMember<WebGLBuffer> m_vertexAttrib0Buffer;
     long m_vertexAttrib0BufferSize;
     GLfloat m_vertexAttrib0BufferValue[4];
     bool m_forceAttrib0BufferRefill;
     bool m_vertexAttrib0UsedBefore;
 
-    RefPtr<WebGLProgram> m_currentProgram;
-    RefPtr<WebGLFramebuffer> m_framebufferBinding;
-    RefPtr<WebGLRenderbuffer> m_renderbufferBinding;
-    class TextureUnitState {
-    public:
-        RefPtr<WebGLTexture> m_texture2DBinding;
-        RefPtr<WebGLTexture> m_textureCubeMapBinding;
-    };
-    Vector<TextureUnitState> m_textureUnits;
+    RefPtrWillBeMember<WebGLProgram> m_currentProgram;
+    RefPtrWillBeMember<WebGLFramebuffer> m_framebufferBinding;
+    RefPtrWillBeMember<WebGLRenderbuffer> m_renderbufferBinding;
+
+    WillBeHeapVector<TextureUnitState> m_textureUnits;
     unsigned long m_activeTextureUnit;
 
-    RefPtr<WebGLTexture> m_blackTexture2D;
-    RefPtr<WebGLTexture> m_blackTextureCubeMap;
+    RefPtrWillBeMember<WebGLTexture> m_blackTexture2D;
+    RefPtrWillBeMember<WebGLTexture> m_blackTextureCubeMap;
 
     Vector<GLenum> m_compressedTextureFormats;
 
@@ -507,7 +515,7 @@ protected:
     GLenum m_unpackColorspaceConversion;
     bool m_contextLost;
     LostContextMode m_contextLostMode;
-    RefPtr<WebGLContextAttributes> m_requestedAttributes;
+    RefPtrWillBeMember<WebGLContextAttributes> m_requestedAttributes;
 
     bool m_layerCleared;
     GLfloat m_clearColor[4];
@@ -564,11 +572,13 @@ protected:
         const char* const* prefixes() const;
         bool matchesNameWithPrefixes(const String&) const;
 
-        virtual PassRefPtr<WebGLExtension> getExtension(WebGLRenderingContextBase*) = 0;
+        virtual PassRefPtrWillBeRawPtr<WebGLExtension> getExtension(WebGLRenderingContextBase*) = 0;
         virtual bool supported(WebGLRenderingContextBase*) const = 0;
         virtual const char* extensionName() const = 0;
         virtual void loseExtension() = 0;
 
+        virtual void trace(Visitor*) { }
+
     private:
         bool m_draft;
         const char* const* m_prefixes;
@@ -577,11 +587,9 @@ protected:
     template <typename T>
     class TypedExtensionTracker FINAL : public ExtensionTracker {
     public:
-        TypedExtensionTracker(RefPtr<T>& extensionField, ExtensionFlags flags, const char* const* prefixes)
-            : ExtensionTracker(flags, prefixes)
-            , m_extensionField(extensionField)
-            , m_extension(nullptr)
+        static PassOwnPtr<TypedExtensionTracker<T> > create(RefPtrWillBeMember<T>& extensionField, ExtensionFlags flags, const char* const* prefixes)
         {
+            return adoptPtr(new TypedExtensionTracker<T>(extensionField, flags, prefixes));
         }
 
         virtual ~TypedExtensionTracker()
@@ -592,7 +600,7 @@ protected:
             }
         }
 
-        virtual PassRefPtr<WebGLExtension> getExtension(WebGLRenderingContextBase* context) OVERRIDE
+        virtual PassRefPtrWillBeRawPtr<WebGLExtension> getExtension(WebGLRenderingContextBase* context) OVERRIDE
         {
             if (!m_extension) {
                 m_extension = T::create(context);
@@ -621,20 +629,38 @@ protected:
             }
         }
 
+        virtual void trace(Visitor* visitor) OVERRIDE
+        {
+            visitor->trace(m_extension);
+            ExtensionTracker::trace(visitor);
+        }
+
     private:
-        RefPtr<T>& m_extensionField;
+        TypedExtensionTracker(RefPtrWillBeMember<T>& extensionField, ExtensionFlags flags, const char* const* prefixes)
+            : ExtensionTracker(flags, prefixes)
+            , m_extensionField(extensionField)
+            , m_extension(nullptr)
+        {
+        }
+
+        // FIXME: Oilpan: off-heap (vector) collections with OwnPtr<>s

[Mads Ager (chromium), 2014/07/11 06:47:23]

We are working hard on getting rid of tracing of off-heap collections and
OwnPtrs. We cannot support them because we will need to be able to
conservatively scan objects which means that all pointers that have to be traced
have to be in the object directly. Can we change these tracker objects to be
heap allocated and use OwnPtrWillBeMember? As far as I can tell we shouldn't be
using the destructors of these tracker objects anyway with oilpan.

[sof, 2014/07/11 14:00:08]

I've been trying to recount all reasons for moving these ExtensionTrackers off
the heap earlier in this review (they were on the heap initially), but with the
weak member from WebGLExtension to its context, I can't see the problem wrt
safety or being observably different than in non-Oilpan.

So, let's try again. Moved it back, but been spending quite a while trying to
make sense of a crash that I'm running into with that in place --
http://pastebin.com/7pSz1vha

i.e., entering an empty vtbl entry?(!) when tracing some hash table. It
definitely is triggered by that heap vector and the ExtensionTracker
objects...bizarre.

Anyone else having run into something like this?

[sof, 2014/07/12 07:22:09]

If this seemingly unrelated bug ends up requiring more work and investigation,
I'm tempted to revert back to having these extension trackers off the heap here.
Keeping a large CL like this alive across ToT changes isn't ideal.

[sof, 2014/07/13 20:42:18]

Some more info/background:
 
- The crash happens when a GC is triggered when needing to expand

    HeapHashSet<WeakMember<DocumentVisibilityObserver>>

- DocumentVisibilityObserver is a Mixin, and HTMLCanvasElement derives from it.

- Such weak-membered mixins aren't common, I'd allege.

- The crash happens when tracing hashtable backing store, the marking of an
element fails.

- It is clearly unsafe to assume that the backing store being traced is that of
the hashtable being expanded (I've been unable to trap the crash in gdb), but
the two does seem connected..
- I've gone over the details to check if NeedsAdjustAndMark<> is somehow  not
returning the expected value for the class types involved here, but haven't
found anything.

Might the recent ephemeron changes not treat Mixins correctly, somehow?

[sof, 2014/07/14 09:20:16]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...

has some other reported instances of the same crash stack. The linux-dbg crash
stack there shows that it is indeed crashing when tracing the HeapHashSet<> in
question here. It is invoking the Mixin mark() version, but the non-null object
reference is invalid.

I've verified that zeroedBackingMalloc() doesn't let through non-zero'ed blocks
in debug builds. (But is that guaranteed to happen in release builds?)

+        // to objects with trace() methods are currently supported,
+        // but not specially recognized by the clang plugin.
+        GC_PLUGIN_IGNORE("340522")
+        RefPtrWillBeMember<T>& m_extensionField;
         // ExtensionTracker holds it's own reference to the extension to ensure
         // that it is not deleted before this object's destructor is called
-        RefPtr<T> m_extension;
+        GC_PLUGIN_IGNORE("340522")
+        RefPtrWillBeMember<T> m_extension;
     };
 
     bool m_extensionEnabled[WebGLExtensionNameCount];
-    Vector<ExtensionTracker*> m_extensions;
+    Vector<OwnPtr<ExtensionTracker> > m_extensions;
 
     template <typename T>
-    void registerExtension(RefPtr<T>& extensionPtr, ExtensionFlags flags = ApprovedExtension, const char* const* prefixes = 0)
+    void registerExtension(RefPtrWillBeMember<T>& extensionPtr, ExtensionFlags flags = ApprovedExtension, const char* const* prefixes = 0)
     {
-        m_extensions.append(new TypedExtensionTracker<T>(extensionPtr, flags, prefixes));
+        m_extensions.append(TypedExtensionTracker<T>::create(extensionPtr, flags, prefixes));
     }
 
     bool extensionSupportedAndAllowed(const ExtensionTracker*);
@@ -908,4 +934,6 @@ DEFINE_TYPE_CASTS(WebGLRenderingContextBase, CanvasRenderingContext, context, co
 
 } // namespace WebCore
 
+WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS(WebCore::WebGLRenderingContextBase::TextureUnitState);
+
 #endif // WebGLRenderingContextBase_h