Unprefix the CSS 'filter' property

Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latter
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Intent to Ship: https://groups.google.com/a/chromium.org/d/topic/blink-dev/ZVT2kxuFMaA/discussion

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441, 618160, 550249, 535786, 244295, 109224
Committed: https://crrev.com/7a0f0e2eb0abdd68fb377d5b789682acfe10757d
Cr-Commit-Position: refs/heads/master@{#400752}

[fs, 2016-06-13 13:15:13]

Description was changed from

==========
Unprefix the 'filter' property

BUG=613441
==========

to

==========
Unprefix the CSS 'filter' property

BUG=613441
==========

[fs, 2016-06-13 14:31:56]

Description was changed from

==========
Unprefix the CSS 'filter' property

BUG=613441
==========

to

==========
Unprefix the CSS 'filter' property

https://codereview.chromium.org/1987943002

BUG=613441
==========

[fs, 2016-06-14 09:52:53]

Description was changed from

==========
Unprefix the CSS 'filter' property

https://codereview.chromium.org/1987943002

BUG=613441
==========

to

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latterxx
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441
==========

[fs, 2016-06-14 09:56:18]

Description was changed from

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latterxx
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441
==========

to

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latterxx
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224
==========

[fs, 2016-06-14 09:56:19]

fs@opera.com changed reviewers:
+ alancutter@chromium.org, chrishtr@chromium.org, pdr@chromium.org,
timloh@chromium.org

[fs, 2016-06-14 10:46:58]

This is largely a rebase of https://codereview.chromium.org/1987943002 -
core/css and core/animation should be pretty much the same. I've rewritten one
of the new (JS) tests (unprefixed.html) to use testharness.
The bigger change I've done though is to let PaintLayer handle the outermost
<svg> root, since that seemed to mesh better (== be less hacky) with the new
filter-based box-reflect.

[fs, 2016-06-14 10:50:25]

Description was changed from

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latterxx
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224
==========

to

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latterxx
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ZVT2kxuFMaA/discus...

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224
==========

[Timothy Loh, 2016-06-14 12:03:46]

core/css lgtm++

[alancutter (OOO until 2018), 2016-06-15 04:31:46]

core/animation lgtm.

BTW there's a small vim leak in the description; "latterxx".

[fs, 2016-06-15 08:01:26]

Description was changed from

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latterxx
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ZVT2kxuFMaA/discus...

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224
==========

to

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latter
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ZVT2kxuFMaA/discus...

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224
==========

[fs, 2016-06-15 08:02:24]

On 2016/06/15 at 04:31:46, alancutter wrote:
> core/animation lgtm.
> 
> BTW there's a small vim leak in the description; "latterxx".

Oops. Fixed, thanks. (And it's not a vim leak - I swear! =P)

[pdr., 2016-06-15 09:50:32]

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp:596: // quash
it for now. FIXME: why is this so, why the assert?
Nit: TODO(fs) instead of FIXME

I've been using FIXME too but I recently learned this was changed in the blink
style guide.

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGResources.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGResources.cpp:235: if
(filterOperations.size() == 1) {
Why do we only use the first filter op instead of looping over all of them?

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/SVGPaintContext.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/SVGPaintContext.cpp:82: // Layer takes care
of root opacity and blend mode.
Can you assert that a layer is present? (Similarly below for filter)

Something like:
if (isSVGRoot) {
    // The PaintLayer will take care of compositing the root.
    DCHECK(m_object.hasLayer());
} else {
    applyCompositingIfNecessary();
}

[fs, 2016-06-15 11:02:43]

Quick upload before running to the taxi - what could possibly go wrong!

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp:596: // quash
it for now. FIXME: why is this so, why the assert?
On 2016/06/15 at 09:50:32, pdr. wrote:
> Nit: TODO(fs) instead of FIXME
> 
> I've been using FIXME too but I recently learned this was changed in the blink
style guide.

I think I inherited this one from noel =). Fixed.

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGResources.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGResources.cpp:235: if
(filterOperations.size() == 1) {
On 2016/06/15 at 09:50:32, pdr. wrote:
> Why do we only use the first filter op instead of looping over all of them?

Because we only want to support the same "subset" as before - i.e a single
url(...). For now.

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/SVGPaintContext.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/SVGPaintContext.cpp:82: // Layer takes care
of root opacity and blend mode.
On 2016/06/15 at 09:50:32, pdr. wrote:
> Can you assert that a layer is present? (Similarly below for filter)
> 
> Something like:
> if (isSVGRoot) {
>     // The PaintLayer will take care of compositing the root.
>     DCHECK(m_object.hasLayer());
> } else {
>     applyCompositingIfNecessary();
> }

Added something along those lines.

[pdr., 2016-06-15 11:53:27]

LGTM

[chrishtr, 2016-06-15 15:56:03]

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:251: if
(!layoutObject->isSVGRoot())
Can a non-SVGRoot SVG object have a PaintLayer?

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp (right):

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp:596: // quash
it for now. TODO(fs): why is this so, why the assert?
Why is this new with this CL?

[fs, 2016-06-15 22:02:10]

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/SVGPaintContext.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/SVGPaintContext.cpp:82: // Layer takes care
of root opacity and blend mode.
On 2016/06/15 at 11:02:43, fs wrote:
> On 2016/06/15 at 09:50:32, pdr. wrote:
> > Can you assert that a layer is present? (Similarly below for filter)
> > 
> > Something like:
> > if (isSVGRoot) {
> >     // The PaintLayer will take care of compositing the root.
> >     DCHECK(m_object.hasLayer());
> > } else {
> >     applyCompositingIfNecessary();
> > }
> 
> Added something along those lines.

Had to adjust this again, because while "has filter" implies "has layer", "has
layer" does not necessarily imply "has filter"; i.e this is not an equivalence.
Since I don't want to add conditional DCHECKs, are there any particular thing
you'd want me to check here?

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:251: if
(!layoutObject->isSVGRoot())
On 2016/06/15 at 15:56:03, chrishtr wrote:
> Can a non-SVGRoot SVG object have a PaintLayer?

Not at the moment (and unlikely ever...)

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp (right):

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp:596: // quash
it for now. TODO(fs): why is this so, why the assert?
On 2016/06/15 at 15:56:03, chrishtr wrote:
> Why is this new with this CL?

I haven't looked into this in depth yet, I just took this at face-value from the
CL I "adopted". It's possible my adjustments made it redundant.

[chrishtr, 2016-06-16 04:55:21]

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp (right):

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp:596: // quash
it for now. TODO(fs): why is this so, why the assert?
On 2016/06/15 at 22:02:10, fs wrote:
> On 2016/06/15 at 15:56:03, chrishtr wrote:
> > Why is this new with this CL?
> 
> I haven't looked into this in depth yet, I just took this at face-value from
the CL I "adopted". It's possible my adjustments made it redundant.

Could you investigate a bit? This could be a real bug.

https://codereview.chromium.org/2065593002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2065593002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:251: if
(!layoutObject->isSVGRoot())
DCHECK(layoutObject->isSVGRoot())

[fs, 2016-06-16 23:06:41]

https://codereview.chromium.org/2065593002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2065593002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:251: if
(!layoutObject->isSVGRoot())
On 2016/06/16 at 04:55:21, chrishtr wrote:
> DCHECK(layoutObject->isSVGRoot())

|layoutObject| can be a non-SVG element here. Or did you mean after the
continue?

[chrishtr, 2016-06-17 06:33:45]

https://codereview.chromium.org/2065593002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2065593002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:251: if
(!layoutObject->isSVGRoot())
On 2016/06/16 at 23:06:41, fs wrote:
> On 2016/06/16 at 04:55:21, chrishtr wrote:
> > DCHECK(layoutObject->isSVGRoot())
> 
> |layoutObject| can be a non-SVG element here. Or did you mean after the
continue?

Right, ok. Ignore my comment then.

[fs, 2016-06-20 10:47:05]

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

[fs, 2016-06-20 10:47:07]

+junov for the 2 trivial changes in modules/canvas2d (will TBR after timeout)

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/SVGPaintContext.cpp (right):

https://codereview.chromium.org/2065593002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/SVGPaintContext.cpp:82: // Layer takes care
of root opacity and blend mode.
On 2016/06/15 at 22:02:09, fs wrote:
> On 2016/06/15 at 11:02:43, fs wrote:
> > On 2016/06/15 at 09:50:32, pdr. wrote:
> > > Can you assert that a layer is present? (Similarly below for filter)
> > > 
> > > Something like:
> > > if (isSVGRoot) {
> > >     // The PaintLayer will take care of compositing the root.
> > >     DCHECK(m_object.hasLayer());
> > > } else {
> > >     applyCompositingIfNecessary();
> > > }
> > 
> > Added something along those lines.
> 
> Had to adjust this again, because while "has filter" implies "has layer", "has
layer" does not necessarily imply "has filter"; i.e this is not an equivalence.
Since I don't want to add conditional DCHECKs, are there any particular thing
you'd want me to check here?

Fixed up to something that passes trybots.

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp (right):

https://codereview.chromium.org/2065593002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGLayoutTreeAsText.cpp:596: // quash
it for now. TODO(fs): why is this so, why the assert?
On 2016/06/16 at 04:55:20, chrishtr wrote:
> On 2016/06/15 at 22:02:10, fs wrote:
> > On 2016/06/15 at 15:56:03, chrishtr wrote:
> > > Why is this new with this CL?
> > 
> > I haven't looked into this in depth yet, I just took this at face-value from
the CL I "adopted". It's possible my adjustments made it redundant.
> 
> Could you investigate a bit? This could be a real bug.

Dropped without incident in PS7.

[chrishtr, 2016-06-20 16:01:41]

lgtm

[Justin Novosad, 2016-06-20 18:13:25]

On 2016/06/20 16:01:41, chrishtr wrote:
> lgtm

lgtm

[fs, 2016-06-20 19:04:49]

The CQ bit was checked by fs@opera.com

[fs, 2016-06-20 19:04:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org, timloh@chromium.org, pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2065593002/#ps120001
(title: "Remove disabler. Try different DCHECK expressions.")

[commit-bot: I haz the power, 2016-06-20 19:05:24]

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

[commit-bot: I haz the power, 2016-06-20 20:00:03]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-06-20 20:02:06]

Description was changed from

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latter
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ZVT2kxuFMaA/discus...

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224
==========

to

==========
Unprefix the CSS 'filter' property

Parse 'filter' in the same way as '-webkit-filter', and make the latter
an alias of the former.
For SVG content, only the "url(...)" function is allowed still, with the
exception of the outermost <svg> (LayoutSVGRoot), since that "has a box".

Update tests to use 'filter' where reasonable and applicable.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/ZVT2kxuFMaA/discus...

Based on https://codereview.chromium.org/1987943002 by noel@chromium.org.

BUG=613441,618160,550249,535786,244295,109224

Committed: https://crrev.com/7a0f0e2eb0abdd68fb377d5b789682acfe10757d
Cr-Commit-Position: refs/heads/master@{#400752}
==========

[commit-bot: I haz the power, 2016-06-20 20:02:08]

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