Print Preview: Fix data/ errors

Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

Reduces print preview closure compiler errors to 91, and eliminates
all remaining warnings.

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

Review-Url: https://codereview.chromium.org/2862203002
Cr-Commit-Position: refs/heads/master@{#470114}
Committed: https://chromium.googlesource.com/chromium/src/+/d4356478ea0f780d9aaebefad24439437bc7b13e

[rbpotter, 2017-05-05 04:17:31]

Description was changed from

==========
Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

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

to

==========
Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

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

[rbpotter, 2017-05-05 04:20:06]

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

[commit-bot: I haz the power, 2017-05-05 04:20:25]

Dry run: CQ is trying da patch.

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

[rbpotter, 2017-05-05 04:21:16]

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

[commit-bot: I haz the power, 2017-05-05 04:21:37]

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 05:06:19]

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

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

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rbpotter, 2017-05-05 16:29:17]

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

[commit-bot: I haz the power, 2017-05-05 16:29:42]

Dry run: CQ is trying da patch.

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

[rbpotter, 2017-05-05 16:29:44]

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

[commit-bot: I haz the power, 2017-05-05 17:28:17]

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

[commit-bot: I haz the power, 2017-05-05 17:28:18]

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

[dpapad, 2017-05-05 17:32:54]

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/destination_match.js (right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/destination_match.js:1: // Copyright
(c) 2017 The Chromium Authors. All rights reserved.
Nit (optional): The "(c)" part of the licenses is not needed anymore.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/invitation_store.js (right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/invitation_store.js:10:
print_preview.InvitationStoreLoadStatus = {
This enum was marked as private before. Can we still keep it private (even
thought we defined it outside of the cr.define()? (or does the compiler not
accept that?)

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/print_ticket_store.js (right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/print_ticket_store.js:444: * @return
{!string} Google Cloud Print print ticket.
Nit: "!" is implied for strings (same for number and boolean), therefore can be
removed.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/custom_margins.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/custom_margins.js:20: 
Nit (optional): You might be able to create a shorter alias here as follows

var CustomMarginsOrientation =
print_preview.ticket_items.CustomMarginsOrientation;

and use CustomMarginsOrientation in actual code below this point to keep it as
short as it was.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/landscape.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/landscape.js:26:
customMargins) {
Indentation was correct before, but not anymore.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/page_range.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/page_range.js:69: *
@return {string} The value of the ticket item as a string
Nit: Period missing at the end.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/ticket_item.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/ticket_item.js:150: if
(this.appState_ && this.field_) {
Is this 100% equivalent? What if |this.field_| was legitimately the value of 0,
or the empty string ''? Are those not valid values for |this.field_|?

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/ticket_item.js:160: *
@protected
Maybe add @abstract here and below (not necessarily in this CL)? See
https://github.com/google/closure-compiler/wiki/%40abstract-classes-and-methods.

[rbpotter, 2017-05-05 17:34:26]

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

[commit-bot: I haz the power, 2017-05-05 17:35:07]

Dry run: CQ is trying da patch.

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

[rbpotter, 2017-05-05 18:25:15]

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

[rbpotter, 2017-05-05 18:25:47]

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/destination_match.js (right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/destination_match.js:1: // Copyright
(c) 2017 The Chromium Authors. All rights reserved.
On 2017/05/05 17:32:50, dpapad wrote:
> Nit (optional): The "(c)" part of the licenses is not needed anymore.

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/invitation_store.js (right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/invitation_store.js:10:
print_preview.InvitationStoreLoadStatus = {
On 2017/05/05 17:32:50, dpapad wrote:
> This enum was marked as private before. Can we still keep it private (even
> thought we defined it outside of the cr.define()? (or does the compiler not
> accept that?)

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/print_ticket_store.js (right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/print_ticket_store.js:444: * @return
{!string} Google Cloud Print print ticket.
On 2017/05/05 17:32:50, dpapad wrote:
> Nit: "!" is implied for strings (same for number and boolean), therefore can
be
> removed.

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/custom_margins.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/custom_margins.js:20: 
On 2017/05/05 17:32:50, dpapad wrote:
> Nit (optional): You might be able to create a shorter alias here as follows
> 
> var CustomMarginsOrientation =
> print_preview.ticket_items.CustomMarginsOrientation;
> 
> and use CustomMarginsOrientation in actual code below this point to keep it as
> short as it was.

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/landscape.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/landscape.js:26:
customMargins) {
On 2017/05/05 17:32:51, dpapad wrote:
> Indentation was correct before, but not anymore.

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/page_range.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/page_range.js:69: *
@return {string} The value of the ticket item as a string
On 2017/05/05 17:32:51, dpapad wrote:
> Nit: Period missing at the end.

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/print_preview/data/ticket_items/ticket_item.js
(right):

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/ticket_item.js:150: if
(this.appState_ && this.field_) {
On 2017/05/05 17:32:53, dpapad wrote:
> Is this 100% equivalent? What if |this.field_| was legitimately the value of
0,
> or the empty string ''? Are those not valid values for |this.field_|?

Done.

https://codereview.chromium.org/2862203002/diff/40001/chrome/browser/resource...
chrome/browser/resources/print_preview/data/ticket_items/ticket_item.js:160: *
@protected
On 2017/05/05 17:32:52, dpapad wrote:
> Maybe add @abstract here and below (not necessarily in this CL)? See
>
https://github.com/google/closure-compiler/wiki/%40abstract-classes-and-methods.

Done.

[commit-bot: I haz the power, 2017-05-05 18:25:51]

Dry run: CQ is trying da patch.

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

[rbpotter, 2017-05-05 18:32:56]

Description was changed from

==========
Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

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

to

==========
Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

Reduces print preview closure compiler errors to 91, and eliminates
all remaining warnings.

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

[dpapad, 2017-05-05 19:27:01]

lgtm

[rbpotter, 2017-05-05 21:03:52]

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

[commit-bot: I haz the power, 2017-05-05 21:04:20]

Dry run: CQ is trying da patch.

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

[rbpotter, 2017-05-05 21:57:52]

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

[commit-bot: I haz the power, 2017-05-05 21:58:51]

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-06 01:39:36]

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

[commit-bot: I haz the power, 2017-05-06 01:39:37]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[rbpotter, 2017-05-08 19:02:51]

The CQ bit was checked by rbpotter@chromium.org

[rbpotter, 2017-05-08 19:02:52]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2862203002/#ps120001
(title: "Fix destination resolver")

[commit-bot: I haz the power, 2017-05-08 19:04:42]

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-08 20:44:01]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1494270171716490,
"parent_rev": "af6c18d4d596a269b7e437dc400162b67f79dab2", "commit_rev":
"d4356478ea0f780d9aaebefad24439437bc7b13e"}

[commit-bot: I haz the power, 2017-05-08 20:44:11]

Description was changed from

==========
Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

Reduces print preview closure compiler errors to 91, and eliminates
all remaining warnings.

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

to

==========
Print Preview: Fix data/ errors

Fix closure compiler errors in data/ directory of print_preview.

Reduces print preview closure compiler errors to 91, and eliminates
all remaining warnings.

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

Review-Url: https://codereview.chromium.org/2862203002
Cr-Commit-Position: refs/heads/master@{#470114}
Committed:
https://chromium.googlesource.com/chromium/src/+/d4356478ea0f780d9aaebefad244...
==========

[commit-bot: I haz the power, 2017-05-08 20:44:12]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/d4356478ea0f780d9aaebefad244...