Fixed WebglRenderingContextBase toImageData

Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by convertToBlob,
wrongly used DrawingBuffer::transferToStaticBitmapImage() to obtain the image data.
This results in black image when toImageData() is called back-to-back, because
transferToStaticBitmapImage() discards the backbuffer when preserveDrawingBuffer
(a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData do;
but it removes all unnecessary steps in HTMLCanvasElement that deals with
ImageBuffer(Surface) and other extra steps that do not apply to OffscreenCanvas,
and also handles the additional situation when toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/7922f33fe97e510c7259ebc077bd04ea2657e704
Cr-Commit-Position: refs/heads/master@{#441511}

[xlai (Olivia), 2017-01-02 00:07:53]

Description was changed from

==========
Fixed WebglRenderingContextBase toImageData

BUG=
==========

to

==========
Fixed WebglRenderingContextBase toImageData

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[xlai (Olivia), 2017-01-02 21:34:54]

Description was changed from

==========
Fixed WebglRenderingContextBase toImageData

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fixed WebglRenderingContextBase toImageData

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[xlai (Olivia), 2017-01-02 21:34:55]

xlai@chromium.org changed reviewers:
+ junov@chromium.org, kbr@chromium.org

[xlai (Olivia), 2017-01-02 21:38:07]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-02 21:38:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xlai (Olivia), 2017-01-02 22:00:21]

Description was changed from

==========
Fixed WebglRenderingContextBase toImageData

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob, wrongly used DrawingBuffer::transferToStaticBitmapImage() to
obtain the image data. This results in black image when toImageData()
is called back-to-back, because transferToStaticBitmapImage() discards
the backbuffer when preserveDrawingBuffer (a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do; but it takes away all
unnecessary steps in HTMLCanvasElement that deals with ImageBuffer(Surface) and
other extra steps that do
not apply to OffscreenCanvas, and also handles the additional situation when
toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[xlai (Olivia), 2017-01-02 22:01:18]

Description was changed from

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob, wrongly used DrawingBuffer::transferToStaticBitmapImage() to
obtain the image data. This results in black image when toImageData()
is called back-to-back, because transferToStaticBitmapImage() discards
the backbuffer when preserveDrawingBuffer (a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do; but it takes away all
unnecessary steps in HTMLCanvasElement that deals with ImageBuffer(Surface) and
other extra steps that do
not apply to OffscreenCanvas, and also handles the additional situation when
toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob, 
wrongly used DrawingBuffer::transferToStaticBitmapImage() to obtain the image
data. 
This results in black image when toImageData() is called back-to-back, because 
transferToStaticBitmapImage() discards the backbuffer when preserveDrawingBuffer

(a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do; 
but it removes all unnecessary steps in HTMLCanvasElement that deals with 
ImageBuffer(Surface) and other extra steps that do not apply to OffscreenCanvas,

and also handles the additional situation when toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[xlai (Olivia), 2017-01-02 22:50:04]

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/DEPS (right):

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/DEPS:4: "+skia/ext",
I saw that "skia/ext" is allowed in
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/notifi....
So I don't see any reasons why it cannot be allowed in modules/webgl.

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:789:
gpu::gles2::GLES2Interface* gl = SharedGpuContext::gl();
This one works for both main and worker threads.

[commit-bot: I haz the power, 2017-01-02 22:54:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-02 22:54:21]

Dry run: This issue passed the CQ dry run.

[Ken Russell (switch to Gerrit), 2017-01-04 04:01:12]

Very nice and good tests. Sorry for the delay reviewing this. LGTM

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/DEPS (right):

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/DEPS:4: "+skia/ext",
On 2017/01/02 22:50:04, xlai (Olivia) wrote:
> I saw that "skia/ext" is allowed in
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/notifi....
> So I don't see any reasons why it cannot be allowed in modules/webgl.

Acknowledged.

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
(right):

https://codereview.chromium.org/2602253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:789:
gpu::gles2::GLES2Interface* gl = SharedGpuContext::gl();
On 2017/01/02 22:50:04, xlai (Olivia) wrote:
> This one works for both main and worker threads.

Acknowledged.

[Justin Novosad, 2017-01-04 20:30:01]

lgtm

[xlai (Olivia), 2017-01-04 21:40:44]

The CQ bit was checked by xlai@chromium.org

[commit-bot: I haz the power, 2017-01-04 21:41:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 23:37:26]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483566044269220,
"parent_rev": "d787de5bca597a01a8291fa1297ae8fdcb3f7c99", "commit_rev":
"13aa5945f5e9db0674a08b4452400f8b994d5bfc"}

[commit-bot: I haz the power, 2017-01-04 23:37:58]

Description was changed from

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob, 
wrongly used DrawingBuffer::transferToStaticBitmapImage() to obtain the image
data. 
This results in black image when toImageData() is called back-to-back, because 
transferToStaticBitmapImage() discards the backbuffer when preserveDrawingBuffer

(a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do; 
but it removes all unnecessary steps in HTMLCanvasElement that deals with 
ImageBuffer(Surface) and other extra steps that do not apply to OffscreenCanvas,

and also handles the additional situation when toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob, 
wrongly used DrawingBuffer::transferToStaticBitmapImage() to obtain the image
data. 
This results in black image when toImageData() is called back-to-back, because 
transferToStaticBitmapImage() discards the backbuffer when preserveDrawingBuffer

(a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do; 
but it removes all unnecessary steps in HTMLCanvasElement that deals with 
ImageBuffer(Surface) and other extra steps that do not apply to OffscreenCanvas,

and also handles the additional situation when toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2602253002
==========

[commit-bot: I haz the power, 2017-01-04 23:37:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2017-01-04 23:41:29]

Description was changed from

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob, 
wrongly used DrawingBuffer::transferToStaticBitmapImage() to obtain the image
data. 
This results in black image when toImageData() is called back-to-back, because 
transferToStaticBitmapImage() discards the backbuffer when preserveDrawingBuffer

(a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do; 
but it removes all unnecessary steps in HTMLCanvasElement that deals with 
ImageBuffer(Surface) and other extra steps that do not apply to OffscreenCanvas,

and also handles the additional situation when toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2602253002
==========

to

==========
Fixed WebglRenderingContextBase toImageData

Previously, WebglRenderingContextBase::toImageData(), which is used by
convertToBlob,
wrongly used DrawingBuffer::transferToStaticBitmapImage() to obtain the image
data.
This results in black image when toImageData() is called back-to-back, because
transferToStaticBitmapImage() discards the backbuffer when preserveDrawingBuffer
(a user-set flag) is false.

The new implementation in this patch follows what HTMLCanvasElement::toImageData
do;
but it removes all unnecessary steps in HTMLCanvasElement that deals with
ImageBuffer(Surface) and other extra steps that do not apply to OffscreenCanvas,
and also handles the additional situation when toImageData can be called on
worker thread.

BUG=657531
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/7922f33fe97e510c7259ebc077bd04ea2657e704
Cr-Commit-Position: refs/heads/master@{#441511}
==========

[commit-bot: I haz the power, 2017-01-04 23:41:30]

Patchset 4 (id:??) landed as
https://crrev.com/7922f33fe97e510c7259ebc077bd04ea2657e704
Cr-Commit-Position: refs/heads/master@{#441511}