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

Issue 408083002: Revert of Enforce clearing renderers' paint invalidation state (Closed)

Created:
6 years, 5 months ago by Peter Kasting
Modified:
6 years, 5 months ago
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr., rune+blink, zoltan1
Project:
blink
Visibility:
Public.

Description

Revert of Enforce clearing renderers' paint invalidation state (https://codereview.chromium.org/389573008/) Reason for revert: Causing consistent assertion failures on Mac. See e.g. http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.6%20%28dbg%29/builds/16532/steps/webkit_tests/logs/stdio . A sample stack: STDERR: SHOULD NEVER BE REACHED STDERR: ../../third_party/WebKit/Source/core/rendering/RenderObject.h(258) : void blink::RenderObject::assertRendererClearedPaintInvalidationState() const STDERR: 1 0x99d08a7 blink::RenderObject::assertRendererClearedPaintInvalidationState() const STDERR: 2 0x99bc4ee blink::RenderObject::assertSubtreeClearedPaintInvalidationState() const STDERR: 3 0x99af007 blink::FrameView::invalidateTreeIfNeeded() STDERR: 4 0x99b82ee blink::FrameView::invalidateTreeIfNeededRecursive() STDERR: 5 0x99b710d blink::FrameView::updateLayoutAndStyleForPainting() STDERR: 6 0x9d169f2 blink::PageAnimator::updateLayoutAndStyleForPainting(blink::LocalFrame*) STDERR: 7 0x709b4fb blink::PageWidgetDelegate::layout(blink::Page*, blink::LocalFrame*) STDERR: 8 0x720c8a4 blink::WebViewImpl::layout() STDERR: 9 0xfc6ee content::EventSender::MouseMoveTo(gin::Arguments*) STDERR: 10 0xf9be7 content::EventSenderBindings::MouseMoveTo(gin::Arguments*) STDERR: 11 0x113572 base::internal::RunnableAdapter<void (content::EventSenderBindings::*)(gin::Arguments*)>::Run(content::EventSenderBindings*, gin::Arguments* const&) STDERR: 12 0x1134af base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void (content::EventSenderBindings::*)(gin::Arguments*)>, void ()(content::EventSenderBindings* const&, gin::Arguments* const&)>::MakeItSo(base::internal::RunnableAdapter<void (content::EventSenderBindings::*)(gin::Arguments*)>, content::EventSenderBindings* const&, gin::Arguments* const&) STDERR: 13 0x11342b base::internal::Invoker<0, base::internal::BindState<base::internal::RunnableAdapter<void (content::EventSenderBindings::*)(gin::Arguments*)>, void ()(content::EventSenderBindings*, gin::Arguments*), void ()()>, void ()(content::EventSenderBindings*, gin::Arguments*)>::Run(base::internal::BindStateBase*, content::EventSenderBindings* const&, gin::Arguments* const&) STDERR: 14 0x113c03 base::Callback<void ()(content::EventSenderBindings*, gin::Arguments*)>::Run(content::EventSenderBindings* const&, gin::Arguments* const&) const STDERR: 15 0x113b5e gin::internal::Invoker<void, content::EventSenderBindings*, gin::Arguments*, void, void, void, void>::Go(gin::Arguments*, base::Callback<void ()(content::EventSenderBindings*, gin::Arguments*)> const&, content::EventSenderBindings* const&, gin::Arguments* const&) STDERR: 16 0x113ab5 gin::internal::Dispatcher<void ()(content::EventSenderBindings*, gin::Arguments*)>::DispatchToCallback(v8::FunctionCallbackInfo<v8::Value> const&) STDERR: 17 0x4f44b1e v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) STDERR: 18 0x4f9c17e _ZN2v88internalL21Builtin_HandleApiCallEiPPNS0_6ObjectEPNS0_7IsolateE STDERR: 19 0x4150a4b6 Original issue's description: > Enforce clearing renderers' paint invalidation state > > The ASSERT checks that no flags cleared by clearPaintInvalidationState() > is still set up after calling repaint-after-compositing-update. > > The ASSERT is skipped for SVG subtree as we have an optimization that > skips them. We could reproduce the exact condition but it seems not > very useful to do so. > > The change caught several bugs (yay!): > - one in shouldCheckForPaintInvalidationAfterLayout() > that made the ASSERT trip on fast/repaint/table-row.html. > - one where setShouldDoFullPaintInvalidationAfterLayout didn't > mark the containing block chain for invalidation. > > BUG=385169 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178089 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178569 TBR=dsinclair@chromium.org,falken@chromium.org,leviw@chromium.org,jchaffraix@chromium.org NOTREECHECKS=true NOTRY=true BUG=385169 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178602

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -126 lines) Patch
M LayoutTests/TestExpectations View 1 chunk +0 lines, -21 lines 0 comments Download
M LayoutTests/compositing/repaint/resize-repaint-expected.txt View 1 chunk +0 lines, -4 lines 0 comments Download
M LayoutTests/css3/flexbox/repaint-column-reverse-expected.txt View 1 chunk +4 lines, -1 line 0 comments Download
M LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt View 1 chunk +1 line, -3 lines 0 comments Download
M LayoutTests/fast/box-shadow/negative-shadow-box-shrink-expected.txt View 1 chunk +1 line, -3 lines 0 comments Download
M LayoutTests/fast/repaint/add-table-overpaint-expected.txt View 1 chunk +0 lines, -3 lines 0 comments Download
M LayoutTests/fast/repaint/box-shadow-inset-repaint-expected.txt View 1 chunk +1 line, -3 lines 0 comments Download
M LayoutTests/fast/repaint/content-into-overflow-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/fast/repaint/overflow-into-content-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/fast/repaint/repaint-table-row-in-composited-document-expected.txt View 1 chunk +5 lines, -1 line 0 comments Download
M LayoutTests/fast/repaint/table-row-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/fast/repaint/table-section-repaint-expected.txt View 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/fast/repaint/transform-disable-layoutstate-expected.txt View 1 chunk +4 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/block-no-inflow-children-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/bugzilla-5699-expected.txt View 1 chunk +1 line, -3 lines 0 comments Download
M LayoutTests/platform/linux/fast/repaint/bugzilla-6278-expected.txt View 1 chunk +4 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/make-children-non-inline-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/outline-change-invalidation-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/platform/linux/fast/repaint/selection-clear-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/stacked-diacritics-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/linux/fast/repaint/table-collapsed-border-expected.txt View 1 chunk +0 lines, -7 lines 0 comments Download
M LayoutTests/platform/linux/fast/repaint/table-shrink-row-repaint-expected.txt View 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/platform/linux/svg/custom/scrolling-embedded-svg-file-image-repaint-problem-expected.txt View 1 chunk +1 line, -2 lines 0 comments Download
M LayoutTests/platform/linux/svg/custom/use-setAttribute-crash-expected.txt View 1 chunk +1 line, -2 lines 0 comments Download
M Source/core/frame/FrameView.cpp View 1 chunk +0 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/RenderObject.h View 5 chunks +7 lines, -55 lines 0 comments Download
M Source/core/rendering/RenderObject.cpp View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Peter Kasting
Created Revert of Enforce clearing renderers' paint invalidation state
6 years, 5 months ago (2014-07-21 23:37:26 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkasting@chromium.org/408083002/1
6 years, 5 months ago (2014-07-21 23:38:25 UTC) #2
commit-bot: I haz the power
6 years, 5 months ago (2014-07-21 23:39:19 UTC) #3
Message was sent while issue was closed.
Change committed as 178602

Powered by Google App Engine
This is Rietveld 408576698