Revamp OffscreenCanvas commit flow

Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef
Cr-Commit-Position: refs/heads/master@{#439356}

[xlai (Olivia), 2016-12-15 18:42:11]

Description was changed from

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

BUG=664547, 619138, 662498
==========

to

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[xlai (Olivia), 2016-12-15 18:46:31]

Description was changed from

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[xlai (Olivia), 2016-12-15 19:13:57]

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

[xlai (Olivia), 2016-12-15 19:14:36]

PTAL. Thanks!

https://codereview.chromium.org/2584643002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeTest.cpp
(left):

https://codereview.chromium.org/2584643002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeTest.cpp:77:
bool success = this->surfaceLayerBridge()->createSurfaceLayer(50, 50);
This unit test was added before we had end-to-end tests. But now we already have
many GPU pixel tests and this unit test becomes not helpful. The only function
it was testing here is now gone so I remove the unit test.

[Fady Samuel, 2016-12-15 19:54:52]

A few comments but this is looking great so far! Thanks! :D

https://codereview.chromium.org/2584643002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/offscreen_canvas_surface_manager.cc (right):

https://codereview.chromium.org/2584643002/diff/20001/content/browser/rendere...
content/browser/renderer_host/offscreen_canvas_surface_manager.cc:36: if
(surface_iter == registered_surface_instances_.end()) {
no need for braces.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:65:
m_webLayer =
You don't need to create a new layer every time the surfaceId changes. Only when
it was invalid and now is valid.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp:36:
: m_surfaceIdAllocator(new cc::SurfaceIdAllocator()),
You don't need this on the heap, just make it inplace.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:14:
SetClient(OffscreenCanvasSurfaceClient client,
I think we prefer not to have a SetClient interface but rather have a Factory
interface: OffscreenCanvasSurfaceFactory, that takes in both
OffscreenCanvasSurface and OffscreenCanvasSurfaceClient.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:21:
interface OffscreenCanvasSurfaceClient {
nit: Add TODO(fsamuel, xlai): Replace this with DisplayCompositorClient

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:28:
interface OffscreenCanvasCompositorFrameSinkProvider {
nit: Add TODO(fsamuel, xlai): Replace this with DisplayCompositor

[Justin Novosad, 2016-12-15 22:19:35]

This looks great!

https://codereview.chromium.org/2584643002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h
(right):

https://codereview.chromium.org/2584643002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h:54:
scoped_refptr<cc::SolidColorLayer> m_solidColorLayer;
It would be cleaner to have a single reference to a layer.  This should work:
scoped_refptr<cc::Layer> m_layer;  
Unless there is a reason for both layers to exist at the same time.
BTW, I noticed that in OnSurfaceCreated you are not discarding m_solidColorLayer
is that on purpose?

[xlai (Olivia), 2016-12-15 22:20:12]

https://codereview.chromium.org/2584643002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/offscreen_canvas_surface_manager.cc (right):

https://codereview.chromium.org/2584643002/diff/20001/content/browser/rendere...
content/browser/renderer_host/offscreen_canvas_surface_manager.cc:36: if
(surface_iter == registered_surface_instances_.end()) {
On 2016/12/15 19:54:52, Fady Samuel wrote:
> no need for braces.

Done.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:65:
m_webLayer =
On 2016/12/15 19:54:52, Fady Samuel wrote:
> You don't need to create a new layer every time the surfaceId changes. Only
when
> it was invalid and now is valid.

Done. But I'm wondering, when SurfaceId, and possibly dsf, size is changed, do I
just need to call SetSurfaceId function to replace the attribute values on the
existing SurfaceLayer? <== this is a question about resizing functionality not
in this patch.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp:36:
: m_surfaceIdAllocator(new cc::SurfaceIdAllocator()),
On 2016/12/15 19:54:52, Fady Samuel wrote:
> You don't need this on the heap, just make it inplace.

Done.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:14:
SetClient(OffscreenCanvasSurfaceClient client,
On 2016/12/15 19:54:52, Fady Samuel wrote:
> I think we prefer not to have a SetClient interface but rather have a Factory
> interface: OffscreenCanvasSurfaceFactory, that takes in both
> OffscreenCanvasSurface and OffscreenCanvasSurfaceClient.

Done.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:21:
interface OffscreenCanvasSurfaceClient {
On 2016/12/15 19:54:52, Fady Samuel wrote:
> nit: Add TODO(fsamuel, xlai): Replace this with DisplayCompositorClient

Done.

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:28:
interface OffscreenCanvasCompositorFrameSinkProvider {
On 2016/12/15 19:54:52, Fady Samuel wrote:
> nit: Add TODO(fsamuel, xlai): Replace this with DisplayCompositor

Done.

[Fady Samuel, 2016-12-15 22:26:01]

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2584643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:65:
m_webLayer =
On 2016/12/15 22:20:12, xlai (Olivia) wrote:
> On 2016/12/15 19:54:52, Fady Samuel wrote:
> > You don't need to create a new layer every time the surfaceId changes. Only
> when
> > it was invalid and now is valid.
> 
> Done. But I'm wondering, when SurfaceId, and possibly dsf, size is changed, do
I
> just need to call SetSurfaceId function to replace the attribute values on the
> existing SurfaceLayer? <== this is a question about resizing functionality not
> in this patch.

Yes, you just call SetSurfaceId

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
(right):

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp:48:
provider->CreateCompositorFrameSink(surfaceId,
This doesn't need to take in a full surfaceId. Could you please also change this
to take in just a FrameSinkId? Thanks

[Fady Samuel, 2016-12-15 22:28:12]

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom
(left):

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:24:
CreateCompositorFrameSink(cc.mojom.SurfaceId surface_id,
This should only take in a FrameSinkId

[xlai (Olivia), 2016-12-15 22:46:28]

https://codereview.chromium.org/2584643002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h
(right):

https://codereview.chromium.org/2584643002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h:54:
scoped_refptr<cc::SolidColorLayer> m_solidColorLayer;
On 2016/12/15 22:19:35, Justin Novosad wrote:
> It would be cleaner to have a single reference to a layer.  This should work:
> scoped_refptr<cc::Layer> m_layer;  
> Unless there is a reason for both layers to exist at the same time.
> BTW, I noticed that in OnSurfaceCreated you are not discarding
m_solidColorLayer
> is that on purpose?

Done. I shouldn't keep the SolidColorLayer; that was a mistake. With only one
cc::Layer ref ptr now, this issue should be resolved.

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
(right):

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp:48:
provider->CreateCompositorFrameSink(surfaceId,
On 2016/12/15 22:26:01, Fady Samuel wrote:
> This doesn't need to take in a full surfaceId. Could you please also change
this
> to take in just a FrameSinkId? Thanks

Done.

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom
(left):

https://codereview.chromium.org/2584643002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/offscreencanvas/offscreen_canvas_surface.mojom:24:
CreateCompositorFrameSink(cc.mojom.SurfaceId surface_id,
On 2016/12/15 22:28:12, Fady Samuel wrote:
> This should only take in a FrameSinkId

Done.

[Fady Samuel, 2016-12-15 22:55:51]

LGTM + nit

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:67:
surfaceLayer->SetSurfaceId(m_currentSurfaceId, deviceScaleFactor,
nit: TODO(xlai): Update this on resize.

[Justin Novosad, 2016-12-15 23:05:20]

lgtm

[xlai (Olivia), 2016-12-16 01:33:18]

xlai@chromium.org changed reviewers:
+ kenrb@chromium.org

[xlai (Olivia), 2016-12-16 01:34:51]

xlai@chromium.org changed reviewers:
+ haraken@chromium.org

[xlai (Olivia), 2016-12-16 01:35:57]

xlai@chromium.org changed reviewers:
+ jbauman@chromium.org

[xlai (Olivia), 2016-12-16 01:37:50]

Adding more reviewers.
kenrb@: Please review offscreen_canvas_surface.mojom
haraken@: Please review ScriptValueSerializer/Deserializer and
third_party/WebKit/public/
jbauman@: Please review cc/surfaces/

Thanks!

[haraken, 2016-12-16 01:42:19]

bindings/ LGTM

[jbauman, 2016-12-16 02:19:26]

lgtm

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:51:
int32_t width,
I guess this could still glitch if the surface created callback is received and
the layer is added before the first frame for the surface is submitted?

[kenrb, 2016-12-16 18:35:51]

mojo lgtm

[xlai (Olivia), 2016-12-16 21:34:06]

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:51:
int32_t width,
On 2016/12/16 02:19:26, jbauman wrote:
> I guess this could still glitch if the surface created callback is received
and
> the layer is added before the first frame for the surface is submitted?

Nope this is impossible to happen. Only when the first frame of the surface is
submitted, then
the OffscreenCanvasCompositorFrameSink will start working and trigger Surface
creation, which
then trigger OffscreenCanvasSurfaceManager::OnSurfaceCreated as a Surface
observer, which will then finds the OffscreenCanvasSurfaceImpl that is
responsible in sending the surface created callback to here and the renderer
will respond by creating a SurfaceLayer.

https://codereview.chromium.org/2584643002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:67:
surfaceLayer->SetSurfaceId(m_currentSurfaceId, deviceScaleFactor,
On 2016/12/15 22:55:51, Fady Samuel wrote:
> nit: TODO(xlai): Update this on resize.

Done.

[xlai (Olivia), 2016-12-16 21:48:21]

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

[commit-bot: I haz the power, 2016-12-16 21:49:34]

Dry run: 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, 2016-12-16 21:56:06]

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

[commit-bot: I haz the power, 2016-12-16 21:56:07]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xlai (Olivia), 2016-12-17 02:34:29]

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

[commit-bot: I haz the power, 2016-12-17 02:34:54]

Dry run: 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, 2016-12-17 02:54:37]

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

[commit-bot: I haz the power, 2016-12-17 02:54:39]

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

[xlai (Olivia), 2016-12-17 05:51:22]

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

[commit-bot: I haz the power, 2016-12-17 05:51:34]

Dry run: 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, 2016-12-17 05:53:24]

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

[commit-bot: I haz the power, 2016-12-17 05:53:26]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[xlai (Olivia), 2016-12-17 20:19:18]

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

[commit-bot: I haz the power, 2016-12-17 20:19:23]

Dry run: 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, 2016-12-17 22:30:25]

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

[commit-bot: I haz the power, 2016-12-17 22:30:26]

Dry run: This issue passed the CQ dry run.

[xlai (Olivia), 2016-12-17 22:50:56]

The CQ bit was checked by xlai@chromium.org

[xlai (Olivia), 2016-12-17 22:50:56]

The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org,
fsamuel@chromium.org, haraken@chromium.org, junov@chromium.org,
jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2584643002/#ps200001
(title: "rebase and fix conflict")

[xlai (Olivia), 2016-12-17 22:51:04]

The CQ bit was unchecked by xlai@chromium.org

[commit-bot: I haz the power, 2016-12-17 22:51:04]

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

[xlai (Olivia), 2016-12-17 22:52:46]

Description was changed from

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[xlai (Olivia), 2016-12-17 22:52:46]

xlai@chromium.org changed reviewers:
+ avi@chromium.org

[xlai (Olivia), 2016-12-17 22:52:58]

The CQ bit was checked by xlai@chromium.org

[commit-bot: I haz the power, 2016-12-17 22:53:05]

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, 2016-12-17 22:56:39]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1482015178480180,
"parent_rev": "1b5caffec88ef9720952bc81047baa9f72ecc13d", "commit_rev":
"f59182b1da56247df75776a79747ab84a75b3087"}

[commit-bot: I haz the power, 2016-12-17 22:57:04]

Description was changed from

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[commit-bot: I haz the power, 2016-12-17 22:57:06]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-12-17 22:59:09]

Description was changed from

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

to

==========
Revamp OffscreenCanvas commit flow

This patch aims to simplify the existing work flow of OffscreenCanvas commit()
and help OffscreenCanvas transition to the new Display Compositor architecture.
This facilitates the next step of adding resizing functionality of
OffscreenCanvas.

Major changes:
1. OffscreenCanvas now use SurfaceIdAllocator in the renderer/worker, instead
of being in the browser. This is consistent with the new display compositor
architecture.
2. SurfaceLayer for HTMLCanvasElement is only created when Surface is created
in the browser; before that, a dummy SolidColorLayer is put as a placeholder.
3. CanvasSurfaceLayerBridge becomes the client of OffscreenCanvasSurfaceImpl,
so that it can receive IPC messages from browser.
4. FrameSinkId is now generated in renderer/main by CanvasSurfaceLayerBridge,
which remains unchanged throughout the lifecycle of canvas; LocalFrameId is
generated in renderer/worker by OffscreenCanvasFrameDispatcher. At this time it
is only generated once, but will be generated repeatedly when resizing
functionality is added in the future patches.

What is being simplified:
1. Sync IPC GetSurfaceId is removed, reducing overhead in HTMLCanvasElement.
transferControlToOffscreen() API call.
2. When OffscreenCanvas is transferred from main thread to worker thread, there
are now only 2 integers that need to be copied, which constitute the
FrameSinkId. Before this, there were 5 integers that need to be copied. As a
result, there are a lot less attributes that need to be kept by OffscreenCanvas
and CanvasSurfaceLayerBridge.

TBR=avi@chromium.org
BUG=664547, 619138, 662498
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef
Cr-Commit-Position: refs/heads/master@{#439356}
==========

[commit-bot: I haz the power, 2016-12-17 22:59:10]

Patchset 11 (id:??) landed as
https://crrev.com/05aa05390801f402c336ce2156014d7ef055e9ef
Cr-Commit-Position: refs/heads/master@{#439356}