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

Issue 379073002: Revert of Divorce PaintInvalidationState from LayoutState (https://codereview.chromium.org/36083300… (Closed)

Created:
6 years, 5 months ago by Yuta Kitamura
Modified:
6 years, 5 months ago
CC:
blink-reviews, blink-reviews-rendering, kouhei+svg_chromium.org, chromiumbugtracker_adobe.com, zoltan1, philipj_slow, eae+blinkwatch, leviw+renderwatch, eric.carlson_apple.com, fs, blink-layers+watch_chromium.org, feature-media-reviews_chromium.org, ed+blinkwatch_opera.com, krit, f(malita), gyuyoung.kim_webkit.org, jchaffraix+rendering, pdr., rwlbuis, Stephen Chennney, rune+blink
Project:
blink
Visibility:
Public.

Description

Revert of Divorce PaintInvalidationState from LayoutState (https://codereview.chromium.org/360833002/), and Use reference for paintInvalidationContainer param of invalidatePaintIfNeeded (https://codereview.chromium.org/374833007) Reason for revert: This is the second attempt of reverting r177621; this time r177694 is reverted together to avoid compile error. Some change between 177596:177623 made DrMemory bot angry: <http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Content%20Browser%20%28DrMemory%20full%29%20%283%29/builds/283>;. It was not immediately obvious which change was to blame, but r177621 seemed most suspicious among others. Let me try to revert this change speculatively and see how it goes. If this was not the culprit, I will revert this revert. r177694 had to be reverted together, since some portion of it depended on r177621. Original issue's description (r177621): > Divorce PaintInvalidationState from LayoutState > > We don't actually benefit from most of the work LayoutState was doing to > keep track of offsets during layout. Instead, we were doing all that > additional work in both invalidation and layout but only using them for > invalidation. > > The new PaintInvalidationState inherits the fast paint-offset caching > from LayoutState, but goes the extra mile to identify new repaint containers, > re-enable itself when disabled and an invalidation boundary is crossed, and > work for invalidation containers other than the root. > > LayoutState was also improperly accumulating its layoutOffset using things > like scroll offsets, which resulted in incorrect rendering on > fast/multicol/shrink-to-column-height-for-pagination.html. This is fixed > with this patch because we no longer consider paint-specific offsets at > all in LayoutState. > > BUG=363834 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177621 Original issue's description (r177694): > Use reference for paintInvalidationContainer param of invalidatePaintIfNeeded > > BTW also change getPaintInvalidationReason, incrementallyInvalidatePaint > and fullyInvalidatePaint which are called only by > invalidatePaintIfNeeded. > > Review URL: https://codereview.chromium.org/374833007 > > git-svn-id: svn://svn.chromium.org/blink/trunk@177694 bbb929c8-8fbe-4397-9dbb-9b2b20218538 TBR=jchaffraix@chromium.org,chrishtr@chromium.org,dsinclair@chromium.org,esprehn@chromium.org,leviw@chromium.org,wangxianzhu@chromium.org BUG=363834 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177725

Patch Set 1 #

Patch Set 2 : Also reverts rebaselines #

Patch Set 3 : Correct revert order #

Unified diffs Side-by-side diffs Delta from patch set Stats (+449 lines, -497 lines) Patch
M LayoutTests/fast/repaint/reflection-repaint-test-expected.txt View 1 1 chunk +0 lines, -15 lines 0 comments Download
A + LayoutTests/platform/android/fast/multicol/shrink-to-column-height-for-pagination-expected.txt View 1 0 chunks +-1 lines, --1 lines 0 comments Download
D LayoutTests/platform/linux-x86/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
M LayoutTests/platform/linux/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
A + LayoutTests/platform/linux/fast/multicol/shrink-to-column-height-for-pagination-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
A + LayoutTests/platform/linux/fast/repaint/reflection-repaint-test-expected.txt View 1 1 chunk +2 lines, -1 line 0 comments Download
M LayoutTests/platform/mac-lion/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
M LayoutTests/platform/mac-mountainlion/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
D LayoutTests/platform/mac-retina/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
M LayoutTests/platform/mac-snowleopard/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
M LayoutTests/platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.png View 1 Binary file 0 comments Download
A + LayoutTests/platform/mac/fast/multicol/shrink-to-column-height-for-pagination-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
A + LayoutTests/platform/mac/fast/repaint/reflection-repaint-test-expected.txt View 1 1 chunk +2 lines, -1 line 0 comments Download
A + LayoutTests/platform/win-xp/fast/repaint/reflection-repaint-test-expected.txt View 1 1 chunk +2 lines, -1 line 0 comments Download
A + LayoutTests/platform/win/fast/repaint/reflection-repaint-test-expected.txt View 1 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/core.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/frame/FrameView.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/rendering/LayoutState.h View 2 chunks +24 lines, -2 lines 0 comments Download
M Source/core/rendering/LayoutState.cpp View 4 chunks +79 lines, -7 lines 0 comments Download
D Source/core/rendering/PaintInvalidationState.h View 1 chunk +0 lines, -61 lines 0 comments Download
D Source/core/rendering/PaintInvalidationState.cpp View 1 chunk +0 lines, -137 lines 0 comments Download
M Source/core/rendering/RenderBlock.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderBlock.cpp View 5 chunks +19 lines, -26 lines 0 comments Download
M Source/core/rendering/RenderBlockFlow.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBox.h View 4 chunks +6 lines, -6 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 13 chunks +57 lines, -38 lines 0 comments Download
M Source/core/rendering/RenderBoxModelObject.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderFlowThread.cpp View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderInline.h View 2 chunks +5 lines, -5 lines 0 comments Download
M Source/core/rendering/RenderInline.cpp View 7 chunks +34 lines, -27 lines 0 comments Download
M Source/core/rendering/RenderLayer.h View 1 chunk +3 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderLayer.cpp View 4 chunks +22 lines, -9 lines 0 comments Download
M Source/core/rendering/RenderListBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderListBox.cpp View 1 chunk +5 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderListItem.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderMediaControlElements.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/RenderObject.h View 8 chunks +14 lines, -15 lines 0 comments Download
M Source/core/rendering/RenderObject.cpp View 15 chunks +32 lines, -30 lines 0 comments Download
M Source/core/rendering/RenderReplaced.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderReplaced.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderTableCell.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderTableCell.cpp View 2 chunks +7 lines, -6 lines 0 comments Download
M Source/core/rendering/RenderTableCol.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderTableCol.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderText.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderText.cpp View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderView.h View 5 chunks +25 lines, -11 lines 0 comments Download
M Source/core/rendering/RenderView.cpp View 5 chunks +6 lines, -5 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGBlock.h View 1 chunk +4 lines, -4 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGBlock.cpp View 3 chunks +9 lines, -9 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGContainer.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGForeignObject.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGForeignObject.cpp View 2 chunks +3 lines, -2 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGGradientStop.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGHiddenContainer.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGInline.h View 1 chunk +3 lines, -3 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGInline.cpp View 1 chunk +6 lines, -6 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGModelObject.h View 2 chunks +4 lines, -4 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGModelObject.cpp View 4 chunks +14 lines, -14 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGRoot.h View 1 chunk +3 lines, -3 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGRoot.cpp View 4 chunks +9 lines, -6 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGText.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGText.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/rendering/svg/SVGRenderSupport.h View 2 chunks +3 lines, -4 lines 0 comments Download
M Source/core/rendering/svg/SVGRenderSupport.cpp View 3 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Yuta Kitamura
The CQ bit was checked by yutak@chromium.org
6 years, 5 months ago (2014-07-09 04:30:31 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yutak@chromium.org/379073002/1
6 years, 5 months ago (2014-07-09 04:30:53 UTC) #2
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink ...
6 years, 5 months ago (2014-07-09 05:42:28 UTC) #3
Yuta Kitamura
The CQ bit was unchecked by yutak@chromium.org
6 years, 5 months ago (2014-07-09 05:49:55 UTC) #4
Yuta Kitamura
The CQ bit was checked by yutak@chromium.org
6 years, 5 months ago (2014-07-09 05:55:43 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yutak@chromium.org/379073002/40001
6 years, 5 months ago (2014-07-09 05:56:12 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: mac_blink_rel on tryserver.blink ...
6 years, 5 months ago (2014-07-09 07:04:25 UTC) #7
commit-bot: I haz the power
Change committed as 177725
6 years, 5 months ago (2014-07-09 07:42:19 UTC) #8
leviw_travelin_and_unemployed
6 years, 5 months ago (2014-07-09 17:02:24 UTC) #9
Message was sent while issue was closed.
Can we get a bug to track this? The DrMemory url on this doesn't work for me :-/

Powered by Google App Engine
This is Rietveld 408576698