Make OffscreenCanvas resizeable

Make OffscreenCanvas resizeable

This change implements all the script observable aspects of changing
the size of an OffscreenCanvas object.  What is missing after this
change is to correctly handle the compositing layer updates for the
commit() flow.

BUG=662498
Committed: https://crrev.com/09cffb4f307ead2a20776a6e6b0d9cc1ca3e5ff7
Cr-Commit-Position: refs/heads/master@{#432881}

[Justin Novosad, 2016-11-16 22:52:11]

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

[Justin Novosad, 2016-11-16 22:52:45]

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

[Justin Novosad, 2016-11-16 22:52:45]

PTAL

[commit-bot: I haz the power, 2016-11-16 22:53:27]

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

[Justin Novosad, 2016-11-16 22:57:59]

Description was changed from

==========
Make OffscreenCanvas resizeable

This change implements all the script observable aspect of changing the
size of an OffscreenCanvas object.  What is missing is to correctly
handle the compositing layer updates for the commit() flow.

BUG=662498
==========

to

==========
Make OffscreenCanvas resizeable

This change implements all the script observable aspects of changing
the size of an OffscreenCanvas object.  What is missing after this
change is to correctly handle the compositing layer updates for the
commit() flow.

BUG=662498
==========

[Justin Novosad, 2016-11-16 22:58:18]

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:111: void
BaseRenderingContext2D::unwindStateStack() {
code moved from CanvasRenderingContext2D.cpp

[commit-bot: I haz the power, 2016-11-17 00:58:57]

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

[commit-bot: I haz the power, 2016-11-17 00:58:58]

Dry run: This issue passed the CQ dry run.

[xidachen, 2016-11-17 02:12:23]

lgtm

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-resize.html
(right):

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-resize.html:10:
canvas.height = 40;
I am guessing that when you make the pull request for resize, you will make the
width && height mutable?

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp (right):

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp:44: void
OffscreenCanvas::setSize(const IntSize& size) {
OK, I am a little confused here. The logic here sounds perfectly correct.
However, when I look at HTMLCanvasElement::setSize(), the function is not
getting called by setWidth() or setHeight(). That means setWidth() and
setHeight() in HTMLCanvasElement doesn't really do anything to its context, is
my understanding correct?

[Justin Novosad, 2016-11-17 15:30:12]

On 2016/11/17 02:12:23, xidachen wrote:
> lgtm
> 
>
https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/LayoutTe...
> File third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-resize.html
> (right):
> 
>
https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/LayoutTe...
> third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-resize.html:10:
> canvas.height = 40;
> I am guessing that when you make the pull request for resize, you will make
the
> width && height mutable?

That, and more...
> 
>
https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
(right):
> 
>
https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp:44: void
> OffscreenCanvas::setSize(const IntSize& size) {
> OK, I am a little confused here. The logic here sounds perfectly correct.
> However, when I look at HTMLCanvasElement::setSize(), the function is not
> getting called by setWidth() or setHeight(). That means setWidth() and
> setHeight() in HTMLCanvasElement doesn't really do anything to its context, is
> my understanding correct?

HTMLCAnvasElement::setSize is only called in unit tests.
The way HTMLCanvasElement does the update is in
HTMLCanvasElement::parseAttribute.  This is a subtle difference that was
necessary for strict compliance with how width/height are spec'ed on <canvas>.

[Justin Novosad, 2016-11-17 15:30:18]

The CQ bit was checked by junov@chromium.org

[commit-bot: I haz the power, 2016-11-17 15:30:43]

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-11-17 15:35:00]

Description was changed from

==========
Make OffscreenCanvas resizeable

This change implements all the script observable aspects of changing
the size of an OffscreenCanvas object.  What is missing after this
change is to correctly handle the compositing layer updates for the
commit() flow.

BUG=662498
==========

to

==========
Make OffscreenCanvas resizeable

This change implements all the script observable aspects of changing
the size of an OffscreenCanvas object.  What is missing after this
change is to correctly handle the compositing layer updates for the
commit() flow.

BUG=662498
==========

[commit-bot: I haz the power, 2016-11-17 15:35:01]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-17 15:37:46]

Description was changed from

==========
Make OffscreenCanvas resizeable

This change implements all the script observable aspects of changing
the size of an OffscreenCanvas object.  What is missing after this
change is to correctly handle the compositing layer updates for the
commit() flow.

BUG=662498
==========

to

==========
Make OffscreenCanvas resizeable

This change implements all the script observable aspects of changing
the size of an OffscreenCanvas object.  What is missing after this
change is to correctly handle the compositing layer updates for the
commit() flow.

BUG=662498

Committed: https://crrev.com/09cffb4f307ead2a20776a6e6b0d9cc1ca3e5ff7
Cr-Commit-Position: refs/heads/master@{#432881}
==========

[commit-bot: I haz the power, 2016-11-17 15:37:47]

Patchset 1 (id:??) landed as
https://crrev.com/09cffb4f307ead2a20776a6e6b0d9cc1ca3e5ff7
Cr-Commit-Position: refs/heads/master@{#432881}

[xlai (Olivia), 2016-11-21 16:26:58]

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
(right):

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp:104:
m_imageBuffer = nullptr;
This function's implementation doesn't look correct.

Once offscreenCanvas'size is changed, the existing image buffer of its 2d
context is set to nullpointer, means everything that has drawn to 2d context so
far is lost.

[xlai (Olivia), 2016-11-21 19:56:17]

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
(right):

https://codereview.chromium.org/2508943003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp:104:
m_imageBuffer = nullptr;
On 2016/11/21 16:26:57, xlai (Olivia) wrote:
> This function's implementation doesn't look correct.
> 
> Once offscreenCanvas'size is changed, the existing image buffer of its 2d
> context is set to nullpointer, means everything that has drawn to 2d context
so
> far is lost. 

Ah I just realize that the image buffer (with a new size) is always recreated in
every call to transferToStaticBitmapImage() if it is nullptr.