Cleanup color space resolution for ReferenceFilterBuilder

Cleanup color space resolution for ReferenceFilterBuilder

There's some dodgy-looking code, used by ReferenceFilterBuilder::build
to compute a ColorSpace for each filter primitive. Convert it to
compute an EColorInterpolation instead, and then resolve the ColorSpace
from that similarly to how LayoutSVGResourceFilter::buildPrimitives
does it. Also try to make it "cascade" the value in a more proper way,
rather than just selecting the value from the filter or the primitive.
This moves the two code-paths a bit closer to each other.

Also convert ReferenceFilterBuilder::build(...) to take references as
arguments where it makes sense.

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

[fs, 2015-04-27 15:11:49]

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

[fs, 2015-04-27 17:03:37]

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

[fs, 2015-04-27 18:59:11]

effect-reference-colorspace{,-hw}.html

  Previously either the <filter>s or the <fe*>s color-interpolation-filters
would be picked, but now the computed style from the each corresponding layout
object is used.

effect-reference-external.html

  Since there's no style (in the external document), and no inline style,
previously this would end up as 'linearRGB' (being the default in the
FilterEffect constructor). Now it is resolved via 'auto' into 'sRGB' (device
RGB).

[Stephen White, 2015-04-27 19:33:04]

I want to take a look at this, but I may not be able to get to it today or
tomorrow. If I don't respond by Wednesday, ping me. Thanks!

[Stephen White, 2015-04-29 21:20:41]

LGTM w/nit

https://codereview.chromium.org/1106973004/diff/20001/LayoutTests/TestExpecta...
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1106973004/diff/20001/LayoutTests/TestExpecta...
LayoutTests/TestExpectations:48: crbug.com/109224
css3/filters/effect-reference-colorspace.html [ NeedsRebaseline ]
Tangentially, we need better tests, whose results are more visually verifiable.
There's such an SVG test here:
https://code.google.com/p/chromium/issues/detail?id=473486#c4

If you feel up to it, it might be worth changing this to a CSS reference test
(doesn't have to be in this patch).

https://codereview.chromium.org/1106973004/diff/20001/Source/core/layout/svg/...
File Source/core/layout/svg/ReferenceFilterBuilder.cpp (right):

https://codereview.chromium.org/1106973004/diff/20001/Source/core/layout/svg/...
Source/core/layout/svg/ReferenceFilterBuilder.cpp:76: // Otherwise, try to
determine the property value "manually" (used by external SVG files.)
Nit: a little strange to have "Otherwise, " here now that the first comment has
been removed. Suggest removing it.

[fs, 2015-04-30 08:29:23]

https://codereview.chromium.org/1106973004/diff/20001/LayoutTests/TestExpecta...
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1106973004/diff/20001/LayoutTests/TestExpecta...
LayoutTests/TestExpectations:48: crbug.com/109224
css3/filters/effect-reference-colorspace.html [ NeedsRebaseline ]
On 2015/04/29 21:20:41, Stephen White wrote:
> Tangentially, we need better tests, whose results are more visually
verifiable.
> There's such an SVG test here:
> https://code.google.com/p/chromium/issues/detail?id=473486#c4

I'm not allowed to see that bug it seems =)

> If you feel up to it, it might be worth changing this to a CSS reference test
> (doesn't have to be in this patch).

I can look into this - suppose we should drop the/use a better image.

https://codereview.chromium.org/1106973004/diff/20001/Source/core/layout/svg/...
File Source/core/layout/svg/ReferenceFilterBuilder.cpp (right):

https://codereview.chromium.org/1106973004/diff/20001/Source/core/layout/svg/...
Source/core/layout/svg/ReferenceFilterBuilder.cpp:76: // Otherwise, try to
determine the property value "manually" (used by external SVG files.)
On 2015/04/29 21:20:41, Stephen White wrote:
> Nit: a little strange to have "Otherwise, " here now that the first comment
has
> been removed. Suggest removing it.

Replaced it with "No layout has been performed, ...".

[fs, 2015-04-30 08:29:27]

The CQ bit was checked by fs@opera.com

[fs, 2015-04-30 08:29:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
senorblanco@chromium.org
Link to the patchset: https://codereview.chromium.org/1106973004/#ps40001
(title: "Touchup comment.")

[commit-bot: I haz the power, 2015-04-30 08:30:06]

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

[Stephen White, 2015-04-30 11:04:25]

On 2015/04/30 08:29:23, fs wrote:
>
https://codereview.chromium.org/1106973004/diff/20001/LayoutTests/TestExpecta...
> File LayoutTests/TestExpectations (right):
> 
>
https://codereview.chromium.org/1106973004/diff/20001/LayoutTests/TestExpecta...
> LayoutTests/TestExpectations:48: crbug.com/109224
> css3/filters/effect-reference-colorspace.html [ NeedsRebaseline ]
> On 2015/04/29 21:20:41, Stephen White wrote:
> > Tangentially, we need better tests, whose results are more visually
> verifiable.
> > There's such an SVG test here:
> > https://code.google.com/p/chromium/issues/detail?id=473486#c4
> 
> I'm not allowed to see that bug it seems =)

Whoops; fixed.

> > If you feel up to it, it might be worth changing this to a CSS reference
test
> > (doesn't have to be in this patch).
> 
> I can look into this - suppose we should drop the/use a better image.
> 
>
https://codereview.chromium.org/1106973004/diff/20001/Source/core/layout/svg/...
> File Source/core/layout/svg/ReferenceFilterBuilder.cpp (right):
> 
>
https://codereview.chromium.org/1106973004/diff/20001/Source/core/layout/svg/...
> Source/core/layout/svg/ReferenceFilterBuilder.cpp:76: // Otherwise, try to
> determine the property value "manually" (used by external SVG files.)
> On 2015/04/29 21:20:41, Stephen White wrote:
> > Nit: a little strange to have "Otherwise, " here now that the first comment
> has
> > been removed. Suggest removing it.
> 
> Replaced it with "No layout has been performed, ...".

[commit-bot: I haz the power, 2015-04-30 11:18:34]

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