Skip paint property update and visual rect update if no geometry change

Skip paint property update and visual rect update if no geometry change

This avoids unnecessary visual rect computation and update of paint
offset related paint properties.

BUG=697081, 685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2732573003
Cr-Commit-Position: refs/heads/master@{#461348}
Committed: https://chromium.googlesource.com/chromium/src/+/81f28ed19f771fe93406496d1d23da137abcd602

[Xianzhu, 2017-03-03 04:18:07]

Description was changed from

==========
Skip paint property update and visual rect update if no geometry change

BUG=685179
==========

to

==========
Skip paint property update and visual rect update if no geometry change

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

[Xianzhu, 2017-03-03 06:54:47]

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

[commit-bot: I haz the power, 2017-03-03 06:55:00]

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-03 06:57:18]

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

[commit-bot: I haz the power, 2017-03-03 06:57:19]

Dry run: 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-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[Xianzhu, 2017-03-03 07:27:28]

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

[commit-bot: I haz the power, 2017-03-03 07:27:41]

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-03 07:56:43]

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

[commit-bot: I haz the power, 2017-03-03 07:56:44]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Xianzhu, 2017-03-04 06:08:08]

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

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

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-04 07:32:57]

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

[commit-bot: I haz the power, 2017-03-04 07:32:58]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Xianzhu, 2017-03-04 20:21:00]

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

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

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

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

[commit-bot: I haz the power, 2017-03-04 21:50:27]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Xianzhu, 2017-03-09 18:49:53]

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

[commit-bot: I haz the power, 2017-03-09 18:50:32]

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-09 20:03:13]

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

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

Dry run: 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-09 23:22:34]

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

[Xianzhu, 2017-03-09 23:22:57]

Description was changed from

==========
Skip paint property update and visual rect update if no geometry change

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

to

==========
Skip paint property update and visual rect update if no geometry change

BUG=697081,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

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

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-09 23:23:58]

Description was changed from

==========
Skip paint property update and visual rect update if no geometry change

BUG=697081,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Skip paint property update and visual rect update if no geometry change

This avoids unnecessary visual rect computation and update of paint
offset related paint properties.

BUG=697081,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-03-10 00:17:02]

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

[commit-bot: I haz the power, 2017-03-10 01:31:16]

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

[commit-bot: I haz the power, 2017-03-10 01:31:17]

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

[chrishtr, 2017-03-10 01:54:52]

Hi,

Do you have perf data showing improvements from this CL?

[Xianzhu, 2017-03-10 02:36:36]

On 2017/03/10 01:54:52, chrishtr wrote:
> Hi,
> 
> Do you have perf data showing improvements from this CL?

It improves performance of pure style changes without geometry change. Some
examples are:

mean_pixels_checkerboarded/text_constant_full_page_raster_NNNNN_pixels_per_second,
20%~50% improvement:

https://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf...
https://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_11_perf_...

blink_perf.paint/color-changes.html prePaint time 25% improvement:

https://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_11_perf_...

[pdr., 2017-03-10 06:21:39]

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3450:
setNeedsPaintOffsetAndVisualRectUpdate();
Can this be called during paint invalidation? I think this might be what the
PropertyTreeBuilderContext DCHECKS are for (parentUpdated). If so, WDYT about
adding a DCHECK that needsPaintOffsetAndVisualRectUpdate is not set to true
during the prepaint walk?

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.h:1646: void
setShouldDoFullPaintInvalidationWithoutGeometryChange(
It looks like this change is basically recognizing that some paint invalidation
changes don't need paint properties so we can leave the properties untouched. Is
that correct?

If so, I wonder if we could make the distinction more explicit? No need to go
implement this, I am just trying to see if I understand the problem.
Could we break apart our invalidation bits into:
1) a set of bits for pure paint invalidation (e.g., color changes)
2) a set of bits for paint offset invalidation
3) a set of bits for property invalidation
We could then early-out of the whole walk if nothing is set. Paint invalidation
walk must continue if the 1 or 2 are set. Property tree building must continue
if 2 or 3 are set. This lets us early-out from the paint invalidation code in a
similar way that this patch early-outs from the property tree building code.

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:264: if
(needsWalkForPaintPropertyUpdate) {
Trying to think of ways to simplify these DCHECKS... What if we split out the
heap-allocated prepaint context into two heap-allocated objects (paint inval
context and paint prop context). If there is no paint property update, just
don't allocate the paint prop context at all. A null ptr makes the invalid
property tree context even more obvious, and this approach might save on
allocations too since the unneeded property tree context stops being allocated
at all.

[Xianzhu, 2017-03-10 17:43:05]

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3450:
setNeedsPaintOffsetAndVisualRectUpdate();
On 2017/03/10 06:21:39, pdr. wrote:
> Can this be called during paint invalidation? I think this might be what the
> PropertyTreeBuilderContext DCHECKS are for (parentUpdated). If so, WDYT about
> adding a DCHECK that needsPaintOffsetAndVisualRectUpdate is not set to true
> during the prepaint walk?

This can be called during paint invalidation (e.g. when we find paint offset
change in SPv2), but it should only change flags of objects that have not been
walked. The xxxUpdated flags can partly ensure the rule. We can add additional
flags in LayoutObject to enforce the rule. Will think more about this.

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.h:1646: void
setShouldDoFullPaintInvalidationWithoutGeometryChange(
On 2017/03/10 06:21:39, pdr. wrote:
> It looks like this change is basically recognizing that some paint
invalidation
> changes don't need paint properties so we can leave the properties untouched.
Is
> that correct?

Yes.

> If so, I wonder if we could make the distinction more explicit? No need to go
> implement this, I am just trying to see if I understand the problem.
> Could we break apart our invalidation bits into:
> 1) a set of bits for pure paint invalidation (e.g., color changes)
> 2) a set of bits for paint offset invalidation
> 3) a set of bits for property invalidation
> We could then early-out of the whole walk if nothing is set. Paint
invalidation
> walk must continue if the 1 or 2 are set. Property tree building must continue
> if 2 or 3 are set. This lets us early-out from the paint invalidation code in
a
> similar way that this patch early-outs from the property tree building code.

I think with the patch we have a set of flags that are basically equivalent to
your proposal:
1) mayNeedPaintInvalidation and fullPaintInvalidationReasons;
2) needsPaintOffsetAndVisualRectUpdate
3) needsPaintPropertyUpdate

The patch is different from your proposal at the API level: I let
setMayNeedPaintInvalidation() and setShouldDoFullPaintInvalidation() set both 1
and 2, and add functions with "WithoutGeometryChange" to set 1 only. This is
because the cases that can use "WithoutGeometyChange" are fewer than those
needing geometry change. If we set 1 and 2 separately at API level, we would
need to change one call to two calls for each of the remaining
setMayNeedPaintInvalidation() and setShouldDoFullPaintInvalidation() calls.

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:264: if
(needsWalkForPaintPropertyUpdate) {
On 2017/03/10 06:21:39, pdr. wrote:
> Trying to think of ways to simplify these DCHECKS... What if we split out the
> heap-allocated prepaint context into two heap-allocated objects (paint inval
> context and paint prop context). If there is no paint property update, just
> don't allocate the paint prop context at all. A null ptr makes the invalid
> property tree context even more obvious, and this approach might save on
> allocations too since the unneeded property tree context stops being allocated
> at all.

This looks a good idea. Will try.

[Xianzhu, 2017-03-13 19:08:09]

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right):

https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:264: if
(needsWalkForPaintPropertyUpdate) {
On 2017/03/10 17:43:05, Xianzhu wrote:
> On 2017/03/10 06:21:39, pdr. wrote:
> > Trying to think of ways to simplify these DCHECKS... What if we split out
the
> > heap-allocated prepaint context into two heap-allocated objects (paint inval
> > context and paint prop context). If there is no paint property update, just
> > don't allocate the paint prop context at all. A null ptr makes the invalid
> > property tree context even more obvious, and this approach might save on
> > allocations too since the unneeded property tree context stops being
allocated
> > at all.
> 
> This looks a good idea. Will try.

I tried the method, but found that we still need the flags for checking
under-invalidation of setNeedsPaintOffsetAndVisualRectUpdate().

It works as follows when DCHECK_IS_ON():

- Even if !needsWalkForPaintPropertyUpdate(), we still create
treeBuilderContext, and update paint offset, paint properties and visual rect,
for checking.

- However, the forced updates don't affect the 'updated' flags so that we can
still check for prerequisites.

Without the 'updated' flags, we either can't check for changes, or can't check
for prerequisites.

[pdr., 2017-03-14 03:23:48]

On 2017/03/13 at 19:08:09, wangxianzhu wrote:
>
https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right):
> 
>
https://codereview.chromium.org/2732573003/diff/160001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:264: if
(needsWalkForPaintPropertyUpdate) {
> On 2017/03/10 17:43:05, Xianzhu wrote:
> > On 2017/03/10 06:21:39, pdr. wrote:
> > > Trying to think of ways to simplify these DCHECKS... What if we split out
the
> > > heap-allocated prepaint context into two heap-allocated objects (paint
inval
> > > context and paint prop context). If there is no paint property update,
just
> > > don't allocate the paint prop context at all. A null ptr makes the invalid
> > > property tree context even more obvious, and this approach might save on
> > > allocations too since the unneeded property tree context stops being
allocated
> > > at all.
> > 
> > This looks a good idea. Will try.
> 
> I tried the method, but found that we still need the flags for checking
under-invalidation of setNeedsPaintOffsetAndVisualRectUpdate().
> 
> It works as follows when DCHECK_IS_ON():
> 
> - Even if !needsWalkForPaintPropertyUpdate(), we still create
treeBuilderContext, and update paint offset, paint properties and visual rect,
for checking.
> 
> - However, the forced updates don't affect the 'updated' flags so that we can
still check for prerequisites.
> 
> Without the 'updated' flags, we either can't check for changes, or can't check
for prerequisites.

I worry PrePaintTreeWalk::walk is getting hard to read with all of the DCHECK
blocks. I also worry that our extra DCHECK code will start to hide other
DCHECKs. I am not sure what to do. Throwing out an idea... what do you think
about relaxing the DCHECKs so they catch fewer global mistakes and focus just on
local checking? For example, we could remove the DCHECK-only forced call to
updatePropertiesForChildren and instead have the visual rect update DCHECKs only
catch cases where the update code was called?

[Xianzhu, 2017-03-14 05:58:48]

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

[commit-bot: I haz the power, 2017-03-14 05:59:00]

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-14 06:39:16]

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

[commit-bot: I haz the power, 2017-03-14 06:39:19]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[pdr., 2017-03-16 19:58:30]

Hi Xianzhu, I don't want to delay this too long because it's a good patch. It
looks like you updated the patch but didn't send it out for another review. Is
this ready to roll?

[Xianzhu, 2017-03-16 20:05:05]

On 2017/03/16 19:58:30, pdr. wrote:
> Hi Xianzhu, I don't want to delay this too long because it's a good patch. It
> looks like you updated the patch but didn't send it out for another review. Is
> this ready to roll?

I'm still looking at some test failures. Will upload a new patch after I fix the
failures. Some of my CLs you've reviewed in this week are actual part of this.

In the new patch I will separate out the DCHECKs into classes like
FindPropertiesNeedingUpdate to avoid them from disturbing the main flow.

[pdr., 2017-03-16 20:45:05]

On 2017/03/16 at 20:05:05, wangxianzhu wrote:
> On 2017/03/16 19:58:30, pdr. wrote:
> > Hi Xianzhu, I don't want to delay this too long because it's a good patch.
It
> > looks like you updated the patch but didn't send it out for another review.
Is
> > this ready to roll?
> 
> I'm still looking at some test failures. Will upload a new patch after I fix
the failures. Some of my CLs you've reviewed in this week are actual part of
this.
> 
> In the new patch I will separate out the DCHECKs into classes like
FindPropertiesNeedingUpdate to avoid them from disturbing the main flow.

Sounds like a great plan, thanks

[Xianzhu, 2017-03-17 21:32:46]

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

[commit-bot: I haz the power, 2017-03-17 21:33:56]

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-17 23:03:25]

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

[commit-bot: I haz the power, 2017-03-17 23:03:26]

Dry run: 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_...)

[Xianzhu, 2017-03-18 00:46:23]

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

[commit-bot: I haz the power, 2017-03-18 00:47:18]

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-18 01:38:33]

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

[commit-bot: I haz the power, 2017-03-18 01:38:34]

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

[Xianzhu, 2017-03-20 16:52:09]

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

[commit-bot: I haz the power, 2017-03-20 16:52:54]

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 17:47:50]

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

[commit-bot: I haz the power, 2017-03-20 17:47:52]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

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

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

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

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-21 03:45:26]

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

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

Dry run: 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-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Xianzhu, 2017-03-21 04:22:47]

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

[commit-bot: I haz the power, 2017-03-21 04:23:06]

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

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

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

Dry run: 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_...)

[Xianzhu, 2017-03-23 19:43:06]

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

[commit-bot: I haz the power, 2017-03-23 19:44:01]

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-23 19:59:05]

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

[commit-bot: I haz the power, 2017-03-23 19:59:06]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Xianzhu, 2017-03-23 21:24:09]

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

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

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-23 22:04:32]

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

[commit-bot: I haz the power, 2017-03-23 22:04:33]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Xianzhu, 2017-03-24 21:23:47]

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

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

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-24 21:26:52]

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

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

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-24 22:16:35]

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

[commit-bot: I haz the power, 2017-03-24 22:16:36]

Dry run: 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_...)

[Xianzhu, 2017-03-25 03:42:26]

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

[commit-bot: I haz the power, 2017-03-25 03:42:29]

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-25 04:53:41]

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

[commit-bot: I haz the power, 2017-03-25 04:53:42]

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

[Xianzhu, 2017-03-27 18:22:04]

I think this CL is ready for review :)

Ptal.

[pdr., 2017-03-27 21:15:02]

This is great, LGTM.

https://codereview.chromium.org/2732573003/diff/420001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/core/paint/FindPaintOffsetAndVisualRectNeedingUpdate.h
(right):

https://codereview.chromium.org/2732573003/diff/420001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FindPaintOffsetAndVisualRectNeedingUpdate.h:1:
// Copyright 2017 The Chromium Authors. All rights reserved.
Nice file, I like this.

[Xianzhu, 2017-03-30 21:56:28]

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

[commit-bot: I haz the power, 2017-03-30 21:57:13]

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-30 22:01:24]

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

[commit-bot: I haz the power, 2017-03-30 22:01:26]

Dry run: 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-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  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-31 03:35:14]

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

[commit-bot: I haz the power, 2017-03-31 03:35:37]

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-31 04:41:41]

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

[commit-bot: I haz the power, 2017-03-31 04:41:43]

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-04-02 20:28:40]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2017-04-02 20:28:40]

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/2732573003/#ps480001
(title: "-")

[commit-bot: I haz the power, 2017-04-02 20:28:49]

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-04-02 21:45:32]

CQ is committing da patch.

Bot data: {"patchset_id": 480001, "attempt_start_ts": 1491164920287900,
"parent_rev": "b519ac382d16fb4f3c969fbcf56722030cdca062", "commit_rev":
"81f28ed19f771fe93406496d1d23da137abcd602"}

[commit-bot: I haz the power, 2017-04-02 21:46:36]

Description was changed from

==========
Skip paint property update and visual rect update if no geometry change

This avoids unnecessary visual rect computation and update of paint
offset related paint properties.

BUG=697081,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Skip paint property update and visual rect update if no geometry change

This avoids unnecessary visual rect computation and update of paint
offset related paint properties.

BUG=697081,685179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2732573003
Cr-Commit-Position: refs/heads/master@{#461348}
Committed:
https://chromium.googlesource.com/chromium/src/+/81f28ed19f771fe93406496d1d23...
==========

[commit-bot: I haz the power, 2017-04-02 21:46:39]

Committed patchset #25 (id:480001) as
https://chromium.googlesource.com/chromium/src/+/81f28ed19f771fe93406496d1d23...

[Timothy Loh, 2017-04-03 01:42:21]

A revert of this CL (patchset #25 id:480001) has been created in
https://codereview.chromium.org/2792863002/ by timloh@chromium.org.

The reason for reverting is: Looks like this is making tests fail on most
builders, e.g.

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty....