LayoutSVGImage should work correctly even when image size is unavailable

third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.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 51 matching lines @@
   return height * intrinsicRatio.width() / intrinsicRatio.height();
 }
 
 static float resolveHeightForRatio(float width,
                                    const FloatSize& intrinsicRatio) {
   return width * intrinsicRatio.height() / intrinsicRatio.width();
 }
 
 FloatSize LayoutSVGImage::calculateObjectSize() const {
   ImageResourceContent* cachedImage = m_imageResource->cachedImage();
-  if (!cachedImage || cachedImage->errorOccurred())
+  if (!cachedImage || cachedImage->errorOccurred() ||
+      !cachedImage->isSizeAvailable())

[fs, 2016/12/20 10:08:12]

Would this not affect other similar code-paths simply because they either check
for the "empty" condition, or because the don't happen to use the (zero)
dimensions as denominators in a division? I'm not opposed to having this check
as such, but I wonder if makes sense to diverge w.r.t other similar uses.

[yhirano, 2016/12/20 12:06:30]

I'm not sure (I'm not familiar with the layout / image code). It looks dangerous
to me to read size when it is not available.

It is possible that we need more protection against 0-width and 0-height, but it
is a different issue, I think.

[fs, 2016/12/20 12:15:45]

My observation was mostly that other code seems to rely on that "no size
available" implies "size 0x0" (meaning one less property of the image to check.)
I'm fine with landing this change as-is though.

[hiroshige, 2016/12/21 17:51:06]

If we can replace isSizeAvailable() here and in ImageResource-related code with
0x0 size check, then it would be simpler (one less property that
ImageResourceContent should keep track), but it doesn't block this CL.

     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));
@@ skipping 119 matching lines @@
 
 void LayoutSVGImage::addOutlineRects(Vector<LayoutRect>& rects,
                                      const LayoutPoint&,
                                      IncludeBlockVisualOverflowOrNot) const {
   // this is called from paint() after the localTransform has already been
   // applied
   rects.append(LayoutRect(visualRectInLocalSVGCoordinates()));
 }
 
 }  // namespace blink