Add ImageBitmapRenderingContext (experimental)

This change adds the ImageBitmapRenderingContext component
of the OffscreenCanvas[1] feature proposal. It is currently
hidden behind the 'experimental canvas features' runtime
flag.

[1] https://wiki.whatwg.org/wiki/OffscreenCanvas

BUG=563861
Committed: https://crrev.com/0090816b130a929829e025a2386e23228983e058
Cr-Commit-Position: refs/heads/master@{#375198}

[Justin Novosad, 2016-02-09 22:50:25]

Description was changed from

==========
WIP (not for review) Add ImageBitmapRenderingContext
==========

to

==========
This change adds the ImageBitmapRenderingContext component
of the OffscreenCanvas[1] feature proposal. It is currently
hidden behind the 'experimental canvas features' runtime
flag.

[1] https://wiki.whatwg.org/wiki/OffscreenCanvas

BUG=563861
==========

[Justin Novosad, 2016-02-09 22:51:51]

junov@chromium.org changed reviewers:
+ haraken@chromium.org, senorblanco@google.com

[Justin Novosad, 2016-02-09 22:53:26]

senorblanco: PTAL
haraken: for InitModules.cpp OWNERS

[haraken, 2016-02-10 00:15:37]

On 2016/02/09 22:53:26, Justin Novosad wrote:
> senorblanco: PTAL
> haraken: for InitModules.cpp OWNERS

InitModules.cpp LGTM

[Stephen White, 2016-02-10 15:34:54]

senorblanco@chromium.org changed reviewers:
+ senorblanco@chromium.org

[Stephen White, 2016-02-10 15:34:55]

A great start!

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap-alpha.html
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap-alpha.html:5:
testRunner.waitUntilDone();
Ibid.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap-alpha.html:15:
var dstCanvas = document.getElementById('dstCanvas');
Ibid.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap-no-alpha.html
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap-no-alpha.html:5:
testRunner.waitUntilDone();
Ibid.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap-no-alpha.html:15:
var dstCanvas = document.getElementById('dstCanvas');
Ibid.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap.html
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap.html:5:
testRunner.waitUntilDone();
Nit: weird indentation here.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap.html:15:
var dstCanvas = document.getElementById('dstCanvas');
Nit: weird indentation here.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:464: // TODO(junov):
Paint is currently only implemented by ImageBitmap contexts,
MicroNit: comma splice. Use a semicolon, or split into two sentences.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:466: /// themselves
(implement paint).
MicroNit: ///

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:24:
m_image = imageBitmap->bitmapImage();
colorPicker->pickColorColorPickPick().  :)

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:30:
// TODO(junov): crbug.com/585607 Eliminate this readback and use an
ExternalTextureLayer
Should we do this check only for display-list canvas, if possible?

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:33:
// silent failure
Is a silent failure spec-compliant? If not, please file a bug.

[Justin Novosad, 2016-02-10 15:50:21]

New Patch

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap.html
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/imagebitmap/transferImageBitmap.html:5:
testRunner.waitUntilDone();
On 2016/02/10 15:34:55, Stephen White wrote:
> Nit: weird indentation here.

Hah, looks like the presubmit check don't check for tab characters in html
files.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:24:
m_image = imageBitmap->bitmapImage();
On 2016/02/10 15:34:55, Stephen White wrote:
> colorPicker->pickColorColorPickPick().  :)

LOL!!!  The old days...

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:30:
// TODO(junov): crbug.com/585607 Eliminate this readback and use an
ExternalTextureLayer
On 2016/02/10 15:34:55, Stephen White wrote:
> Should we do this check only for display-list canvas, if possible?
It is not display list canvas that is the problem, It's blink display lists
(impl-side rasterization).

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:33:
// silent failure
On 2016/02/10 15:34:55, Stephen White wrote:
> Is a silent failure spec-compliant? If not, please file a bug.

The handling of general OOM failures is not specified AFAIK
It is either this or crash (or draw a sad canvas?).  The pre-existing policy in
2D canvas is to let the canvas be blank if we fail to allocate it's backing
store. This is the ImageBitmap equivalent of the same behavior.

[Stephen White, 2016-02-10 15:56:09]

LGTM

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:30:
// TODO(junov): crbug.com/585607 Eliminate this readback and use an
ExternalTextureLayer
On 2016/02/10 15:50:21, Justin Novosad wrote:
> On 2016/02/10 15:34:55, Stephen White wrote:
> > Should we do this check only for display-list canvas, if possible?
> It is not display list canvas that is the problem, It's blink display lists
> (impl-side rasterization).

Right, but isn't impl-side rasterization only utilized in display list mode?
Otherwise we do raster in the renderer
process, no? I was thinking that if there was a way to detect it, we could do
GPU raster without a readback (and 
without an external texture layer) when in non-display-list mode. Then again,
I'm not certain I understand if/how 
this new context interacts with the different canvas rendering modes.

Doesn't have to be in this patch, just some food for thought.

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:33:
// silent failure
On 2016/02/10 15:50:21, Justin Novosad wrote:
> On 2016/02/10 15:34:55, Stephen White wrote:
> > Is a silent failure spec-compliant? If not, please file a bug.
> 
> The handling of general OOM failures is not specified AFAIK
> It is either this or crash (or draw a sad canvas?).  The pre-existing policy
in
> 2D canvas is to let the canvas be blank if we fail to allocate it's backing
> store. This is the ImageBitmap equivalent of the same behavior.

Acknowledged.

[Justin Novosad, 2016-02-10 16:09:29]

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:30:
// TODO(junov): crbug.com/585607 Eliminate this readback and use an
ExternalTextureLayer
There is no "display list mode". The concept of display list mode only exist in
2D canvases. This is an imagebitmap canvas. All this context does is dump a
bitmap into the containing layer's GraphicsContext. The GraphicsContext records
into a blink DisplayListItem that gets rasterized by the compositor. This flow
does not support GPU-backed bitmaps, ever. The only way to prevent the readback
is to set up an "ImageBitmapLayerBridge", which is what crbug.com/585607 is
about.

[Justin Novosad, 2016-02-10 16:10:18]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-02-10 16:10:19]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1598923002/#ps40001
(title: "Removed tabs in html files")

[commit-bot: I haz the power, 2016-02-10 16:11:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1598923002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1598923002/40001

[Stephen White, 2016-02-10 16:11:34]

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp
(right):

https://codereview.chromium.org/1598923002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/imagebitmap/ImageBitmapRenderingContext.cpp:30:
// TODO(junov): crbug.com/585607 Eliminate this readback and use an
ExternalTextureLayer
On 2016/02/10 16:09:29, Justin Novosad wrote:
> There is no "display list mode". The concept of display list mode only exist
in
> 2D canvases. This is an imagebitmap canvas. All this context does is dump a
> bitmap into the containing layer's GraphicsContext. The GraphicsContext
records
> into a blink DisplayListItem that gets rasterized by the compositor. This flow
> does not support GPU-backed bitmaps, ever. The only way to prevent the
readback
> is to set up an "ImageBitmapLayerBridge", which is what crbug.com/585607 is
> about.

Acknowledged. These are not the nits you're looking for. You may go about your
business.

[commit-bot: I haz the power, 2016-02-10 17:28:03]

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

[commit-bot: I haz the power, 2016-02-10 17:28:03]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Justin Novosad, 2016-02-10 20:45:22]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-02-10 20:45:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, senorblanco@chromium.org
Link to the patchset: https://codereview.chromium.org/1598923002/#ps60001
(title: "Adding missing adoptRef")

[commit-bot: I haz the power, 2016-02-10 20:47:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1598923002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1598923002/60001

[commit-bot: I haz the power, 2016-02-10 22:17:35]

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

[commit-bot: I haz the power, 2016-02-10 22:17:36]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Justin Novosad, 2016-02-10 22:18:18]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-02-10 22:18:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, senorblanco@chromium.org
Link to the patchset: https://codereview.chromium.org/1598923002/#ps80001
(title: "another missing adoptRef")

[commit-bot: I haz the power, 2016-02-10 22:21:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1598923002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1598923002/80001

[commit-bot: I haz the power, 2016-02-11 00:31:20]

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

[commit-bot: I haz the power, 2016-02-11 00:31:21]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Justin Novosad, 2016-02-12 16:35:09]

On 2016/02/11 00:31:21, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

Looks like unrelated flake. re-trying...

[Justin Novosad, 2016-02-12 16:35:16]

The CQ bit was checked by junov@chromium.org

[commit-bot: I haz the power, 2016-02-12 16:35:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1598923002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1598923002/80001

[commit-bot: I haz the power, 2016-02-12 18:20:45]

Description was changed from

==========
This change adds the ImageBitmapRenderingContext component
of the OffscreenCanvas[1] feature proposal. It is currently
hidden behind the 'experimental canvas features' runtime
flag.

[1] https://wiki.whatwg.org/wiki/OffscreenCanvas

BUG=563861
==========

to

==========
This change adds the ImageBitmapRenderingContext component
of the OffscreenCanvas[1] feature proposal. It is currently
hidden behind the 'experimental canvas features' runtime
flag.

[1] https://wiki.whatwg.org/wiki/OffscreenCanvas

BUG=563861
==========

[commit-bot: I haz the power, 2016-02-12 18:20:46]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-02-16 22:43:37]

Description was changed from

==========
This change adds the ImageBitmapRenderingContext component
of the OffscreenCanvas[1] feature proposal. It is currently
hidden behind the 'experimental canvas features' runtime
flag.

[1] https://wiki.whatwg.org/wiki/OffscreenCanvas

BUG=563861
==========

to

==========
This change adds the ImageBitmapRenderingContext component
of the OffscreenCanvas[1] feature proposal. It is currently
hidden behind the 'experimental canvas features' runtime
flag.

[1] https://wiki.whatwg.org/wiki/OffscreenCanvas

BUG=563861

Committed: https://crrev.com/0090816b130a929829e025a2386e23228983e058
Cr-Commit-Position: refs/heads/master@{#375198}
==========

[commit-bot: I haz the power, 2016-02-16 22:43:38]

Patchset 5 (id:??) landed as
https://crrev.com/0090816b130a929829e025a2386e23228983e058
Cr-Commit-Position: refs/heads/master@{#375198}