Adds SVGImageElement as a CanvasImageSource

Adds SVGImageElement to CanvasImageSource

Relevant spec: https://html.spec.whatwg.org/multipage/scripting.html#image-sources-for-2d-rendering-contexts:htmlorsvgimageelement

SVGImageElement should be part of CanvasImageSource union typedef, i.e., be allowed as an source for drawImage(). This CL does that, while creating a common interface for both HTMLImageElement and SVGImageElement.


BUG=695662
Review-Url: https://codereview.chromium.org/2723093004
Cr-Commit-Position: refs/heads/master@{#459289}
Committed: https://chromium.googlesource.com/chromium/src/+/11a275c5522ae4dbf8e3dee5c7246e74141b5142

[fserb, 2017-03-01 19:20:55]

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

[fserb, 2017-03-01 20:44:09]

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

[commit-bot: I haz the power, 2017-03-01 20:44:41]

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

[Justin Novosad, 2017-03-01 20:56:30]

Code Looks good modulo tests.

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:212: if (!cachedImage())
{
What happened to that extra check I saw you put in your local build?

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:230: return
!cachedImage()->isAccessAllowed(destinationSecurityOrigin);
This needs to be tested in a layout test.  Example:
LayoutTests/http/security/canvas-remote-read-remote-image.html

You basically need a version of that test that uses an SVGImageElement.  Also,
new tests should use testharness.js.

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

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:37:
SVGImageElement or
I'm not sure there is a way to put a union member behind a flag, so let's just
wait for lgtm to ship from API owners.

[fs, 2017-03-01 21:20:40]

fs@opera.com changed reviewers:
+ fs@opera.com

[fs, 2017-03-01 21:20:42]

Drive-by

https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:207: PassRefPtr<Image>
SVGImageElement::getSourceImageForCanvas(
This (and other method implementations) appear to be missing the intrinsic
sizing handling from HTMLImageElement. AFAIK the spec pretty much calls out that
SVGImageElement and HTMLImageElement are treated identically so there would seem
to be a decent chance of using the same code for these (something keyed off the
ImageLoader would probably make sense?)

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

https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:31: //
but there's a problem with our IDL code generator for typedef-in-typedef,
Presumably this is also why the ImageBitmapSource typedef has the "flattened"
union... Should SVGImageElement be included there as well? (I don't see why
not.) Not mentioned in the intent (as either "in" or "out".)

[Justin Novosad, 2017-03-01 23:03:20]

On 2017/03/01 21:20:42, fs wrote:

>
https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:31: //
> but there's a problem with our IDL code generator for typedef-in-typedef,
> Presumably this is also why the ImageBitmapSource typedef has the "flattened"
> union... Should SVGImageElement be included there as well? (I don't see why
> not.) Not mentioned in the intent (as either "in" or "out".)

Good catch. The spec defines ImageBitmapSource as:
typedef (CanvasImageSource or
         Blob or
         ImageData) ImageBitmapSource;

So there is a cascading effect here. Let's clarify the intent thread to include
this, and take care of it in a separate CL.

[Justin Novosad, 2017-03-01 23:05:10]

Oops, previous l-g-t-m was accidental. So to be clear: not lgtm (for now)

[commit-bot: I haz the power, 2017-03-01 23:05:34]

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

[commit-bot: I haz the power, 2017-03-01 23:05:35]

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

[fserb, 2017-03-06 21:05:13]

Refactored CanvasHTMLSVGImage out. PTAL.

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:212: if (!cachedImage())
{
On 2017/03/01 at 20:56:29, Justin Novosad wrote:
> What happened to that extra check I saw you put in your local build?

it's there. :)

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:230: return
!cachedImage()->isAccessAllowed(destinationSecurityOrigin);
On 2017/03/01 at 20:56:30, Justin Novosad wrote:
> This needs to be tested in a layout test.  Example:
LayoutTests/http/security/canvas-remote-read-remote-image.html
> 
> You basically need a version of that test that uses an SVGImageElement.  Also,
new tests should use testharness.js.

done.

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

https://codereview.chromium.org/2723093004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:37:
SVGImageElement or
On 2017/03/01 at 20:56:30, Justin Novosad wrote:
> I'm not sure there is a way to put a union member behind a flag, so let's just
wait for lgtm to ship from API owners.

k

https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:207: PassRefPtr<Image>
SVGImageElement::getSourceImageForCanvas(
On 2017/03/01 at 21:20:41, fs wrote:
> This (and other method implementations) appear to be missing the intrinsic
sizing handling from HTMLImageElement. AFAIK the spec pretty much calls out that
SVGImageElement and HTMLImageElement are treated identically so there would seem
to be a decent chance of using the same code for these (something keyed off the
ImageLoader would probably make sense?)

Done.

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

https://codereview.chromium.org/2723093004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:31: //
but there's a problem with our IDL code generator for typedef-in-typedef,
On 2017/03/01 at 21:20:41, fs wrote:
> Presumably this is also why the ImageBitmapSource typedef has the "flattened"
union... Should SVGImageElement be included there as well? (I don't see why
not.) Not mentioned in the intent (as either "in" or "out".)

I'll check on ImageBitmapSource on a later CL

[fserb, 2017-03-06 21:05:20]

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

[commit-bot: I haz the power, 2017-03-06 21:06:11]

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

[Justin Novosad, 2017-03-06 21:37:32]

Could you also add tests where the image resource of the svg <image> is an svg
file?

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html
(right):

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html:8:
stTest.step(function() {
The code below does not need to be inside a 'step' call. Only the event listener
needs it.

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasImageSource.h:33: #include
"platform/weborigin/KURL.h"
Why do we need this all of a sudden?

[commit-bot: I haz the power, 2017-03-06 22:24:47]

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

[commit-bot: I haz the power, 2017-03-06 22:24:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fs, 2017-03-06 22:46:16]

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html
(right):

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html:8:
stTest.step(function() {
On 2017/03/06 at 21:37:32, Justin Novosad wrote:
> The code below does not need to be inside a 'step' call. Only the event
listener needs it.

I believe there's a positive side to using a step though - if something here
throws the test will fail (rather than time out.) IIRC.

This could be written as:

async_test(function(t) {
  ...stuff...
  image.addEventListener('load', t.step_func_done(...));
  ...more stuff...
}, 'Description...');

for slightly less harness mongering.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html:20:
image.addEventListener('load', stTest.step_func(function() {
Could use step_func_done (and remove the explicit .done())

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:2: * Copyright
(C) 2017 Google Inc. All rights reserved.
Same comment as in the header.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:106: return
false;
Isn't this what the base-class has?

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:115: FloatSize
defaultObjectSize;  // (width(), height()) ??
Yes, looks like this needs to move down to *ImageElement. Maybe make a IntSize
sourceSize(const FloatSize& defaultObjectSize) helper in this class though?

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:3: * Copyright
(C) 2017 Google Inc. All rights reserved.
Use short copyright.
(https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...)

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:24: #include
"bindings/core/v8/ScriptWrappable.h"
What's ScriptWrappable here

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:28: #include
"platform/heap/Visitor.h"
Not used? (In general it seems very little besides CanvasImageSource.h would
need to be included here, so maybe double-check and clean up as needed here.)

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:32: class
CORE_EXPORT CanvasHTMLSVGImage : public CanvasImageSource {
The ...HTMLSVGImage suffix feels a bit weird. Maybe CanvasImageElementSource or
something instead?

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:34: virtual
ImageResourceContent* cachedImage() const = 0;
It's a bit unfortunate that this ended up as virtual, seeing as though it's
called pretty often. If this remains, try to make it so it's not called too much
per method. (HTML/SVGImageElement are both final, so hopefully it's
devirtualized there.)
That being said, I still think using the ImageLoader would be better - it
already has a Element reference, so the element() method here wouldn't be
needed, and imageLoaded() can be trivially implemented from that as well. The
only problem seems to be that both HTMLImageElement and SVGImageElement have
imageLoader() methods that return their derived ImageLoader types, so a
different name might be needed to avoid clashing.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:61: private:
Drop?

[fserb, 2017-03-07 20:44:03]

Thanks for the comments, it looks much better now.
Added a few svg file tests.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html
(right):

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html:8:
stTest.step(function() {
On 2017/03/06 at 22:46:15, fs wrote:
> On 2017/03/06 at 21:37:32, Justin Novosad wrote:
> > The code below does not need to be inside a 'step' call. Only the event
listener needs it.
> 
> I believe there's a positive side to using a step though - if something here
throws the test will fail (rather than time out.) IIRC.
> 
> This could be written as:
> 
> async_test(function(t) {
>   ...stuff...
>   image.addEventListener('load', t.step_func_done(...));
>   ...more stuff...
> }, 'Description...');
> 
> for slightly less harness mongering.

done.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/canvas-remote-read-remote-svg-image.html:20:
image.addEventListener('load', stTest.step_func(function() {
On 2017/03/06 at 22:46:15, fs wrote:
> Could use step_func_done (and remove the explicit .done())

done.

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:2: * Copyright
(C) 2017 Google Inc. All rights reserved.
On 2017/03/06 at 22:46:15, fs wrote:
> Same comment as in the header.

done. sorry for that.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:106: return
false;
On 2017/03/06 at 22:46:15, fs wrote:
> Isn't this what the base-class has?

pure virtual on base class, do you want me to change that?

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:115: FloatSize
defaultObjectSize;  // (width(), height()) ??
On 2017/03/06 at 22:46:15, fs wrote:
> Yes, looks like this needs to move down to *ImageElement. Maybe make a IntSize
sourceSize(const FloatSize& defaultObjectSize) helper in this class though?

did something like that. sourceDefaultSize() and left the rest as is.

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:3: * Copyright
(C) 2017 Google Inc. All rights reserved.
On 2017/03/06 at 22:46:15, fs wrote:
> Use short copyright.
(https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...)

done.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:24: #include
"bindings/core/v8/ScriptWrappable.h"
On 2017/03/06 at 22:46:15, fs wrote:
> What's ScriptWrappable here

sry, my setup used to lint about those and it doesn't for Blink. :/

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:28: #include
"platform/heap/Visitor.h"
On 2017/03/06 at 22:46:15, fs wrote:
> Not used? (In general it seems very little besides CanvasImageSource.h would
need to be included here, so maybe double-check and clean up as needed here.)

done.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:32: class
CORE_EXPORT CanvasHTMLSVGImage : public CanvasImageSource {
On 2017/03/06 at 22:46:15, fs wrote:
> The ...HTMLSVGImage suffix feels a bit weird. Maybe CanvasImageElementSource
or something instead?

done.

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:34: virtual
ImageResourceContent* cachedImage() const = 0;
On 2017/03/06 at 22:46:15, fs wrote:
> It's a bit unfortunate that this ended up as virtual, seeing as though it's
called pretty often. If this remains, try to make it so it's not called too much
per method. (HTML/SVGImageElement are both final, so hopefully it's
devirtualized there.)
> That being said, I still think using the ImageLoader would be better - it
already has a Element reference, so the element() method here wouldn't be
needed, and imageLoaded() can be trivially implemented from that as well. The
only problem seems to be that both HTMLImageElement and SVGImageElement have
imageLoader() methods that return their derived ImageLoader types, so a
different name might be needed to avoid clashing.

The ImageLoader is a great idea. I moved that to be the only virtual and moved
the other helper functions private on the class.
It's still virtual, but as you said, they are both final, so maybe it's ok to
leave like that?

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:61: private:
On 2017/03/06 at 22:46:15, fs wrote:
> Drop?

used now.

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasImageSource.h:33: #include
"platform/weborigin/KURL.h"
On 2017/03/06 at 21:37:32, Justin Novosad wrote:
> Why do we need this all of a sudden?

it wasn't failing before, because every time you include this you included
something else that had KURL. It broke when I added as the only header on my
code. There's an undefined "KURL&" somewhere down there.

[fserb, 2017-03-07 20:45:58]

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

[commit-bot: I haz the power, 2017-03-07 20:47:08]

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

[fs, 2017-03-07 22:01:25]

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.cpp:106: return
false;
On 2017/03/07 at 20:44:02, fserb wrote:
> On 2017/03/06 at 22:46:15, fs wrote:
> > Isn't this what the base-class has?
> 
> pure virtual on base class, do you want me to change that?

Sorry, maybe I quick-scanned too quickly and got it mixed up with the impl for
isOpaque. This is fine.

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

https://codereview.chromium.org/2723093004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/canvas/CanvasHTMLSVGImage.h:34: virtual
ImageResourceContent* cachedImage() const = 0;
On 2017/03/07 at 20:44:02, fserb wrote:
> On 2017/03/06 at 22:46:15, fs wrote:
> > It's a bit unfortunate that this ended up as virtual, seeing as though it's
called pretty often. If this remains, try to make it so it's not called too much
per method. (HTML/SVGImageElement are both final, so hopefully it's
devirtualized there.)
> > That being said, I still think using the ImageLoader would be better - it
already has a Element reference, so the element() method here wouldn't be
needed, and imageLoaded() can be trivially implemented from that as well. The
only problem seems to be that both HTMLImageElement and SVGImageElement have
imageLoader() methods that return their derived ImageLoader types, so a
different name might be needed to avoid clashing.
> 
> The ImageLoader is a great idea. I moved that to be the only virtual and moved
the other helper functions private on the class.
> It's still virtual, but as you said, they are both final, so maybe it's ok to
leave like that?

Yeah, let's hope so. (Feel free to verify that it indeed is.)

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/fast/canvas/resources/circle.svg (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/canvas/resources/circle.svg:1: <?xml
version="1.0"?>
You can drop the XML decl (this line) and the doctype (which is useless, even if
seeing a SVG 1.0 doctype is fun! =P)

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLImageElement.h (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLImageElement.h:200: HTMLImageLoader&
imageLoader() const override { return *m_imageLoader; }
(This is a bit scary-looking, hopefully it's future-safe enough.)

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.cpp
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.cpp:7:
#include "core/html/HTMLImageLoader.h"
Nit: using core/loader/ImageLoader.h would suffice, or?

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.h
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.h:20:
virtual FloatSize sourceDefaultSize() = 0;
Should probably be sourceDefaultObjectSize for extra clarity.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.h:47: const
Element* element() const;
Nit: Could be an Element& even

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:199: return FloatSize(0,
0);
To be somewhat equivalent with the HTMLImageElement version, I think this would
be something like:

if (layoutObject())
  return toLayoutSVGImage(layoutObject())->objectBoundingBox().size();
SVGLengthContext lengthContext(this);
return FloatSize(m_width->currentValue()->value(lengthContext),
                 m_height->currentValue()->value(lengthContext));

I wonder if these width/heights shouldn't be the <canvas> width/height - reading
step 3 of https://html.spec.whatwg.org/multipage/scripting.html#drawing-images

[commit-bot: I haz the power, 2017-03-07 23:20:42]

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

[commit-bot: I haz the power, 2017-03-07 23:20:43]

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

[fserb, 2017-03-08 18:31:59]

fs, just a comment. I'll get to the rest later.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:199: return FloatSize(0,
0);
On 2017/03/07 at 22:01:25, fs wrote:
> To be somewhat equivalent with the HTMLImageElement version, I think this
would be something like:
> 
> if (layoutObject())
>   return toLayoutSVGImage(layoutObject())->objectBoundingBox().size();
> SVGLengthContext lengthContext(this);
> return FloatSize(m_width->currentValue()->value(lengthContext),
>                  m_height->currentValue()->value(lengthContext));
> 
> I wonder if these width/heights shouldn't be the <canvas> width/height -
reading step 3 of
https://html.spec.whatwg.org/multipage/scripting.html#drawing-images

Thanks for the code snippet, I'll implement it like this.

Regarding the spec thing, those functions are messed up
(sourceWidth/sourceHeight). They are not called by the CanvasImageSource code
path at all (they are used for BitmapImageSource). On the canvas side, we always
get called through getSourceImageForCanvas() which receives a
"defaultObjectSize" (which is the canvas size) -- I've added tests to make sure
that step 3 works (it's the 6th id on the test, iirc). Since those are not used
for the actual canvas rendering, I interpreted them as "the inherit dimension of
the image" and therefore I think your code makes sense (but I'll go over it
again for Bitmap source, maybe even remove it completely). 

Does this make sense to you?

[fs, 2017-03-08 18:57:16]

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:199: return FloatSize(0,
0);
On 2017/03/08 at 18:31:59, fserb wrote:
> On 2017/03/07 at 22:01:25, fs wrote:
> > To be somewhat equivalent with the HTMLImageElement version, I think this
would be something like:
> > 
> > if (layoutObject())
> >   return toLayoutSVGImage(layoutObject())->objectBoundingBox().size();
> > SVGLengthContext lengthContext(this);
> > return FloatSize(m_width->currentValue()->value(lengthContext),
> >                  m_height->currentValue()->value(lengthContext));
> > 
> > I wonder if these width/heights shouldn't be the <canvas> width/height -
reading step 3 of
https://html.spec.whatwg.org/multipage/scripting.html#drawing-images
> 
> Thanks for the code snippet, I'll implement it like this.
> 
> Regarding the spec thing, those functions are messed up
(sourceWidth/sourceHeight). They are not called by the CanvasImageSource code
path at all (they are used for BitmapImageSource).

Ok (ImageBitmapSource though, right?). Codesearch seems to only show them being
called in modules/shapedetection though. Did it miss something?

> On the canvas side, we always get called through getSourceImageForCanvas()
which receives a "defaultObjectSize" (which is the canvas size) -- I've added
tests to make sure that step 3 works (it's the 6th id on the test, iirc). Since
those are not used for the actual canvas rendering, I interpreted them as "the
inherit dimension of the image" and therefore I think your code makes sense (but
I'll go over it again for Bitmap source, maybe even remove it completely). 

If they are indeed only used by the shapedetection code, it makes perfect sense
to revisit later. So, if you want to go with the above, that would be fine with
me. I suspect that these are buggy/broken though, and should probably be
replaced/removed.

[fs, 2017-03-13 13:56:34]

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:31: //
but there's a problem with our IDL code generator for typedef-in-typedef,
I saw crbug.com/700225, and that some fixes had landed there. Don't know if the
issue here was with nested unions of actually typedefs, anyway, might be worth
to revisit (here or in the ImageBitmapSource followup.)

[Justin Novosad, 2017-03-13 14:16:44]

I looked at the test failures. They just need to be rebaselined, it seems.

[Justin Novosad, 2017-03-13 14:22:28]

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-svg-expected.html
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-svg-expected.html:25:
<circle cx="50%" cy="50%" r="50%" style="fill:blue;" />
This one shows a minor discrepancy in the pixel results.  Perhaps it would go
away if you just referenced circle.svg.  If not, maybe you can remove this
specific case from the test. It is possible that the anti-aliasing is too
numerically sensitive for this case to be used in a ref test.

[fserb, 2017-03-21 19:15:22]

Thanks for the comments.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-svg-expected.html
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-svg-expected.html:25:
<circle cx="50%" cy="50%" r="50%" style="fill:blue;" />
On 2017/03/13 at 14:22:26, Justin Novosad wrote:
> This one shows a minor discrepancy in the pixel results.  Perhaps it would go
away if you just referenced circle.svg.  If not, maybe you can remove this
specific case from the test. It is possible that the anti-aliasing is too
numerically sensitive for this case to be used in a ref test.

I didn't see that. But yeah, referencing circle.svg makes sense, I guess? At
least we guarantee that we have the same problem as a regular svg image element.
;)

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/fast/canvas/resources/circle.svg (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/canvas/resources/circle.svg:1: <?xml
version="1.0"?>
On 2017/03/07 at 22:01:24, fs wrote:
> You can drop the XML decl (this line) and the doctype (which is useless, even
if seeing a SVG 1.0 doctype is fun! =P)

done.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLImageElement.h (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLImageElement.h:200: HTMLImageLoader&
imageLoader() const override { return *m_imageLoader; }
On 2017/03/07 at 22:01:24, fs wrote:
> (This is a bit scary-looking, hopefully it's future-safe enough.)

I double checked with some people and nobody could come up with anything really
bad that could happen here. I'm convinced it's fine. :) (famous last words)

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.cpp
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.cpp:7:
#include "core/html/HTMLImageLoader.h"
On 2017/03/07 at 22:01:24, fs wrote:
> Nit: using core/loader/ImageLoader.h would suffice, or?

done.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.h
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.h:20:
virtual FloatSize sourceDefaultSize() = 0;
On 2017/03/07 at 22:01:25, fs wrote:
> Should probably be sourceDefaultObjectSize for extra clarity.

done

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/canvas/CanvasImageElementSource.h:47: const
Element* element() const;
On 2017/03/07 at 22:01:25, fs wrote:
> Nit: Could be an Element& even

done.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:199: return FloatSize(0,
0);
On 2017/03/08 at 18:57:16, fs wrote:
> On 2017/03/08 at 18:31:59, fserb wrote:
> > On 2017/03/07 at 22:01:25, fs wrote:
> > > To be somewhat equivalent with the HTMLImageElement version, I think this
would be something like:
> > > 
> > > if (layoutObject())
> > >   return toLayoutSVGImage(layoutObject())->objectBoundingBox().size();
> > > SVGLengthContext lengthContext(this);
> > > return FloatSize(m_width->currentValue()->value(lengthContext),
> > >                  m_height->currentValue()->value(lengthContext));
> > > 
> > > I wonder if these width/heights shouldn't be the <canvas> width/height -
reading step 3 of
https://html.spec.whatwg.org/multipage/scripting.html#drawing-images
> > 
> > Thanks for the code snippet, I'll implement it like this.
> > 
> > Regarding the spec thing, those functions are messed up
(sourceWidth/sourceHeight). They are not called by the CanvasImageSource code
path at all (they are used for BitmapImageSource).
> 
> Ok (ImageBitmapSource though, right?). Codesearch seems to only show them
being called in modules/shapedetection though. Did it miss something?
> 
> > On the canvas side, we always get called through getSourceImageForCanvas()
which receives a "defaultObjectSize" (which is the canvas size) -- I've added
tests to make sure that step 3 works (it's the 6th id on the test, iirc). Since
those are not used for the actual canvas rendering, I interpreted them as "the
inherit dimension of the image" and therefore I think your code makes sense (but
I'll go over it again for Bitmap source, maybe even remove it completely). 
> 
> If they are indeed only used by the shapedetection code, it makes perfect
sense to revisit later. So, if you want to go with the above, that would be fine
with me. I suspect that these are buggy/broken though, and should probably be
replaced/removed.

yep. Only for shapedetection, (which I assumed is only used for
ImageBitmapSource, no?)
Anyway, moot point. I've used your snippet and will look into those again for
ImageBitmapSoure.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:31: //
but there's a problem with our IDL code generator for typedef-in-typedef,
On 2017/03/13 at 13:56:34, fs wrote:
> I saw crbug.com/700225, and that some fixes had landed there. Don't know if
the issue here was with nested unions of actually typedefs, anyway, might be
worth to revisit (here or in the ImageBitmapSource followup.)

Arghh, that took me a while to figure out. So, they've added support for it on
the IDL compiler, but I think the C++ generator is still not using it, so we are
still seeing broken behaviour when doing nested things. I'll keep the comment
and add a TODO for me to revisit this.

[fs, 2017-03-21 21:05:42]

LGTM, thanks

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLImageElement.h (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLImageElement.h:200: HTMLImageLoader&
imageLoader() const override { return *m_imageLoader; }
On 2017/03/21 at 19:15:22, fserb wrote:
> On 2017/03/07 at 22:01:24, fs wrote:
> > (This is a bit scary-looking, hopefully it's future-safe enough.)
> 
> I double checked with some people and nobody could come up with anything
really bad that could happen here. I'm convinced it's fine. :) (famous last
words)

Ok.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/svg/SVGImageElement.cpp (right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/svg/SVGImageElement.cpp:199: return FloatSize(0,
0);
On 2017/03/21 at 19:15:22, fserb wrote:
> On 2017/03/08 at 18:57:16, fs wrote:
> > On 2017/03/08 at 18:31:59, fserb wrote:
> > > On 2017/03/07 at 22:01:25, fs wrote:
> > > > To be somewhat equivalent with the HTMLImageElement version, I think
this would be something like:
> > > > 
> > > > if (layoutObject())
> > > >   return toLayoutSVGImage(layoutObject())->objectBoundingBox().size();
> > > > SVGLengthContext lengthContext(this);
> > > > return FloatSize(m_width->currentValue()->value(lengthContext),
> > > >                  m_height->currentValue()->value(lengthContext));
> > > > 
> > > > I wonder if these width/heights shouldn't be the <canvas> width/height -
reading step 3 of
https://html.spec.whatwg.org/multipage/scripting.html#drawing-images
> > > 
> > > Thanks for the code snippet, I'll implement it like this.
> > > 
> > > Regarding the spec thing, those functions are messed up
(sourceWidth/sourceHeight). They are not called by the CanvasImageSource code
path at all (they are used for BitmapImageSource).
> > 
> > Ok (ImageBitmapSource though, right?). Codesearch seems to only show them
being called in modules/shapedetection though. Did it miss something?
> > 
> > > On the canvas side, we always get called through getSourceImageForCanvas()
which receives a "defaultObjectSize" (which is the canvas size) -- I've added
tests to make sure that step 3 works (it's the 6th id on the test, iirc). Since
those are not used for the actual canvas rendering, I interpreted them as "the
inherit dimension of the image" and therefore I think your code makes sense (but
I'll go over it again for Bitmap source, maybe even remove it completely). 
> > 
> > If they are indeed only used by the shapedetection code, it makes perfect
sense to revisit later. So, if you want to go with the above, that would be fine
with me. I suspect that these are buggy/broken though, and should probably be
replaced/removed.
> 
> yep. Only for shapedetection, (which I assumed is only used for
ImageBitmapSource, no?)
> Anyway, moot point. I've used your snippet and will look into those again for
ImageBitmapSoure.

Acknowledged.

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl
(right):

https://codereview.chromium.org/2723093004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:31: //
but there's a problem with our IDL code generator for typedef-in-typedef,
On 2017/03/21 at 19:15:22, fserb wrote:
> On 2017/03/13 at 13:56:34, fs wrote:
> > I saw crbug.com/700225, and that some fixes had landed there. Don't know if
the issue here was with nested unions of actually typedefs, anyway, might be
worth to revisit (here or in the ImageBitmapSource followup.)
> 
> Arghh, that took me a while to figure out. So, they've added support for it on
the IDL compiler, but I think the C++ generator is still not using it, so we are
still seeing broken behaviour when doing nested things. I'll keep the comment
and add a TODO for me to revisit this.

Roger that.

[Justin Novosad, 2017-03-21 21:08:57]

lgtm

[fserb, 2017-03-21 21:10:47]

The CQ bit was checked by fserb@chromium.org

[commit-bot: I haz the power, 2017-03-21 21:11:17]

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, 2017-03-21 21:30:35]

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

[commit-bot: I haz the power, 2017-03-21 21:30:36]

Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fserb, 2017-03-21 21:44:42]

The CQ bit was checked by fserb@chromium.org

[commit-bot: I haz the power, 2017-03-21 21:45:10]

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

[fserb, 2017-03-21 21:54:49]

The CQ bit was unchecked by fserb@chromium.org

[fserb, 2017-03-21 21:59:20]

fserb@chromium.org changed reviewers:
+ esprehn@chromium.org

[fserb, 2017-03-21 21:59:20]

esprehn: third_party/WebKit/Source/bindings/modules/v8/generated.gni

[esprehn, 2017-03-21 22:37:03]

lgtm

[esprehn, 2017-03-21 22:37:26]

Can you write a better change description and link to some mailing lists or
specs?

[fserb, 2017-03-22 18:30:54]

Description was changed from

==========
Adds SVGImageElement to CanvasImageSource

BUG=695662
==========

to

==========
Adds SVGImageElement to CanvasImageSource

Relevant spec:
https://html.spec.whatwg.org/multipage/scripting.html#image-sources-for-2d-re...

SVGImageElement should be part of CanvasImageSource union typedef, i.e., be
allowed as an source for drawImage(). This CL does that, while creating a common
interface for both HTMLImageElement and SVGImageElement.


BUG=695662
==========

[fserb, 2017-03-22 18:31:07]

The CQ bit was checked by fserb@chromium.org

[fserb, 2017-03-22 18:31:08]

done. thanks.

[commit-bot: I haz the power, 2017-03-22 18:32:14]

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, 2017-03-22 20:41:18]

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

[commit-bot: I haz the power, 2017-03-22 20:41:20]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fserb, 2017-03-23 19:38:23]

The CQ bit was checked by fserb@chromium.org

[fserb, 2017-03-23 19:38:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, fs@opera.com, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/2723093004/#ps160001
(title: "Fixed more tests")

[commit-bot: I haz the power, 2017-03-23 19:39:06]

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, 2017-03-23 20:52:31]

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

[commit-bot: I haz the power, 2017-03-23 20:52:33]

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_...)

[fserb, 2017-03-23 21:26:21]

The CQ bit was checked by fserb@chromium.org

[fserb, 2017-03-23 21:26:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, fs@opera.com, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/2723093004/#ps200001
(title: "Moved from circle to rect, to avoid aliasing")

[commit-bot: I haz the power, 2017-03-23 21:26:59]

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, 2017-03-24 00:04:14]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1490304381075960,
"parent_rev": "7c149655e1546c3bb4478f4dca931e5ad35d4073", "commit_rev":
"11a275c5522ae4dbf8e3dee5c7246e74141b5142"}

[commit-bot: I haz the power, 2017-03-24 00:06:08]

Description was changed from

==========
Adds SVGImageElement to CanvasImageSource

Relevant spec:
https://html.spec.whatwg.org/multipage/scripting.html#image-sources-for-2d-re...

SVGImageElement should be part of CanvasImageSource union typedef, i.e., be
allowed as an source for drawImage(). This CL does that, while creating a common
interface for both HTMLImageElement and SVGImageElement.


BUG=695662
==========

to

==========
Adds SVGImageElement to CanvasImageSource

Relevant spec:
https://html.spec.whatwg.org/multipage/scripting.html#image-sources-for-2d-re...

SVGImageElement should be part of CanvasImageSource union typedef, i.e., be
allowed as an source for drawImage(). This CL does that, while creating a common
interface for both HTMLImageElement and SVGImageElement.


BUG=695662

Review-Url: https://codereview.chromium.org/2723093004
Cr-Commit-Position: refs/heads/master@{#459289}
Committed:
https://chromium.googlesource.com/chromium/src/+/11a275c5522ae4dbf8e3dee5c724...
==========

[commit-bot: I haz the power, 2017-03-24 00:06:10]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/11a275c5522ae4dbf8e3dee5c724...