Make OffscreenCanvas Webgl Context work with preserveDrawingBuffer flag

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

Index: third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
index 42972294f3d78d5904bd7dc548708e646f034219..609f776a2863240263e66359c2148648946cb99a 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -749,10 +749,22 @@ ScriptPromise WebGLRenderingContextBase::commit(
   if (!drawingBuffer()) {
     return offscreenCanvas()->commit(nullptr, false, scriptState);
   }
-  // TODO(crbug.com/646864): Make commit() work correctly with
-  // { preserveDrawingBuffer : true }.
+
+  RefPtr<StaticBitmapImage> image;
+  if (creationAttributes().preserveDrawingBuffer()) {
+    int width = drawingBuffer()->size().width();
+    int height = drawingBuffer()->size().height();
+    SkImageInfo imageInfo =
+        SkImageInfo::Make(width, height, kRGBA_8888_SkColorType,
+                          creationAttributes().alpha() ? kPremul_SkAlphaType
+                                                       : kOpaque_SkAlphaType);
+    image = StaticBitmapImage::create(makeImageSnapshotCopy(imageInfo));
+  } else {
+    image = drawingBuffer()->transferToStaticBitmapImage();
+  }
+
   return offscreenCanvas()->commit(
-      std::move(drawingBuffer()->transferToStaticBitmapImage()),
+      std::move(image),
       drawingBuffer()->contextProvider()->isSoftwareRendering(), scriptState);
 }
 
@@ -782,6 +794,27 @@ PassRefPtr<Image> WebGLRenderingContextBase::getImage(
   return buffer->newImageSnapshot(hint, reason);
 }
 
+sk_sp<SkImage> WebGLRenderingContextBase::makeImageSnapshotCopy(
+    SkImageInfo& imageInfo) {
+  drawingBuffer()->resolveAndBindForReadAndDraw();
+  gpu::gles2::GLES2Interface* gl = SharedGpuContext::gl();
+
+  SkSurfaceProps disableLCDProps(0, kUnknown_SkPixelGeometry);
+  sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(
+      SharedGpuContext::gr(), SkBudgeted::kYes, imageInfo, 0,
+      imageInfo.alphaType() == kOpaque_SkAlphaType ? nullptr
+                                                   : &disableLCDProps);
+  GLuint textureId = skia::GrBackendObjectToGrGLTextureInfo(
+                         surface->getTextureHandle(
+                             SkSurface::kDiscardWrite_TextureHandleAccess))
+                         ->fID;
+
+  drawingBuffer()->copyToPlatformTexture(
+      gl, textureId, GL_RGBA, GL_UNSIGNED_BYTE, 0, true, false, IntPoint(0, 0),
+      IntRect(IntPoint(0, 0), drawingBuffer()->size()), BackBuffer);
+  return surface->makeImageSnapshot();
+}
+
 ImageData* WebGLRenderingContextBase::toImageData(SnapshotReason reason) {
   ImageData* imageData = nullptr;
   // TODO(ccameron): WebGL should produce sRGB images.
@@ -795,27 +828,11 @@ ImageData* WebGLRenderingContextBase::toImageData(SnapshotReason reason) {
 
     int width = drawingBuffer()->size().width();
     int height = drawingBuffer()->size().height();
-    OpacityMode opacityMode = creationAttributes().alpha() ? NonOpaque : Opaque;
-
-    drawingBuffer()->resolveAndBindForReadAndDraw();
-    gpu::gles2::GLES2Interface* gl = SharedGpuContext::gl();
-    SkImageInfo imageInfo = SkImageInfo::Make(
-        width, height, kRGBA_8888_SkColorType,
-        Opaque == opacityMode ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
-    SkSurfaceProps disableLCDProps(0, kUnknown_SkPixelGeometry);
-    sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(
-        SharedGpuContext::gr(), SkBudgeted::kYes, imageInfo, 0,
-        Opaque == opacityMode ? nullptr : &disableLCDProps);
-    GLuint textureId = skia::GrBackendObjectToGrGLTextureInfo(
-                           surface->getTextureHandle(
-                               SkSurface::kDiscardWrite_TextureHandleAccess))
-                           ->fID;
-
-    drawingBuffer()->copyToPlatformTexture(
-        gl, textureId, GL_RGBA, GL_UNSIGNED_BYTE, 0, true, false,
-        IntPoint(0, 0), IntRect(IntPoint(0, 0), drawingBuffer()->size()),
-        BackBuffer);
-    sk_sp<SkImage> snapshot = surface->makeImageSnapshot();
+    SkImageInfo imageInfo =
+        SkImageInfo::Make(width, height, kRGBA_8888_SkColorType,
+                          creationAttributes().alpha() ? kPremul_SkAlphaType
+                                                       : kOpaque_SkAlphaType);
+    sk_sp<SkImage> snapshot = makeImageSnapshotCopy(imageInfo);
     if (snapshot) {
       imageData = ImageData::create(drawingBuffer()->size());
       snapshot->readPixels(imageInfo, imageData->data()->data(),
@@ -4118,7 +4135,12 @@ void WebGLRenderingContextBase::readPixelsHelper(GLint x,
     return;
   // Due to WebGL's same-origin restrictions, it is not possible to
   // taint the origin using the WebGL API.
-  ASSERT(canvas()->originClean());
+  if (canvas()) {

[Justin Novosad, 2017/01/11 21:40:04]

The way this is written, canvas() will get called on builds that have DCHECKs
disabled.  Either use #if or ternary operator inside DCHECK.

[Ken Russell (switch to Gerrit), 2017/01/12 02:01:45]

Yes, let's not call this in release builds. You could use:

#if DCHECK_IS_ON()
...
#endif

but the ternary operator seems reasonable in this case.

[xlai (Olivia), 2017/01/12 20:04:47]

Done.

+    DCHECK(canvas()->originClean());
+  } else {  // Calling from OffscreenCanvas webgl context
+    DCHECK(offscreenCanvas()->originClean());
+  }
+
   // Validate input parameters.
   if (!pixels) {
     synthesizeGLError(GL_INVALID_VALUE, "readPixels",