Bug 225175

Summary: [GPU Process] REGRESSION(r272888): Don't assert the validity of the dataURL mimeType inside GPU Process
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, graouts, kkinnunen, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221885
Attachments:
Description Flags
Patch
none
Patch none

Description Said Abou-Hallawa 2021-04-28 17:17:43 PDT
We disabled image decoding inside GPUProcess in r272888. MIMETypeRegistry::isSupportedImageMIMETypeForEncoding() will return false for any mimeType if it is called inside GPUProcess. So ImageBufferCGBackend::toCFData() should not assert the validity of the mimeType in this case.

Note: If for any reason the mimeType is altered while sending the message RemoteRenderingBackend.GetDataURLForImageBuffer to GPU Process, ImageBufferCGBackend::toCFData() will fail peacefully if the "system" frameworks do not support the mimeType.
Comment 1 Said Abou-Hallawa 2021-04-28 17:18:23 PDT
<rdar://76286963>
Comment 2 Said Abou-Hallawa 2021-04-28 18:02:48 PDT
Created attachment 427317 [details]
Patch
Comment 3 Said Abou-Hallawa 2021-04-28 18:07:33 PDT
Created attachment 427319 [details]
Patch
Comment 4 Said Abou-Hallawa 2021-04-28 18:54:50 PDT
If this is too much to fix an assertion, I can remove it altogether.
Comment 5 Said Abou-Hallawa 2021-04-28 19:08:51 PDT
This is the call stack we hit when GPUP for 2D canvas is enabled and toDataURL() is called:

ASSERTION FAILED: MIMETypeRegistry::isSupportedImageMIMETypeForEncoding(mimeType)
./platform/graphics/cg/ImageBufferCGBackend.cpp(169) : virtual RetainPtr<CFDataRef> WebCore::ImageBufferCGBackend::toCFData(const WTF::String &, Optional<double>, WebCore::PreserveResolution) const
1   0x131ea1879 WTFCrash
2   0x142d26e0b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x146c83a27 WebCore::ImageBufferCGBackend::toCFData(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution) const
4   0x146c86212 WebCore::ImageBufferIOSurfaceBackend::toCFData(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution) const
5   0x146c8457a WebCore::ImageBufferCGBackend::toDataURL(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution) const
6   0x11b7e8dbe WebCore::ConcreteImageBuffer<WebKit::ImageBufferShareableMappedIOSurfaceBackend>::toDataURL(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution) const
7   0x11b7daf88 WebKit::RemoteRenderingBackend::getDataURLForImageBuffer(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType>, WTF::CompletionHandler<void (WTF::String&&)>&&)
8   0x11b7a276c void IPC::callMemberFunctionImpl<WebKit::RemoteRenderingBackend, void (WebKit::RemoteRenderingBackend::*)(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType>, WTF::CompletionHandler<void (WTF::String&&)>&&), void (WTF::String const&), std::1::tuple<WTF::String, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType> >, 0ul, 1ul, 2ul, 3ul>(WebKit::RemoteRenderingBackend*, void (WebKit::RemoteRenderingBackend::*)(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType>, WTF::CompletionHandler<void (WTF::String&&)>&&), WTF::CompletionHandler<void (WTF::String const&)>&&, std::1::tuple<WTF::String, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType> >&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>)
9   0x11b79ffb2 void IPC::callMemberFunction<WebKit::RemoteRenderingBackend, void (WebKit::RemoteRenderingBackend::*)(WTF::String const&, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType>, WTF::CompletionHandler<void (WTF::String&&)>&&), void (WTF::String const&), std::1::tuple<WTF::String, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType> >, std::1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul> >(std::__1::tuple<WTF::String, WTF::Optional<double>, WebCore::PreserveResolution, WTF::ObjectIdentifier<WebCore::RenderingResourceIdentifierType> >&&, WTF::CompletionHandler<void (WTF::String const&)>&&, WebKi
Comment 6 Antoine Quint 2021-04-29 00:31:49 PDT
Comment on attachment 427319 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp:178
> +    ASSERT_IMPLIES(!isInGPUProcess(), MIMETypeRegistry::isSupportedImageMIMETypeForEncoding(mimeType));

Wenson suggested that having a `isInGPUProcess()` check in WebCore platform code was probably not a good idea.
Comment 7 Antoine Quint 2021-04-29 00:32:54 PDT
(In reply to Antoine Quint from comment #6)
> Comment on attachment 427319 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=427319&action=review
> 
> > Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp:178
> > +    ASSERT_IMPLIES(!isInGPUProcess(), MIMETypeRegistry::isSupportedImageMIMETypeForEncoding(mimeType));
> 
> Wenson suggested that having a `isInGPUProcess()` check in WebCore platform
> code was probably not a good idea.

Although I suppose this merely checks that this can only be false when in the GPU process.
Comment 8 EWS 2021-04-29 00:45:54 PDT
Committed r276753 (237149@main): <https://commits.webkit.org/237149@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427319 [details].
Comment 9 Said Abou-Hallawa 2021-04-29 01:27:21 PDT
A follow-up patch: r276758: <https://commits.webkit.org/r276758>.