Move network settings metrics to JS

Move network settings metrics to JS

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

[stevenjb, 2014-09-12 23:19:33]

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

[stevenjb, 2014-09-12 23:19:34]

Moving all UI metrics to the JS.

[armansito, 2014-09-15 14:44:52]

lgtm with nits.

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos/internet_detail.js (right):

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos/internet_detail.js:101: function
sendMetrics(type, state) {
Should this be called "sendShowDetailsStateMetrics" or something along those
lines? I think "sendMetrics" is a bit too generic and this function seems to be
getting used for technology + connection state.

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos/internet_detail.js:111: if (type ==
'VPN') {
else if?

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos/internet_detail.js:115: }
Maybe you could reduce the repetitions here. Something like:

   if (type === undefined)
     return;

   var technology = (type == 'WiFi') ? 'Wifi' : type;
   sendChromeMetricsAction('Options_NetworkShowDetails' + technology);
   if (state != 'NotConnected')
     sendChromeMetricsAction('Options_NetworkShowDetails' + technology +
'Connected');

Up to you though, since this is mostly cosmetic.

[stevenjb, 2014-09-15 22:51:51]

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
File chrome/browser/resources/options/chromeos/internet_detail.js (right):

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos/internet_detail.js:101: function
sendMetrics(type, state) {
On 2014/09/15 14:44:52, armansito wrote:
> Should this be called "sendShowDetailsStateMetrics" or something along those
> lines? I think "sendMetrics" is a bit too generic and this function seems to
be
> getting used for technology + connection state.

Done.

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos/internet_detail.js:111: if (type ==
'VPN') {
On 2014/09/15 14:44:52, armansito wrote:
> else if?

Done.

https://codereview.chromium.org/570743002/diff/1/chrome/browser/resources/opt...
chrome/browser/resources/options/chromeos/internet_detail.js:115: }
On 2014/09/15 14:44:52, armansito wrote:
> Maybe you could reduce the repetitions here. Something like:
> 
>    if (type === undefined)
>      return;
> 
>    var technology = (type == 'WiFi') ? 'Wifi' : type;
>    sendChromeMetricsAction('Options_NetworkShowDetails' + technology);
>    if (state != 'NotConnected')
>      sendChromeMetricsAction('Options_NetworkShowDetails' + technology +
> 'Connected');
> 
> Up to you though, since this is mostly cosmetic.
I prefer not to concatenate strings here for searchability.

[stevenjb, 2014-09-16 01:25:35]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2014-09-16 01:28:20]

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

[commit-bot: I haz the power, 2014-09-16 03:43:13]

Committed patchset #3 (id:40001) as e6933486b9eeaedd104c6e8ff81c8a84458295b6

[commit-bot: I haz the power, 2014-09-16 03:45:00]

Patchset 3 (id:??) landed as
https://crrev.com/0bd565a3deff6119417bc23b9afdae05e7d6b580
Cr-Commit-Position: refs/heads/master@{#295001}