[iOS] Create a UIPrintFormatter for WKWebView
Created attachment 231590 [details] Patch
Created attachment 231695 [details] [iOS] Create a UIPrintFormatter for WKWebView
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.
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."
Created attachment 231704 [details] [iOS] Create a UIPrintFormatter for WKWebView
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.
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)?
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).
Created attachment 231719 [details] [iOS] Create a UIPrintFormatter for WKWebView
(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.
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.
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.
Comment on attachment 231719 [details] [iOS] Create a UIPrintFormatter for WKWebView Clearing flags on attachment: 231719 Committed r169175: <http://trac.webkit.org/changeset/169175>
All reviewed patches have been landed. Closing bug.