[webkit-reviews] review denied: [Bug 197384] [CG] Add support for HEIF-sequence (‘public.heics’) images : [Attachment 369059] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 197384: [CG] Add support for HEIF-sequence (‘public.heics’) images
https://bugs.webkit.org/show_bug.cgi?id=197384

Attachment 369059: Patch

https://bugs.webkit.org/attachment.cgi?id=369059&action=review




--- Comment #11 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 369059
  --> https://bugs.webkit.org/attachment.cgi?id=369059
Patch

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

r- because I don't think this is correctly resetting state between tests.

> Source/WebCore/platform/graphics/cg/UTIRegistry.cpp:81
>  void setAdditionalSupportedImageTypes(const Vector<String>& imageTypes)

This seem more like appendAdditionSupportedImageTypes, since you can't
unregister a type, right?

That also implies that you're never cleaning up this state between tests
correctly.

> Source/WebCore/platform/graphics/cg/UTIRegistry.cpp:94
> +void setAdditionalSupportedImageTypes(const String& imageTypes)

If this is only used for testing, maybe append "ForTesting" to the function
name.