[SPInvalidation] Update paint properties when SVG resource change

[SPInvalidation] Update paint properties when SVG resource change

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

Committed: https://crrev.com/828836bbf1f4c4e0c4f5ffbe5d4c47058f4f5010
Cr-Commit-Position: refs/heads/master@{#439329}

[Xianzhu, 2016-12-16 19:23:26]

Description was changed from

==========
[SPInvalidation] Update paint properties when SVG resource change

BUG=645667
==========

to

==========
[SPInvalidation] Update paint properties when SVG resource change

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

[Xianzhu, 2016-12-16 19:23:52]

wangxianzhu@chromium.org changed reviewers:
+ pdr@chromium.org

[Xianzhu, 2016-12-16 19:27:57]

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

[commit-bot: I haz the power, 2016-12-16 19:29:03]

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

[pdr., 2016-12-16 21:13:37]

LGTM with a question

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:181: //
Invalidate paint properties to update effects if any.
This invalidation is very different from the one below. Are both required for
the test to pass?

I think both this change and the ones in PaintLayerResourceInfo make sense, just
want to make sure a new test passes.

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerResourceInfo.cpp (right):

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerResourceInfo.cpp:54:
layoutObject->setNeedsPaintPropertyUpdate();
Can you add a comment here and below:
// The effect paint property nodes depend on SVG filters so we need
// to update these properties when filter resources change.

[Xianzhu, 2016-12-16 22:57:41]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-16 22:57:42]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2581243003/#ps20001
(title: "Add a test for SVG referencing filter resource")

[commit-bot: I haz the power, 2016-12-16 22:57:49]

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

[Xianzhu, 2016-12-16 22:58:08]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-16 23:00:05]

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:181: //
Invalidate paint properties to update effects if any.
On 2016/12/16 21:13:37, pdr. wrote:
> This invalidation is very different from the one below. Are both required for
> the test to pass?
>

Added a test for this. However the test doesn't fail in
SlimmingPaintInvalidation mode without this patch because of a bug of
PaintPropertyTreeBuilder that it fails to create an effect node for an SVG
element referencing a filter resource (so FindPaintPropertiesToUpdate doesn't
find the under-invalidation). This doesn't block SlimmingPaintInvalidation
because we can still paint the filter correctly. Will file a bug for this.
 
> I think both this change and the ones in PaintLayerResourceInfo make sense,
just
> want to make sure a new test passes.

[Xianzhu, 2016-12-16 23:01:43]

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerResourceInfo.cpp (right):

https://codereview.chromium.org/2581243003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerResourceInfo.cpp:54:
layoutObject->setNeedsPaintPropertyUpdate();
On 2016/12/16 21:13:37, pdr. wrote:
> Can you add a comment here and below:
> // The effect paint property nodes depend on SVG filters so we need
> // to update these properties when filter resources change.

Done.

[Xianzhu, 2016-12-16 23:02:42]

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

[Xianzhu, 2016-12-16 23:02:46]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-16 23:02:46]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2581243003/#ps40001
(title: "Add comments")

[commit-bot: I haz the power, 2016-12-16 23:03:38]

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-12-17 01:16:34]

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

[commit-bot: I haz the power, 2016-12-17 01:16:35]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[pdr., 2016-12-17 01:39:56]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-12-17 01:40:39]

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-12-17 04:15:19]

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

[commit-bot: I haz the power, 2016-12-17 04:15:26]

Failed to apply patch for third_party/WebKit/LayoutTests/TestExpectations:
While running git apply --index -p1;
  error: patch failed: third_party/WebKit/LayoutTests/TestExpectations:105
  error: third_party/WebKit/LayoutTests/TestExpectations: patch does not apply

Patch:       third_party/WebKit/LayoutTests/TestExpectations
Index: third_party/WebKit/LayoutTests/TestExpectations
diff --git a/third_party/WebKit/LayoutTests/TestExpectations
b/third_party/WebKit/LayoutTests/TestExpectations
index
b2b35bb836395c925bf502e005874a66fc3cc24d..361085e47243c6f6471299a55c022cb3920c7041
100644
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -105,6 +105,8 @@ crbug.com/646176
virtual/spinvalidation/compositing/contents-opaque/visibility-h
 crbug.com/646176
virtual/spinvalidation/compositing/composited-iframe-hidden.html [ Pass ]
 crbug.com/646176 virtual/spinvalidation/printing/fixed-positioned-scrolled.html
[ Pass ]
 crbug.com/646176 virtual/spinvalidation/printing/overflow-auto.html [ Pass ]
+crbug.com/646176
virtual/spinvalidation/css3/filters/effect-reference-rename.html [ Pass ]
+crbug.com/646176
virtual/spinvalidation/css3/filters/effect-reference-rename-2.html [ Pass ]
 
 crbug.com/645667
virtual/spinvalidation/paint/invalidation/filter-repaint-accelerated-on-accelerated-filter.html
[ Crash ]
 crbug.com/645667
virtual/spinvalidation/paint/invalidation/compositing/resize-repaint.html [
Crash ]

[Xianzhu, 2016-12-17 04:20:46]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-17 04:20:47]

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

[commit-bot: I haz the power, 2016-12-17 04:21:05]

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-12-17 06:14:50]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481948446825590,
"parent_rev": "b400f6eb73a7119f82aeb10c3dfc8fc83a1889a3", "commit_rev":
"2ecba49c910acb5bbd389f9d251755134a412704"}

[commit-bot: I haz the power, 2016-12-17 06:15:27]

Description was changed from

==========
[SPInvalidation] Update paint properties when SVG resource change

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

to

==========
[SPInvalidation] Update paint properties when SVG resource change

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

Review-Url: https://codereview.chromium.org/2581243003
==========

[commit-bot: I haz the power, 2016-12-17 06:15:29]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-17 06:17:46]

Description was changed from

==========
[SPInvalidation] Update paint properties when SVG resource change

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

Review-Url: https://codereview.chromium.org/2581243003
==========

to

==========
[SPInvalidation] Update paint properties when SVG resource change

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

Committed: https://crrev.com/828836bbf1f4c4e0c4f5ffbe5d4c47058f4f5010
Cr-Commit-Position: refs/heads/master@{#439329}
==========

[commit-bot: I haz the power, 2016-12-17 06:17:47]

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