Support createImageBitmap from a SVG image

third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp

Index: third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
diff --git a/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp b/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
index 1e2a4d913e4257302da47d10620e9cddfa12c8b6..e73370f43a5bde09da714b42086c81e40c02e955 100644
--- a/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
+++ b/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
@@ -168,6 +168,24 @@ static float resolveHeightForRatio(float width, const FloatSize& intrinsicRatio)
     return width * intrinsicRatio.height() / intrinsicRatio.width();
 }
 
+bool SVGImage::hasIntrinsicSize() const
+{
+    SVGSVGElement* svg = svgRootElement(m_page.get());
+    if (!svg)
+        return false;
+
+    LayoutSVGRoot* layoutObject = toLayoutSVGRoot(svg->layoutObject());
+    if (!layoutObject)
+        return false;
+
+    LayoutReplaced::IntrinsicSizingInfo intrinsicSizingInfo;
+    layoutObject->computeIntrinsicSizingInfo(intrinsicSizingInfo);

[Justin Novosad, 2016/04/14 20:45:14]

Is this call at all expensive? I am concerned that we will end up calling it
twice (here, and in concreteObjectSize immediately after).  Perhaps the concrete
sizing info should be cached?  Or perhaps you could instead have a method
bool getSizingInfo(LayoutReplaced::IntrinsicSizingInfo* result);
Then concreteObjectSize could take an optional sizingInfo argument and only
compute the sizingInfo if the argument was omitted.

If computeIntrinsicSizingInfo is trivially cheap, then never mind.

[davve, 2016/04/15 07:24:31]

I don't think it's particularly expensive. But at the surface it seems
unnecessary to go through the layout object. Might as well go straight
for:

SVGSVGElement* svg = svgRootElement(m_page.get());
if (!svg)
    return false;
return svg->hasIntrinsicWidth() && svg->hasIntrinsicHeight();

That said, it's very strict to require both intrinsic width and
intrinsic height for SVG with an aspect ratio (from the viewBox
attribute). Example:

<svg width="10" viewBox="0 0 1 1"/>

This SVG can be considered to have an intrinsic height 10 even though
the attribute is not there, since the viewBox attribute specifies a
1:1 aspect ratio. It would probably work just fine to let through this
case.

I can't load the spec link at the moment, so I'm not sure what the
spec says about it. Personally I think it's a bit too strict to
disallow it (and the corresponding case for width.)

Regardless, it may be something we should add a test for.

[davve, 2016/04/15 09:31:18]

Now being able to read the spec text, it says "... has no intrinsic
dimensions". I take that as wanting to allow the case I posted above.

As I understand it, when createImageBitmap is called the size of the
SVG has already been calculated and saved using
concreteObjectSize(defaultObjectSize=300x150) (called when the SVG
finishes loading in SVGImage::dataChanged()). The sizes we wish to
reject is size based on that 300x150 rectangle.

(side-note: I wish to someday get rid of that 300x150 rect that in
most cases, as in this one, is just wrong),

One idea is to use SVGImage::concreteObjectSize() here with an empty
default size and just check the result for being non-empty. So adding
and using something like:

bool SVGImage::hasIntrinsicDimensions() const
{
    return !concreteObjectSize(FloatSize()).isEmpty();
}

This leaves the issue with how to handle SVG with explicit zero size:

<svg width="0" height="0" viewBox="0 0 1 1"/>

I think this qualifies as having no intrinsic dimensions, but I'm not
sure. https://github.com/whatwg/html/issues/1057 filed for
clarification.

WDYT?

[xidachen, 2016/04/15 12:24:04]

Thanks David. I am going with 
return !concreteObjectSize(FloatSize()).isEmpty();

I will land this CL for now, but for sure needs more test cases later on.

+
+    if (intrinsicSizingInfo.hasWidth && intrinsicSizingInfo.hasHeight)
+        return true;
+    return false;
+}
+
 FloatSize SVGImage::concreteObjectSize(const FloatSize& defaultObjectSize) const
 {
     SVGSVGElement* svg = svgRootElement(m_page.get());