To fix crbug.com/85132, I need a webkit public header file to provide different types of print scaling options. Actual fix for the bug(crbug.com/85132) include ppapi changes, chromium changes and webkit changes. In order to prevent any build breakage while committing the individual CL's, I would like to commit this header file first followed by the patch in https://bugs.webkit.org/show_bug.cgi?id=84312.
Created attachment 138374 [details] Patch
Created attachment 138375 [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 138375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138375&action=review How do you plan to use this enum? It is often easier to review API changes when it is clear how the parts will fit together. This looks like an unused enum :-) > Source/WebKit/chromium/public/WebPrintScalingOption.h:36 > +enum WebPrintScalingOption { please fix naming: http://trac.webkit.org/wiki/ChromiumWebKitAPI#Enums
Created attachment 138395 [details] Patch
Comment on attachment 138375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138375&action=review >> Source/WebKit/chromium/public/WebPrintScalingOption.h:36 >> +enum WebPrintScalingOption { > > please fix naming: > http://trac.webkit.org/wiki/ChromiumWebKitAPI#Enums I assume you meant to change the enum value names. Done.
Sorry I didn't state it clearly in my description. You can see the usage of this enum in the latest patch uploaded in https://bugs.webkit.org/show_bug.cgi?id=84312. And you can also find the usage of this enum in the following chromium side CL's. http://codereview.chromium.org/10083059/ http://codereview.chromium.org/10168013/ Thanks.
Patch in bug 84312 uses WebPrintFitToPrintableArea enum value. After addressing all the review comments in this header file, I will update the patch in bug 84312 accordingly.
ping
Comment on attachment 138395 [details] Patch OK, R=me Normally, it is better to include a change like this with the patch that actually requires it.
Thanks for reviewing the patch. I understand your concern. I am just trying to avoid chrome canary build breakages. By landing this header file first, it easier for me to land the individual patches. Will it help if I add more information in the ChangeLog file? Can you also please review the other webkit patch (https://bugs.webkit.org/show_bug.cgi?id=84312) which actually uses this enum to make sure you are fine with the workflow?
Comment on attachment 138395 [details] Patch Clearing flags on attachment: 138395 Committed r115464: <http://trac.webkit.org/changeset/115464>
All reviewed patches have been landed. Closing bug.