Bug 203445 - RenderTreeNeedsLayoutChecker asserts with css-position/position-absolute* tests
Summary: RenderTreeNeedsLayoutChecker asserts with css-position/position-absolute* tests
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on: 209023 209022
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-25 21:12 PDT by Simon Fraser (smfr)
Modified: 2020-03-12 15:47 PDT (History)
10 users (show)

See Also:


Attachments
Patch (7.66 KB, patch)
2020-03-12 12:20 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (7.65 KB, patch)
2020-03-12 12:47 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2019-10-25 21:12:26 PDT
These two tests:

imported/w3c/web-platform-tests/css/css-position/position-absolute-container-dynamic-002.html
imported/w3c/web-platform-tests/css/css-position/position-absolute-crash-chrome-005.html

hit:

ERROR: post-layout: dirty renderer(s)
./page/FrameViewLayoutContext.cpp(81) : auto WebCore::RenderTreeNeedsLayoutChecker::~RenderTreeNeedsLayoutChecker()::(anonymous class)::operator()(const WebCore::RenderObject &) const

(B)lock/(I)nline/I(N)line-block, (A)bsolute/Fi(X)ed/(R)elative/Stic(K)y, (F)loating, (O)verflow clip, Anon(Y)mous, (G)enerated, has(L)ayer, (C)omposited, (+)Dirty style, (+)Dirty layout
B---YGLC --  RenderView at (0,0) size 800x600 renderer->(0x34da015f0)
B-----L- --    HTML RenderBlock at (0,0) size 800x216 renderer->(0x34da01ad0) node->(0x3513e8d80)
B------- --      BODY RenderBody at (8,8) size 784x200 renderer->(0x34da01c00) node->(0x3513f0d80)
BR----L- --        DIV RenderBlock at (0,0) size 784x200 renderer->(0x34db0e280) node->(0x3513dce80)
B--O--L- --          DIV RenderBlock at (0,0) size 200x200 renderer->(0x34db0e3b0) node->(0x3513dcf00)
B------- --            DIV RenderBlock at (0,0) size 200x100 renderer->(0x34db0e4e0) node->(0x3513dcf80)
BA----L- -+*           DIV RenderBlock at (0,200) size 200x100 renderer->(0x34db0e610) node->(0x3513dd000) layout->[normal child]

SHOULD NEVER BE REACHED
./page/FrameViewLayoutContext.cpp(83) : auto WebCore::RenderTreeNeedsLayoutChecker::~RenderTreeNeedsLayoutChecker()::(anonymous class)::operator()(const WebCore::RenderObject &) const
1   0x3446e0af9 WTFCrash
2   0x32a8d0a4b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x32de4d36d WebCore::RenderTreeNeedsLayoutChecker::~RenderTreeNeedsLayoutChecker()::'lambda'(WebCore::RenderObject const&)::operator()(WebCore::RenderObject const&) const
4   0x32de4d2c2 WebCore::RenderTreeNeedsLayoutChecker::~RenderTreeNeedsLayoutChecker()
5   0x32de43225 WebCore::RenderTreeNeedsLayoutChecker::~RenderTreeNeedsLayoutChecker()
6   0x32de427d6 WebCore::FrameViewLayoutContext::layout()
7   0x32d0e70f6 WebCore::Document::updateLayout()
8   0x32d0e860e WebCore::Document::updateLayoutIgnorePendingStylesheets(WebCore::Document::RunPostLayoutTasks)
9   0x32d1b35c9 WebCore::Element::offsetTop()
10  0x32d1b33ac WebCore::Element::offsetTopForBindings()
11  0x32b38b68d WebCore::jsHTMLElementOffsetTopGetter(JSC::JSGlobalObject&, WebCore::JSHTMLElement&, JSC::ThrowScope&)
12  0x32b2d0910 long long WebCore::IDLAttribute<WebCore::JSHTMLElement>::get<&(WebCore::jsHTMLElementOffsetTopGetter(JSC::JSGlobalObject&, WebCore::JSHTMLElement&, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)3>(JSC::JSGlobalObject&, long long, char const*)
13  0x32b2d07f8 WebCore::jsHTMLElementOffsetTop(JSC::JSGlobalObject*, long long, JSC::PropertyName)
14  0x345eaa54f JSC::PropertySlot::customGetter(JSC::JSGlobalObject*, JSC::PropertyName) const
15  0x344c309e1 JSC::PropertySlot::getValue(JSC::JSGlobalObject*, JSC::PropertyName) const
16  0x345580a55 JSC::JSValue::get(JSC::JSGlobalObject*, JSC::PropertyName, JSC::PropertySlot&) const
17  0x345a4dd10 llint_slow_path_get_by_id
18  0x344bd749e llint_entry
19  0x344beafaf llint_entry
20  0x344be9e0e llint_entry
21  0x344be9e0e llint_entry
22  0x344bcd6e3 vmEntryToJavaScript
23  0x345932757 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
24  0x345931d19 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*)
25  0x345c7bfbc JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
26  0x345c7c168 JSC::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
27  0x32cbc40d8 WebCore::JSExecState::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
28  0x32cbc3e37 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&, WebCore::ExceptionDetails*)
29  0x32cbc41ad WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&, WebCore::ExceptionDetails*)
30  0x32d2cde01 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&)
31  0x32d2cc06a WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)
LEAK: 1 WebPageProxy
Comment 1 Radar WebKit Bug Importer 2020-03-11 09:36:01 PDT
<rdar://problem/60326406>
Comment 2 zalan 2020-03-12 12:20:08 PDT
Created attachment 393394 [details]
Patch
Comment 3 Simon Fraser (smfr) 2020-03-12 12:32:33 PDT
Comment on attachment 393394 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=393394&action=review

> Source/WebCore/rendering/RenderBlock.cpp:380
> +    auto outOfFlowDescendantsHaveNewContainingBlock = (hadTransform && !willHaveTransform) || (newStyle.position() == PositionType::Static && !willHaveTransform);

auto -> bool otherwise at a glance I might think this is a "containing block"

It's crazy that we don't have an "is containing block for out of flow descendants" helper.

Why does this code not look at the oldStyle.position()?

> Source/WebCore/rendering/RenderBlock.cpp:1783
> +        while (parent && !is<RenderBlock>(parent))
>              parent = parent->parent();

Weird that there's no helper for this.

> Source/WebCore/rendering/RenderBlock.cpp:1787
> +        if (renderer->isFixedPositioned())
> +            view().setNeedsLayout();

Blank line above. Is this special-cased for fixed necessary or just an optimization to avoid calling containingBlock() below?

> Source/WebCore/rendering/RenderBlock.cpp:1788
> +        else if (auto* newContainingBlock = this->containingBlock()) {

Don't need this->
Comment 4 zalan 2020-03-12 12:47:48 PDT
Created attachment 393398 [details]
Patch
Comment 5 zalan 2020-03-12 15:41:20 PDT
Apparently these test cases require separate patches.