Bug 219905 - ASSERTION FAILED: &layoutState().establishedFormattingState(layoutBox.formattingContextRoot()) == this in WebCore::Layout::FormattingState::boxGeometry
Summary: ASSERTION FAILED: &layoutState().establishedFormattingState(layoutBox.formatt...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
Keywords: InRadar
Depends on:
Reported: 2020-12-15 11:28 PST by Ryan Haddad
Modified: 2021-04-12 13:31 PDT (History)
7 users (show)

See Also:

crash log (70.44 KB, text/plain)
2020-12-15 11:30 PST, Ryan Haddad
no flags Details
Patch (3.32 KB, patch)
2021-04-08 19:53 PDT, zalan
simon.fraser: review+
Details | Formatted Diff | Diff
Patch (2.17 KB, patch)
2021-04-12 12:45 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 Ryan Haddad 2020-12-15 11:28:56 PST
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)

Comment 1 Ryan Haddad 2020-12-15 11:29:51 PST
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);
Comment 2 Ryan Haddad 2020-12-15 11:30:40 PST
Created attachment 416269 [details]
crash log
Comment 3 Radar WebKit Bug Importer 2020-12-15 11:31:28 PST
Comment 4 Ryan Haddad 2020-12-15 11:32:37 PST
The first instance of this I see appears to be at https://trac.webkit.org/changeset/269627/webkit
Comment 5 Truitt Savell 2021-01-12 15:21:43 PST
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
Comment 6 Truitt Savell 2021-01-13 10:17:32 PST
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
Comment 7 Truitt Savell 2021-01-13 13:39:14 PST
marked this test as crashing in https://trac.webkit.org/changeset/271454/webkit
Comment 8 Ryan Haddad 2021-03-22 14:12:35 PDT
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.
Comment 9 zalan 2021-04-08 19:53:35 PDT
Created attachment 425577 [details]
Comment 10 zalan 2021-04-09 20:01:05 PDT
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.
Comment 11 zalan 2021-04-12 12:45:42 PDT
Created attachment 425776 [details]
Comment 12 EWS 2021-04-12 13:31:31 PDT
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].