Make network settings functions more closely match networkingPrivate

Make network settings functions more closely match networkingPrivate

This is in preparation for using the networkingPrivate API directly
for the code that has been re-factored so far.

BUG=279351
Committed: https://crrev.com/b7e5dc39e133101cfc10097a48379fbbc4cdea8c
Cr-Commit-Position: refs/heads/master@{#295134}

[stevenjb, 2014-09-16 02:15:15]

stevenjb@chromium.org changed reviewers:
+ armansito@chromium.org, pneubeck@chromium.org

[stevenjb, 2014-09-16 02:15:15]

Note: A lot of the complexity in this CL surrounds moving logic from
InternetOptionsHandler::EnableCellularCallback to the JS. This also involves
passing some additional non ONC properties to the JS, but I think it is an
important step in moving the control logic out of InternetOptionsHandler and
into the JS code.

Eventually I think we will need to provide SIM lock related methods to
networkingPrivate, but that should clearly be done in a separate CL, and I think
this will make that step a bit more straightforward.

[armansito, 2014-09-16 16:40:42]

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/internet_detail.js (right):

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/internet_detail.js:1166: * properties,
including the HUID and service path to be used for requiesting
nit: requiesting -> requesting

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/internet_detail.js:1167: * the ONC
properites. Note: currently GUID is only used to confirm that the
nit: properites -> properties

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/network_list.js (right):

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/network_list.js:66: * Indicates if
cellular device supports network scanning.
Shouldn't this be something like "Indicates the current SIM lock type of the
cellular device"?

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/network_list.js:73: * Indicates if
cellular device supports network scanning.
Copy-paste?

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
(right):

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:104: //
networkingPrivate methods:
Add TODO?

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:542: //
networkingPrivate implementation methods
Add a note saying that these will be superseded by the real networkingPrivate
implementation?

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:906:
LOG(ERROR) << "Unknown inetenet options command: " << command;
nit: inetenet -> internet

[stevenjb, 2014-09-16 17:32:58]

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/internet_detail.js (right):

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/internet_detail.js:1166: * properties,
including the HUID and service path to be used for requiesting
On 2014/09/16 16:40:41, armansito wrote:
> nit: requiesting -> requesting

Done.

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/internet_detail.js:1167: * the ONC
properites. Note: currently GUID is only used to confirm that the
On 2014/09/16 16:40:41, armansito wrote:
> nit: properites -> properties

Done.

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/network_list.js (right):

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/network_list.js:66: * Indicates if
cellular device supports network scanning.
On 2014/09/16 16:40:41, armansito wrote:
> Shouldn't this be something like "Indicates the current SIM lock type of the
> cellular device"?

Done.

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/network_list.js:73: * Indicates if
cellular device supports network scanning.
On 2014/09/16 16:40:41, armansito wrote:
> Copy-paste?

Done.

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
(right):

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:104: //
networkingPrivate methods:
On 2014/09/16 16:40:42, armansito wrote:
> Add TODO?

Done.

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:542: //
networkingPrivate implementation methods
On 2014/09/16 16:40:42, armansito wrote:
> Add a note saying that these will be superseded by the real networkingPrivate
> implementation?

Done.

https://codereview.chromium.org/544173013/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:906:
LOG(ERROR) << "Unknown inetenet options command: " << command;
On 2014/09/16 16:40:42, armansito wrote:
> nit: inetenet -> internet

Done.

[armansito, 2014-09-16 17:38:56]

lgtm with nits

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/network_list.js (right):

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/network_list.js:73: * Indicates that
the SIM card is absent on the cellular device.
nit: I would say "Indicates whether..." since it's a boolean.

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
(right):

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:543: // the
matching networkingPrivate methods, crbug.com/279351.
nit: I guess by replacing we mean that these methods here will be removed
entirely, correct? I would make that a bit more clear.

[stevenjb, 2014-09-16 18:27:20]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2014-09-16 18:27:33]

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/network_list.js (right):

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/network_list.js:73: * Indicates that
the SIM card is absent on the cellular device.
On 2014/09/16 17:38:56, armansito wrote:
> nit: I would say "Indicates whether..." since it's a boolean.

Done.

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
(right):

https://codereview.chromium.org/544173013/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:543: // the
matching networkingPrivate methods, crbug.com/279351.
On 2014/09/16 17:38:56, armansito wrote:
> nit: I guess by replacing we mean that these methods here will be removed
> entirely, correct? I would make that a bit more clear.

Done.

[armansito, 2014-09-16 18:28:11]

lgtm

[commit-bot: I haz the power, 2014-09-16 18:29:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/544173013/60001

[commit-bot: I haz the power, 2014-09-16 20:06:10]

Committed patchset #4 (id:60001) as b9691d7494d688cfe0e73084ecae5de2acd353fd

[commit-bot: I haz the power, 2014-09-16 20:06:47]

Patchset 4 (id:??) landed as
https://crrev.com/b7e5dc39e133101cfc10097a48379fbbc4cdea8c
Cr-Commit-Position: refs/heads/master@{#295134}