Implement OffscreenCanvas.getContext('webgl')

third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h

Index: third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
index af0f0c9a5a667046d04bc01bb6861afcedc76649..ca9844691f9404621cd49a27234c60ad76158025 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
@@ -35,6 +35,7 @@
 #include "core/dom/TypedFlexibleArrayBufferView.h"
 #include "core/html/canvas/CanvasRenderingContext.h"
 #include "core/layout/LayoutBoxModelObject.h"
+#include "modules/offscreencanvas/OffscreenCanvasRenderingContext.h"
 #include "modules/webgl/WebGLContextAttributes.h"
 #include "modules/webgl/WebGLExtensionName.h"
 #include "modules/webgl/WebGLTexture.h"
@@ -141,7 +142,8 @@ private:
     Member<WebGLFramebuffer> m_readFramebufferBinding;
 };
 
-class MODULES_EXPORT WebGLRenderingContextBase : public CanvasRenderingContext {
+class MODULES_EXPORT WebGLRenderingContextBase : public CanvasRenderingContext, public OffscreenCanvasRenderingContext, public ScriptWrappable {

[xlai (Olivia), 2016/04/20 20:42:16]

[continued from previous comment] But then why is ScriptWrappable in WebGLBase,
instead of being in WebGL2 and WebGL separately? Are they sharing the same
interface in scripts?

[xidachen, 2016/04/20 20:52:09]

WebGLBase has a idl and it is the actual interface for WebGLRenderingContext.
WebGL2RenderingContext just extend the WebGLBase.

[Ken Russell (switch to Gerrit), 2016/04/21 01:05:24]

Based on past experience with multiple inheritance I think that making
WebGLRenderingContextBase inherit from both CanvasRenderingContext and
OffscreenCanvasRenderingContext is not a good direction to take.

The introduction of OffscreenCanvasRenderingContext as a concept does not appear
necessary to me. All that needs to happen is:

 - The few methods that deal with the HTMLCanvasElement in
CanvasRenderingContext2D, WebGLRenderingContext, etc. and their base classes
need to be considered optional.

 - Methods need to be added to the base class (CanvasRenderingContext) allowing
it to be used with OffscreenCanvas.

 - CanvasRenderingContextFactory needs to be extended with a new factory method
taking OffscreenCanvas instead of HTMLCanvasElement as the owner of the context.

Fundamentally, the context classes will have to understand their two modes of
operation -- rendering to either an on-screen or off-screen canvas. Using
multiple inheritance to achieve this is deeply problematic. Pointers to the
objects change value depending on which area of the code is touching it.
Accidental diamond inheritance causes behaviors that can't be easily explained.
Interaction with the JavaScript layer is made more confusing. There are many
other reasons for preferring single inheritance.

Please make this change. I think that using multiple inheritance for the WebGL
contexts is not a good design decision and I can't support moving the code in
this direction.

[xlai (Olivia), 2016/04/21 15:31:54]

First, I agree with merging OffscreenCanvasRenderingContext and
CanvasRenderingContext into one class (so does the merging of non-offscreen and
offscreen factory classes into one). It is also possible to implement by:
- Adding one more create function in Factory that takes in OffscreenCanvas* and
returns CanvasRenderingContext*. This create function is invoked by
OffscreenCanvas.getContext() only.
- From what I see, CanvasRenderingContext only interacts with HTMLCanvasElement
by having a member instance m_canvas; we can set it null in the case of
OffscreenCanvas and have one more member instance called m_offscreencanvas.
- Adding all out offscreen context types to the list of enum in
CanvasRenderingContext.ContextType with prefix "Offscreen" to distinguish them
with the existing rendering context types.

Second, I disagree with changing the status quo in
OffscreenCanvasRenderingContext2D and CanvasRenderingContext2D. I think attempts
to merge these two classes will make things messy. Right now,
BaseRenderingContext2D is parent of 3 classes--PaintRenderingContext2D,
CanvasRenderingContext2D, OffscreenCanvasRenderingContext2D. It is problematic
to adapt BaseRC2D for only the canvas cases, without affecting the PaintRC2D.
Also, there is no reason to let OffCRC2D and CRC2D to become one class, as many
functions that solely exist in CRC2D (hitregion, focus) are not applicable to
OffscreenCanvas.

[Ken Russell (switch to Gerrit), 2016/04/21 19:52:00]

OK. Please do as you see fit with CanvasRenderingContext2D and
OffscreenCanvasRenderingContext2D, but please merge the base class so that
CanvasRenderingContext is the sole base class for all of the derived classes.
Thanks.

Also please note that https://wiki.whatwg.org/wiki/OffscreenCanvas doesn't
define an OffscreenCanvasRenderingContext2D type. The behavior of these methods
that make no sense on OffscreenCanvases needs to be specified, or a
CanvasRenderingContext2DBase needs to be defined with the common methods, and
CanvasRenderingContext2D specified to add the methods that only make sense with
on-screen canvases.

+    USING_GARBAGE_COLLECTED_MIXIN(WebGLRenderingContextBase);
 public:
     ~WebGLRenderingContextBase() override;
 
@@ -154,6 +156,7 @@ public:
     static unsigned getWebGLVersion(const CanvasRenderingContext*);
 
     static PassOwnPtr<WebGraphicsContext3DProvider> createWebGraphicsContext3DProvider(HTMLCanvasElement*, WebGLContextAttributes, unsigned webGLVersion);
+    static PassOwnPtr<WebGraphicsContext3DProvider> createWebGraphicsContext3DProvider(ScriptState*, WebGLContextAttributes, unsigned webGLVersion);
     static void forceNextWebGLContextCreationToFail();
 
     int drawingBufferWidth() const;
@@ -409,7 +412,6 @@ public:
     // to (first) be able to detach its WebGLContextObjects, before
     // they're later swept and finalized.
     EAGERLY_FINALIZE();
-    DECLARE_EAGER_FINALIZATION_OPERATOR_NEW();
     DECLARE_VIRTUAL_TRACE();
 
     // Returns approximate gpu memory allocated per pixel.
@@ -448,6 +450,7 @@ protected:
     friend class ScopedFramebufferRestorer;
 
     WebGLRenderingContextBase(HTMLCanvasElement*, PassOwnPtr<WebGraphicsContext3DProvider>, const WebGLContextAttributes&);
+    WebGLRenderingContextBase(OffscreenCanvas*, PassOwnPtr<WebGraphicsContext3DProvider>, const WebGLContextAttributes&);
     PassRefPtr<DrawingBuffer> createDrawingBuffer(PassOwnPtr<WebGraphicsContext3DProvider>);
     void setupFlags();
 
@@ -1080,6 +1083,9 @@ protected:
     static WebGLRenderingContextBase* oldestEvictedContext();
 
     CrossThreadWeakPersistentThisPointer<WebGLRenderingContextBase> createWeakThisPointer() { return CrossThreadWeakPersistentThisPointer<WebGLRenderingContextBase>(this); }
+
+private:
+    WebGLRenderingContextBase(HTMLCanvasElement*, OffscreenCanvas*, PassOwnPtr<WebGraphicsContext3DProvider>, const WebGLContextAttributes&);
 };
 
 DEFINE_TYPE_CASTS(WebGLRenderingContextBase, CanvasRenderingContext, context, context->is3d(), context.is3d());