Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(41)

Issue 264963004: Mark when we may have been invalidated to early out on repaintTreeAfterLayout. (Closed)

Created:
6 years, 7 months ago by dsinclair
Modified:
6 years, 7 months ago
CC:
blink-reviews, Manuel Rego, fs, eric.carlson_apple.com, kouhei+svg_chromium.org, rwlbuis, krit, bemjb+rendering_chromium.org, dsinclair, jchaffraix+rendering, pdr., rune+blink, mstensho+blink_opera.com, jfernandez, zoltan1, eae+blinkwatch, philipj_slow, feature-media-reviews_chromium.org, gyuyoung.kim_webkit.org, blink-reviews-rendering, svillar, leviw+renderwatch, blink-layers+watch_chromium.org, ed+blinkwatch_opera.com, f(malita), Stephen Chennney
Visibility:
Public.

Description

Mark when we may have been invalidated to early out on repaintTreeAfterLayout. This CL adds in a flag to mark if we may have been invalidated. If the flag is set we will propagate the flag up to our parents. This allows us to early out of the repaintTreeAfterLayout walk if at any point the node does not need to be invalidated. This improves the performance of the large-table-with-collapsed-borders-and-colspans-wider-than-table.html test by ~17% and removes some over-invalidations that were output. I visually verified the items in TestExpectations to make sure they matched. BUG=320139, 368739 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173348

Patch Set 1 #

Patch Set 2 : Rebase to master #

Patch Set 3 : #

Patch Set 4 : #

Total comments: 3

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : Rebase to master #

Patch Set 8 : #

Patch Set 9 : #

Total comments: 14

Patch Set 10 : Review updates (TestExpectations commented out to show changes on bots) #

Patch Set 11 : With expectations. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -1 line) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 10 1 chunk +18 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBlock.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBlockFlow.cpp View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderDeprecatedFlexibleBox.cpp View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderFlexibleBox.cpp View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderObject.h View 1 2 3 4 5 6 7 8 9 3 chunks +20 lines, -1 line 0 comments Download
M Source/core/rendering/RenderObject.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
dsinclair
PTAL.
6 years, 7 months ago (2014-05-02 16:56:53 UTC) #1
fs
I was wondering when this change would show up =). Looks a bit "sprinkly" though... ...
6 years, 7 months ago (2014-05-02 17:21:41 UTC) #2
Julien - ping for review
As discussed this is not cool to add another boolean to be set up for ...
6 years, 7 months ago (2014-05-02 17:27:06 UTC) #3
dsinclair
PTAL. Using layoutDidGet called along with the new flag to make this less invasive. https://codereview.chromium.org/264963004/diff/60001/Source/core/rendering/svg/RenderSVGImage.cpp ...
6 years, 7 months ago (2014-05-02 19:18:52 UTC) #4
leviw_travelin_and_unemployed
https://codereview.chromium.org/264963004/diff/150001/LayoutTests/fast/repaint/box-inline-resize.html File LayoutTests/fast/repaint/box-inline-resize.html (right): https://codereview.chromium.org/264963004/diff/150001/LayoutTests/fast/repaint/box-inline-resize.html#newcode4 LayoutTests/fast/repaint/box-inline-resize.html:4: <script src="../../resources/run-after-display.js"></script> It's odd that this test change is ...
6 years, 7 months ago (2014-05-05 18:52:32 UTC) #5
Julien - ping for review
https://codereview.chromium.org/264963004/diff/150001/Source/core/rendering/RenderObject.h File Source/core/rendering/RenderObject.h (right): https://codereview.chromium.org/264963004/diff/150001/Source/core/rendering/RenderObject.h#newcode998 Source/core/rendering/RenderObject.h:998: Unneeded space. https://codereview.chromium.org/264963004/diff/150001/Source/core/rendering/RenderObject.h#newcode1019 Source/core/rendering/RenderObject.h:1019: // Make sure our parent ...
6 years, 7 months ago (2014-05-05 19:02:34 UTC) #6
dsinclair
PTAL. The patchset 10 has the try bots running without the TestExpectation changes so you'll ...
6 years, 7 months ago (2014-05-05 19:38:25 UTC) #7
leviw_travelin_and_unemployed
This LGTM.
6 years, 7 months ago (2014-05-05 21:31:43 UTC) #8
dsinclair
The CQ bit was checked by dsinclair@chromium.org
6 years, 7 months ago (2014-05-05 23:14:59 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dsinclair@chromium.org/264963004/190001
6 years, 7 months ago (2014-05-05 23:15:35 UTC) #10
commit-bot: I haz the power
6 years, 7 months ago (2014-05-05 23:28:39 UTC) #11
Message was sent while issue was closed.
Change committed as 173348

Powered by Google App Engine
This is Rietveld 408576698