Bug 249689

Summary: REGRESSION(255641@main): Web process crash in WebCore::isDescendantOfFullScreenLayer when when fullscreening video on reddit.com
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: Layout and RenderingAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, bugs-noreply, mcatanzaro, ntim, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=249690
Attachments:
Description Flags
gdb.txt none

Description Michael Catanzaro 2022-12-20 18:02:20 PST 
Created attachment 464130 [details]
gdb.txt

Visit https://www.reddit.com/r/IdiotsInCars/comments/zqehls/they_said_my_headlights_were_off_and_i_ran_the/ or any other reddit video and play the video, then click the fullscreen button. In Ephy Tech Preview with WebKitGTK 2.39.3, the web process will crash and the UI process hangs. I'll report a separate bug for the UI process hang. The crash looks like a cross-platform issue. Note in particular this=0x0 in frame 2, so the RenderLayerCompositor decided to use a nullptr RenderLayerModelObject. I'll attach a full backtrace with all member variables. Wonder if this reproduces in Safari.

#0  std::__uniq_ptr_impl<WebCore::RenderLayer, std::default_delete<WebCore::RenderLayer> >::_M_ptr() const
    (this=0xa8) at /usr/include/c++/12.1.0/bits/unique_ptr.h:191
#1  std::unique_ptr<WebCore::RenderLayer, std::default_delete<WebCore::RenderLayer> >::get() const (this=0xa8)
    at /usr/include/c++/12.1.0/bits/unique_ptr.h:462
#2  WebCore::RenderLayerModelObject::layer() const (this=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerModelObject.h:48
#3  WebCore::isDescendantOfFullScreenLayer(WebCore::RenderLayer const&) (layer=...)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerCompositor.cpp:2669
#4  0x00007f1687c58ed5 in WebCore::RenderLayerCompositor::requiresCompositingForPosition(WebCore::RenderLayerModelObject&, WebCore::RenderLayer const&, WebCore::RenderLayerCompositor::RequiresCompositingData&) const
    (this=0x7f16760202a0, renderer=..., layer=..., queryData=...)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerCompositor.cpp:3352
#5  0x00007f1687c59104 in WebCore::RenderLayerCompositor::requiresCompositingLayer(WebCore::RenderLayer const&, WebCore::RenderLayerCompositor::RequiresCompositingData&) const (this=0x7f16760202a0, layer=<optimized out>, queryData=...)
    at /usr/include/c++/12.1.0/bits/unique_ptr.h:191
#6  0x00007f1687c59216 in WebCore::RenderLayerCompositor::needsToBeComposited(WebCore::RenderLayer const&, WebCore::RenderLayerCompositor::RequiresCompositingData&) const (this=0x7f16760202a0, layer=..., queryData=<optimized out>)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerCompositor.cpp:2612
#7  0x00007f1687c5ebd5 in WebCore::RenderLayerCompositor::updateBacking(WebCore::RenderLayer&, WebCore::RenderLayerCompositor::RequiresCompositingData&, WebCore::RenderLayerCompositor::BackingSharingState*, WebCore::RenderLayerCompositor::BackingRequired)
    (this=0x7f16760202a0, layer=..., queryData=..., backingSharingState=<optimized out>, backingRequired=<optimized out>) at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerCompositor.cpp:1827
#8  0x00007f1687c5ef55 in WebCore::RenderLayerCompositor::layerStyleChanged(WebCore::StyleDifference, WebCore::RenderLayer&, WebCore::RenderStyle const*)
    (this=0x7f16760202a0, diff=WebCore::StyleDifference::NewStyle, layer=..., oldStyle=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerCompositor.cpp:1697
#9  0x00007f1687c645e1 in WebCore::RenderLayer::styleChanged(WebCore::StyleDifference, WebCore::RenderStyle const*)
    (this=0x7f148e5a1870, diff=WebCore::StyleDifference::NewStyle, oldStyle=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayer.cpp:5371
#10 0x00007f1687c6486b in WebCore::RenderLayerModelObject::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
    (this=this@entry=0x7f14161f8440, diff=diff@entry=WebCore::StyleDifference::NewStyle, oldStyle=oldStyle@entry=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderLayerModelObject.cpp:168
#11 0x00007f1687bcf09c in WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
     (this=this@entry=0x7f14161f8440, diff=diff@entry=WebCore::StyleDifference::NewStyle, oldStyle=oldStyle@entry=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderBox.cpp:319
#12 0x00007f1687b9211f in WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
    (this=this@entry=0x7f14161f8440, diff=diff@entry=WebCore::StyleDifference::NewStyle, oldStyle=oldStyle@entry=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderBlock.cpp:459
#13 0x00007f1687b92612 in WebCore::RenderBlockFlow::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) (this=0x7f14161f8440, diff=WebCore::StyleDifference::NewStyle, oldStyle=0x0)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/RenderBlockFlow.cpp:2147
#14 0x00007f1687def62b in WebCore::RenderTreeUpdater::createRenderer(WebCore::Element&, WebCore::RenderStyle&&)
    (this=this@entry=0x7ffea77cc4f0, element=..., style=...) at /usr/include/c++/12.1.0/bits/unique_ptr.h:191
#15 0x00007f1687def809 in WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&) (this=this@entry=0x7ffea77cc4f0, element=..., elementUpdate=...)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:352
#16 0x00007f1687df174c in WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&)
    (this=0x7ffea77cc4f0, root=<optimized out>)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:187
#17 0x00007f1687df1e43 in WebCore::RenderTreeUpdater::commit(std::unique_ptr<WebCore::Style::Update const, std::defaul--Type <RET> for more, q to quit, c to continue without paging--
t_delete<WebCore::Style::Update const> >)
     (this=0x7ffea77cc4f0, styleUpdate=std::unique_ptr<const WebCore::Style::Update> = {...})
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:114
#18 0x00007f1687102d4c in WebCore::Document::updateRenderTree(std::unique_ptr<WebCore::Style::Update const, std::default_delete<WebCore::Style::Update const> >)
     (this=this@entry=0x7f1626140c00, styleUpdate=std::unique_ptr<const WebCore::Style::Update> = {...})
    at /usr/include/c++/12.1.0/bits/unique_ptr.h:189
#19 0x00007f168711f13b in WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType)
    (this=this@entry=0x7f1626140c00, type=<optimized out>, type@entry=WebCore::Document::ResolveStyleType::Normal)
    at /usr/include/c++/12.1.0/tuple:199
#20 0x00007f168711f8be in WebCore::Document::updateStyleIfNeeded() (this=0x7f1626140c00)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/dom/Document.cpp:2258
#21 WebCore::Document::updateStyleIfNeeded() (this=0x7f1626140c00)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/dom/Document.cpp:2233
#22 0x00007f1687120aab in WebCore::Document::updateLayoutIfDimensionsOutOfDate(WebCore::Element&, WebCore::DimensionsCheck) (this=0x7f1626140c00, element=..., dimensionsCheck=dimensionsCheck@entry=WebCore::HeightDimensionsCheck)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/dom/Document.cpp:2360
#23 0x00007f1687140b29 in WebCore::Element::clientHeight() (this=0x7f160209c850)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/dom/Element.cpp:1419
#24 0x00007f1686408c51 in WebCore::jsElement_clientHeightGetter
    (thisObject=<optimized out>, lexicalGlobalObject=<optimized out>)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/_builddir/WebCore/DerivedSources/JSElement.cpp:3050
#25 WebCore::IDLAttribute<WebCore::JSElement>::get<WebCore::jsElement_clientHeightGetter, (WebCore::CastedThisErrorBehavior)3> (attributeName=..., thisValue=<optimized out>, lexicalGlobalObject=<optimized out>)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/WebCore/bindings/js/JSDOMAttribute.h:88
#26 WebCore::jsElement_clientHeight(JSC::JSGlobalObject*, JSC::EncodedJSValue, JSC::PropertyName)
    (lexicalGlobalObject=<optimized out>, thisValue=<optimized out>, attributeName=...)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/_builddir/WebCore/DerivedSources/JSElement.cpp:3055
#27 0x00007f16844f2708 in WTF::FunctionPtr<(WTF::PtrTag)57072, long (JSC::JSGlobalObject*, long, JSC::PropertyName), (WTF::FunctionAttributes)1>::operator()(JSC::JSGlobalObject*, long, JSC::PropertyName) const
    (this=0x7ffea77cca50, in#2=..., in#1=<optimized out>, in#0=<optimized out>)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/_builddir/WTF/Headers/wtf/FunctionPtr.h:101
#28 JSC::PropertySlot::customGetter(JSC::VM&, JSC::PropertyName) const
    (this=this@entry=0x7ffea77ccc20, vm=<optimized out>, propertyName=..., propertyName@entry=...)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/JavaScriptCore/runtime/PropertySlot.cpp:47
#29 0x00007f16841602bf in JSC::PropertySlot::getValue(JSC::JSGlobalObject*, JSC::PropertyName) const
    (propertyName=..., globalObject=<optimized out>, this=<optimized out>)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/JavaScriptCore/runtime/PropertySlot.h:405
#30 JSC::JSValue::get(JSC::JSGlobalObject*, JSC::PropertyName, JSC::PropertySlot&) const
    (slot=..., propertyName=..., globalObject=<optimized out>, this=0x7ffea77ccbd8)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/JavaScriptCore/runtime/JSCJSValueInlines.h:1045
#31 JSC::LLInt::performLLIntGetByID(JSC::BytecodeIndex, JSC::CodeBlock*, JSC::JSGlobalObject*, JSC::JSValue, JSC::Identifier const&, JSC::GetByIdModeMetadata&)
    (bytecodeIndex=..., codeBlock=0x7f14c5945b70, globalObject=<optimized out>, baseValue=..., ident=..., metadata=...) at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:814
#32 0x00007f1684160ded in JSC::LLInt::llint_slow_path_get_by_id(JSC::CallFrame*, JSC::JSInstruction const*)
    (callFrame=0x7ffea77cce20, pc=0x7f1676a3b25e)
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:888
#33 0x00007f16835a8734 in llint_op_get_by_id ()
    at /usr/lib/debug/source/sdk/webkitgtk-6.0.bst/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:118
#34 0x00007f14b56da9e8 in  ()
Comment 1 Michael Catanzaro 2022-12-20 18:05:32 PST 
The associated UI process hang is bug #249690 (which I'll assume, without evidence, to be GTK-specific).
Comment 2 Sam Sneddon [:gsnedders] 2022-12-21 07:27:16 PST 
> Wonder if this reproduces in Safari.

I can't reproduce it anywhere in Safari (either STP or stable, macOS Ventura), FWIW.
Comment 3 Michael Catanzaro 2023-01-04 13:01:46 PST 
There is a bug in RenderLayerCompositor::isDescendantOfFullScreenLayer, here:

    auto* fullScreenRenderer = dynamicDowncast<RenderLayerModelObject>(fullScreenElement->renderer());
    auto* fullScreenLayer = fullScreenRenderer->layer();
    if (!fullScreenRenderer || !fullScreenLayer)
        return FullScreenDescendant::NotApplicable;

The code first assumes that fullScreenRenderer is not nullptr (as if the dynamicDowncast cannot fail) and uses it unconditionally. Then it checks to see if it's nullptr on the very next line. No good. The downcast is surely failing here. There might be a platform-specific reason for that, but this is a cross-platform bug.
Comment 4 Michael Catanzaro 2023-01-04 14:00:06 PST 
Actually that's the only problem here. Fullscreen works fine with that fixed.
Comment 5 Brent Fulgham 2023-01-04 14:07:25 PST 
Certainly seems worth fixing!
Comment 6 Michael Catanzaro 2023-01-04 14:17:43 PST 
Pull request: https://github.com/WebKit/WebKit/pull/8213
Comment 7 Radar WebKit Bug Importer 2023-01-04 14:24:13 PST 
<rdar://problem/103888322>
Comment 8 EWS 2023-01-06 18:54:50 PST 
Committed 258593@main (e29dfab61f35): <https://commits.webkit.org/258593@main>

Reviewed commits have been landed. Closing PR #8213 and removing active labels.