Bug 240843 - Re-enable HEIC tests
Summary: Re-enable HEIC tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikos Mouchtaris
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-23 17:36 PDT by Nikos Mouchtaris
Modified: 2022-05-26 16:15 PDT (History)
1 user (show)

See Also:


Attachments
Patch (8.97 KB, patch)
2022-05-23 18:26 PDT, Nikos Mouchtaris
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikos Mouchtaris 2022-05-23 17:36:26 PDT
Although we are not supporting HEIC images in web content, we want to re-enable these tests as this process is still being used in Mail/other apps.
Comment 1 Nikos Mouchtaris 2022-05-23 17:49:07 PDT
rdar://90658250
Comment 2 Nikos Mouchtaris 2022-05-23 18:26:55 PDT
Created attachment 459694 [details]
Patch
Comment 3 Alexey Proskuryakov 2022-05-23 19:28:30 PDT
Comment on attachment 459694 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:434
> +    if (MacApplication::isAppleMail() || CocoaApplication::isWebkitTestRunner()) {

What are we trying to achieve with this check? It's not clear to me if this is security sensitive, but anyone can build a "WebKitTestRunner". Maybe a "Mail" too, depending on how the check is implemented.

> LayoutTests/platform/mac/TestExpectations:2305
> +[ Catalina+ ] fast/images/animated-heics-draw.html [ Pass ]
> +[ Catalina+ ] fast/images/animated-heics-verify.html [ Pass ]
> +[ BigSur+ ] fast/images/heic-as-background-image.html [ Pass ]

OS version qualifiers are not needed here, Big Sur is the oldest supported OS version.
Comment 4 Nikos Mouchtaris 2022-05-24 11:09:37 PDT
Pull request: https://github.com/WebKit/WebKit/pull/979
Comment 5 Nikos Mouchtaris 2022-05-24 11:10:31 PDT
(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 459694 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=459694&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:434
> > +    if (MacApplication::isAppleMail() || CocoaApplication::isWebkitTestRunner()) {
> 
> What are we trying to achieve with this check? It's not clear to me if this
> is security sensitive, but anyone can build a "WebKitTestRunner". Maybe a
> "Mail" too, depending on how the check is implemented.
> 

This is what allows the tests to decode the HEIC images. This just checks the bundle identifier, which is the same as the mail check. I don't think this would have any security implications, but I will ask Per Arne to review to be sure.
Comment 6 EWS 2022-05-24 14:05:45 PDT
Committed r294764 (250930@main): <https://commits.webkit.org/250930@main>

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