Remove dependency on m_previousPositionFromPaintInvalidationBacking for spv2

Remove dependency on m_previousPositionFromPaintInvalidationBacking for spv2

- Set layer needsRepaint when any paint invalidation flag is set, so
  that we can check paint offset changes during paint.
- Always use paintsWithTransform in paintLayer to avoid paint offset
  changes of descendants when the layer is moved-only.

BUG=508383, 510908
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=202486

[pdr., 2015-09-13 23:29:58]

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

[pdr., 2015-09-13 23:29:59]

Mind if I fold this patch into one I'm working on to not paint non-comp scroll
children?

[Xianzhu, 2015-09-14 17:32:06]

On 2015/09/13 23:29:59, pdr wrote:
> Mind if I fold this patch into one I'm working on to not paint non-comp scroll
> children?

Not at all. Please go ahead.

Fwiw the change in DeprecatedPaintLayerPainter.cpp lets all layer painting go
through the transform path (making every DPL a transform node), so that contents
of layers will be painted in coordinates of the layer and don't need repaint
when the position of the layer changes. This is mainly to resolve the issue of
repaint of LayoutInlines and LayoutTexts on paint offset change (see "Details"
below). However, try bot results showed many failures. I haven't looked into
them yet.

Details: Before this change, location changes of line boxes are detected by
LayoutInlines and LayoutTexts in LayoutObject::invalidatePaintIfNeeded(), and
line boxes will be invalidated in LayoutInline::invalidateDisplayItems() and
LayoutText::invalidateDisplayItems(). With this change, though we have
paint-offset-change detection in LayoutObjectDrawingRecorder, it doesn't cover
line boxes because they are not LayoutObjects. Making all layer painting go
through the transform path can avoid this issue. Location change of lineboxes
caused by line flow layout can still be handled by normal logic for
layout-caused invalidations.

Perhaps two choices:
1. make every DPL a transform node and fix failures caused by this;
2. keep the current DPL painting logic, and add location/paint-offset change
detection for line boxes.

1 seems better to reduce invalidations, but I'm not sure if it would cause
trouble of cc layerization.

[Xianzhu, 2015-09-14 19:49:02]

The following solution might work for paint-offset-change detection for line
boxes:

1. Add another constructor of LayoutObjectDrawingRecorder accepting InlineBox as
the parameter, and use the same logic as the constructor for LayoutObject.
2. This patch should work without the change to DeprecatedPaintLayerPainter.cpp,
to remove dependency to m_previousPositionFromPaintInvalidationBacking.
3. Then we can use the space for m_previousPositionFromPaintInvalidationBacking
to save m_previousPaintOffset. For InlineBoxes, we save and compare paint offset
of the containing LayoutObject.

I can work on this.

[Xianzhu, 2015-09-15 01:27:10]

According to the try bot results, this CL works better on
https://codereview.chromium.org/1340133003/. Will look into the remaining
failures which may be caused by other non-LayoutObject display item clients.

[pdr., 2015-09-15 03:03:59]

https://codereview.chromium.org/1312423006/diff/80001/Source/core/layout/Layo...
File Source/core/layout/LayoutObject.cpp (left):

https://codereview.chromium.org/1312423006/diff/80001/Source/core/layout/Layo...
Source/core/layout/LayoutObject.cpp:1202: if
(RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
Restoring this (instead of the setNeedsRepaint below) fixes a couple failures.

[Xianzhu, 2015-09-17 03:51:39]

There seems no real failures now. The paint-invalidation-object mismatches are
because
- Orders of paint invalidations are different (location-changes are invalidated
later);
- We didn't track paint offset invalidations.
The latest patch added tracking of paint offset invalidations.

[Xianzhu, 2015-09-17 18:21:34]

Ptal.

I have checked the results of all failures, and there seemed no real painting
failures. I didn't update the tests because we are still trying to find a better
way for expectations of these tests. I added failure expectations of them
instead.

[pdr., 2015-09-17 19:04:58]

This is great and with this patch we are one line away from non-composited
scrolling without over-invalidation.

https://codereview.chromium.org/1312423006/diff/180001/Source/core/layout/Lay...
File Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1312423006/diff/180001/Source/core/layout/Lay...
Source/core/layout/LayoutObject.cpp:3135: // Setting layer-needs-repaint doesn't
mean we'll fully repaint the layer, but
I don't think we need this since we have the same call in
invalidateDisplayItemClients/invalidateDisplayItemClient. Which test requires
this?

[Xianzhu, 2015-09-17 19:16:50]

https://codereview.chromium.org/1312423006/diff/180001/Source/core/layout/Lay...
File Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1312423006/diff/180001/Source/core/layout/Lay...
Source/core/layout/LayoutObject.cpp:3135: // Setting layer-needs-repaint doesn't
mean we'll fully repaint the layer, but
On 2015/09/17 19:04:57, pdr wrote:
> I don't think we need this since we have the same call in
> invalidateDisplayItemClients/invalidateDisplayItemClient. Which test requires
> this?

This is required by subsequence caching. Many tests will fail without this. This
patch removed one source of paint invalidations during paint invalidation stage:
change of positionFromPaintInvalidation which no longer causes
invalidateDisplayItemClients/invalidateDisplayItemClient.

In next steps of spv2 paint invalidation, I will move invalidation checking (on
change of LayoutBox geometry) into painting stage, which also requires this
change.

[pdr., 2015-09-17 19:46:35]

On 2015/09/17 at 19:16:50, wangxianzhu wrote:
>
https://codereview.chromium.org/1312423006/diff/180001/Source/core/layout/Lay...
> File Source/core/layout/LayoutObject.cpp (right):
> 
>
https://codereview.chromium.org/1312423006/diff/180001/Source/core/layout/Lay...
> Source/core/layout/LayoutObject.cpp:3135: // Setting layer-needs-repaint
doesn't mean we'll fully repaint the layer, but
> On 2015/09/17 19:04:57, pdr wrote:
> > I don't think we need this since we have the same call in
> > invalidateDisplayItemClients/invalidateDisplayItemClient. Which test
requires
> > this?
> 
> This is required by subsequence caching. Many tests will fail without this.
This patch removed one source of paint invalidations during paint invalidation
stage: change of positionFromPaintInvalidation which no longer causes
invalidateDisplayItemClients/invalidateDisplayItemClient.
> 
> In next steps of spv2 paint invalidation, I will move invalidation checking
(on change of LayoutBox geometry) into painting stage, which also requires this
change.

LGTM

[Xianzhu, 2015-09-17 20:09:47]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2015-09-17 20:09:58]

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

[commit-bot: I haz the power, 2015-09-17 21:10:58]

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

[commit-bot: I haz the power, 2015-09-17 21:10:59]

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

[Xianzhu, 2015-09-17 21:24:19]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-09-17 21:24:21]

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/1312423006/#ps200001
(title: "Update TestExpectations")

[commit-bot: I haz the power, 2015-09-17 21:24:47]

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

[commit-bot: I haz the power, 2015-09-18 00:56:57]

Committed patchset #11 (id:200001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=202486