Make sure a container size is always set for SVG images in <svg:image>

Source/core/rendering/svg/RenderSVGImage.cpp

 /*
  * Copyright (C) 2006 Alexander Kellett <lypanov@kde.org>
  * Copyright (C) 2006 Apple Computer, Inc.
  * Copyright (C) 2007 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2007, 2008, 2009 Rob Buis <buis@kde.org>
  * Copyright (C) 2009 Google, Inc.
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  * Copyright (C) 2010 Patrick Gansterer <paroga@paroga.com>
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 59 matching lines @@
     // scaling. This can be achieved by setting the image's container size to
     // its viewport size (i.e. if a viewBox is available - use that - else use intrinsic size.)
     // See: http://www.w3.org/TR/SVG/single-page.html, 7.8 The ‘preserveAspectRatio’ attribute.
     Length intrinsicWidth;
     Length intrinsicHeight;
     FloatSize intrinsicRatio;
     cachedImage.computeIntrinsicDimensions(intrinsicWidth, intrinsicHeight, intrinsicRatio);
     return intrinsicRatio;
 }
 
+static bool containerSizeIsSetForRenderer(ImageResource& cachedImage, const RenderObject* renderer)

[pdr., 2015/01/29 19:32:21]

Can we put this on ImageResource?

[fs, 2015/01/30 09:30:20]

I considered that, but since it didn't seem universally useful (to me) - since
this is a strange semantic that arises from the way m_svgImageCache is set
up/works. I.e. it'd look something like:

bool ImageResource::hasContainerSizeForRenderer(const ImageResourceClient*
renderer)
{
    ASSERT(renderer);
    if (!m_image)
        return m_pendingContainerSizeRequests.contains(renderer);
    if (!m_image->isSVGImage())
        return !m_image->usesContainerSize() || !m_image->size().isZero();
    return !!m_svgImageCache->imageForRenderer(renderer);
}

and some of that felt pretty hand-wavy... I can do it in a follow-up though if
you prefer.

+{
+    const Image* image = cachedImage.image();
+    // If a container size has been specified for this renderer, then
+    // imageForRenderer() will return the SVGImageForContainer while image()
+    // will return the underlying SVGImage.
+    return !image->isSVGImage() || image != cachedImage.imageForRenderer(renderer);
+}
+
 bool RenderSVGImage::updateImageViewport()
 {
     SVGImageElement* image = toSVGImageElement(element());
     FloatRect oldBoundaries = m_objectBoundingBox;
 
     SVGLengthContext lengthContext(image);
     m_objectBoundingBox = FloatRect(image->x()->currentValue()->value(lengthContext), image->y()->currentValue()->value(lengthContext), image->width()->currentValue()->value(lengthContext), image->height()->currentValue()->value(lengthContext));
     bool boundsChanged = oldBoundaries != m_objectBoundingBox;
 
     bool updatedViewport = false;
     ImageResource* cachedImage = m_imageResource->cachedImage();
     if (cachedImage && cachedImage->usesImageContainerSize()) {
         FloatSize imageViewportSize = computeImageViewportSize(*cachedImage);
-        if (LayoutSize(imageViewportSize) != m_imageResource->imageSize(style()->effectiveZoom())) {
+        if (LayoutSize(imageViewportSize) != m_imageResource->imageSize(style()->effectiveZoom())
+            || !containerSizeIsSetForRenderer(*cachedImage, this)) {
             m_imageResource->setContainerSizeForRenderer(roundedIntSize(imageViewportSize));
             updatedViewport = true;
         }
     }
 
     m_needsBoundariesUpdate |= boundsChanged;
     return updatedViewport || boundsChanged;
 }
 
 void RenderSVGImage::layout()
@@ skipping 79 matching lines @@
 
 void RenderSVGImage::addFocusRingRects(Vector<LayoutRect>& rects, const LayoutPoint&) const
 {
     // this is called from paint() after the localTransform has already been applied
     LayoutRect contentRect = LayoutRect(paintInvalidationRectInLocalCoordinates());
     if (!contentRect.isEmpty())
         rects.append(contentRect);
 }
 
 } // namespace blink