Make OffscreenCanvas resizable

Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Committed: https://crrev.com/cfc7fa46621dc8a79511c60cb8cb5bfb3e412031
Cr-Commit-Position: refs/heads/master@{#440150}

[Fady Samuel, 2016-12-19 20:12:40]

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

[Fady Samuel, 2016-12-19 20:12:40]

https://codereview.chromium.org/2588053003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2588053003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:116:
surfaceLayer->SetSurfaceInfo(info, true);
I think you also need to update the layer bounds?
https://cs.chromium.org/chromium/src/content/renderer/child_frame_compositing...

[xlai (Olivia), 2016-12-19 22:16:07]

Description was changed from

==========
[WIP] OffscreenCanvas resize

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

to

==========
[WIP] OffscreenCanvas resize

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;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

[xlai (Olivia), 2016-12-19 22:25:03]

Description was changed from

==========
[WIP] OffscreenCanvas resize

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;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

to

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in 
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
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;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

[xlai (Olivia), 2016-12-19 22:25:03]

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

[xlai (Olivia), 2016-12-19 22:30:14]

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

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

junov@: PTAL at the code.
kbr@: PTAL at the gpu pixel test.

fsamuel@: FYI for general interest.

Thanks!

https://codereview.chromium.org/2588053003/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp
(right):

https://codereview.chromium.org/2588053003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:116:
surfaceLayer->SetSurfaceInfo(info, true);
On 2016/12/19 20:12:40, Fady Samuel wrote:
> I think you also need to update the layer bounds?
>
https://cs.chromium.org/chromium/src/content/renderer/child_frame_compositing...

Although there is no visible effect seen at the moment, I agree that 
it is more correct to update layer bounds.

[Ken Russell (switch to Gerrit), 2016-12-20 06:01:24]

The pixel tests basically lgtm. A few comments.

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
File content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html (right):

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html:1: <!DOCTYPE HTML>
Is it worth naming the new test something like "...resize_on_worker"?

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html:5: associated test in
content/test/gpu/page_sets/pixel_tests.py. This will ensure
Could you update the comment to point to
content/test/gpu/gpu_tests/pixel_test_pages.py please?

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html:62: worker.onmessage
= function (e) {
The fact that the onmessage handler is set after calling postMessage looks racy
-- is it? If so, could you swap the order?

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
File content/test/data/gpu/pixel_offscreenCanvas_webgl_resize.html (right):

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_webgl_resize.html:1: <!DOCTYPE HTML>
Similar comment: ...resize_on_worker?

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_webgl_resize.html:5: associated test
in content/test/gpu/page_sets/pixel_tests.py. This will ensure
Same fix here.

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_webgl_resize.html:62:
worker.onmessage = function (e) {
Same race question here.

https://codereview.chromium.org/2588053003/diff/20001/content/test/gpu/gpu_te...
File content/test/gpu/gpu_tests/pixel_test_pages.py (right):

https://codereview.chromium.org/2588053003/diff/20001/content/test/gpu/gpu_te...
content/test/gpu/gpu_tests/pixel_test_pages.py:244: base_name +
'_OffscreenCanvas2DResize',
Same naming question: perhaps ResizeOnWorker?

[Justin Novosad, 2016-12-20 16:44:54]

lgtm.

Deferring to Fady for review of compositor interactions.

[xlai (Olivia), 2016-12-20 18:13:39]

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
File content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html (right):

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html:1: <!DOCTYPE HTML>
On 2016/12/20 06:01:24, Ken Russell wrote:
> Is it worth naming the new test something like "...resize_on_worker"?

Done.

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html:5: associated test in
content/test/gpu/page_sets/pixel_tests.py. This will ensure
On 2016/12/20 06:01:24, Ken Russell wrote:
> Could you update the comment to point to
> content/test/gpu/gpu_tests/pixel_test_pages.py please?

Done.

https://codereview.chromium.org/2588053003/diff/20001/content/test/data/gpu/p...
content/test/data/gpu/pixel_offscreenCanvas_2d_resize.html:62: worker.onmessage
= function (e) {
On 2016/12/20 06:01:24, Ken Russell wrote:
> The fact that the onmessage handler is set after calling postMessage looks
racy
> -- is it? If so, could you swap the order?

Done. I will follow up with another CL that edits the other existing tests too.

https://codereview.chromium.org/2588053003/diff/20001/content/test/gpu/gpu_te...
File content/test/gpu/gpu_tests/pixel_test_pages.py (right):

https://codereview.chromium.org/2588053003/diff/20001/content/test/gpu/gpu_te...
content/test/gpu/gpu_tests/pixel_test_pages.py:244: base_name +
'_OffscreenCanvas2DResize',
On 2016/12/20 06:01:24, Ken Russell wrote:
> Same naming question: perhaps ResizeOnWorker?

Done.

[Fady Samuel, 2016-12-20 18:45:13]

lgtm

[Justin Novosad, 2016-12-20 22:06:09]

Tests look great.
lgtm with clean-up of width/height args that are no longer used.

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

https://codereview.chromium.org/2588053003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:1411: void
HTMLCanvasElement::OnWebLayerReplaced(int width, int height) {
Revert

https://codereview.chromium.org/2588053003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.h (right):

https://codereview.chromium.org/2588053003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.h:193: void
OnWebLayerReplaced(int width, int height) override;
Revert

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

https://codereview.chromium.org/2588053003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:121:
m_observer->OnWebLayerReplaced(width, height);
remove width+height args

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

https://codereview.chromium.org/2588053003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.h:30:
virtual void OnWebLayerReplaced(int width, int height) = 0;
Revert

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

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

[commit-bot: I haz the power, 2016-12-20 22:21:46]

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-21 00:34:47]

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

[commit-bot: I haz the power, 2016-12-21 00:34:48]

Dry run: Try jobs failed on following builders:
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[xlai (Olivia), 2016-12-21 01:05:01]

Description was changed from

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in 
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
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;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

to

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in 
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

[xlai (Olivia), 2016-12-21 01:05:22]

The CQ bit was checked by xlai@chromium.org

[xlai (Olivia), 2016-12-21 01:05:23]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
fsamuel@chromium.org, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/2588053003/#ps100001
(title: "remove redundant width and height")

[commit-bot: I haz the power, 2016-12-21 01:05:58]

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-21 01:12:44]

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

[commit-bot: I haz the power, 2016-12-21 01:12:47]

Failed to apply patch for content/test/gpu/gpu_tests/pixel_expectations.py:
While running git apply --index -p1;
  error: patch failed: content/test/gpu/gpu_tests/pixel_expectations.py:24
  error: content/test/gpu/gpu_tests/pixel_expectations.py: patch does not apply

Patch:       content/test/gpu/gpu_tests/pixel_expectations.py
Index: content/test/gpu/gpu_tests/pixel_expectations.py
diff --git a/content/test/gpu/gpu_tests/pixel_expectations.py
b/content/test/gpu/gpu_tests/pixel_expectations.py
index
d61c3680a71006215a134857dbccf1c11c3c1925..75e021bace9a0cdd6184c90d654ec738fe1a1e4c
100644
--- a/content/test/gpu/gpu_tests/pixel_expectations.py
+++ b/content/test/gpu/gpu_tests/pixel_expectations.py
@@ -24,6 +24,10 @@ class PixelExpectations(GpuTestExpectations):
     self.Fail('Pixel_ScissorTestWithPreserveDrawingBuffer',
         ['android'], bug=521588)
 
+    # TODO(xlai): Check / generate reference images.
+    self.Fail('Pixel_OffscreenCanvas2DResizeOnWorker', bug=662498)
+    self.Fail('Pixel_OffscreenCanvasWebglResizeOnWorker', bug=662498)
+
     # TODO(ccameron) fix these on Mac Retina
     # self.Fail('Pixel_CSS3DBlueBox', ['mac'], bug=533690)

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

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

[commit-bot: I haz the power, 2016-12-21 16:06:31]

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-21 17:58:46]

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

[commit-bot: I haz the power, 2016-12-21 17:58:47]

Dry run: This issue passed the CQ dry run.

[xlai (Olivia), 2016-12-21 18:03:05]

The CQ bit was checked by xlai@chromium.org

[xlai (Olivia), 2016-12-21 18:03:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, junov@chromium.org, kbr@chromium.org
Link to the patchset: https://codereview.chromium.org/2588053003/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-21 18:03:38]

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-21 18:08:40]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1482343385593980,
"parent_rev": "861a3e87b7361424137f15684c7dfb1571fb09d7", "commit_rev":
"8647014cce12648e556ae6765e950a8cd0c93ca2"}

[commit-bot: I haz the power, 2016-12-21 18:09:11]

Description was changed from

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in 
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
==========

to

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in 
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

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

[commit-bot: I haz the power, 2016-12-21 18:09:13]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-12-21 18:10:50]

Description was changed from

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in 
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

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

to

==========
Make OffscreenCanvas resizable

When OffscreenCanvas's width or height is changed, we reshape the
OffscreenCanvasFrameDispatcher's size and mark
m_changeSizeForNextCommit as true. In the next incoming commit(),
OffscreenCanvasFrameDispatcher picks up the new size and generate
new LocalFrameId. On browser side, Surface will be re-created once
a different LocalFrameId is received (all is handled in
CompositorFrameSinkSupport), prompting OffscreenCanvasSurfaceImpl
to send OnSurfaceCreated message back to CanvasSurfaceLayerBridge,
which will then change the SurfaceInfo in the existing SurfaceLayer.

BUG=662498
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Committed: https://crrev.com/cfc7fa46621dc8a79511c60cb8cb5bfb3e412031
Cr-Commit-Position: refs/heads/master@{#440150}
==========

[commit-bot: I haz the power, 2016-12-21 18:10:51]

Patchset 7 (id:??) landed as
https://crrev.com/cfc7fa46621dc8a79511c60cb8cb5bfb3e412031
Cr-Commit-Position: refs/heads/master@{#440150}