[webkit-reviews] review granted: [Bug 178502] [CG] PostScript images should be supported if they are sub-resource images : [Attachment 327768] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 178502: [CG] PostScript images should be supported if they are sub-resource
images
https://bugs.webkit.org/show_bug.cgi?id=178502

Attachment 327768: Patch

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




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

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

> Source/WebCore/loader/cache/CachedImage.h:98
> +    bool isPDFRequest() const;
> +    bool isPostScriptRequest() const;

It's odd to ask whether an image is a "request". Maybe
isPDFResource()/isPostScriptResource()?

> Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp:363
> +    CGPSConverterCallbacks callbacks = { 0, 0, 0, 0, 0, 0, 0, 0 };

Could be CGPSConverterCallbacks callbacks = { };

> Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp:369
> +    CGPSConverterConvert(converter.get(), provider.get(), consumer.get(),
0);

How expensive is this? Can it happen off the main thread?

> Source/WebCore/platform/graphics/cg/PDFDocumentImage.h:58
> +    WEBCORE_EXPORT static RetainPtr<CFMutableDataRef>
convertPostScriptDataToPDF(RetainPtr<CFDataRef> postScriptData);

This should return a CFDataRef, right? The caller doesn't need mutability.

Can the argument be a  RetainPtr<CFDataRef>&& ?