Add StyleDifference::needsVisualRectUpdate

Add StyleDifference::needsVisualRectUpdate

It's set when a style change will change visual rect (without changing
layout, as visual rect update is implied by layout change).

BTW optimized paint invalidation of box with resizer. We no longer
force full paint invalidation for box with resizer when resized.
The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
if needed.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2761863002
Cr-Commit-Position: refs/heads/master@{#458313}
Committed: https://chromium.googlesource.com/chromium/src/+/4e0d0f9ddd68e59dccda0a8c9a096c1813a1bd44

[Xianzhu, 2017-03-20 17:12:56]

Description was changed from

==========
Add StyleDifference::needsVisualRectUpdate

It's set when a style change will change visual rect (without changing
layout, as visual rect update is implied by layout change).

BTW optimized paint invalidation of box with resizer. We no longer
force full paint invalidation for box with resizer when resized.
The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
if needed.
==========

to

==========
Add StyleDifference::needsVisualRectUpdate

It's set when a style change will change visual rect (without changing
layout, as visual rect update is implied by layout change).

BTW optimized paint invalidation of box with resizer. We no longer
force full paint invalidation for box with resizer when resized.
The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
if needed.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-03-20 17:13:19]

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

[commit-bot: I haz the power, 2017-03-20 17:13:51]

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-20 18:53:56]

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

[commit-bot: I haz the power, 2017-03-20 18:53:57]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2017-03-20 19:53:16]

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

[commit-bot: I haz the power, 2017-03-20 19:53:50]

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-20 20:36:32]

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

[pdr., 2017-03-20 21:23:04]

https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1015: if (visibility() !=
other.visibility())
Where was this logic before this patch? Can we remove the old code that was
detecting visibility changes?

https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1019: if (resize() !=
other.resize())
The old code seemed to check against RESIZE_NONE whereas this checks against
changes. Is that the optimization you're referring to in the change description?

[Xianzhu, 2017-03-20 22:26:35]

https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1015: if (visibility() !=
other.visibility())
On 2017/03/20 21:23:04, pdr. wrote:
> Where was this logic before this patch? Can we remove the old code that was
> detecting visibility changes?

This is to make https://codereview.chromium.org/2732573003/ work. Previously
visibility change only caused needsPaintInvalidationObject which just caused
shouldDoFullPaintInvalidationWithoutGeometryChange().

Visibility affects visual rects through the visibility conditions in
localVisualRect() methods.

In the new patch I removed visibility change condition from
diffNeedsPaintInvalidationObject because visibility changes cause either empty
or non-empty visual rects, and diffNeedsVisualRectUpdate() is good.

https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1019: if (resize() !=
other.resize())
On 2017/03/20 21:23:04, pdr. wrote:
> The old code seemed to check against RESIZE_NONE whereas this checks against
> changes. Is that the optimization you're referring to in the change
description?

The old code always invalidates the whole box container resizer when the box is
resized. With this change we'll just incrementally invalidate the box if needed
and invalidate the resizer if needed.

paint/invalidation/textarea-appearance-none-resize-handle-expected.txt shows the
effect of the optimization.

Here I don't check RESIZE_NONE for safety of future changes about visual size of
resizer according to different resizer styles.

[pdr., 2017-03-20 22:29:17]

On 2017/03/20 at 22:26:35, wangxianzhu wrote:
>
https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):
> 
>
https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/style/ComputedStyle.cpp:1015: if (visibility()
!= other.visibility())
> On 2017/03/20 21:23:04, pdr. wrote:
> > Where was this logic before this patch? Can we remove the old code that was
> > detecting visibility changes?
> 
> This is to make https://codereview.chromium.org/2732573003/ work. Previously
visibility change only caused needsPaintInvalidationObject which just caused
shouldDoFullPaintInvalidationWithoutGeometryChange().
> 
> Visibility affects visual rects through the visibility conditions in
localVisualRect() methods.
> 
> In the new patch I removed visibility change condition from
diffNeedsPaintInvalidationObject because visibility changes cause either empty
or non-empty visual rects, and diffNeedsVisualRectUpdate() is good.
> 
>
https://codereview.chromium.org/2761863002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/style/ComputedStyle.cpp:1019: if (resize() !=
other.resize())
> On 2017/03/20 21:23:04, pdr. wrote:
> > The old code seemed to check against RESIZE_NONE whereas this checks against
> > changes. Is that the optimization you're referring to in the change
description?
> 
> The old code always invalidates the whole box container resizer when the box
is resized. With this change we'll just incrementally invalidate the box if
needed and invalidate the resizer if needed.
> 
> paint/invalidation/textarea-appearance-none-resize-handle-expected.txt shows
the effect of the optimization.
> 
> Here I don't check RESIZE_NONE for safety of future changes about visual size
of resizer according to different resizer styles.

LGTM

[Xianzhu, 2017-03-20 22:34:16]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-03-20 22:34:58]

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

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

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

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

[wkorman, 2017-03-21 00:20:03]

On 2017/03/20 23:57:57, commit-bot: I haz the power wrote:
> 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_...)

Just FYI I will wait for this patch to land before I submit
http://crrev.com/2763643003 (and will update my patch to reflect these
revisions).

[Xianzhu, 2017-03-21 01:06:25]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2017-03-21 01:06:26]

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/2761863002/#ps80001
(title: "Still keep visibility change detection for full paint invalidation.
Will optimize in later CLs")

[commit-bot: I haz the power, 2017-03-21 01:07: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-21 01:57:55]

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

[commit-bot: I haz the power, 2017-03-21 01:57:56]

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

[pdr., 2017-03-21 02:08:56]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2017-03-21 02:09:33]

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-21 03:08:04]

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

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

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[pdr., 2017-03-21 03:11:14]

On 2017/03/21 at 03:08:05, commit-bot wrote:
> Try jobs failed on following builders:
>   linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

I've filed https://crbug.com/703478 for this. For now, I guess re-click CQ? :/

[Xianzhu, 2017-03-21 03:43:23]

The CQ bit was checked by wangxianzhu@chromium.org

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

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-21 05:00:55]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490067803301610,
"parent_rev": "1025d669c602e25b072bcc1632cfe4845af1d9f4", "commit_rev":
"4e0d0f9ddd68e59dccda0a8c9a096c1813a1bd44"}

[commit-bot: I haz the power, 2017-03-21 05:01:34]

Description was changed from

==========
Add StyleDifference::needsVisualRectUpdate

It's set when a style change will change visual rect (without changing
layout, as visual rect update is implied by layout change).

BTW optimized paint invalidation of box with resizer. We no longer
force full paint invalidation for box with resizer when resized.
The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
if needed.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Add StyleDifference::needsVisualRectUpdate

It's set when a style change will change visual rect (without changing
layout, as visual rect update is implied by layout change).

BTW optimized paint invalidation of box with resizer. We no longer
force full paint invalidation for box with resizer when resized.
The resizer will be invlaidated in PaintInvalidationCapableScrollableArea
if needed.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2761863002
Cr-Commit-Position: refs/heads/master@{#458313}
Committed:
https://chromium.googlesource.com/chromium/src/+/4e0d0f9ddd68e59dccda0a8c9a09...
==========

[commit-bot: I haz the power, 2017-03-21 05:01:35]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/4e0d0f9ddd68e59dccda0a8c9a09...

[Xianzhu, 2017-03-27 16:07:49]

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

The reason for reverting is: BUG=702748.