[JS Changes] Support NumCopies print preset

Support NumCopies print preset

Chrome side javascript changes to support NumCopies print preset
option. Copies in print preview dialog should be set to value set for
NumCopies in pdf source document.

BUG=169120
Committed: https://crrev.com/2604a9784e1c28e790533555bc3cfe97333289b1
Cr-Commit-Position: refs/heads/master@{#307454}

[Nikhil, 2014-07-19 13:43:18]

Javascript changes to update print preview dialog to set copies based on
NumCopies property set in source pdf document.

Please note, this patch depends on https://codereview.chromium.org/375253002/
(review in progress)

PTAL, Thanks!

[Nikhil, 2014-11-19 13:45:50]

n.bansal@samsung.com changed reviewers:
+ alekseys@chromium.org

[Nikhil, 2014-11-19 13:45:50]

vitalybuka@, alekseys@ - PTAL at changes to update print preview dialog to set
copies based on NumCopies property set in source pdf document.

Blink and Chrome patches required to support this have already landed. Please
see :
[*] Blink - https://codereview.chromium.org/591053002/
[*] Chrome - https://codereview.chromium.org/375253002/

Please take a look :)

Thanks!

[Vitaly Buka (NO REVIEWS), 2014-11-20 16:40:47]

lgtm
please also wait Aleksey's feedback.

[Aleksey Shlyapnikov, 2014-11-20 18:56:56]

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1007: if
(event.optionsFromDocument.copies) {
Check this.printTicketStore_.copies.isCapabilityAvailable() too.

What is the expected behavior sending PDF with embedded copies to 'Save to PDF'
printer?

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1009:
event.optionsFromDocument.copies);
Two more spaces indent.

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/print_preview/print_preview_ui.cc (right):

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/print_preview/print_preview_ui.cc:633:
printing::kSettingFitToPageEnabled,
Why are you using kSettingFitToPageEnabled name here? It makes no sense.

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/print_preview/print_preview_ui.cc:636:
web_ui()->CallJavascriptFunction("printPresetOptionsFromDocument", options);
Since you're not calling printScalingDisabledForSourcePDF anymore, please remove
the entire call stack from JS.

[Nikhil, 2014-11-21 10:45:32]

vitalybuka@, alekseys@ - Thanks for reviewing this! I've made changes as per
review comments. PTAL.

Thanks!

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1007: if
(event.optionsFromDocument.copies) {
On 2014/11/20 18:56:56, Aleksey Shlyapnikov wrote:
> Check this.printTicketStore_.copies.isCapabilityAvailable() too.
> 
> What is the expected behavior sending PDF with embedded copies to 'Save to
PDF'
> printer?

NumCopies property spec doesn't say anything specific to 'Save to PDF' printer.
Please see :
http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/PDF32000_2008.pdf

Also, I tried opening print preview dialog with 'Save to PDF' as printer, and I
don't see copies option in the dialog for both pdf and html pages. 

I think NumCopies should be ignored for 'Save to PDF' printer as we would need
only one pdf file. Please let me know WDYT.

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1009:
event.optionsFromDocument.copies);
On 2014/11/20 18:56:56, Aleksey Shlyapnikov wrote:
> Two more spaces indent.

Done.

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/print_preview/print_preview_ui.cc (right):

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/print_preview/print_preview_ui.cc:633:
printing::kSettingFitToPageEnabled,
On 2014/11/20 18:56:56, Aleksey Shlyapnikov wrote:
> Why are you using kSettingFitToPageEnabled name here? It makes no sense.

Thanks for reviewing this! I've introduced kSettingDisableScaling for this. PTAL
:)

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/print_preview/print_preview_ui.cc:636:
web_ui()->CallJavascriptFunction("printPresetOptionsFromDocument", options);
On 2014/11/20 18:56:56, Aleksey Shlyapnikov wrote:
> Since you're not calling printScalingDisabledForSourcePDF anymore, please
remove
> the entire call stack from JS.

Done.

[Aleksey Shlyapnikov, 2014-11-21 22:50:42]

Thank you for making the change!

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/40001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1007: if
(event.optionsFromDocument.copies) {
On 2014/11/21 10:45:32, Nikhil wrote:
> On 2014/11/20 18:56:56, Aleksey Shlyapnikov wrote:
> > Check this.printTicketStore_.copies.isCapabilityAvailable() too.
> > 
> > What is the expected behavior sending PDF with embedded copies to 'Save to
> PDF'
> > printer?
> 
> NumCopies property spec doesn't say anything specific to 'Save to PDF'
printer.
> Please see :
>
http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/PDF32000_2008.pdf
> 
> Also, I tried opening print preview dialog with 'Save to PDF' as printer, and
I
> don't see copies option in the dialog for both pdf and html pages. 
> 
> I think NumCopies should be ignored for 'Save to PDF' printer as we would need
> only one pdf file. Please let me know WDYT.

I agree and that's why I suggested to add
this.printTicketStore_.copies.isCapabilityAvailable() check here.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/print_preview/native_layer.js (right):

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/native_layer.js:658: * @param {object}
options Print preset options as mentioned in source
Please define parameter type (see onDidGetDefaultPageLayout_, for example)

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:351:
this.onDisableScaling_.bind(this));
Please remove _all_ obsolete code.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:993: },
Please remove _all_ obsolete code.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:997: * updates the print
preset options from source document.
Just one sentence:

Updates printing options according to source document presets.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:998: * @param {event}
event Contains options from source document.
event -> Event

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1006:
event.optionsFromDocument.copies) {
Let's swap checks and add sanity check:

if (event.optionsFromDocument.copies > 0 &&
    this.printTicketStore_.copies.isCapabilityAvailable()) {

[Nikhil, 2014-11-24 12:57:30]

alekseys@ - Thanks for reviewing this! I've updated the code as per your review
comments. PTAL.

Thanks!

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/print_preview/native_layer.js (right):

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/native_layer.js:658: * @param {object}
options Print preset options as mentioned in source
On 2014/11/21 22:50:42, Aleksey Shlyapnikov wrote:
> Please define parameter type (see onDidGetDefaultPageLayout_, for example)

Done.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:351:
this.onDisableScaling_.bind(this));
On 2014/11/21 22:50:42, Aleksey Shlyapnikov wrote:
> Please remove _all_ obsolete code.

Done.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:993: },
On 2014/11/21 22:50:42, Aleksey Shlyapnikov wrote:
> Please remove _all_ obsolete code.

Done.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:997: * updates the print
preset options from source document.
On 2014/11/21 22:50:42, Aleksey Shlyapnikov wrote:
> Just one sentence:
> 
> Updates printing options according to source document presets.

Done.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:998: * @param {event}
event Contains options from source document.
On 2014/11/21 22:50:42, Aleksey Shlyapnikov wrote:
> event -> Event

Done.

https://codereview.chromium.org/407733002/diff/60001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:1006:
event.optionsFromDocument.copies) {
On 2014/11/21 22:50:42, Aleksey Shlyapnikov wrote:
> Let's swap checks and add sanity check:
> 
> if (event.optionsFromDocument.copies > 0 &&
>     this.printTicketStore_.copies.isCapabilityAvailable()) {

Done.

[Aleksey Shlyapnikov, 2014-11-24 19:47:58]

lgtm

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/print_preview/native_layer.js (right):

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
chrome/browser/resources/print_preview/native_layer.js:656: * Update print
preset options from source PDF document.
Update -> Updates

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
chrome/browser/resources/print_preview/native_layer.js:658: * @param
{{disableScaling: boolean, copies: number}}
Keep more text on this line:
     * @param {{disableScaling: boolean, copies: number}} options Specifies
     *     printing options according to source document presets.

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:988: // Reset
fit-to-page selection and update document info.
Remove this comment, it states the obvious now.

[Nikhil, 2014-11-25 06:36:13]

alekseys@ - Thank you for the quick reviews! I've addressed final nits. PTAL.

Thanks!

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/print_preview/native_layer.js (right):

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
chrome/browser/resources/print_preview/native_layer.js:656: * Update print
preset options from source PDF document.
On 2014/11/24 19:47:58, Aleksey Shlyapnikov wrote:
> Update -> Updates

Done.

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
chrome/browser/resources/print_preview/native_layer.js:658: * @param
{{disableScaling: boolean, copies: number}}
On 2014/11/24 19:47:58, Aleksey Shlyapnikov wrote:
> Keep more text on this line:
>      * @param {{disableScaling: boolean, copies: number}} options Specifies
>      *     printing options according to source document presets.

Done.

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/print_preview/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/80001/chrome/browser/resources...
chrome/browser/resources/print_preview/print_preview.js:988: // Reset
fit-to-page selection and update document info.
On 2014/11/24 19:47:58, Aleksey Shlyapnikov wrote:
> Remove this comment, it states the obvious now.

Done.

[Nikhil, 2014-11-26 09:11:19]

alekseys@ - Shall I go ahead and submit this? I've addressed the final nits.
PTAL.

Thanks!

[Aleksey Shlyapnikov, 2014-12-01 17:53:00]

Indeed, go ahead and submit it. You got my lgtm already.

[Nikhil, 2014-12-01 18:07:50]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-12-01 18:08:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/407733002/120001

[commit-bot: I haz the power, 2014-12-01 19:18:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-01 19:18:03]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Nikhil, 2014-12-03 10:28:00]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-12-03 10:28:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/407733002/140001

[commit-bot: I haz the power, 2014-12-03 11:33:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-03 11:33:24]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Aleksey Shlyapnikov, 2014-12-03 19:24:12]

The test failure is real. You need to figure out why this test is failing and
fix it.

[Nikhil, 2014-12-04 10:19:31]

alekseys@ - I've updated the test case that was failing. I've also added a new
test case for NumCopies print preset.

If you think it's better to have just one test case to test all print preset
properties, then please let me know. I'll combine the tests accordingly.

Please take a look.

Thanks!

[Aleksey Shlyapnikov, 2014-12-04 18:45:57]

lgtm

https://codereview.chromium.org/407733002/diff/180001/chrome/test/data/webui/...
File chrome/test/data/webui/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/180001/chrome/test/data/webui/...
chrome/test/data/webui/print_preview.js:480:
printPresetOptionsEvent.optionsFromDocument = printPresetOptions;
Since you're not reusing them, inline them here:

printPresetOptionsEvent.optionsFromDocument = {disableScaling: true};

https://codereview.chromium.org/407733002/diff/180001/chrome/test/data/webui/...
chrome/test/data/webui/print_preview.js:502: var printPresetOptions =
{disableScaling: true, copies: 2};
var printPresetOptions = {
  disableScaling: true,
  copies: 2
};

[Aleksey Shlyapnikov, 2014-12-04 18:45:58]

lgtm

[Nikhil, 2014-12-09 11:00:22]

alekseys@ - Thanks for reviewing this! I've addressed final nits for the test
case. CQing this now :)

Thanks!

[Nikhil, 2014-12-09 11:00:36]

https://codereview.chromium.org/407733002/diff/180001/chrome/test/data/webui/...
File chrome/test/data/webui/print_preview.js (right):

https://codereview.chromium.org/407733002/diff/180001/chrome/test/data/webui/...
chrome/test/data/webui/print_preview.js:480:
printPresetOptionsEvent.optionsFromDocument = printPresetOptions;
On 2014/12/04 18:45:57, Aleksey Shlyapnikov wrote:
> Since you're not reusing them, inline them here:
> 
> printPresetOptionsEvent.optionsFromDocument = {disableScaling: true};

Done.

https://codereview.chromium.org/407733002/diff/180001/chrome/test/data/webui/...
chrome/test/data/webui/print_preview.js:502: var printPresetOptions =
{disableScaling: true, copies: 2};
On 2014/12/04 18:45:57, Aleksey Shlyapnikov wrote:
> var printPresetOptions = {
>   disableScaling: true,
>   copies: 2
> };

Done.

[Nikhil, 2014-12-09 11:00:45]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-12-09 11:01:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/407733002/200001

[commit-bot: I haz the power, 2014-12-09 12:26:54]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2014-12-09 12:27:38]

Patchset 11 (id:??) landed as
https://crrev.com/2604a9784e1c28e790533555bc3cfe97333289b1
Cr-Commit-Position: refs/heads/master@{#307454}