Bug 209097 – Fix the "deliver cached ranges" logic in PDFPlugin (and other small cleanups)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brady Eidson]

Fix the "deliver cached ranges" logic in PDFPlugin (and other small cleanups)

It was pretty broken, but not noticed because we rarely hit that code path.

Now we do, so the brokenness was obvious.

Noticed at the same time was needing to respond to some requests on the main thread after all data was loaded, along with a new PDFKit SPI to allow for that.

[Brady Eidson]

Created attachment 393561 [details]
Patch

[Brady Eidson]

<rdar://problem/60396297>

[Timothy Hatcher]

Comment on attachment 393561 [details]
Patch

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

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1441
> +    if ([m_pdfDocument.get() respondsToSelector:NSSelectorFromString(@"setHasHighLatencyDataProvider:")])
> +        [m_pdfDocument.get() setHasHighLatencyDataProvider:NO];

You might need to forward declare this to call it like this. Otherwise you will need to do an objc_msgSend or performSelector:. If you forward declare, you can just do @selector(setHasHighLatencyDataProvider:) instead of NSSelectorFromString.

[Timothy Hatcher]

Needs rebased.

[Brady Eidson]

(In reply to Timothy Hatcher from comment #3)
> Comment on attachment 393561 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=393561&action=review
> 
> > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1441
> > +    if ([m_pdfDocument.get() respondsToSelector:NSSelectorFromString(@"setHasHighLatencyDataProvider:")])
> > +        [m_pdfDocument.get() setHasHighLatencyDataProvider:NO];
> 
> You might need to forward declare this to call it like this. Otherwise you
> will need to do an objc_msgSend or performSelector:. If you forward declare,
> you can just do @selector(setHasHighLatencyDataProvider:) instead of
> NSSelectorFromString.

Good call.

[Brady Eidson]

Oh I know why everything is red - Locally, it's built on top of the patch in https://bugs.webkit.org/show_bug.cgi?id=209063

Will figure something out.

[Brady Eidson]

Created attachment 393599 [details]
PFL

[Brady Eidson]

Created attachment 393600 [details]
PFL

[Brady Eidson]

Created attachment 393602 [details]
PFL

[WebKit Commit Bot]

Comment on attachment 393602 [details]
PFL

Clearing flags on attachment: 393602

Committed r258477: <https://trac.webkit.org/changeset/258477>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.