[SPv2] Fix property node invalidation for CSS masks

[SPv2] Fix property node invalidation for CSS masks

When CSS masks were implemented a while back, invalidation wasn't implemented
at all. This CL is an amendment and also added under-invalidation checks.

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

Review-Url: https://codereview.chromium.org/2736463002
Cr-Commit-Position: refs/heads/master@{#455317}
Committed: https://chromium.googlesource.com/chromium/src/+/b381061316947dbceed438908696566dc32fc44e

[trchen, 2017-03-04 00:52:58]

Description was changed from

==========
[SPv2] Fix property node invalidation for CSS masks

When CSS masks were implemented a while back, invalidation wasn't implemented
at all. This CL is an amendment and also added under-invalidation checks.

BUG=696811
==========

to

==========
[SPv2] Fix property node invalidation for CSS masks

When CSS masks were implemented a while back, invalidation wasn't implemented
at all. This CL is an amendment and also added under-invalidation checks.

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

[trchen, 2017-03-04 00:56:19]

Patchset #1 (id:1) has been deleted

[trchen, 2017-03-04 00:56:46]

trchen@chromium.org changed reviewers:
+ chrishtr@chromium.org, wangxianzhu@chromium.org

[trchen, 2017-03-04 00:57:02]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-04 00:57:13]

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

[Xianzhu, 2017-03-04 01:15:59]

As this affects not only paint property update but also paint invalidation, can
you also add a paint invalidation test?

https://codereview.chromium.org/2736463002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2736463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:2288: // the size of the
box.
Does this apply to SPv1?

[trchen, 2017-03-04 01:24:09]

On 2017/03/04 01:15:59, Xianzhu wrote:
> As this affects not only paint property update but also paint invalidation,
can
> you also add a paint invalidation test?

The same test catches paint invalidation issue too.
The DCHECK_EQ here
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...
fails without that change.

>
https://codereview.chromium.org/2736463002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):
> 
>
https://codereview.chromium.org/2736463002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutBox.cpp:2288: // the size of the
> box.
> Does this apply to SPv1?

Yes for mask, no for clip-path. Clip-path in SPv1 uses special meta display
items.

[Xianzhu, 2017-03-04 02:04:37]

lgtm.

Please update the description about the fact that this also affects SPv1, and
merge it into M58.

[trchen, 2017-03-04 02:42:29]

On 2017/03/04 02:04:37, Xianzhu wrote:
> lgtm.
> 
> Please update the description about the fact that this also affects SPv1, and
> merge it into M58.

For clarification, this CL won't change SPv1 behavior. For SPv1 the conditions
were juggled around but end result is still the same.

Orig: (hasClipRelatedProperty() || hasControlClip() || hasMask())
= hasClip() || hasOverflowClip() || hasClipPath() || style()->containsPaint() ||
hasControlClip() || hasMask()

With this CL: (hasMask() || hasClipPath()) || (shouldClipOverflow() ||
hasClip())
= hasMask() || hasClipPath() || hasOverflowClip() || styleRef().containsPaint()
|| hasControlClip() || hasClip()

[commit-bot: I haz the power, 2017-03-04 02:55:24]

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

[commit-bot: I haz the power, 2017-03-04 02:55:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[trchen, 2017-03-04 03:08:10]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-04 03:08:21]

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, 2017-03-04 04:38:35]

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

[commit-bot: I haz the power, 2017-03-04 04:38:36]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[trchen, 2017-03-06 22:39:18]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-06 22:39:55]

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

[trchen, 2017-03-06 22:41:17]

On 2017/03/04 02:42:29, trchen wrote:
> On 2017/03/04 02:04:37, Xianzhu wrote:
> > lgtm.
> > 
> > Please update the description about the fact that this also affects SPv1,
and
> > merge it into M58.
> 
> For clarification, this CL won't change SPv1 behavior. For SPv1 the conditions
> were juggled around but end result is still the same.
> 
> Orig: (hasClipRelatedProperty() || hasControlClip() || hasMask())
> = hasClip() || hasOverflowClip() || hasClipPath() || style()->containsPaint()
||
> hasControlClip() || hasMask()
> 
> With this CL: (hasMask() || hasClipPath()) || (shouldClipOverflow() ||
> hasClip())
> = hasMask() || hasClipPath() || hasOverflowClip() ||
styleRef().containsPaint()
> || hasControlClip() || hasClip()

Alright I was wrong. shouldClipOverflow() is overridden by SVGRoot and the
change does introduce subtle over-invalidation for SVGRoots. Changing to a
version that's closer to the original. :/

[commit-bot: I haz the power, 2017-03-07 00:46:51]

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

[commit-bot: I haz the power, 2017-03-07 00:46:52]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[trchen, 2017-03-07 02:30:24]

The CQ bit was checked by trchen@chromium.org

[trchen, 2017-03-07 02:30:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2736463002/#ps40001
(title: "Avoid SVGRoot can of worms")

[commit-bot: I haz the power, 2017-03-07 02:31:45]

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, 2017-03-07 05:46:58]

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

[commit-bot: I haz the power, 2017-03-07 05:46:59]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[trchen, 2017-03-07 21:09:56]

The CQ bit was checked by trchen@chromium.org

[commit-bot: I haz the power, 2017-03-07 21:11:00]

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, 2017-03-08 01:10:24]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488920996750040,
"parent_rev": "e4ff7160c06948baf8dd3d3141c00c8cee40fc3b", "commit_rev":
"b381061316947dbceed438908696566dc32fc44e"}

[commit-bot: I haz the power, 2017-03-08 01:12:11]

Description was changed from

==========
[SPv2] Fix property node invalidation for CSS masks

When CSS masks were implemented a while back, invalidation wasn't implemented
at all. This CL is an amendment and also added under-invalidation checks.

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

to

==========
[SPv2] Fix property node invalidation for CSS masks

When CSS masks were implemented a while back, invalidation wasn't implemented
at all. This CL is an amendment and also added under-invalidation checks.

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

Review-Url: https://codereview.chromium.org/2736463002
Cr-Commit-Position: refs/heads/master@{#455317}
Committed:
https://chromium.googlesource.com/chromium/src/+/b381061316947dbceed438908696...
==========

[commit-bot: I haz the power, 2017-03-08 01:12:12]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b381061316947dbceed438908696...