Printing an actual WKView means that we need to check for destination all the time, making code slower and more complicated. Also, it will be more difficult to get right when we start printing from a separate thread.
Created attachment 79857 [details] proposed patch
Attachment 79857 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/UIProcess/API/mac/WKPrintingView.h:33: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
IIRC the Japan office is working on printing for Chromium and might be curious to see this go by.
Comment on attachment 79857 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=79857&action=review > Source/WebKit2/UIProcess/API/mac/WKPrintingView.h:26 > +#import <Cocoa/Cocoa.h> I see this imported from WebKit2Prefix.h. Isn’t that enough? > Source/WebKit2/UIProcess/API/mac/WKPrintingView.h:40 > + Vector<WebCore::IntRect> _webPrintingPageRects; > + double _webTotalScaleFactorForPrinting; Any reason for the “web” prefix in these two? Should this be a CGFloat instead of a double? > Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:32 > +@interface NSView (Details) In WebKit1, the convention for a category name like this is (WebNSViewDetails). > Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:42 > +static float currentPrintOperationScale() Seems like callers to this function already have the current operation, so this could have been a function that takes an NSPrintOperation * or simply an instance method in an NSOperation category. But I see that you’re just moving this code around. > Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:83 > + originalTopMargin = [info topMargin]; > + originalBottomMargin = [info bottomMargin]; -topMargin and -bottomMargin return CGFloats. Maybe this code should use more CGFloats (or doubles) and fewer floats. > Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:99 > +// Return the number of pages available for printing > +- (BOOL)knowsPageRange:(NSRangePointer)range Actually, it returns YES. I’m not sure the comment helps.
> + Vector<WebCore::IntRect> _webPrintingPageRects; > + double _webTotalScaleFactorForPrinting; > > Any reason for the “web” prefix in these two? Just consistency with the third one. > Should this be a CGFloat instead of a double? Not sure, why? I need a double for total scale factor in cross-platform code, and using a standard type as much as possible seems preferable. > In WebKit1, the convention for a category name like this is (WebNSViewDetails). It doesn't seem like WebKit2 follows this convention, and I'm not sure if there is any difference. But I've changed it to WebNSViewDetails. > Seems like callers to this function already have the current operation, so this could have been > a function that takes an NSPrintOperation * or simply an instance method in an NSOperation > category. But I see that you’re just moving this code around. I now see that starting with 10.6, NSPrintInfo has a proper scalingFactor method. > -topMargin and -bottomMargin return CGFloats. Maybe this code should use more CGFloats (or doubles) and fewer floats. I felt uncomfortable using CGFloats with -doubleValue and +numberWithDouble, so changed to double.
Created attachment 79863 [details] patch for landing
(In reply to comment #5) > > + Vector<WebCore::IntRect> _webPrintingPageRects; > > + double _webTotalScaleFactorForPrinting; > > > > Any reason for the “web” prefix in these two? > > Just consistency with the third one. The first one is “webFrame” because “frame” has a different meaning in the context of NSViews. > > Should this be a CGFloat instead of a double? > > Not sure, why? I need a double for total scale factor in cross-platform code, and using a standard type as much as possible seems preferable. Agreed.
The commit-queue encountered the following flaky tests while processing attachment 79863 [details]: animations/suspend-resume-animation.html bug 48161 (author: cmarrin@apple.com) The commit-queue is continuing to process your patch.
Comment on attachment 79863 [details] patch for landing Clearing flags on attachment: 79863 Committed r76470: <http://trac.webkit.org/changeset/76470>
All reviewed patches have been landed. Closing bug.