Bug 223864

Summary: [GPU Process] Don't record DisplayList items if the GPU Process has been terminated
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Said Abou-Hallawa 2021-03-28 22:59:53 PDT 
Otherwise the following ASSERT_NOT_REACHED() will fire in Debug builds:

SHOULD NEVER BE REACHED
/Volumes/Data/worker/macOS-AppleSilicon-Big-Sur-Debug-Build-EWS/build/Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h(300) : virtual WebCore::DisplayList::ItemBufferHandle WebKit::RemoteImageBufferProxy<WebKit::ImageBufferShareableMappedIOSurfaceBackend>::createItemBuffer(size_t) [BackendType = WebKit::ImageBufferShareableMappedIOSurfaceBackend]
1   0x132ef36e0 WTFCrash
2   0x10445c158 WTF::Ref<WebKit::IPCTestingAPI::JSIPC, WTF::RawPtrTraits<WebKit::IPCTestingAPI::JSIPC> > WTF::makeRef<WebKit::IPCTestingAPI::JSIPC>(WebKit::IPCTestingAPI::JSIPC&)
3   0x10410e9a0 WebKit::RemoteImageBufferProxy<WebKit::ImageBufferShareableMappedIOSurfaceBackend>::createItemBuffer(unsigned long)
4   0x117900710 WebCore::DisplayList::ItemBuffer::createItemBuffer(unsigned long)
5   0x117900ad0 WebCore::DisplayList::ItemBuffer::swapWritableBufferIfNeeded(unsigned long)
6   0x1179266c8 void WebCore::DisplayList::ItemBuffer::append<WebCore::DisplayList::Restore>()
7   0x11792667c void WebCore::DisplayList::DisplayList::append<WebCore::DisplayList::Restore>()
8   0x117908f80 void WebCore::DisplayList::Recorder::append<WebCore::DisplayList::Restore>()
9   0x117908eec WebCore::DisplayList::Recorder::restore()
10  0x1177182fc WebCore::GraphicsContext::restore()
11  0x113843a68 WebCore::GraphicsContextStateSaver::~GraphicsContextStateSaver()
12  0x113830910 WebCore::GraphicsContextStateSaver::~GraphicsContextStateSaver()
13  0x11681cd8c std::__1::default_delete<WebCore::GraphicsContextStateSaver>::operator()(WebCore::GraphicsContextStateSaver*) const
14  0x11681cd10 std::__1::unique_ptr<WebCore::GraphicsContextStateSaver, std::__1::default_delete<WebCore::GraphicsContextStateSaver> >::reset(WebCore::GraphicsContextStateSaver*)
15  0x1168103a8 std::__1::unique_ptr<WebCore::GraphicsContextStateSaver, std::__1::default_delete<WebCore::GraphicsContextStateSaver> >::operator=(std::nullptr_t)
16  0x1168100bc WebCore::CanvasBase::setImageBuffer(WTF::RefPtr<WebCore::ImageBuffer, WTF::RawPtrTraits<WebCore::ImageBuffer>, WTF::DefaultRefDerefTraits<WebCore::ImageBuffer> >&&) const
17  0x116882658 WebCore::HTMLCanvasElement::~HTMLCanvasElement()
18  0x11688275c WebCore::HTMLCanvasElement::~HTMLCanvasElement()
19  0x1168827c8 WebCore::HTMLCanvasElement::~HTMLCanvasElement()
20  0x11651df34 WebCore::Node::removedLastRef()
21  0x113afc6a4 WebCore::Node::deref() const
22  0x11440ce58 WTF::DefaultRefDerefTraits<WebCore::Node>::derefIfNotNull(WebCore::Node*)
23  0x11440ce10 WTF::RefPtr<WebCore::Node, WTF::RawPtrTraits<WebCore::Node>, WTF::DefaultRefDerefTraits<WebCore::Node> >::~RefPtr()
24  0x11440cb30 WTF::RefPtr<WebCore::Node, WTF::RawPtrTraits<WebCore::Node>, WTF::DefaultRefDerefTraits<WebCore::Node> >::~RefPtr()
25  0x115a9cb98 WTF::RefPtr<WebCore::Node, WTF::RawPtrTraits<WebCore::Node>, WTF::DefaultRefDerefTraits<WebCore::Node> >::operator=(WTF::RefPtr<WebCore::Node, WTF::RawPtrTraits<WebCore::Node>, WTF::DefaultRefDerefTraits<WebCore::Node> > const&)
26  0x116311fa4 WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&)
27  0x116312038 WebCore::removeDetachedChildrenInContainer(WebCore::ContainerNode&)
28  0x1162eaff8 WebCore::ContainerNode::removeDetachedChildren()
29  0x1162eb530 WebCore::ContainerNode::~ContainerNode()
30  0x116445e50 WebCore::Element::~Element()
31  0x1165e8df4 WebCore::StyledElement::~StyledElement()
LEAK: 2 WebPageProxy

In release builds will end up allocating a local ItemBufferHandle which is unusual scenario for a RemoteImageBuffer.
Comment 1 Said Abou-Hallawa 2021-03-28 23:05:02 PDT 
<rdar://73586187>
Comment 2 Said Abou-Hallawa 2021-03-28 23:09:17 PDT 
Created attachment 424526 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-03-29 14:43:27 PDT 
Comment on attachment 424526 [details]
Patch

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

> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:68
> -        virtual void willAppendItemOfType(ItemType) { }
> +        virtual bool canAppendItemOfType(ItemType) { return false; }

What is the caller supposed to do with the return value?
Comment 4 Said Abou-Hallawa 2021-03-29 15:33:18 PDT 
Comment on attachment 424526 [details]
Patch

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

>> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:68
>> +        virtual bool canAppendItemOfType(ItemType) { return false; }
> 
> What is the caller supposed to do with the return value?

If the return value is 'false', the caller will not append the DisplayList::Item to the DisplayList. The append() method in this header file bails out early if the return value is 'false'
Comment 5 EWS 2021-03-29 15:38:30 PDT 
Committed r275185: <https://commits.webkit.org/r275185>

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