MD Settings: Internet: Cleanup JS

MD Settings: Internet: Cleanup JS

This CL:
* Primarily appends '_' to private properties.
* Removes remaining function parameters used just for data binding dependencies.
* Includes some clang formatting.

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

Committed: https://crrev.com/7894d3ce0f6755053be0cc7cb4ada055cb2f8755
Cr-Commit-Position: refs/heads/master@{#416138}

[stevenjb, 2016-08-31 20:22:56]

Description was changed from

==========
MD Settings: Internet: Cleanup JS

BUG=609156
==========

to

==========
MD Settings: Internet: Cleanup JS

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

[stevenjb, 2016-08-31 20:26:08]

Description was changed from

==========
MD Settings: Internet: Cleanup JS

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

to

==========
MD Settings: Internet: Cleanup JS

This CL:
* Primarily appends '_' to private properties.
* Removes remaining function parameters used just for data binding dependencies.
* Includes some clang formatting.


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

[stevenjb, 2016-08-31 20:26:19]

stevenjb@chromium.org changed reviewers:
+ dschuyler@chromium.org

[dschuyler, 2016-08-31 20:52:05]

I didn't mark all the places where an @private
could go. I did mark a few.

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.js:49: /**
The network AutoConnect state. */
Should each of these be @private
now that they have the trailing
underscore?

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js:39:
* @type {!Array<!CrOnc.NetworkStateProperties>}
@private?

[stevenjb, 2016-08-31 21:33:45]

PTAL

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.js:49: /**
The network AutoConnect state. */
On 2016/08/31 20:52:05, dschuyler wrote:
> Should each of these be @private
> now that they have the trailing
> underscore?

Huh. I didn't think that was valid for properties, but I looked again and did
find some use cases. I will add it.

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.js:39:
* @type {!Array<!CrOnc.NetworkStateProperties>}
On 2016/08/31 20:52:05, dschuyler wrote:
> @private?

Acknowledged.

[Dan Beam, 2016-08-31 21:59:48]

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_apnlist.js (right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_apnlist.js:176: * @param
{!{detail: !{selected: string}}} event
nit: CustomEvent (and then cast detail to {selected: string})

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_apnlist.js:230: * @param
{string} selectedApn
are you mixing this.* with @params?

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_ip_config.html
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_ip_config.html:20:
edit-field-types="[[getIPEditFields_(editable, ipConfig_, automatic_)]]"
wrap

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_ip_config.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_ip_config.js:192: *
@param {!{detail: { field: string, value: string}}} event The
nit: spurious space, \sfield

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_property_list.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_property_list.js:116:
computeFilter_() {
nit: computeFilter_() { -> computeFilter_: function() {

if you ever want to vulcanize ;)

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.html:98: <div
hidden$="[[useSameProxy_]]" class="layout vertical">
these should probably both be dom-if, no?

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_summary_item.html
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_summary_item.html:63:
<template is="dom-if" if="[[networksDomIfIsTrue_(expanded_, wasExpanded_)]]">
wrap

[stevenjb, 2016-08-31 22:10:17]

PTAL

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_apnlist.js (right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_apnlist.js:230: * @param
{string} selectedApn
On 2016/08/31 21:59:47, Dan Beam wrote:
> are you mixing this.* with @params?

I'm not sure I understand your question.

I am avoiding including properties as parameters (using this.foo instead). If a
property is a dependency and a non-property parameter is needed, I append the
properties at the end in the HTML. Does that seem reasonable?

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_ip_config.html
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_ip_config.html:20:
edit-field-types="[[getIPEditFields_(editable, ipConfig_, automatic_)]]"
On 2016/08/31 21:59:47, Dan Beam wrote:
> wrap

Done.

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_ip_config.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_ip_config.js:192: *
@param {!{detail: { field: string, value: string}}} event The
On 2016/08/31 21:59:47, Dan Beam wrote:
> nit: spurious space, \sfield

Done.

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_property_list.js
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_property_list.js:116:
computeFilter_() {
On 2016/08/31 21:59:47, Dan Beam wrote:
> nit: computeFilter_() { -> computeFilter_: function() {
> 
> if you ever want to vulcanize ;)

Done.

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.html:98: <div
hidden$="[[useSameProxy_]]" class="layout vertical">
On 2016/08/31 21:59:47, Dan Beam wrote:
> these should probably both be dom-if, no?

Maybe? Not sure how aggressive we want to be about minor performance
improvements in sub-pages? Certainly don't want to change that in this CL.

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_summary_item.html
(right):

https://codereview.chromium.org/2300783002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_summary_item.html:63:
<template is="dom-if" if="[[networksDomIfIsTrue_(expanded_, wasExpanded_)]]">
On 2016/08/31 21:59:47, Dan Beam wrote:
> wrap

Done.

[Dan Beam, 2016-09-01 00:11:39]

https://codereview.chromium.org/2300783002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_apnlist.js (right):

https://codereview.chromium.org/2300783002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_apnlist.js:238:
isOtherSelected_: function(selectedApn) {
why are you using selectedApn and not discarding the @param and using
selectedApn_ instead?  you removed networkProperties in favor of
this.networkProperties.  what is the difference?

[stevenjb, 2016-09-01 15:18:45]

https://codereview.chromium.org/2300783002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_apnlist.js (right):

https://codereview.chromium.org/2300783002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_apnlist.js:238:
isOtherSelected_: function(selectedApn) {
On 2016/09/01 00:11:39, Dan Beam wrote:
> why are you using selectedApn and not discarding the @param and using
> selectedApn_ instead?  you removed networkProperties in favor of
> this.networkProperties.  what is the difference?

Because selectedApn is not the same as this.selectedApn_ in this case. The
naming is confusing, I can fix that.

[Dan Beam, 2016-09-01 15:52:40]

https://codereview.chromium.org/2300783002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_apnlist.js (right):

https://codereview.chromium.org/2300783002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_apnlist.js:238:
isOtherSelected_: function(selectedApn) {
On 2016/09/01 15:18:45, stevenjb wrote:
> On 2016/09/01 00:11:39, Dan Beam wrote:
> > why are you using selectedApn and not discarding the @param and using
> > selectedApn_ instead?  you removed networkProperties in favor of
> > this.networkProperties.  what is the difference?
> 
> Because selectedApn is not the same as this.selectedApn_ in this case. The
> naming is confusing, I can fix that.

i suspected something like this, but couldn't tell whether it was actually a
parameter instead of a member.  if it's really parameterizing this function,
ignore me.

[stevenjb, 2016-09-01 16:30:17]

Renamed selectedApn.

[dschuyler, 2016-09-01 21:15:57]

lgtm

https://codereview.chromium.org/2300783002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_apnlist.js (right):

https://codereview.chromium.org/2300783002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_apnlist.js:238:
isOtherSelected_: function(accessPointName) {
:) I like |accessPointName| more than |selectedApn|
Thanks!

https://codereview.chromium.org/2300783002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_summary_item.js
(right):

https://codereview.chromium.org/2300783002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary_item.js:166:
knownNetworksIsVisible_: function() {
optional: 
var networkState = this.activeNetworkState;
return !!networkState && networkState.Type == CrOnc.Type.WI_FI;

[stevenjb, 2016-09-01 23:07:57]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-09-01 23:07:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org
Link to the patchset: https://codereview.chromium.org/2300783002/#ps80001
(title: "Feedback")

[commit-bot: I haz the power, 2016-09-01 23:08:32]

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, 2016-09-02 00:12:08]

Description was changed from

==========
MD Settings: Internet: Cleanup JS

This CL:
* Primarily appends '_' to private properties.
* Removes remaining function parameters used just for data binding dependencies.
* Includes some clang formatting.


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

to

==========
MD Settings: Internet: Cleanup JS

This CL:
* Primarily appends '_' to private properties.
* Removes remaining function parameters used just for data binding dependencies.
* Includes some clang formatting.


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

[commit-bot: I haz the power, 2016-09-02 00:12:09]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-02 00:14:44]

Description was changed from

==========
MD Settings: Internet: Cleanup JS

This CL:
* Primarily appends '_' to private properties.
* Removes remaining function parameters used just for data binding dependencies.
* Includes some clang formatting.


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

to

==========
MD Settings: Internet: Cleanup JS

This CL:
* Primarily appends '_' to private properties.
* Removes remaining function parameters used just for data binding dependencies.
* Includes some clang formatting.

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

Committed: https://crrev.com/7894d3ce0f6755053be0cc7cb4ada055cb2f8755
Cr-Commit-Position: refs/heads/master@{#416138}
==========

[commit-bot: I haz the power, 2016-09-02 00:14:45]

Patchset 5 (id:??) landed as
https://crrev.com/7894d3ce0f6755053be0cc7cb4ada055cb2f8755
Cr-Commit-Position: refs/heads/master@{#416138}