Bug 225175 - [GPU Process] REGRESSION(r272888): Don't assert the validity of the dataURL mimeType inside GPU Process
Summary: [GPU Process] REGRESSION(r272888): Don't assert the validity of the dataURL m...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-28 17:17 PDT by Said Abou-Hallawa
Modified: 2021-04-29 01:27 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.89 KB, patch)
2021-04-28 18:02 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.93 KB, patch)
2021-04-28 18:07 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>.