Bug 206107 - Nullptr crash in WebCore::findPlaceForCounter with display: contents sibling
Summary: Nullptr crash in WebCore::findPlaceForCounter with display: contents sibling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-10 16:20 PST by Jack
Modified: 2020-01-28 09:05 PST (History)
11 users (show)

See Also:


Attachments
Patch (5.28 KB, patch)
2020-01-27 18:04 PST, Jack
no flags Details | Formatted Diff | Diff
Patch (5.26 KB, patch)
2020-01-27 23:27 PST, Jack
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jack 2020-01-10 16:20:59 PST
<rdar://problem/56723217>
Comment 1 Jack 2020-01-27 17:20:58 PST
Root cause of the crash:
In function “findPlaceForCounter”, function “previousSiblingOrParent” will skip all sliblings before a sibling that has display content and finds its parent. On the other hand, function “previousInPreOrder” will search through all the siblings.

In this particular test case, when previousInPreOrder is inserting a counter for “SELECT”, previousSiblingOrParent function starts from TIME but jump to BODY since OL’s renderer is null.

This root cause if verified by modifying function previousSiblingOrParent such that it will return the next slibling if previous one has display content.

*BODY	0x60c00009a300 (renderer 0x61200004d740) 
	SELECT	0x613000064f80 (renderer 0x6150000a0d00) 
	OL	0x60e000058720 (renderer 0x0) 
		TIME	0x60c00009a540 (renderer 0x6110000da240) 
	Q	0x60c00009a600 (renderer 0x6110000da380)  STYLE=counter-increment: c 1;
	#text	0x60800004c720 "\n"
Comment 2 Jack 2020-01-27 17:35:14 PST
Also verified the new patch against the test case attached in https://bugs.webkit.org/show_bug.cgi?id=205290.
Comment 3 Jack 2020-01-27 18:04:05 PST
Created attachment 388953 [details]
Patch
Comment 4 Antti Koivisto 2020-01-27 23:12:14 PST
Comment on attachment 388953 [details]
Patch

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

> Source/WebCore/rendering/RenderCounter.cpp:80
> +    Element* previous = ElementTraversal::pseudoAwarePreviousSibling(*element);

Could be auto*

> Source/WebCore/rendering/RenderCounter.cpp:87
> +    RenderElement* renderer = element->renderer();

auto*

> Source/WebCore/rendering/RenderCounter.cpp:90
> +    if (renderer && renderer->isPseudoElement()) {
> +        return renderer->generatingElement();
>      }

WebKit coding style doesn't use { } around single line blocks.

> Source/WebCore/rendering/RenderCounter.cpp:104
> +    Element* previous = previousSiblingOrParentElement(renderer.element());

auto*
Comment 5 Jack 2020-01-27 23:27:13 PST
Created attachment 388973 [details]
Patch
Comment 6 WebKit Commit Bot 2020-01-28 09:05:42 PST
Comment on attachment 388973 [details]
Patch

Clearing flags on attachment: 388973

Committed r255244: <https://trac.webkit.org/changeset/255244>
Comment 7 WebKit Commit Bot 2020-01-28 09:05:44 PST
All reviewed patches have been landed.  Closing bug.