Use an SVGElementProxy in ReferenceClipPathOperation

Use an SVGElementProxy in ReferenceClipPathOperation

This transforms ReferenceClipPathOperation into using the SVGElementProxy
mechanism. Currently only for PaintLayer clients.

PaintLayerFilterInfo is generalized to PaintLayerResourceInfo and used as
the proxy/resource client for the 'clip-path' property. This enables change
notifications to flow back to the PaintLayer from the <clipPath> subtree.

The SVGElementProxySet is made a little bit generic by moving it to
SVGElementRareData, however it's still only made available for the few element
types that are used with it.

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/ad7c7f0de20e3ad160f063f1990c69ce97f7e16f
Cr-Commit-Position: refs/heads/master@{#432193}

[fs, 2016-11-08 17:46:49]

Description was changed from

==========
Use a SVGElementProxy in ReferenceClipPathOperation

BUG=391604
==========

to

==========
Use a SVGElementProxy in ReferenceClipPathOperation

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[fs, 2016-11-08 17:47:44]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 17:48:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 19:43:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-08 19:43:45]

Dry run: This issue passed the CQ dry run.

[fs, 2016-11-11 15:12:51]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 15:13:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-11 16:48:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 16:48:59]

Dry run: This issue passed the CQ dry run.

[fs, 2016-11-14 11:37:51]

Description was changed from

==========
Use a SVGElementProxy in ReferenceClipPathOperation

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use a SVGElementProxy in ReferenceClipPathOperation

This transforms ReferenceClipPathOperation into using the SVGElementProxy
mechanism. Currently only for PaintLayer clients.

PaintLayerFilterInfo is generalized to PaintLayerResourceInfo and used as
the proxy/resource client for the 'clip-path' property. This enables change
notifications to flow back to the PaintLayer from the <clipPath> subtree.

The SVGElementProxySet is made a little bit generic by moving it to
SVGElementRareData, however it's still only made available for the few element
types that are used with it.

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[fs, 2016-11-14 13:28:43]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 13:28:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 15:03:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-14 15:03:18]

Dry run: This issue passed the CQ dry run.

[fs, 2016-11-14 16:31:07]

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

[fs, 2016-11-14 16:31:21]

Description was changed from

==========
Use a SVGElementProxy in ReferenceClipPathOperation

This transforms ReferenceClipPathOperation into using the SVGElementProxy
mechanism. Currently only for PaintLayer clients.

PaintLayerFilterInfo is generalized to PaintLayerResourceInfo and used as
the proxy/resource client for the 'clip-path' property. This enables change
notifications to flow back to the PaintLayer from the <clipPath> subtree.

The SVGElementProxySet is made a little bit generic by moving it to
SVGElementRareData, however it's still only made available for the few element
types that are used with it.

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use an SVGElementProxy in ReferenceClipPathOperation

This transforms ReferenceClipPathOperation into using the SVGElementProxy
mechanism. Currently only for PaintLayer clients.

PaintLayerFilterInfo is generalized to PaintLayerResourceInfo and used as
the proxy/resource client for the 'clip-path' property. This enables change
notifications to flow back to the PaintLayer from the <clipPath> subtree.

The SVGElementProxySet is made a little bit generic by moving it to
SVGElementRareData, however it's still only made available for the few element
types that are used with it.

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-11-14 20:56:14]

lgtm

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ClipPathOperation.cpp (right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ClipPathOperation.cpp:10:
m_elementProxy->addClient(client);
Why have these helper methods if elementProxy() is also available as a method on
this
class?

[pdr., 2016-11-15 04:14:35]

LGTM

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp
(right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp:144: //
TODO(fs): Doesn't work with forward or external SVG references
I think the new id tracking technology may fix this. Is this TODO (and the one
in ClipPathClipper.cpp) still needed with this patch?

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGElement.cpp (right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGElement.cpp:1178: // Limit to specific
element types.
Nit: the comment above SVGElementProxySet needs to be updated to mention
clippath. Maybe just point that comment to this code?

[fs, 2016-11-15 14:09:40]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-15 14:09:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2016-11-15 14:10:57]

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp
(right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp:144: //
TODO(fs): Doesn't work with forward or external SVG references
On 2016/11/15 at 04:14:35, pdr. wrote:
> I think the new id tracking technology may fix this. Is this TODO (and the one
in ClipPathClipper.cpp) still needed with this patch?

I think the bit about 'forward' should be fixed, but I haven't dug into these
bugs to see what exactly they were about. (The crbug.com/391604 reference should
obviously be removed, so I'll do that =).)

The 'external' bit (crbug.com/109212) should still be true (for 'clip-path'.)

(Ended up removing mentions of "forward", and updated to reference
crbug.com/109212 in both cases.)

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ClipPathOperation.cpp (right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ClipPathOperation.cpp:10:
m_elementProxy->addClient(client);
On 2016/11/14 at 20:56:13, chrishtr wrote:
> Why have these helper methods if elementProxy() is also available as a method
on this
> class?

I guess I thought it looked prettier... (I did this for the corresponding filter
case too.) Maybe I should delegate findElement() too for consistency then?

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/svg/SVGElement.cpp (right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/svg/SVGElement.cpp:1178: // Limit to specific
element types.
On 2016/11/15 at 04:14:35, pdr. wrote:
> Nit: the comment above SVGElementProxySet needs to be updated to mention
clippath. Maybe just point that comment to this code?

Updated that comment to point to this method.

[chrishtr, 2016-11-15 16:24:22]

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ClipPathOperation.cpp (right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ClipPathOperation.cpp:10:
m_elementProxy->addClient(client);
On 2016/11/15 at 14:10:56, fs wrote:
> On 2016/11/14 at 20:56:13, chrishtr wrote:
> > Why have these helper methods if elementProxy() is also available as a
method on this
> > class?
> 
> I guess I thought it looked prettier... (I did this for the corresponding
filter case too.) Maybe I should delegate findElement() too for consistency
then?

Maybe. I don't feel that strongly about it or my original comment, up to you.

[commit-bot: I haz the power, 2016-11-15 16:24:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-15 16:24:42]

Dry run: This issue passed the CQ dry run.

[fs, 2016-11-15 16:29:32]

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ClipPathOperation.cpp (right):

https://codereview.chromium.org/2484153003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ClipPathOperation.cpp:10:
m_elementProxy->addClient(client);
On 2016/11/15 at 16:24:22, chrishtr wrote:
> On 2016/11/15 at 14:10:56, fs wrote:
> > On 2016/11/14 at 20:56:13, chrishtr wrote:
> > > Why have these helper methods if elementProxy() is also available as a
method on this
> > > class?
> > 
> > I guess I thought it looked prettier... (I did this for the corresponding
filter case too.) Maybe I should delegate findElement() too for consistency
then?
> 
> Maybe. I don't feel that strongly about it or my original comment, up to you.

Noted. I've dropped the accessor (elementProxy()) and added a delegation to
findElement too.

[fs, 2016-11-15 16:30:28]

The CQ bit was checked by fs@opera.com

[fs, 2016-11-15 16:30:28]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/2484153003/#ps60001
(title: "Rebase; fix comments; findElement")

[commit-bot: I haz the power, 2016-11-15 16:30:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-15 16:36:06]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-15 16:38:25]

Description was changed from

==========
Use an SVGElementProxy in ReferenceClipPathOperation

This transforms ReferenceClipPathOperation into using the SVGElementProxy
mechanism. Currently only for PaintLayer clients.

PaintLayerFilterInfo is generalized to PaintLayerResourceInfo and used as
the proxy/resource client for the 'clip-path' property. This enables change
notifications to flow back to the PaintLayer from the <clipPath> subtree.

The SVGElementProxySet is made a little bit generic by moving it to
SVGElementRareData, however it's still only made available for the few element
types that are used with it.

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use an SVGElementProxy in ReferenceClipPathOperation

This transforms ReferenceClipPathOperation into using the SVGElementProxy
mechanism. Currently only for PaintLayer clients.

PaintLayerFilterInfo is generalized to PaintLayerResourceInfo and used as
the proxy/resource client for the 'clip-path' property. This enables change
notifications to flow back to the PaintLayer from the <clipPath> subtree.

The SVGElementProxySet is made a little bit generic by moving it to
SVGElementRareData, however it's still only made available for the few element
types that are used with it.

BUG=391604
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2016-11-15 16:38:26]

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