SVG Image intrinsic size should be used if css style size is 'auto'

third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp

Index: third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp b/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
index 91867b267e3846e7871142f9b3e0af1f11131e9a..f81dcc8ccf2334bfca5e76f14a6f3b03aaea1ad9 100644
--- a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
@@ -66,11 +66,17 @@ void LayoutSVGImage::updateBoundingBox()
     FloatRect oldBoundaries = m_objectBoundingBox;
 
     SVGLengthContext lengthContext(element());
+    ImageResource* cachedImage = m_imageResource->cachedImage();
+    FloatSize intrinsicSize;
+    if (cachedImage && !cachedImage->errorOccurred())
+        intrinsicSize = FloatSize(cachedImage->getImage()->size());

[fs, 2016/08/16 12:43:08]

I checked the spec, and these seem to be the relevant sections:

https://svgwg.org/svg2-draft/embedded.html#Placement
https://svgwg.org/svg2-draft/geometry.html#Sizing

(those can be added to the description)

I notice that the initial value is set to '0' for the attribute (the former
link) while it is 'auto' for the properties. Should probably get that cleared up
in the spec. (This will affect existing content too.) I'm not 100% sure that the
different descriptions of 'auto' are useful (and equivalent either), so that's
one other thing to consider also filing a spec issue about. (I'd go with the
description from the second link, since that seems to just refer to CSS sizing.)

[Shanmuga Pandi, 2016/08/19 07:20:28]

I filed a spec bug:
https://github.com/w3c/svgwg/issues/237

+
     m_objectBoundingBox = FloatRect(
         lengthContext.valueForLength(styleRef().svgStyle().x(), styleRef(), SVGLengthMode::Width),
         lengthContext.valueForLength(styleRef().svgStyle().y(), styleRef(), SVGLengthMode::Height),
-        lengthContext.valueForLength(styleRef().width(), styleRef(), SVGLengthMode::Width),
-        lengthContext.valueForLength(styleRef().height(), styleRef(), SVGLengthMode::Height));
+        styleRef().width().isAuto() ? intrinsicSize.width() : lengthContext.valueForLength(styleRef().width(), styleRef(), SVGLengthMode::Width),
+        styleRef().height().isAuto() ? intrinsicSize.height() : lengthContext.valueForLength(styleRef().height(), styleRef(), SVGLengthMode::Height));
+
     m_needsBoundariesUpdate |= oldBoundaries != m_objectBoundingBox;
 }
 
@@ -146,6 +152,16 @@ void LayoutSVGImage::imageChanged(WrappedImagePtr, const IntRect*)
     // representation of this image/layout object.
     LayoutSVGResourceContainer::markForLayoutAndParentResourceInvalidation(this, false);
 
+    if (styleRef().width().isAuto() || styleRef().height().isAuto()) {
+        FloatRect oldBoundaries = m_objectBoundingBox;
+        updateBoundingBox();
+        FloatRect newBoundaries = m_objectBoundingBox;
+        if (oldBoundaries.size() != newBoundaries.size()) {

[fs, 2016/08/16 12:43:08]

Just make updateBoundingBox() return a bool instead.

[Shanmuga Pandi, 2016/08/19 07:20:28]

Done.

+            setNeedsLayoutAndFullPaintInvalidation(LayoutInvalidationReason::SizeChanged);

[fs, 2016/08/16 12:43:08]

Maybe just make this setNeedsLayout and then a fall-through to mark for paint
invalidation.

Also, make sure there's a test for this code-path.

[Shanmuga Pandi, 2016/08/19 07:20:28]

Done.

+            return;
+        }
+    }
+
     setShouldDoFullPaintInvalidation();
 }