Teach SVGImageForContainer::imageForCurrentFrame about the URL

Teach SVGImageForContainer::imageForCurrentFrame about the URL

Pass URL from SVGImageForContainer to SVGImage when fetching an
snapshot for current frame.

BUG=565282
Committed: https://crrev.com/e53ccb7b664054419f1564648f80839477d5b7b7
Cr-Commit-Position: refs/heads/master@{#363466}

[davve, 2015-12-03 13:22:05]

davve@opera.com changed reviewers:
+ fs@opera.com

[davve, 2015-12-03 13:22:06]

Patterns use imageForCurrentFrame() to generated the pattern image. Might as
well pass on the URL when we already sit on it.

[fs, 2015-12-03 15:45:19]

https://codereview.chromium.org/1498683003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp
(right):

https://codereview.chromium.org/1498683003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp:52: return
m_image->imageForCurrentFrameForContainer(m_url);
Shouldn't we potentially be passing containerSize here as well, or is that
something for a rainier day?

[davve, 2015-12-03 21:07:56]

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

[davve, 2015-12-03 21:07:57]

Adding pdr too, in case something like this has come up before at some point.

https://codereview.chromium.org/1498683003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp
(right):

https://codereview.chromium.org/1498683003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp:52: return
m_image->imageForCurrentFrameForContainer(m_url);
On 2015/12/03 15:45:19, fs wrote:
> Shouldn't we potentially be passing containerSize here as well, or is that
> something for a rainier day?

Potentially, yes. I haven't really investigated the implications.
imageForCurrentFrame() uses the intrinsic size (with 300x150 fallback) as
container size, which does cause some issues. Especially the 2:1 ratio it gives
for SVG images without any intrinsic dimensions.

I'll investigate more tomorrow, but I wouldn't mind following up in another CL.

[fs, 2015-12-03 21:15:11]

https://codereview.chromium.org/1498683003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp
(right):

https://codereview.chromium.org/1498683003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp:52: return
m_image->imageForCurrentFrameForContainer(m_url);
On 2015/12/03 at 21:07:57, David Vest wrote:
> On 2015/12/03 15:45:19, fs wrote:
> > Shouldn't we potentially be passing containerSize here as well, or is that
> > something for a rainier day?
> 
> Potentially, yes. I haven't really investigated the implications.
imageForCurrentFrame() uses the intrinsic size (with 300x150 fallback) as
container size, which does cause some issues. Especially the 2:1 ratio it gives
for SVG images without any intrinsic dimensions.
> 
> I'll investigate more tomorrow, but I wouldn't mind following up in another
CL.

Keeping it for later SGTM.

[pdr., 2015-12-03 22:27:15]

I can't think of this ever coming up before, but it seems like a good change to
me. LGTM.

So many great SVGImage changes lately!

[fs, 2015-12-03 22:51:24]

On 2015/12/03 at 22:27:15, pdr wrote:
> I can't think of this ever coming up before, but it seems like a good change
to me. LGTM.
> 
> So many great SVGImage changes lately!

Keep it up, and pdr might dance ballongdansen! =P

lgtm

[fs, 2015-12-03 23:00:23]

On 2015/12/03 at 22:51:24, fs wrote:
> On 2015/12/03 at 22:27:15, pdr wrote:
> > I can't think of this ever coming up before, but it seems like a good change
to me. LGTM.
> > 
> > So many great SVGImage changes lately!
> 
> Keep it up, and pdr might dance ballongdansen! =P

Yhm, I meant LGTM-dansen of course! *facepalm*

I blame the late hour.

[davve, 2015-12-07 12:15:49]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2015-12-07 12:17:11]

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

[commit-bot: I haz the power, 2015-12-07 13:27:15]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-12-07 13:28:11]

Description was changed from

==========
Teach SVGImageForContainer::imageForCurrentFrame about the URL

Pass URL from SVGImageForContainer to SVGImage when fetching an
snapshot for current frame.

BUG=565282
==========

to

==========
Teach SVGImageForContainer::imageForCurrentFrame about the URL

Pass URL from SVGImageForContainer to SVGImage when fetching an
snapshot for current frame.

BUG=565282

Committed: https://crrev.com/e53ccb7b664054419f1564648f80839477d5b7b7
Cr-Commit-Position: refs/heads/master@{#363466}
==========

[commit-bot: I haz the power, 2015-12-07 13:28:12]

Patchset 1 (id:??) landed as
https://crrev.com/e53ccb7b664054419f1564648f80839477d5b7b7
Cr-Commit-Position: refs/heads/master@{#363466}