Bug 129600

Summary: [WebKit2][iOS] PDF
Product: WebKit Reporter: Tim Horton <thorton>
Component: WebKit2Assignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, mitz, sam
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 129809    
Bug Blocks:    
Attachments:
Description Flags
preliminary
none
patch
none
still preliminary
none
patch
none
split patch andersca: review+

Description Tim Horton 2014-03-02 23:13:27 PST
<rdar://problem/15260216>
Comment 1 Tim Horton 2014-03-02 23:16:41 PST
Created attachment 225633 [details]
preliminary
Comment 2 Tim Horton 2014-03-02 23:36:52 PST
Created attachment 225634 [details]
patch
Comment 3 Tim Horton 2014-03-02 23:38:00 PST
Created attachment 225635 [details]
still preliminary
Comment 4 Tim Horton 2014-03-05 11:35:55 PST
Comment on attachment 225635 [details]
still preliminary

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

> Source/WebKit2/Configurations/WebKit2.xcconfig:33
> +FRAMEWORK_AND_LIBRARY_LDFLAGS_iphonesimulator = -lobjc -framework CFNetwork -framework CoreFoundation -framework CoreGraphics -framework CoreText -framework Foundation -framework GraphicsServices -framework ImageIO -framework UIKit -framework WebKit -lMobileGestalt -lassertion_extension -framework CorePDF;

alphabetize

> Source/WebKit2/UIProcess/Cocoa/CustomRepresentationManager.h:46
> +

another header

> Source/WebKit2/UIProcess/Cocoa/CustomRepresentationManager.h:47
> +- (instancetype)initWithFrame:(CGRect)frame;

nope

> Source/WebKit2/UIProcess/Cocoa/CustomRepresentationManager.h:49
> +- (void)setData:(CFDataRef)data;

web_setCustomRepresentationData or setWebCustomRepresentationData or something like that (and Dan says it should be NSData instead)

> Source/WebKit2/UIProcess/Cocoa/CustomRepresentationManager.h:52
> +- (void)setContentInset:(UIEdgeInsets)insets;

don't need this if we re-use the scrollview

> Source/WebKit2/UIProcess/Cocoa/CustomRepresentationManager.h:62
> +struct CustomRepresentation {
> +    Class representationClass;
> +    String mimeType;
> +};

move into the class
Comment 5 Tim Horton 2014-03-06 01:03:23 PST
Created attachment 225964 [details]
patch

I just noticed that we need to save and restore a few more scrollview properties (minimum and maximum magnification at least) in the case where you are navigating back to a page that's in the page cache (but we can just do it always, I think); I'll fix that up tomorrow.
Comment 6 Tim Horton 2014-03-06 11:08:08 PST
Splitting this up.
Comment 7 Tim Horton 2014-03-07 03:01:27 PST
Created attachment 226106 [details]
split patch

This now depends on https://bugs.webkit.org/show_bug.cgi?id=129809 so it won't apply yet.
Comment 8 Anders Carlsson 2014-03-07 15:56:48 PST
Comment on attachment 226106 [details]
split patch

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

> Source/WebKit2/UIProcess/ios/WKPDFView.mm:46
> +@interface WKPDFView () {
> +    RetainPtr<UIPDFDocument> _pdfDocument;
> +    Vector<UIPDFPageView*> _pageViews;
> +    CGSize _minimumSize;
> +    UIScrollView *_scrollView;
> +}
> +@end

Just move these to the @implementation section instead.

> Source/WebKit2/UIProcess/ios/WKPDFView.mm:55
> +    [self setBackgroundColor:[UIColor grayColor]];

self.backgroundColor =

> Source/WebKit2/UIProcess/ios/WKPDFView.mm:79
> +        [[pageView contentLayer] setContentsScale:self.window.screen.scale];

pageView.contentLayer.contentsScale =
Comment 9 Tim Horton 2014-03-07 22:38:36 PST
http://trac.webkit.org/changeset/165327