Bug 218995 – Remove simpleUserAgentStyleSheet (to fix flaky fast/lists/001.html and fast/lists/001-vertical.html)

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Truitt Savell]

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)

[Radar WebKit Bug Importer]

<rdar://problem/71452387>

[Truitt Savell]

Created attachment 414263 [details]
001-crash-log.txt

[Truitt Savell]

Created attachment 414264 [details]
001-vertical-crash-log.txt

[Ryan Haddad]

This is probably related to https://trac.webkit.org/changeset/269774/webkit

[Truitt Savell]

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

[Truitt Savell]

marked these as skip on Mac wk2 while this is investigated: https://trac.webkit.org/changeset/269964/webkit

[Antoine Quint]

Created attachment 416148 [details]
Patch

[Antti Koivisto]

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?

[Antoine Quint]

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

[Antoine Quint]

Interestingly, this crash can be reduced to just <div style="display: list-item"></div>, but <li></li> won't crash.

[Antoine Quint]

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.

[Antti Koivisto]

Created attachment 416260 [details]
patch

[Antti Koivisto]

Created attachment 416265 [details]
patch

[EWS]

Committed r270886: <https://trac.webkit.org/changeset/270886>

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