Bug 218995

Summary: Remove simpleUserAgentStyleSheet (to fix flaky fast/lists/001.html and fast/lists/001-vertical.html)
Product: WebKit Reporter: Truitt Savell <tsavell>
Component: CSSAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, esprehn+autocc, ews-watchlist, glenn, graouts, graouts, koivisto, kondapallykalyan, pdr, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218894
Attachments:
Description Flags
001-crash-log.txt
none
001-vertical-crash-log.txt
none
Patch
ews-feeder: commit-queue-
patch
none
patch none

Description Truitt Savell 2020-11-16 12:04:56 PST
fast/lists/001.html
fast/lists/001-vertical.html

these two tests started to flakily crash 

History
https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=fast%2Flists%2F001-vertical.html&test=fast%2Flists%2F001.html

Uploaded logs

Application Specific Information:
CRASHING TEST: fast/lists/001.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x000000010a6c9557 WebCore::RenderStyle::clone(WebCore::RenderStyle const&) + 7 (RenderStyle.cpp:108)
1   com.apple.WebCore             	0x000000010a60bae8 WebCore::RenderListItem::computeMarkerStyle() const + 536
2   com.apple.WebCore             	0x000000010a725ed7 WebCore::RenderTreeBuilder::List::updateItemMarker(WebCore::RenderListItem&) + 151 (RenderTreeBuilderList.cpp:99)
3   com.apple.WebCore             	0x000000010a7225da WebCore::RenderTreeBuilder::updateAfterDescendants(WebCore::RenderElement&) + 74
4   com.apple.WebCore             	0x000000010a72d2b7 WebCore::RenderTreeUpdater::popParent() + 103 (RenderTreeUpdater.cpp:238)
5   com.apple.WebCore             	0x000000010a72bf88 WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&) + 680 (RenderTreeUpdater.cpp:159)
Comment 1 Radar WebKit Bug Importer 2020-11-16 12:05:21 PST
<rdar://problem/71452387>
Comment 2 Truitt Savell 2020-11-16 12:06:56 PST
Created attachment 414263 [details]
001-crash-log.txt
Comment 3 Truitt Savell 2020-11-16 12:07:20 PST
Created attachment 414264 [details]
001-vertical-crash-log.txt
Comment 4 Ryan Haddad 2020-11-16 13:19:16 PST
This is probably related to https://trac.webkit.org/changeset/269774/webkit
Comment 5 Truitt Savell 2020-11-16 13:51:07 PST
I can reproduce these crashes with command:
run-webkit-tests --iterations 2000 --exit-after-n-failures 1 --exit-after-n-crashes-or-timeouts 10 --debug-rwt-logging --no-retry --force --no-build -f fast/lists/001-vertical.html fast/lists/001.html
Comment 6 Truitt Savell 2020-11-18 10:01:41 PST
marked these as skip on Mac wk2 while this is investigated: https://trac.webkit.org/changeset/269964/webkit
Comment 7 Antoine Quint 2020-12-14 03:20:43 PST
Created attachment 416148 [details]
Patch
Comment 8 Antti Koivisto 2020-12-14 03:35:54 PST
Comment on attachment 416148 [details]
Patch

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

> Source/WebCore/rendering/RenderListItem.cpp:65
> -        auto markerStyle = getCachedPseudoStyle(PseudoId::Marker, &style());
> -        ASSERT(markerStyle);
> -        return RenderStyle::clone(*markerStyle);
> +        if (auto markerStyle = getCachedPseudoStyle(PseudoId::Marker, &style()))
> +            return RenderStyle::clone(*markerStyle);

There is an universal ::marker rule on UA sheet. It should never compute null. You should look into. why this is happening.

Maybe it is being optimized away by one of the check in TreeResolver::resolvePseudoStyle?
Comment 9 Antoine Quint 2020-12-15 06:36:35 PST
By the way, this crash is easily reproducible for me with just this command:

run-webkit-tests --debug -1 --no-build fast/lists/001.html
Comment 10 Antoine Quint 2020-12-15 06:48:52 PST
Interestingly, this crash can be reduced to just <div style="display: list-item"></div>, but <li></li> won't crash.
Comment 11 Antoine Quint 2020-12-15 07:29:27 PST
Antti helped me figure this out, this is due to simpleUserAgentStyleSheet being used in these test cases and the ::marker UA style not being used.
Comment 12 Antti Koivisto 2020-12-15 10:20:37 PST
Created attachment 416260 [details]
patch
Comment 13 Antti Koivisto 2020-12-15 11:02:47 PST
Created attachment 416265 [details]
patch
Comment 14 EWS 2020-12-16 04:55:35 PST
Committed r270886: <https://trac.webkit.org/changeset/270886>

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