HTMLImageElement.natural{Width,Height} should return intrinsic dimensions

HTMLImageElement.natural{Width,Height} should return intrinsic dimensions

HTML says:

 "The IDL attributes naturalWidth and naturalHeight must return the
  intrinsic width and height of the image, in CSS pixels, if the image is
  available, or else 0."

Change HTMLImageElement::natural{Width,Height} to request the intrinsic
size from ImageResource::imageSizeForRenderer by passing IntrinsicSize as
the third parameter. This makes the attributes return more appropriate
values for SVG-images in <img>.

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

[fs, 2014-07-25 14:24:36]

Wrote the tests based on my interpretation of the spec - Gecko/FF seemed to
agree. That of course doesn't mean they are correct, so please scrutinize...

[Yoav Weiss, 2014-07-25 15:08:54]

On 2014/07/25 14:24:36, fs wrote:
> Wrote the tests based on my interpretation of the spec - Gecko/FF seemed to
> agree. That of course doesn't mean they are correct, so please scrutinize...

That seems reasonable, all in all, but pdr should take a look.

Also - you have a bunch of failures in your expected tests, so the fix may not
be complete (or the test slightly broken?)

[fs, 2014-07-25 15:37:15]

On 2014/07/25 15:08:54, Yoav Weiss wrote:
> On 2014/07/25 14:24:36, fs wrote:
...
> Also - you have a bunch of failures in your expected tests, so the fix may not
> be complete (or the test slightly broken?)

The fails seem to stem from what we store as intrinsic size in SVGImage - i.e.
not directly related to the fix (of returning the appropriate intrinsic
dimension). There's a possibility that the tests are wrong of course, but that's
more hazy =)

[pdr., 2014-07-25 20:02:57]

LGTM if you end up liking the IE results.

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
File LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html
(right):

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html:11:
function makeSvgImageUrl(dimension) {
Maybe call this sizingAttributes instead of dimension?

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html:22:
assert_equals(img.naturalWidth, expected.width, 'naturalWidth');
Can you assert both the width and the height together here? As it is written, we
end up failing the width check so we don't have coverage over the height check.

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html:37:
makeTest("width='500'", { width: 500, height: 0 }, "width in pixels; height
unspecified");
I don't have my IE machine with me, but if my netrenderer results are right this
returns 0,0 on IE11. Could you get the IE results for the other tests to see how
they compare?

I think the results you have here make sense and it's a nice bonus that they
match gecko.

[fs, 2014-07-28 08:57:00]

Can't say I "liked" the IE results, but at least they were consistent... ;-)

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
File LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html
(right):

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html:11:
function makeSvgImageUrl(dimension) {
On 2014/07/25 20:02:56, pdr wrote:
> Maybe call this sizingAttributes instead of dimension?

Done.

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html:22:
assert_equals(img.naturalWidth, expected.width, 'naturalWidth');
On 2014/07/25 20:02:56, pdr wrote:
> Can you assert both the width and the height together here? As it is written,
we
> end up failing the width check so we don't have coverage over the height
check.

Rewrote the assertion(s) to the use the form "WxH".

https://codereview.chromium.org/422503002/diff/1/LayoutTests/fast/dom/HTMLIma...
LayoutTests/fast/dom/HTMLImageElement/image-natural-width-height-svg.html:37:
makeTest("width='500'", { width: 500, height: 0 }, "width in pixels; height
unspecified");
On 2014/07/25 20:02:56, pdr wrote:
> I don't have my IE machine with me, but if my netrenderer results are right
this
> returns 0,0 on IE11. Could you get the IE results for the other tests to see
how
> they compare?

I solicited results for IE (v11) and it seemed to return 0x0 on all the tests...
(hope there was no IE-magic missing, and also had to strip out the harness to
simplify...)

[fs, 2014-07-28 12:10:42]

The CQ bit was checked by fs@opera.com

[fs, 2014-07-28 12:10:47]

The CQ bit was unchecked by fs@opera.com

[fs, 2014-07-28 12:11:29]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-07-28 12:11:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fs@opera.com/422503002/40001

[commit-bot: I haz the power, 2014-07-28 12:15:11]

Change committed as 179030