Bug 209097

Summary: Fix the "deliver cached ranges" logic in PDFPlugin (and other small cleanups)
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, timothy
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
timothy: review+
PFL
none
PFL
none
PFL none

Description Brady Eidson 2020-03-13 17:05:05 PDT 
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.
Comment 1 Brady Eidson 2020-03-13 17:16:50 PDT 
Created attachment 393561 [details]
Patch
Comment 2 Brady Eidson 2020-03-13 17:17:11 PDT 
<rdar://problem/60396297>
Comment 3 Timothy Hatcher 2020-03-13 17:27:01 PDT 
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.
Comment 4 Timothy Hatcher 2020-03-13 17:27:36 PDT 
Needs rebased.
Comment 5 Brady Eidson 2020-03-13 20:33:58 PDT 
(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.
Comment 6 Brady Eidson 2020-03-13 20:34:52 PDT 
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.
Comment 7 Brady Eidson 2020-03-14 17:12:17 PDT 
Created attachment 393599 [details]
PFL
Comment 8 Brady Eidson 2020-03-14 17:39:55 PDT 
Created attachment 393600 [details]
PFL
Comment 9 Brady Eidson 2020-03-14 18:30:21 PDT 
Created attachment 393602 [details]
PFL
Comment 10 WebKit Commit Bot 2020-03-14 20:40:48 PDT 
Comment on attachment 393602 [details]
PFL

Clearing flags on attachment: 393602

Committed r258477: <https://trac.webkit.org/changeset/258477>
Comment 11 WebKit Commit Bot 2020-03-14 20:40:49 PDT 
All reviewed patches have been landed.  Closing bug.