[webkit-reviews] review denied: [Bug 185445] Set colorspace in the PDF plugin. : [Attachment 339869] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has denied  review:
Bug 185445: Set colorspace in the PDF plugin.
https://bugs.webkit.org/show_bug.cgi?id=185445

Attachment 339869: Patch

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




--- Comment #5 from Tim Horton <thorton at apple.com> ---
Comment on attachment 339869
  --> https://bugs.webkit.org/attachment.cgi?id=339869
Patch

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

> Source/WebKit/WebProcess/Plugins/PDF/PDFLayerControllerSPI.h:170
> +- (void) setDeviceColorSpace:(CGColorSpaceRef)colorSpace;

No space after the )

>> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:637
>> +	if ([m_pdfLayerController.get()
respondsToSelector:@selector(setDeviceColorSpace:)])
> 
> No need for the .get()s

No need for the .get()s

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:638
> +	   [m_pdfLayerController.get() setDeviceColorSpace:screenColorSpace()];

What does screenColorSpace do? It seems like it will usually be wrong, and we
should be getting it from the page/view, not the screen (because there are
multiple screens).