Print Preview: Fix top level directory compile errors

Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2863183004
Cr-Commit-Position: refs/heads/master@{#470413}
Committed: https://chromium.googlesource.com/chromium/src/+/881669afc5906b5190097dd5f8108465b238ea25

[rbpotter, 2017-05-05 21:24:24]

Description was changed from

==========
Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
==========

to

==========
Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[rbpotter, 2017-05-05 21:24:29]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[rbpotter, 2017-05-05 21:24:40]

Description was changed from

==========
Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[rbpotter, 2017-05-05 21:24:41]

rbpotter@chromium.org changed reviewers:
+ dpapad@chromium.org

[commit-bot: I haz the power, 2017-05-05 21:25:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 23:14:03]

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

[commit-bot: I haz the power, 2017-05-05 23:14:04]

Dry run: This issue passed the CQ dry run.

[dpapad, 2017-05-05 23:30:29]

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/cloud_print_interface.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/cloud_print_interface.js:269: accept ?
new HttpParam('accept', 'true') :
You can compact (and satisfy the compiler) as follows,

new HttpParam('accept', accept ? 'true' : false)
or
new HttpParam('accept', accept.toString());

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/data/destination_store.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/data/destination_store.js:1131:
this.userInfo_.activeUser, opt_origin);
Is this equivalent to before? It seems that before this.userInfo_.activeUser
could be null or undefined.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/native_layer.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/native_layer.js:1005: * @type {boolean}
Nit (optional): Since you are already doing small lint fixes in this CL, perhaps
consider using the shorthand syntax "@private {Type}" in this file.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/print_preview_animations.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/print_preview_animations.js:181: * @param
{HTMLElement} el The element to be processed.
Could this be !HTMLELement. Same question for !Array in next line.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/print_preview_focus_manager.js
(right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/print_preview_focus_manager.js:26: while
((newEl = el.querySelector('.overlay:not([hidden])')))
That was probably an auto-fix for linter for
http://eslint.org/docs/rules/no-cond-assign. Maybe consider reverting this for
now, I'll probably turn off this rule.

[rbpotter, 2017-05-06 00:02:19]

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/cloud_print_interface.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/cloud_print_interface.js:269: accept ?
new HttpParam('accept', 'true') :
On 2017/05/05 23:30:29, dpapad wrote:
> You can compact (and satisfy the compiler) as follows,
> 
> new HttpParam('accept', accept ? 'true' : false)
> or
> new HttpParam('accept', accept.toString());

Done.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/data/destination_store.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/data/destination_store.js:1131:
this.userInfo_.activeUser, opt_origin);
On 2017/05/05 23:30:29, dpapad wrote:
> Is this equivalent to before? It seems that before this.userInfo_.activeUser
> could be null or undefined.

Modified CloudPrintInterface.search so that it can accept null or
undefined inputs, as it does this || '' step at the very beginning
of the function.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/native_layer.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/native_layer.js:1005: * @type {boolean}
On 2017/05/05 23:30:29, dpapad wrote:
> Nit (optional): Since you are already doing small lint fixes in this CL,
perhaps
> consider using the shorthand syntax "@private {Type}" in this file.

Done.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/print_preview_animations.js (right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/print_preview_animations.js:181: * @param
{HTMLElement} el The element to be processed.
On 2017/05/05 23:30:29, dpapad wrote:
> Could this be !HTMLELement. Same question for !Array in next line.

Done.

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
File chrome/browser/resources/print_preview/print_preview_focus_manager.js
(right):

https://codereview.chromium.org/2863183004/diff/1/chrome/browser/resources/pr...
chrome/browser/resources/print_preview/print_preview_focus_manager.js:26: while
((newEl = el.querySelector('.overlay:not([hidden])')))
On 2017/05/05 23:30:29, dpapad wrote:
> That was probably an auto-fix for linter for
> http://eslint.org/docs/rules/no-cond-assign. Maybe consider reverting this for
> now, I'll probably turn off this rule.

Done.

[rbpotter, 2017-05-06 00:02:24]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-06 00:03:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dpapad, 2017-05-06 00:24:32]

lgtm

[commit-bot: I haz the power, 2017-05-06 02:47:53]

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

[commit-bot: I haz the power, 2017-05-06 02:47:53]

Dry run: This issue passed the CQ dry run.

[rbpotter, 2017-05-08 21:04:53]

The CQ bit was checked by rbpotter@chromium.org

[commit-bot: I haz the power, 2017-05-08 21:05:56]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 01:21:38]

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

[commit-bot: I haz the power, 2017-05-09 01:21:39]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rbpotter, 2017-05-09 18:57:08]

The CQ bit was checked by rbpotter@chromium.org

[commit-bot: I haz the power, 2017-05-09 18:58:34]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 20:19:07]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494356228159430,
"parent_rev": "ccedf2b29faaf7d66f4270a6e2a8cdf70a6d2f37", "commit_rev":
"881669afc5906b5190097dd5f8108465b238ea25"}

[commit-bot: I haz the power, 2017-05-09 20:19:23]

Description was changed from

==========
Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Print Preview: Fix top level directory compile errors

Fix closure compile errors for the print_preview/ top level directory.

Also fix linter errors.

Reduces closure compiler errors to 50.

BUG=717620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2863183004
Cr-Commit-Position: refs/heads/master@{#470413}
Committed:
https://chromium.googlesource.com/chromium/src/+/881669afc5906b5190097dd5f810...
==========

[commit-bot: I haz the power, 2017-05-09 20:19:24]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/881669afc5906b5190097dd5f810...