Make CSSStyleImageValue a member of CanvasImageSource.

Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html

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

[Gleb Lanbin, 2016-09-20 21:22:52]

Patchset #1 (id:1) has been deleted

[Gleb Lanbin, 2016-09-20 21:25:13]

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

[commit-bot: I haz the power, 2016-09-20 21:25:58]

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

[Gleb Lanbin, 2016-09-20 22:13:57]

glebl@chromium.org changed reviewers:
+ ikilpatrick@chromium.org

[commit-bot: I haz the power, 2016-09-20 22:59:00]

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

[commit-bot: I haz the power, 2016-09-20 22:59:00]

Dry run: This issue passed the CQ dry run.

[ikilpatrick, 2016-09-27 18:05:59]

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

[ikilpatrick, 2016-09-27 18:06:00]

approach looks good. +junov.

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp (right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp:56:
PassRefPtr<Image> CSSStyleImageValue::getSourceImageForCanvas(SourceImageStatus*
status, AccelerationHint, SnapshotReason,
would it be better to change this method to accept a layoutobject, to that we
can just call:
StyleImage::image
?

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

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:869: if
(value.isCSSImageValue())
it might be worth making this conditional on CSSPaintAPI as well at the moment,
but see what junov thinks.

[Gleb Lanbin, 2016-09-27 19:37:18]

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp (right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp:56:
PassRefPtr<Image> CSSStyleImageValue::getSourceImageForCanvas(SourceImageStatus*
status, AccelerationHint, SnapshotReason,
On 2016/09/27 18:06:00, ikilpatrick wrote:
> would it be better to change this method to accept a layoutobject, to that we
> can just call:
> StyleImage::image
> ? 

probably, but this method is not defined here. It's defined as a pure virtual
method in CanvasImageSource.

[Justin Novosad, 2016-09-28 15:41:59]

Overall approach is good IMHO

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp (right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp:61: return
m_imageValue->cachedImage()->cachedImage()->getImage()->imageForDefaultFrame();
Confident you don't need any nullptr checks anywhere in this chain?

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/canvas/CanvasImageSource.h (right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasImageSource.h:38: class
FloatSize;
Why are these forward declarations needed?

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl
(right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:28:
typedef (CSSImageValue or
You can't just ship this. This needs to go through the launch process. Until
then you must hide this behind a flag. I am not sure whether you can use
[RuntimeEnabled=xxx] in a union typedef.  If not, you may have to overload
drawImage and createPattern for now.

[Gleb Lanbin, 2016-09-28 16:58:23]

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp (right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp:61: return
m_imageValue->cachedImage()->cachedImage()->getImage()->imageForDefaultFrame();
On 2016/09/28 15:41:58, Justin Novosad wrote:
> Confident you don't need any nullptr checks anywhere in this chain?

Done.

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/canvas/CanvasImageSource.h (right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasImageSource.h:38: class
FloatSize;
On 2016/09/28 15:41:58, Justin Novosad wrote:
> Why are these forward declarations needed?

deleted but I think we need these declarations to avoid hidden dependencies

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

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:869: if
(value.isCSSImageValue())
On 2016/09/27 18:06:00, ikilpatrick wrote:
> it might be worth making this conditional on CSSPaintAPI as well at the
moment,
> but see what junov thinks.

Done.

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl
(right):

https://codereview.chromium.org/2351363002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:28:
typedef (CSSImageValue or
On 2016/09/28 15:41:59, Justin Novosad wrote:
> You can't just ship this. This needs to go through the launch process. Until
> then you must hide this behind a flag. I am not sure whether you can use
> [RuntimeEnabled=xxx] in a union typedef.  If not, you may have to overload
> drawImage and createPattern for now.

done. modified toImageSourceInternal and made isCSSImageValue switch to be
conditional on CSSPaintAPI

[Justin Novosad, 2016-09-28 17:12:33]

lgtm for modules/canvas2d

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

https://codereview.chromium.org/2351363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:873:
return RuntimeEnabledFeatures::cssPaintAPIEnabled() ? value.getAsCSSImageValue()
: nullptr;
Good idea to do it this way.

[Gleb Lanbin, 2016-09-28 17:15:46]

glebl@chromium.org changed reviewers:
+ dpranke@google.com

[Gleb Lanbin, 2016-09-28 17:15:47]

Dirk, can I ask for a rubber stamp LGTM to change
third_party/WebKit/Source/bindings/modules/v8/generated.gni ?

[Gleb Lanbin, 2016-09-28 17:16:29]

Description was changed from

==========
Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM
Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html
==========

to

==========
Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM
Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html
==========

[Gleb Lanbin, 2016-09-28 17:16:29]

glebl@chromium.org changed reviewers:
- dpranke@google.com

[Justin Novosad, 2016-09-28 17:17:35]

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

https://codereview.chromium.org/2351363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:873:
return RuntimeEnabledFeatures::cssPaintAPIEnabled() ? value.getAsCSSImageValue()
: nullptr;
Wait a second... To be correct, a TypeError exception should be thrown if the
feature is disabled.

[Dirk Pranke, 2016-09-28 17:35:46]

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

[Dirk Pranke, 2016-09-28 17:35:46]

lgtm.

[Gleb Lanbin, 2016-09-28 17:47:00]

PTAL

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

https://codereview.chromium.org/2351363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:873:
return RuntimeEnabledFeatures::cssPaintAPIEnabled() ? value.getAsCSSImageValue()
: nullptr;
On 2016/09/28 17:17:35, Justin Novosad wrote:
> Wait a second... To be correct, a TypeError exception should be thrown if the
> feature is disabled.

Done.

[ikilpatrick, 2016-09-28 18:05:14]

lgtm

[Gleb Lanbin, 2016-09-28 18:10:27]

glebl@chromium.org changed reviewers:
+ meade@chromium.org

[Gleb Lanbin, 2016-09-28 18:10:28]

+ meade@
Eddy, could you review and LGTM changes in
third_party/WebKit/Source/core/css/cssom/*

[meade_UTC10, 2016-09-29 04:41:50]

\o/ lgtm!

[Gleb Lanbin, 2016-09-29 04:50:54]

The CQ bit was checked by glebl@chromium.org

[Gleb Lanbin, 2016-09-29 04:50:54]

The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org,
dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2351363002/#ps60001
(title: "throw a TypeError exception if cssPaintAPIEnabled != true")

[commit-bot: I haz the power, 2016-09-29 04:51:15]

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-09-29 05:03:24]

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

[commit-bot: I haz the power, 2016-09-29 05:03:25]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  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-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Gleb Lanbin, 2016-09-29 17:54:59]

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

[commit-bot: I haz the power, 2016-09-29 17:55: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-09-29 19:30:04]

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

[commit-bot: I haz the power, 2016-09-29 19:30:05]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Gleb Lanbin, 2016-09-29 22:49:56]

The CQ bit was checked by glebl@chromium.org

[Gleb Lanbin, 2016-09-29 22:49:56]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org,
ikilpatrick@chromium.org, junov@chromium.org, dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2351363002/#ps80001
(title: "Override isAccelerated")

[commit-bot: I haz the power, 2016-09-29 22:50:34]

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-09-30 00:56:13]

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

[commit-bot: I haz the power, 2016-09-30 00:56:15]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Gleb Lanbin, 2016-09-30 00:57:27]

The CQ bit was checked by glebl@chromium.org

[commit-bot: I haz the power, 2016-09-30 00:57:52]

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-09-30 01:54:11]

Description was changed from

==========
Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM
Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html
==========

to

==========
Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM
Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html
==========

[commit-bot: I haz the power, 2016-09-30 01:54:13]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-09-30 01:57:52]

Description was changed from

==========
Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM
Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html
==========

to

==========
Make CSSStyleImageValue a member of CanvasImageSource.

This will make it possible to call BaseRenderingContext2D.drawImage with CSSOM
Style Image object as an argument.

BUG=567358
TEST=third_party/WebKit/LayoutTests/csspaint/paint2d-image.html

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

[commit-bot: I haz the power, 2016-09-30 01:57:54]

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