Add createGradient and createPattern to OffscreenCanvas2D in worker

Add createGradient and createPattern to OffscreenCanvas2D in worker

Implemented more color-related API functions for OffscreenCRC2D.
But createPattern() on main thread has not been completed due to
crbug.com/563870.

BUG=563856
Committed: https://crrev.com/5433e667fc2a2a1e62e969a8d7539081c4dde9c3
Cr-Commit-Position: refs/heads/master@{#390188}

[xlai (Olivia), 2016-04-26 19:31:33]

Patchset #1 (id:1) has been deleted

[xlai (Olivia), 2016-04-26 19:36:50]

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

[xlai (Olivia), 2016-04-26 19:36:50]

Maybe we should rename Issue 563856 to "Make CanvasRenderingContext2D renderable
in a worker and in a main thread" or create one more issue "...in a main
thread". Implementations for these two cases start to diverge for createPattern
function.

[Justin Novosad, 2016-04-26 19:50:07]

https://codereview.chromium.org/1919363002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1919363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp:54:
// TODO(xlai): implement wouldTaintOrigin for OffscreenCanvas2d on main thread
This comment should refer to a crbug

[xlai (Olivia), 2016-04-26 21:45:24]

Rebased with master, fixed layout tests, edited comments based on junov's
feedback.

https://codereview.chromium.org/1919363002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/CanvasPattern.idl (right):

https://codereview.chromium.org/1919363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasPattern.idl:29:
[RuntimeEnabled=ExperimentalCanvasFeatures,Exposed=Window] void
setTransform(SVGMatrix transform);
because SVGMatrix exists only in main thread, there's no point exposing this
function in worker.

[haraken, 2016-04-27 06:30:51]

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

[haraken, 2016-04-27 06:30:51]

https://codereview.chromium.org/1919363002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1919363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp:46:
if
(ScriptState::current(v8::Isolate::GetCurrent())->getExecutionContext()->isWorkerGlobalScope())
{

ScriptState::current should not be called outside binding layers (unless you
understand what you're doing).

Alternately, you should add [CallWith=ScriptState] to createPattern and pass the
ScriptState down to this method.

[xlai (Olivia), 2016-04-27 19:19:31]

Edited based on haraken@'s feedback

[haraken, 2016-04-27 19:26:28]

On 2016/04/27 19:19:31, xlai (Olivia) wrote:
> Edited based on haraken@'s feedback

LGTM in terms of implementation.

[Justin Novosad, 2016-04-27 19:32:25]

https://codereview.chromium.org/1919363002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1919363002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1055: if
(originClean() && wouldTaintOrigin(imageSource, nullptr))
why no scriptState here? I realize you have not yet exposed drawImage in OCRC2D,
but OCRC2D::wouldTaintOrigin expects scriptState to never be null (no null
check).

[xlai (Olivia), 2016-04-27 19:47:49]

https://codereview.chromium.org/1919363002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1919363002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1055: if
(originClean() && wouldTaintOrigin(imageSource, nullptr))
On 2016/04/27 19:32:25, Justin Novosad wrote:
> why no scriptState here? I realize you have not yet exposed drawImage in
OCRC2D,
> but OCRC2D::wouldTaintOrigin expects scriptState to never be null (no null
> check).

The call here does not invoke OCRC2D::wouldTaintOrigin, because drawImage is not
exposed in OCRC2D yet. It will only invoke either CRC2D::wouldTaintOrigin or
PaintRC2D::wouldTaintOrigin, neither of them require ScriptState*.

Once OCRC2D::wouldTaintOrigin() is completed and drawImage is exposed to OCRC2D,
this line of code will be changed as well.

[Justin Novosad, 2016-04-27 19:49:55]

On 2016/04/27 19:47:49, xlai (Olivia) wrote:
>
https://codereview.chromium.org/1919363002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
> (right):
> 
>
https://codereview.chromium.org/1919363002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1055: if
> (originClean() && wouldTaintOrigin(imageSource, nullptr))
> On 2016/04/27 19:32:25, Justin Novosad wrote:
> > why no scriptState here? I realize you have not yet exposed drawImage in
> OCRC2D,
> > but OCRC2D::wouldTaintOrigin expects scriptState to never be null (no null
> > check).
> 
> The call here does not invoke OCRC2D::wouldTaintOrigin, because drawImage is
not
> exposed in OCRC2D yet. It will only invoke either CRC2D::wouldTaintOrigin or
> PaintRC2D::wouldTaintOrigin, neither of them require ScriptState*.
> 
> Once OCRC2D::wouldTaintOrigin() is completed and drawImage is exposed to
OCRC2D,
> this line of code will be changed as well.

Fair enough lgtm

[xlai (Olivia), 2016-04-27 19:56:27]

The CQ bit was checked by xlai@chromium.org

[commit-bot: I haz the power, 2016-04-27 19:56:44]

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

[commit-bot: I haz the power, 2016-04-27 21:15:43]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-04-30 17:13:13]

Patchset 3 (id:??) landed as
https://crrev.com/5433e667fc2a2a1e62e969a8d7539081c4dde9c3
Cr-Commit-Position: refs/heads/master@{#390188}

[commit-bot: I haz the power, 2016-04-30 17:51:58]

Description was changed from

==========
Add createGradient and createPattern to OffscreenCanvas2D in worker

Implemented more color-related API functions for OffscreenCRC2D.
But createPattern() on main thread has not been completed due to
crbug.com/563870.

BUG=563856
==========

to

==========
Add createGradient and createPattern to OffscreenCanvas2D in worker

Implemented more color-related API functions for OffscreenCRC2D.
But createPattern() on main thread has not been completed due to
crbug.com/563870.

BUG=563856

Committed: https://crrev.com/5433e667fc2a2a1e62e969a8d7539081c4dde9c3
Cr-Commit-Position: refs/heads/master@{#390188}
==========