Bug 209097 - Fix the "deliver cached ranges" logic in PDFPlugin (and other small cleanups)
Summary: Fix the "deliver cached ranges" logic in PDFPlugin (and other small cleanups)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-13 17:05 PDT by Brady Eidson
Modified: 2020-03-14 20:40 PDT (History)
2 users (show)

See Also:

Attachments
Patch (8.77 KB, patch)
2020-03-13 17:16 PDT, Brady Eidson 		timothy: review+
Details | Formatted Diff | Diff
PFL (9.05 KB, patch)
2020-03-14 17:12 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
PFL (9.07 KB, patch)
2020-03-14 17:39 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
PFL (8.88 KB, patch)
2020-03-14 18:30 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.