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

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

If a referenced image had the same dimensions as the referencing <image>,
a container size would not be set. This could cause the wrong scaling (etc)
to be applied if there existed another reference to the same image resource
that ended up applying a different scale-factor.
To ensure that a container size is set in this case, determine if the
referenced image has a container size set or not by checking if what
the "base" image (as returned by ImageResource::image()) differs from the
RenderObject-specific image (as returned by
ImageResource::imageForRenderer)

BUG=453169
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189241

[fs, 2015-01-29 16:37:34]

fs@opera.com changed reviewers:
+ pdr@chromium.org

[fs, 2015-01-29 16:37:34]

The method that just keeps on giving... (stock of brown paper bags soon to be
depleted)

http://stream1.gifsoup.com/view3/3697068/whack-a-mole-o.gif

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

Ahh, our old friend :)

LGTM

https://codereview.chromium.org/889613002/diff/1/Source/core/rendering/svg/Re...
File Source/core/rendering/svg/RenderSVGImage.cpp (right):

https://codereview.chromium.org/889613002/diff/1/Source/core/rendering/svg/Re...
Source/core/rendering/svg/RenderSVGImage.cpp:80: static bool
containerSizeIsSetForRenderer(ImageResource& cachedImage, const RenderObject*
renderer)
Can we put this on ImageResource?

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

On 2015/01/29 19:32:21, pdr wrote:
> Ahh, our old friend :)

Old - certainly. Friend - less certain... =)

https://codereview.chromium.org/889613002/diff/1/Source/core/rendering/svg/Re...
File Source/core/rendering/svg/RenderSVGImage.cpp (right):

https://codereview.chromium.org/889613002/diff/1/Source/core/rendering/svg/Re...
Source/core/rendering/svg/RenderSVGImage.cpp:80: static bool
containerSizeIsSetForRenderer(ImageResource& cachedImage, const RenderObject*
renderer)
On 2015/01/29 19:32:21, pdr wrote:
> Can we put this on ImageResource?

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.

[fs, 2015-01-30 09:30:35]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-01-30 09:30:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/889613002/1

[commit-bot: I haz the power, 2015-01-30 10:31:03]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189241