Fix caret paint invalidation issues

Fix caret paint invalidation issues

- We should save CaretDisplayItemClient::m_previousLayoutBlock only if
  it has not been saved since the last paint invalidation, because we
  use it to invalidate the caret in the layout block of the previous
  paint invalidation.

- Caret painted output can change paint offset without visual rect
  change. Invalidate display item client if the block is doing full
  paint invalidation.

BUG=699044, 698662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2736213002
Cr-Commit-Position: refs/heads/master@{#455657}
Committed: https://chromium.googlesource.com/chromium/src/+/0f9cfc4753603c7b19d8ea434ddd6326d5417894

[Xianzhu, 2017-03-08 05:42:03]

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

[commit-bot: I haz the power, 2017-03-08 05:42:15]

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-08 05:55:26]

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

[commit-bot: I haz the power, 2017-03-08 05:55:30]

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-08 06:41:03]

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

[commit-bot: I haz the power, 2017-03-08 06:41:06]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2017-03-08 17:07:05]

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

[Xianzhu, 2017-03-08 17:07:06]

This CL replaces https://codereview.chromium.org/2737043002/.

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/CaretDisplayItemClientTest.cpp
(right):

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretDisplayItemClientTest.cpp:56:
document().view()->updateLifecycleToLayoutClean();
This is to reproduce the bug situation about partial lifecycle update. The tests
would fail with it without the other changes in this CL.

[chrishtr, 2017-03-08 17:36:06]

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/invalidation/caret-change-paint-offset-keep-visual.html
(right):

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/invalidation/caret-change-paint-offset-keep-visual.html:11:
container.scrollLeft = 56;
Does this layout test exercise the code about
isImmediateFullPaintInvalidationReason?

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp (right):

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp:121: // only
if they have not been saved since the last paint invalidation.
This conditional is needed because there may be multiple layouts between
paint invalidations, right? If so, worth commenting to that effect.

And it seems this is unique to carets because they can be reparented without
an eager invalidation?

[Xianzhu, 2017-03-08 18:48:37]

Description was changed from

==========
Fix caret paint invalidation issues

- We should save CaretDisplayItemClient::m_previousLayoutBlock only if
  it has not been saved since the last paint invalidation, because we
  use it to invalidate the caret in the layout block of the previous
  paint invalidation.

- Caret painted output can change paint offset without visual rect
  change. Invalidate display item client if the block is doing full
  paint invalidation.

BUG=699044,698662
==========

to

==========
Fix caret paint invalidation issues

- We should save CaretDisplayItemClient::m_previousLayoutBlock only if
  it has not been saved since the last paint invalidation, because we
  use it to invalidate the caret in the layout block of the previous
  paint invalidation.

- Caret painted output can change paint offset without visual rect
  change. Invalidate display item client if the block is doing full
  paint invalidation.

BUG=699044,698662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-03-08 19:38:22]

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

[Xianzhu, 2017-03-08 19:38:22]

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/invalidation/caret-change-paint-offset-keep-visual.html
(right):

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/invalidation/caret-change-paint-offset-keep-visual.html:11:
container.scrollLeft = 56;
On 2017/03/08 17:36:06, chrishtr wrote:
> Does this layout test exercise the code about
> isImmediateFullPaintInvalidationReason?

Yes. Line 11 and 12 creates a situation that the input (whose inner editor is
the containing block of the caret) doesn't change its visual, but it needs
repaint because its paint offset changes within its paint invalidation
container. Our paint offset change detection logic will let the containing
block's invalidatePaintIfNeeded() return a full invalidation reason.

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp (right):

https://codereview.chromium.org/2736213002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretDisplayItemClient.cpp:121: // only
if they have not been saved since the last paint invalidation.
On 2017/03/08 17:36:06, chrishtr wrote:
> This conditional is needed because there may be multiple layouts between
> paint invalidations, right? If so, worth commenting to that effect.
> 

Yes. Added comment.

> And it seems this is unique to carets because they can be reparented without
> an eager invalidation?

Yes. Carets are also unique because they are managed by frames, but the
containing blocks invalidate and paint them.

https://codereview.chromium.org/2736213002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/CaretDisplayItemClient.h (left):

https://codereview.chromium.org/2736213002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretDisplayItemClient.h:80:
PaintInvalidationReason layoutBlockPaintInvalidationReason);
This parameter is removed because it just indicates the reason of the
layoutBlock's display item client invalidation reason, not the raster
invalidation reason. We used the parameter to skip caret raster invalidation
which had issue of under-invalidation of raster of caret if caret is moved and
the containing block just invalidated display item client. The new test
move-caret-in-container-change-paint-offset-keep-visual.html could reproduce the
issue.

(Unrelated to this CL: I'm planning to remove the returning value of
ObjectPaintInvalidator::invalidatePaintIfNeeded() because its meaning is not
clear. Will use LayoutObject status and context at callsites instead.)

[chrishtr, 2017-03-08 19:44:46]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-08 19:44:47]

lgtm

[commit-bot: I haz the power, 2017-03-08 19:45:22]

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 21:11:50]

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

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

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_...)

[Xianzhu, 2017-03-08 21:46:54]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2017-03-08 21:46:55]

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

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

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 21:50:14]

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

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

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Xianzhu, 2017-03-08 21:53:22]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2017-03-08 21:53:22]

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

[commit-bot: I haz the power, 2017-03-08 21:53: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-08 22:37:49]

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

[commit-bot: I haz the power, 2017-03-08 22:37:50]

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

[Xianzhu, 2017-03-08 22:38:47]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-03-08 22:39:15]

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 23:49:56]

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

[commit-bot: I haz the power, 2017-03-08 23:49:57]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2017-03-09 00:14:03]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2017-03-09 00:14:04]

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

[commit-bot: I haz the power, 2017-03-09 00:14:32]

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

[Xianzhu, 2017-03-09 00:16:33]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2017-03-09 02:22:00]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-03-09 02:22:48]

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-09 03:47:36]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1489026120089780,
"parent_rev": "6ac6bd671fbbbe77fb3adc2847e0102cb995c57c", "commit_rev":
"0f9cfc4753603c7b19d8ea434ddd6326d5417894"}

[commit-bot: I haz the power, 2017-03-09 03:48:16]

Description was changed from

==========
Fix caret paint invalidation issues

- We should save CaretDisplayItemClient::m_previousLayoutBlock only if
  it has not been saved since the last paint invalidation, because we
  use it to invalidate the caret in the layout block of the previous
  paint invalidation.

- Caret painted output can change paint offset without visual rect
  change. Invalidate display item client if the block is doing full
  paint invalidation.

BUG=699044,698662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Fix caret paint invalidation issues

- We should save CaretDisplayItemClient::m_previousLayoutBlock only if
  it has not been saved since the last paint invalidation, because we
  use it to invalidate the caret in the layout block of the previous
  paint invalidation.

- Caret painted output can change paint offset without visual rect
  change. Invalidate display item client if the block is doing full
  paint invalidation.

BUG=699044,698662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2736213002
Cr-Commit-Position: refs/heads/master@{#455657}
Committed:
https://chromium.googlesource.com/chromium/src/+/0f9cfc4753603c7b19d8ea434ddd...
==========

[commit-bot: I haz the power, 2017-03-09 03:48:17]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/0f9cfc4753603c7b19d8ea434ddd...