Bug 137386 - REGRESSION (r173265): UIWebView PDF loading is completely broken
Summary: REGRESSION (r173265): UIWebView PDF loading is completely broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: PDF (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-03 01:52 PDT by Tim Horton
Modified: 2014-10-03 13:27 PDT (History)
7 users (show)

See Also:


Attachments
patch (2.46 KB, patch)
2014-10-03 01:53 PDT, Tim Horton
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2014-10-03 01:52:46 PDT
The WebPDFViewPlaceholder changes in r173265 completely broke loading by ... removing the code that completes the load.

Let's put it back.

rdar://problem/18524717
Comment 1 Tim Horton 2014-10-03 01:53:50 PDT
Created attachment 239192 [details]
patch
Comment 2 Tim Horton 2014-10-03 01:54:43 PDT
Comment on attachment 239192 [details]
patch

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

> Source/WebKit/ios/WebView/WebPDFViewPlaceholder.mm:287
> +    // Dump the data provider as soon as possible since the CGPDFDocument will not hold onto it.

I'm not sure this comment makes sense to me (I'm just restoring it, but maybe we should refrain).
Comment 3 Tim Horton 2014-10-03 01:56:55 PDT
Comment on attachment 239192 [details]
patch

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

> Source/WebKit/ios/WebView/WebPDFViewPlaceholder.mm:285
> +    _document = CGPDFDocumentCreateWithProvider(provider);

No idea why this is manually memory-managed like this, but we should fix that in a different patch.
Comment 4 David Kilzer (:ddkilzer) 2014-10-03 08:55:43 PDT
Comment on attachment 239192 [details]
patch

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

r=me

>> Source/WebKit/ios/WebView/WebPDFViewPlaceholder.mm:287
>> +    // Dump the data provider as soon as possible since the CGPDFDocument will not hold onto it.
> 
> I'm not sure this comment makes sense to me (I'm just restoring it, but maybe we should refrain).

Yes, let's remove the comment.  It doesn't appear to add anything useful.
Comment 5 Tim Horton 2014-10-03 13:27:17 PDT
http://trac.webkit.org/changeset/174282