To fix crbug.com/85132, I need to pass additional print params such as selected printer paper size, printable area and scale to fit value to the plugin print context. Therefore, I modified printBegin() function accordingly. Please review and let me know your comments.
Created attachment 137828 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 137828 [details] Patch Patch is not ready for review yet. I will update the review flag as soon as it is ready for review. Thanks.
Created attachment 138175 [details] Patch
Created attachment 138378 [details] Patch
Latest patch uses WebPrintScalingOption.h. This is a new header file. I would like to commit that header file first to support Chrome side changes. Therefore, I uploaded that header file patch in https://bugs.webkit.org/show_bug.cgi?id=84608.
Comment on attachment 138378 [details] Patch Attachment 138378 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12518135
Created attachment 139405 [details] Patch
Updated patch and it is ready for review.
ping.
fishd@: Can you please review the latest patch? Thanks.
Comment on attachment 139405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139405&action=review > Source/WebKit/chromium/public/WebFrame.h:468 > + // TODO(kmadhusu@chromium.org): This is a temporary interface to avoid the nit: webkit style doesn't use TODO(user). just use FIXME. > Source/WebKit/chromium/public/WebFrame.h:488 > + virtual int printBegin(const WebSize& printContentSize, I suspect it would be better to bottle up these parameters into a WebPrintParams struct. See for example WebPluginParams. Exclude the constrainToNode parameter so that you can reuse WebPrintParams in the WebPlugin printBegin method. > Source/WebKit/chromium/public/WebPlugin.h:104 > + // TODO(kmadhusu@chromium.org): Change the data type of printScalingOption to nit: TODO(user) -> FIXME Use WebPrintParams here too.
Created attachment 141493 [details] Patch
Created attachment 141495 [details] Patch
Comment on attachment 139405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139405&action=review Addressed review comments. PTAL. Thanks. >> Source/WebKit/chromium/public/WebFrame.h:468 >> + // TODO(kmadhusu@chromium.org): This is a temporary interface to avoid the > > nit: webkit style doesn't use TODO(user). just use FIXME. Fixed. >> Source/WebKit/chromium/public/WebFrame.h:488 >> + virtual int printBegin(const WebSize& printContentSize, > > I suspect it would be better to bottle up these parameters into a WebPrintParams > struct. See for example WebPluginParams. Exclude the constrainToNode parameter > so that you can reuse WebPrintParams in the WebPlugin printBegin method. Done. >> Source/WebKit/chromium/public/WebPlugin.h:104 >> + // TODO(kmadhusu@chromium.org): Change the data type of printScalingOption to > > nit: TODO(user) -> FIXME > > Use WebPrintParams here too. Done.
Comment on attachment 141495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141495&action=review > Source/WebKit/chromium/public/WebPrintParams.h:60 > + , printScalingOption(WebPrintScalingOptionFitToPrintableArea) { } nit: indentation unindent the comma-prefixed rows by 2 spaces > Source/WebKit/chromium/public/WebPrintParams.h:64 > + , printableArea(printableArea) nit: indentation
Created attachment 142063 [details] Patch
Comment on attachment 141495 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141495&action=review Fixed nits. >> Source/WebKit/chromium/public/WebPrintParams.h:60 >> + , printScalingOption(WebPrintScalingOptionFitToPrintableArea) { } > > nit: indentation > > unindent the comma-prefixed rows by 2 spaces Fixed. >> Source/WebKit/chromium/public/WebPrintParams.h:64 >> + , printableArea(printableArea) > > nit: indentation Fixed.
Comment on attachment 142063 [details] Patch Clearing flags on attachment: 142063 Committed r117200: <http://trac.webkit.org/changeset/117200>
All reviewed patches have been landed. Closing bug.