Bug 219905 – ASSERTION FAILED: &layoutState().establishedFormattingState(layoutBox.formattingContextRoot()) == this in WebCore::Layout::FormattingState::boxGeometry

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ryan Haddad]

fast/layoutformattingcontext/subframe-with-display-none-html.html is a flaky crash on debug macOS bots

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x00000002524f11ce WTFCrash + 14 (Assertions.cpp:295)
1   com.apple.WebCore             	0x000000022f6b51ab WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x0000000233145c2d WebCore::Layout::FormattingState::boxGeometry(WebCore::Layout::Box const&) + 141 (FormattingState.cpp:54)
3   com.apple.WebCore             	0x0000000233146343 WebCore::Layout::FormattingContext::computeBorderAndPadding(WebCore::Layout::Box const&, WebCore::Layout::HorizontalConstraints const&) + 51 (FormattingContext.cpp:131)
4   com.apple.WebCore             	0x00000002331730bb WebCore::Layout::BlockFormattingContext::layoutInFlowContent(WebCore::Layout::InvalidationState&, WebCore::Layout::FormattingContext::ConstraintsForInFlowContent const&) + 635 (BlockFormattingContext.cpp:115)
5   com.apple.WebCore             	0x00000002331545e7 WebCore::Layout::LayoutContext::layoutFormattingContextSubtree(WebCore::Layout::ContainerBox const&, WebCore::Layout::InvalidationState&) + 327 (LayoutContext.cpp:111)
6   com.apple.WebCore             	0x00000002331542b7 WebCore::Layout::LayoutContext::layoutWithPreparedRootGeometry(WebCore::Layout::InvalidationState&) + 151 (LayoutContext.cpp:87)
7   com.apple.WebCore             	0x000000023315415b WebCore::Layout::LayoutContext::layout(WebCore::LayoutSize const&, WebCore::Layout::InvalidationState&) + 443 (LayoutContext.cpp:78)
8   com.apple.WebCore             	0x000000023354475d WebCore::FrameViewLayoutContext::layoutUsingFormattingContext() + 349 (FrameViewLayoutContext.cpp:75)
9   com.apple.WebCore             	0x0000000233525c9d WebCore::FrameViewLayoutContext::layout() + 2141 (FrameViewLayoutContext.cpp:234)
10  com.apple.WebCore             	0x0000000233544edf WebCore::FrameViewLayoutContext::layoutTimerFired() + 127 (FrameViewLayoutContext.cpp:469)

https://results.webkit.org/?suite=layout-tests&test=fast%2Flayoutformattingcontext%2Fsubframe-with-display-none-html.html

[Ryan Haddad]

This is the assert from FormattingState.cpp:54
>    // Should never need to mutate a display box outside of the formatting context.
>    > ASSERT(&layoutState().establishedFormattingState(layoutBox.formattingContextRoot()) == this);

[Ryan Haddad]

Created attachment 416269 [details]
crash log

[Radar WebKit Bug Importer]

<rdar://problem/72350516>

[Ryan Haddad]

The first instance of this I see appears to be at https://trac.webkit.org/changeset/269627/webkit

[Truitt Savell]

I am able to reproduce this using command 
rwt fast/layoutformattingcontext/subframe-with-display-none-html.html --iterations 2000 -f --exit-after-n-crashes-or-timeout 1

[Truitt Savell]

there is a large regression range due to a lack of builds during this time. I can reproduce this on 269629 but not on 269601

[Truitt Savell]

marked this test as crashing in https://trac.webkit.org/changeset/271454/webkit

[Ryan Haddad]

I skipped the test in https://trac.webkit.org/changeset/274083/webkit because it kept showing up in the "other crashes" section of test results.

[zalan]

Created attachment 425577 [details]
Patch

[zalan]

So this is the very similar to what was already addressed in bug 219878.

In TestController::resetPreferencesToConsistentValues, first we call WKPreferencesResetAllInternalDebugFeatures() which (in batch) turns on/off the debug features. It enables LFC integration while it disables full LFC. Later in this function we take the boolWebPreferenceFeatures() unordered map and call WKPreferencesSetBoolValueForKeyForTesting() on each entry.
WKPreferencesSetBoolValueForKeyForTesting is not a batch update, so it'll (through the WebPageProxy) issue a preferencesDidChange messages on each call.
When we reach the "LayoutFormattingContextEnabled=true" key/value in this unordered map and update the preferences, WebContentProcess may see both the integration (from WKPreferencesResetAllInternalDebugFeatures) and the full LFC (this call) on.
Later when the loop reaches "LayoutFormattingContextIntegrationEnabled=false" and we update the preferences, everything goes back to normal. However if WebContent process issues a layout in the meantime, we hit this assert.

There are a few ways to address it.
1, use batch update for all the test preferences
2, use some kind of ordered data structure to guarantee that "<!-- webkit-test-runner [ LayoutFormattingContextIntegrationEnabled=false LayoutFormattingContextEnabled=true  ] -->" turns the integration feature off first and then enables full LFC.
3, since these 2 preferences are mutually exclusive, turning full LFC on would also disable the LFC integration bit.

[zalan]

Created attachment 425776 [details]
Patch

[EWS]

Committed r275837 (236405@main): <https://commits.webkit.org/236405@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425776 [details].