Don't update descendant visual rects on border radius change

Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug --story-filter=plus
The failure was because the subtree was marked to update visual rects but
ancestors didn't update paint properties.

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

Review-Url: https://codereview.chromium.org/2821183002
Cr-Commit-Position: refs/heads/master@{#465093}
Committed: https://chromium.googlesource.com/chromium/src/+/2fe404ba675b5ba11b63b3a4bf8d8efdd1f86a09

[Xianzhu, 2017-04-17 23:07:04]

Description was changed from

==========
Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug
--story-filter=plus

BUG=710665
==========

to

==========
Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug
--story-filter=plus

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

[Xianzhu, 2017-04-17 23:15:09]

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

[commit-bot: I haz the power, 2017-04-17 23:15:29]

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

[Xianzhu, 2017-04-18 00:05:22]

Description was changed from

==========
Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug
--story-filter=plus

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

to

==========
Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug
--story-filter=plus
The failure was because the subtree was marked to update visual rects but
ancestors didn't update paint properties.

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

[Xianzhu, 2017-04-18 00:05:38]

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

[chrishtr, 2017-04-18 00:14:28]

lgtm

[Xianzhu, 2017-04-18 00:16:29]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2017-04-18 00:16:29]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-04-18 00:16:56]

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

[pdr., 2017-04-18 00:54:32]

Nice! I'm curious how you tracked this down. I spent a little time looking into
it but didn't get to the bottom of it. This only crashed after some dynamic
changes to the archived WPR google plus page. Did you minimize the crash, or
just work backwards from the crash in a debugger, or something else? If
minimization, did you do the replay.py thing to start up a local server of the
archived WPR page? If a debugger, how'd you get the debugger to attach?

[Xianzhu, 2017-04-18 01:15:26]

On 2017/04/18 00:54:32, pdr. wrote:
> Nice! I'm curious how you tracked this down. I spent a little time looking
into
> it but didn't get to the bottom of it. This only crashed after some dynamic
> changes to the archived WPR google plus page. Did you minimize the crash, or
> just work backwards from the crash in a debugger, or something else? If
> minimization, did you do the replay.py thing to start up a local server of the
> archived WPR page? If a debugger, how'd you get the debugger to attach?

There may be two reasons of DCHECK(parent_context->is_actually_needed) failure:

1. We call setNeedsPaintOffsetAndVisualRectUpdate() (indirectly from
setMayNeedPaintInvalidation() or setShouldDoFullPaintInvalidation()) during
PrePaint, but the ancestors didn't have related flag set so didn't update
PaintPropertyTreeBuilderContext;

2. We set ForcedSubtreeVisualRectUpdate flag during PrePaint but ancestors
didn't have related flag set.

I looked at crbug.com/709798 first before looking at this bug. I added code to
dump layout tree (with additional dumping of the paint flags) when the assert
fails, and found reason 1. Then I tracked down to the caller of
setShouldDoFullPaintInvalidation() during PrePaint.
https://codereview.chromium.org/2823963002/ is a WIP CL for this reason. The
added DCHECK in setNeedsPaintOffsetAndVisualRectUpdate() seems to discover more
issues which need more investigation. It's simple to use gdb because the bug is
a clusterfuzz bug.

For this bug I used the same debug code for the above bug, but didn't find issue
of paint flags in layout tree. Then I added log at the two places where we set
ForcedSubtreeVisualRectUpdate, and found that reason 2 and the flag is set by
clip change. With more logging of clip change I found that the rects didn't
change but HasRadius changed.

For perf test, I think it's possible to use a debugger to attach to the renderer
process which is stopping at a waitForDebugger loop, but I never tried it.

[commit-bot: I haz the power, 2017-04-18 01:52:01]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492474589765330,
"parent_rev": "b59bf346862b89daee2a4f06f02f5d7df59f9164", "commit_rev":
"2fe404ba675b5ba11b63b3a4bf8d8efdd1f86a09"}

[commit-bot: I haz the power, 2017-04-18 01:52:39]

Description was changed from

==========
Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug
--story-filter=plus
The failure was because the subtree was marked to update visual rects but
ancestors didn't update paint properties.

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

to

==========
Don't update descendant visual rects on border radius change

Border radius doesn't affect descendants' visual rects.
This fixes the "Check failed: parent_context.tree_builder_context->
is_actually_needed" failure when running
tools/perf/run_benchmark run rasterize_and_record_micro.top_25 --browser=debug
--story-filter=plus
The failure was because the subtree was marked to update visual rects but
ancestors didn't update paint properties.

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

Review-Url: https://codereview.chromium.org/2821183002
Cr-Commit-Position: refs/heads/master@{#465093}
Committed:
https://chromium.googlesource.com/chromium/src/+/2fe404ba675b5ba11b63b3a4bf8d...
==========

[commit-bot: I haz the power, 2017-04-18 01:52:40]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2fe404ba675b5ba11b63b3a4bf8d...