Invalidate scrolling contents on scrolling contents layer only

Invalidate scrolling contents on scrolling contents layer only

This is required for synchronized painting to avoid assertion failure
caused by extra invalidation on the scrolling container layer (which has
finished painting) when we paint the scrolling contents layer.

BUG=536999
Committed: https://crrev.com/933aa408647976a45828676e6477fa5059d04e60
Cr-Commit-Position: refs/heads/master@{#354589}

[Xianzhu, 2015-10-15 23:50:50]

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

[Xianzhu, 2015-10-15 23:50:51]

About tests:
https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel...

The changes are all about removed object paint invalidation of scrolling
contents on container layers.

[chrishtr, 2015-10-16 00:55:38]

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:411:
compositedLayerMapping->invalidateDisplayItemClientOnScrollingContentsLayer(displayItemClient,
invalidationReason, previousPaintInvalidationRect, newPaintInvalidationRect);
This conditional doesn't need to be protected by
SlimmingPaintSynchronizedPaintingEnabled? Why is it correct to not invalidate
even without the flag?

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:2049:
// FIXME: need to split out paint invalidations for the background.
TODO(wangxianzhu). Also represent in a bug blocking the tracking one so we don't
forget.

[Xianzhu, 2015-10-16 17:38:05]

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:411:
compositedLayerMapping->invalidateDisplayItemClientOnScrollingContentsLayer(displayItemClient,
invalidationReason, previousPaintInvalidationRect, newPaintInvalidationRect);
On 2015/10/16 00:55:38, chrishtr wrote:
> This conditional doesn't need to be protected by
> SlimmingPaintSynchronizedPaintingEnabled? Why is it correct to not invalidate
> even without the flag?

A scrolling content is on the scrolling contents layer, so it's correct not to
invalidate the scrolling container layer, regardless of the flag.

Invalidating on both layers is a bug (crbug.com/416535) which is not fixed
because of crbug.com/429845 and because it is not a big issue. With this CL, we
still invalidate both layers if the invalidation is issued by the container
(while the rect may be for contents but we can't distinguish) to avoid
crbug.com/429845. If an invalidation is issued by scrolling content, we
invalidate the scrolling contents layer only.

crbug.com/416535 doesn't apply for spv2.

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/1408913002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:2049:
// FIXME: need to split out paint invalidations for the background.
On 2015/10/16 00:55:38, chrishtr wrote:
> TODO(wangxianzhu). Also represent in a bug blocking the tracking one so we
don't
> forget.

Actually the FIXME won't apply for spv2 so I removed it. The FIXME was to
invalidate background on the background layer (if any) only.

[chrishtr, 2015-10-16 17:44:14]

Ok let's see how it goes then.

https://codereview.chromium.org/1408913002/diff/50001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/1408913002/diff/50001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:410: if
(this->displayItemClient() != displayItemClient.displayItemClient() && isBox()
&& toLayoutBox(this)->usesCompositedScrolling())
Add a comment explaining this conditional.

[Xianzhu, 2015-10-16 17:52:06]

https://codereview.chromium.org/1408913002/diff/50001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (right):

https://codereview.chromium.org/1408913002/diff/50001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:410: if
(this->displayItemClient() != displayItemClient.displayItemClient() && isBox()
&& toLayoutBox(this)->usesCompositedScrolling())
On 2015/10/16 17:44:13, chrishtr wrote:
> Add a comment explaining this conditional.

Done.

[chrishtr, 2015-10-16 17:54:49]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-10-16 17:54:49]

lgtm

[commit-bot: I haz the power, 2015-10-16 17:55:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1408913002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1408913002/70001

[commit-bot: I haz the power, 2015-10-16 20:59:36]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2015-10-16 21:00:41]

Patchset 5 (id:??) landed as
https://crrev.com/933aa408647976a45828676e6477fa5059d04e60
Cr-Commit-Position: refs/heads/master@{#354589}

[Xianzhu, 2015-12-18 23:23:31]

A revert of this CL (patchset #5 id:70001) has been created in
https://codereview.chromium.org/1533163002/ by wangxianzhu@chromium.org.

The reason for reverting is: The patch caused under-invalidation of some changed
content display items painting themselves on the main layer.

The patch was needed by synchronized painting in paint-invalidation-during-paint
mode. Now we plan to implement the mode in slimming paint v2 only which doesn't
have the main layer/contents layer problem..