[iOS] Create a UIPrintFormatter for WKWebView

• Component: New Bugs
• Status: RESOLVED FIXED

[Andy Estes]

[iOS] Create a UIPrintFormatter for WKWebView

[Andy Estes]

Created attachment 231590 [details]
Patch

[Andy Estes]

Created attachment 231695 [details]
[iOS] Create a UIPrintFormatter for WKWebView

[Jonathan Wells]

Comment on attachment 231695 [details]
[iOS] Create a UIPrintFormatter for WKWebView

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

> Source/WebKit2/ChangeLog:11
> +        returning the data to the UI process asynchronously. When UIPrintInteractionController later calls our

"returning the data to the UI process asynchronously." I think is somewhat dangling. It isn't meant to modify "PDF".

> Source/WebKit2/UIProcess/ios/WKPDFView.mm:31
> +

Extra line here.

[Andy Estes]

Comment on attachment 231695 [details]
[iOS] Create a UIPrintFormatter for WKWebView

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

>> Source/WebKit2/ChangeLog:11
>> +        returning the data to the UI process asynchronously. When UIPrintInteractionController later calls our
> 
> "returning the data to the UI process asynchronously." I think is somewhat dangling. It isn't meant to modify "PDF".

I'll just add a new sentence that says "The PDF data will be returned to the UI process asynchronously."

[Andy Estes]

Created attachment 231704 [details]
[iOS] Create a UIPrintFormatter for WKWebView

[Tim Horton]

Comment on attachment 231704 [details]
[iOS] Create a UIPrintFormatter for WKWebView

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

> Source/WebKit2/Shared/WebPreferencesStore.h:147
> +    macro(ShouldPrintBackgrounds, shouldPrintBackgrounds, Bool, bool, DEFAULT_SHOULD_PRINT_BACKGROUNDS) \

oops!

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1868
> +        return { };

{ }, really?

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1892
> +        if (!_page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DidFinishDrawingPagesToPDF>(_page->pageID(), didFinishLoadingTimeout)) {

terrifying

> Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:35
> +@interface UIPrintFormatter (IPI)

(Details)

> Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:100
> +    CGContextTranslateCTM(context, 0, -CGPDFPageGetBoxRect(pdfPage, kCGPDFMediaBox).size.height);
> +    CGContextClipToRect(context, CGPDFPageGetBoxRect(pdfPage, kCGPDFCropBox));

I cannot remember; are you sure that CGPDFPageGetBoxRect falls back to the next box if the one you ask for isn't available? I guess this is fine if you always control _printedDocument.

[Andy Estes]

Comment on attachment 231704 [details]
[iOS] Create a UIPrintFormatter for WKWebView

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

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1892
>> +        if (!_page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DidFinishDrawingPagesToPDF>(_page->pageID(), didFinishLoadingTimeout)) {
> 
> terrifying

More terrifying than a call to sendSync() (which would be the alternative here)?

[Andy Estes]

Comment on attachment 231704 [details]
[iOS] Create a UIPrintFormatter for WKWebView

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

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1868
>> +        return { };
> 
> { }, really?

Oops. That was from when this method returned a Vector<>. Will change.

>> Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:100
>> +    CGContextClipToRect(context, CGPDFPageGetBoxRect(pdfPage, kCGPDFCropBox));
> 
> I cannot remember; are you sure that CGPDFPageGetBoxRect falls back to the next box if the one you ask for isn't available? I guess this is fine if you always control _printedDocument.

Maybe I should just use the media box, or just not clip at all (thinking about it, clipping should be unnecessary, at least for the PDFs WebKit generates).

[Andy Estes]

Created attachment 231719 [details]
[iOS] Create a UIPrintFormatter for WKWebView

[Tim Horton]

(In reply to comment #8)
> (From update of attachment 231704 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231704&action=review
> 
> >> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1868
> >> +        return { };
> > 
> > { }, really?
> 
> Oops. That was from when this method returned a Vector<>. Will change.
> 
> >> Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:100
> >> +    CGContextClipToRect(context, CGPDFPageGetBoxRect(pdfPage, kCGPDFCropBox));
> > 
> > I cannot remember; are you sure that CGPDFPageGetBoxRect falls back to the next box if the one you ask for isn't available? I guess this is fine if you always control _printedDocument.
> 
> Maybe I should just use the media box, or just not clip at all (thinking about it, clipping should be unnecessary, at least for the PDFs WebKit generates).

Yeah, I think WebKit probably does enough clipping on the other end.

If you ever end up handing PDFs from the web in here, though, you're going to want to do the right thing.

[Andy Estes]

Comment on attachment 231719 [details]
[iOS] Create a UIPrintFormatter for WKWebView

I'm going to take a stab at making the page rect computation asynchronous.

[Tim Horton]

Comment on attachment 231719 [details]
[iOS] Create a UIPrintFormatter for WKWebView

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1867
> +    if (!_page->sendSync(Messages::WebPage::ComputePagesForPrintingAndStartDrawingToPDF(_page->mainFrame()->frameID(), printInfo, firstPage), Messages::WebPage::ComputePagesForPrintingAndStartDrawingToPDF::Reply(pageRects, totalScaleFactor)))

Sam OK'd this.

[WebKit Commit Bot]

Comment on attachment 231719 [details]
[iOS] Create a UIPrintFormatter for WKWebView

Clearing flags on attachment: 231719

Committed r169175: <http://trac.webkit.org/changeset/169175>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.