Teach mapFill{X,Y}Position about initial values to individual background layers

Teach mapFill{X,Y}Position about initial values to individual background layers

In <https://codereview.chromium.org/35303002>;, it was falsely assumed
that the initial values for background-position-{x,y} only came from
the property as a whole, ignoring that the initial value must be taken
into account *for each layer* too. Otherwise we will leak background
positions from one layer to the next, causing the positioning bug the
reporter found.

This is a partial revert of r160243 along with an added js test.

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

[davve, 2014-01-08 13:48:39]

Sorry for the breakage, I misread the code back in October.

[davve, 2014-01-10 08:34:27]

Julien, can you take a look?

[Julien - ping for review, 2014-01-10 13:37:03]

lgtm, I don't need to see the updated change.

https://codereview.chromium.org/128203003/diff/1/LayoutTests/fast/backgrounds...
File LayoutTests/fast/backgrounds/multiple-backgrounds-initial-values.html
(right):

https://codereview.chromium.org/128203003/diff/1/LayoutTests/fast/backgrounds...
LayoutTests/fast/backgrounds/multiple-backgrounds-initial-values.html:21:
shouldBeGreaterThanOrEqual('bgPosLayers[0].search("50px")', '0');
Couldn't we just use the explicit position here instead of a fuzzy check? Your
intent is clear - did we find the values in the strings? - but I don't think
restricting us will matter much in practice.

https://codereview.chromium.org/128203003/diff/1/Source/core/css/resolver/CSS...
File Source/core/css/resolver/CSSToStyleMap.cpp (right):

https://codereview.chromium.org/128203003/diff/1/Source/core/css/resolver/CSS...
Source/core/css/resolver/CSSToStyleMap.cpp:221: if (value->isInitialValue()) {
Shouldn't you also handle the keyword 'inherit' here too?

[davve, 2014-01-10 14:09:48]

https://codereview.chromium.org/128203003/diff/1/LayoutTests/fast/backgrounds...
File LayoutTests/fast/backgrounds/multiple-backgrounds-initial-values.html
(right):

https://codereview.chromium.org/128203003/diff/1/LayoutTests/fast/backgrounds...
LayoutTests/fast/backgrounds/multiple-backgrounds-initial-values.html:21:
shouldBeGreaterThanOrEqual('bgPosLayers[0].search("50px")', '0');
On 2014/01/10 13:37:03, Julien Chaffraix - CET wrote:
> Couldn't we just use the explicit position here instead of a fuzzy check? Your
> intent is clear - did we find the values in the strings? - but I don't think
> restricting us will matter much in practice.

I like the ability to run (and PASS) the test in Firefox. They output a more
compact format, IIRC.

https://codereview.chromium.org/128203003/diff/1/Source/core/css/resolver/CSS...
File Source/core/css/resolver/CSSToStyleMap.cpp (right):

https://codereview.chromium.org/128203003/diff/1/Source/core/css/resolver/CSS...
Source/core/css/resolver/CSSToStyleMap.cpp:221: if (value->isInitialValue()) {
On 2014/01/10 13:37:03, Julien Chaffraix - CET wrote:
> Shouldn't you also handle the keyword 'inherit' here too?

Hm, I don't think so. The added code is for handling absent values in each
layer. You may not set 'inherit' in a way that may reach this code. All
'inherit' values should be handled further up in StyleBuilder::applyProperty,
unless I'm missing something.

[commit-bot: I haz the power, 2014-01-10 14:51:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/davve@opera.com/128203003/1

[commit-bot: I haz the power, 2014-01-10 15:00:11]

Change committed as 164877