Bug 209112 - ASSERTION FAILURE: !result.innerNode() || (request.resultIsElementList() && result.listBasedTestResult().size()) in RenderLayer::hitTestContents()
Summary: ASSERTION FAILURE: !result.innerNode() || (request.resultIsElementList() && r...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-14 13:13 PDT by Daniel Bates
Modified: 2020-03-16 10:35 PDT (History)
14 users (show)

See Also:


Attachments
Test (574 bytes, text/html)
2020-03-14 13:13 PDT, Daniel Bates
no flags Details
Patch (28.42 KB, patch)
2020-03-14 17:42 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (35.94 KB, patch)
2020-03-14 22:23 PDT, Daniel Bates
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2020-03-14 13:13:14 PDT
Steps to reproduce:

Using a debug build + MiniBrowser.

Consider a page with the following markup:

[[
<style>
.icon::before {
    content: url('data:image/svg+xml,<svg xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" version="1.1" height="727px"></svg>');
}

.icon {
    position: absolute;
}
</style>
<body>
    <p>This test FAILED if it causes an assertion failure a debug build.</p>
    <div class="icon"></div>
    <script>
        // Need to use a zero-timer to ensure we run this code AFTER .icon has loaded its content.
        window.setTimeout(() => {
            document.elementsFromPoint(200, 200);
        }, 0);
    </script>
</body>
]]

1. Ensure Settings > Internal Features > Simple line layout is DISABLED (under Debug > WebKit Internal Features in Safari)

2. Ensure Settings > Internal Features > Next-generation line layout integration (LFC) is DISABLED (under Debug > WebKit Internal Features in Safari).

3. Open in a NEW WK2 window in MiniBrowser the above page.

Then the WebContent process with crash due to the ASSERT(!result.innerNode() || (request.resultIsElementList() && result.listBasedTestResult().size()) in RenderLayer::hitTestContents()) failing in RenderLayer::hitTestContents(): <https://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayer.cpp?rev=258400#L5630>.
Comment 1 Daniel Bates 2020-03-14 13:13:40 PDT
Created attachment 393589 [details]
Test

Test case (for convenience)
Comment 2 Daniel Bates 2020-03-14 13:14:43 PDT
(In reply to Daniel Bates from comment #1)
> Created attachment 393589 [details]
> Test
> 
> Test case (for convenience)

If you open this test case from Bugzilla, you may need to reload it to trigger the bug.
Comment 3 Daniel Bates 2020-03-14 13:15:29 PDT
#0  0x000000012d55190e in ::WTFCrash() at /Volumes/.../Source/WTF/wtf/Assertions.cpp:305
#1  0x0000000112c138eb in WTFCrashWithInfo(int, char const*, char const*, int) at /Volumes/.../WebKitBuild/Debug/usr/local/include/wtf/Assertions.h:620
#2  0x00000001168a7bd5 in WebCore::RenderLayer::hitTestContents(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::LayoutRect const&, WebCore::HitTestLocation const&, WebCore::HitTestFilter) const at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5630
#3  0x00000001168a7866 in WebCore::RenderLayer::hitTestContentsForFragments(WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::HitTestLocation const&, WebCore::HitTestFilter, bool&) const at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5533
#4  0x00000001168a5fbb in WebCore::RenderLayer::hitTestLayer(WebCore::RenderLayer*, WebCore::RenderLayer*, WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::LayoutRect const&, WebCore::HitTestLocation const&, bool, WebCore::HitTestingTransformState const*, double*) at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5476
#5  0x00000001168a747d in WebCore::RenderLayer::hitTestList(WebCore::RenderLayer::LayerList, WebCore::RenderLayer*, WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::LayoutRect const&, WebCore::HitTestLocation const&, WebCore::HitTestingTransformState const*, double*, double*, WebCore::HitTestingTransformState const*, bool) at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5676
#6  0x00000001168a5c2b in WebCore::RenderLayer::hitTestLayer(WebCore::RenderLayer*, WebCore::RenderLayer*, WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::LayoutRect const&, WebCore::HitTestLocation const&, bool, WebCore::HitTestingTransformState const*, double*) at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5443
#7  0x00000001168a747d in WebCore::RenderLayer::hitTestList(WebCore::RenderLayer::LayerList, WebCore::RenderLayer*, WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::LayoutRect const&, WebCore::HitTestLocation const&, WebCore::HitTestingTransformState const*, double*, double*, WebCore::HitTestingTransformState const*, bool) at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5676
#8  0x00000001168a5c2b in WebCore::RenderLayer::hitTestLayer(WebCore::RenderLayer*, WebCore::RenderLayer*, WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::LayoutRect const&, WebCore::HitTestLocation const&, bool, WebCore::HitTestingTransformState const*, double*) at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5443
#9  0x00000001168a5285 in WebCore::RenderLayer::hitTest(WebCore::HitTestRequest const&, WebCore::HitTestLocation const&, WebCore::HitTestResult&) at /Volumes/.../Source/WebCore/rendering/RenderLayer.cpp:5238
#10 0x000000011535c2bf in WebCore::Document::hitTest(WebCore::HitTestRequest const&, WebCore::HitTestLocation const&, WebCore::HitTestResult&) at /Volumes/.../Source/WebCore/dom/Document.cpp:8269
#11 0x0000000115340f74 in WebCore::Document::hitTest(WebCore::HitTestRequest const&, WebCore::HitTestResult&) at /Volumes/.../Source/WebCore/dom/Document.cpp:8250
#12 0x0000000115572772 in WebCore::TreeScope::elementsFromPoint(double, double) at /Volumes/.../Source/WebCore/dom/TreeScope.cpp:400
#13 0x00000001134c68f9 in WebCore::jsDocumentPrototypeFunctionElementsFromPointBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*, JSC::ThrowScope&) at /Volumes/.../WebKitBuild/Debug/DerivedSources/WebCore/JSDocument.cpp:5971
#14 0x00000001133cffc2 in long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunctionElementsFromPointBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) at /Volumes/.../Source/WebCore/bindings/js/JSDOMOperation.h:53
#15 0x00000001133cfca4 in WebCore::jsDocumentPrototypeFunctionElementsFromPoint(JSC::JSGlobalObject*, JSC::CallFrame*) at /Volumes/.../WebKitBuild/Debug/DerivedSources/WebCore/JSDocument.cpp:5976
#16 0x0000344110c01178 in 0x344110c01178 ()
#17 0x000000012da511e3 in llint_entry at /Volumes/.../Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1039
#18 0x000000012da33dc3 in vmEntryToJavaScript at /Volumes/.../Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:296
#19 0x000000012e7c0a77 in JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) at /Volumes/.../Source/JavaScriptCore/jit/JITCodeInlines.h:38
#20 0x000000012e7c10ed in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) at /Volumes/.../Source/JavaScriptCore/interpreter/Interpreter.cpp:910
#21 0x000000012eac9b69 in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) at /Volumes/.../Source/JavaScriptCore/runtime/CallData.cpp:59
#22 0x000000012eac9c5a in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) at /Volumes/.../Source/JavaScriptCore/runtime/CallData.cpp:66
#23 0x000000012eac9f4e in JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) at /Volumes/.../Source/JavaScriptCore/runtime/CallData.cpp:87
#24 0x0000000114dd2d78 in WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) at /Volumes/.../Source/WebCore/bindings/js/JSExecState.h:73
#25 0x0000000114e72a64 in WebCore::ScheduledAction::executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue, WebCore::ScriptExecutionContext&) at /Volumes/.../Source/WebCore/bindings/js/ScheduledAction.cpp:118
#26 0x0000000114e72505 in WebCore::ScheduledAction::execute(WebCore::Document&) at /Volumes/.../Source/WebCore/bindings/js/ScheduledAction.cpp:137
#27 0x0000000114e723c3 in WebCore::ScheduledAction::execute(WebCore::ScriptExecutionContext&) at /Volumes/.../Source/WebCore/bindings/js/ScheduledAction.cpp:86
#28 0x0000000115f70559 in WebCore::DOMTimer::fired() at /Volumes/.../Source/WebCore/page/DOMTimer.cpp:340
#29 0x0000000116242304 in WebCore::ThreadTimers::sharedTimerFiredInternal() at /Volumes/.../Source/WebCore/platform/ThreadTimers.cpp:127
#30 0x0000000116249ff1 in WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const at /Volumes/.../Source/WebCore/platform/ThreadTimers.cpp:67
#31 0x0000000116249f9e in WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call() at /Volumes/.../WebKitBuild/Debug/usr/local/include/wtf/Function.h:52
#32 0x0000000112c261d2 in WTF::Function<void ()>::operator()() const at /Volumes/.../WebKitBuild/Debug/usr/local/include/wtf/Function.h:84
#33 0x000000011620ff5b in WebCore::MainThreadSharedTimer::fired() at /Volumes/.../Source/WebCore/platform/MainThreadSharedTimer.cpp:83
#34 0x00000001162a4a86 in WebCore::timerFired(__CFRunLoopTimer*, void*) at /Volumes/.../Source/WebCore/platform/cf/MainThreadSharedTimerCF.cpp:74
Comment 4 Daniel Bates 2020-03-14 17:42:40 PDT
Created attachment 393601 [details]
Patch
Comment 5 Daniel Bates 2020-03-14 21:47:58 PDT
Test failures are progressions.
Comment 6 Daniel Bates 2020-03-14 22:23:06 PDT
Created attachment 393605 [details]
Patch
Comment 7 Daniel Bates 2020-03-14 23:11:34 PDT
Comment on attachment 393605 [details]
Patch

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

> Source/WebCore/ChangeLog:37
> +        was passed, which is nullptr for an anonymouse node, and addNodeToListBasedTestResult() doesn't have enough info

anonymouse => anonymous
Comment 8 Darin Adler 2020-03-15 17:22:26 PDT
Comment on attachment 393605 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:1993
>      return continuation() ? continuation()->element() : element();

Do we have a guarantee these are not the anonymous renderers in before or after content? They can’t be RenderBlock?
Comment 9 Daniel Bates 2020-03-16 10:15:03 PDT
Comment on attachment 393605 [details]
Patch

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

>> Source/WebCore/rendering/RenderBlock.cpp:1993
>>      return continuation() ? continuation()->element() : element();
> 
> Do we have a guarantee these are not the anonymous renderers in before or after content? They can’t be RenderBlock?

🤷‍♂️. I didn't think about it. I only thought about the isRenderView logic. I cannot think of a way using CSS for ::before or ::after content to contain anonymous blocks. Looks like if inserting a renderer for ::before or ::after breaks the inline/block invariants then anonymous blocks are added to the **containing element**.

Here's some examples I was playing with:

1. ::before with display:block

Because ::before generates a block, anonymous blocks must be added and the span split.

<data:text/html,<style>span::before { content: "FL"; display: block; } span::first-letter { color:red;  float:left; }</style><span>before span text<div>span text</div>after span text</span>>

2 . ::before default (display:inline)
<data:text/html,<style>span::before { content: "FL" } span::first-letter { color:red;  float:left; }</style><span>before span text<div>span text</div>after span text</span>>

3. <p> must be split:

<data:text/html,<style>p::before { content: "FL" } p::first-letter { color:red;  float:left; }</style><p>before div text<div>div text</div>after div text</p>>

4. Anonymous blocks must be added for the text before and after then <img>.

<data:text/html,<style>p::before { content: "FL" } p::first-letter { color:red;  float:left; }</style><p>before img text<img style="display: block" height="20" width="20">after img text</p>>
Comment 10 Daniel Bates 2020-03-16 10:15:14 PDT
Thanks for the review.
Comment 11 Daniel Bates 2020-03-16 10:30:51 PDT
Comment on attachment 393605 [details]
Patch

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

>>> Source/WebCore/rendering/RenderBlock.cpp:1993
>>>      return continuation() ? continuation()->element() : element();
>> 
>> Do we have a guarantee these are not the anonymous renderers in before or after content? They can’t be RenderBlock?
> 
> 🤷‍♂️. I didn't think about it. I only thought about the isRenderView logic. I cannot think of a way using CSS for ::before or ::after content to contain anonymous blocks. Looks like if inserting a renderer for ::before or ::after breaks the inline/block invariants then anonymous blocks are added to the **containing element**.
> 
> Here's some examples I was playing with:
> 
> 1. ::before with display:block
> 
> Because ::before generates a block, anonymous blocks must be added and the span split.
> 
> <data:text/html,<style>span::before { content: "FL"; display: block; } span::first-letter { color:red;  float:left; }</style><span>before span text<div>span text</div>after span text</span>>
> 
> 2 . ::before default (display:inline)
> <data:text/html,<style>span::before { content: "FL" } span::first-letter { color:red;  float:left; }</style><span>before span text<div>span text</div>after span text</span>>
> 
> 3. <p> must be split:
> 
> <data:text/html,<style>p::before { content: "FL" } p::first-letter { color:red;  float:left; }</style><p>before div text<div>div text</div>after div text</p>>
> 
> 4. Anonymous blocks must be added for the text before and after then <img>.
> 
> <data:text/html,<style>p::before { content: "FL" } p::first-letter { color:red;  float:left; }</style><p>before img text<img style="display: block" height="20" width="20">after img text</p>>

containing element => originating element

Ok.. I looked at the specs, emphasis mine:

[[

The content property dictates what is rendered inside an element or pseudo-element.

For elements, it has only one purpose: specifying that the element renders as normal, or replacing the element with an image (and possibly some associated "alt text").

For pseudo-elements and margin boxes, it is more powerful. It controls whether the element renders at all, can replace the element with an image, or replace it with ***arbitrary inline content (text and images).***

]]
<https://drafts.csswg.org/css-content-3/#propdef-content>

So, the answer to your question is: No, they cannot have anonymous renderer because they can only have inline content (by def of CSS content).


And just for me:


[[
4.1. Generated Content Pseudo-elements: ::before and ::after

When their computed content value is not none, these pseudo-elements generate boxes as if they were immediate children of their originating element, and can be styled exactly like any normal document-sourced element in the document tree.
]]
<https://drafts.csswg.org/css-pseudo-4/#selectordef-before>


^^^ means that anonymous blocks are generated in the originating element, if needed, when inserted ::before, ::after generated boxes.
Comment 12 Daniel Bates 2020-03-16 10:34:51 PDT
Committed r258508: <https://trac.webkit.org/changeset/258508>
Comment 13 Radar WebKit Bug Importer 2020-03-16 10:35:16 PDT
<rdar://problem/60500448>