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 c49dccecaca90366e94c3ee6f092369eece6e2ff..559f139258643eecd9dc643f9d08666911ff49ad 100644
--- a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
@@ -61,7 +61,34 @@ void LayoutSVGImage::willBeDestroyed()
     LayoutSVGModelObject::willBeDestroyed();
 }
 
-void LayoutSVGImage::updateBoundingBox()
+static float resolveWidthForRatio(float height, const FloatSize& intrinsicRatio)
+{
+    return height * intrinsicRatio.width() / intrinsicRatio.height();
+}
+
+static float resolveHeightForRatio(float width, const FloatSize& intrinsicRatio)
+{
+    return width * intrinsicRatio.height() / intrinsicRatio.width();
+}
+
+FloatSize LayoutSVGImage::calculateObjectSize() const
+{
+    ImageResource* cachedImage = m_imageResource->cachedImage();
+    if (!cachedImage || cachedImage->errorOccurred())
+        return m_objectBoundingBox.size();
+
+    FloatSize intrinsicSize = FloatSize(cachedImage->getImage()->size());
+    if (styleRef().width().isAuto() && styleRef().height().isAuto())
+        return intrinsicSize;
+
+    if (styleRef().height().isAuto())
+        return FloatSize(m_objectBoundingBox.width(), resolveHeightForRatio(m_objectBoundingBox.width(), intrinsicSize));
+
+    DCHECK(styleRef().width().isAuto());
+    return FloatSize(resolveWidthForRatio(m_objectBoundingBox.height(), intrinsicSize), m_objectBoundingBox.height());
+}
+
+bool LayoutSVGImage::updateBoundingBox()

[davve, 2016/09/14 14:13:17]

Having updateBoundingBox() both set a member variable (m_needsBoundariesUpdate)
and return a value looks a bit odd to me. Especially since we now call
updateBoundingBox() from outside layout(), where the m_needsBoundariesUpdate
machinery lives. (I suppose m_needsBoundariesUpdate will be set until the
layout() and will counteract a no-up updateBoundingBox() there?) It may be fine
and dandy but the fallout isn't obvious to me.

[fs, 2016/09/14 14:37:11]

IIRC we have a bug on file to shift some of this stuff to styleDidChange or
thereabouts. I suppose that wouldn't really help with the
determine-if-we-need-to-layout bit though.

[davve, 2016/09/15 12:34:43]

s/no-up/no-op/. Sorry about that. After some more contemplating, I think I've
managed to convince myself this is OK.

 {
     FloatRect oldBoundaries = m_objectBoundingBox;
 
@@ -71,9 +98,15 @@ void LayoutSVGImage::updateBoundingBox()
         lengthContext.valueForLength(styleRef().svgStyle().y(), styleRef(), SVGLengthMode::Height),
         lengthContext.valueForLength(styleRef().width(), styleRef(), SVGLengthMode::Width),
         lengthContext.valueForLength(styleRef().height(), styleRef(), SVGLengthMode::Height));
+
+    if (styleRef().width().isAuto() || styleRef().height().isAuto())
+        m_objectBoundingBox.setSize(calculateObjectSize());
+
     m_needsBoundariesUpdate |= oldBoundaries != m_objectBoundingBox;
     if (element())
         element()->setNeedsResizeObserverUpdate();
+
+    return oldBoundaries.size() != m_objectBoundingBox.size();
 }
 
 void LayoutSVGImage::layout()
@@ -148,6 +181,11 @@ void LayoutSVGImage::imageChanged(WrappedImagePtr, const IntRect*)
     // representation of this image/layout object.
     LayoutSVGResourceContainer::markForLayoutAndParentResourceInvalidation(this, false);
 
+    if (styleRef().width().isAuto() || styleRef().height().isAuto()) {
+        if (updateBoundingBox())
+            setNeedsLayout(LayoutInvalidationReason::SizeChanged);
+    }
+
     setShouldDoFullPaintInvalidation();
 }