Move network settings metrics to JS

chrome/browser/resources/options/chromeos/internet_detail.js

Index: chrome/browser/resources/options/chromeos/internet_detail.js
diff --git a/chrome/browser/resources/options/chromeos/internet_detail.js b/chrome/browser/resources/options/chromeos/internet_detail.js
index 7c97af998f17e39527c36d78b2088317967ed3e6..16cecd5b66be8a898938ed2c226d44c8787c59a0 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -72,14 +72,47 @@ cr.define('options.internet', function() {
   }
 
   /**
+   * @param {string} action An action to send to coreOptionsUserMetricsAction.
+   */
+  function sendChromeMetricsAction(action) {
+    chrome.send('coreOptionsUserMetricsAction', [action]);
+  }
+
+  /**
    * Sends the 'checked' state of a control to chrome for a network.
    * @param {string} path The service path of the network.
    * @param {string} message The message to send to chrome.
    * @param {HTMLInputElement} checkbox The checkbox storing the value to send.
+   * @param {string=} opt_action Optional action to record.
    */
-  function sendCheckedIfEnabled(path, message, checkbox) {
-    if (!checkbox.hidden && !checkbox.disabled)
+  function sendCheckedIfEnabled(path, message, checkbox, opt_action) {
+    if (!checkbox.hidden && !checkbox.disabled) {
       chrome.send(message, [path, checkbox.checked ? 'true' : 'false']);
+      if (opt_action)
+        sendChromeMetricsAction(opt_action);
+    }
+  }
+
+  /**
+   * Send metrics to Chrome when the detailed page is opened.
+   * @param {string} type The ONC type of the network being shown.
+   * @param {string} state The ONC network state.
+   */
+  function sendMetrics(type, state) {

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

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.

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

Done.

+    if (type == 'WiFi') {
+      sendChromeMetricsAction('Options_NetworkShowDetailsWifi');
+      if (state != 'NotConnected')
+        sendChromeMetricsAction('Options_NetworkShowDetailsWifiConnected');
+    } else if (type == 'Cellular') {
+      sendChromeMetricsAction('Options_NetworkShowDetailsCellular');
+      if (state != 'NotConnected')
+        sendChromeMetricsAction('Options_NetworkShowDetailsCellularConnected');
+    }
+    if (type == 'VPN') {

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

else if?

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

Done.

+      sendChromeMetricsAction('Options_NetworkShowDetailsVPN');
+      if (state != 'NotConnected')
+        sendChromeMetricsAction('Options_NetworkShowDetailsVPNConnected');
+    }

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

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]

I prefer not to concatenate strings here for searchability.

   }
 
   /**
@@ -527,8 +560,7 @@ cr.define('options.internet', function() {
                                     !$('proxy-use-pac-url').checked;
       $('auto-proxy-parms').hidden = !$('auto-proxy').checked;
       $('manual-proxy-parms').hidden = !$('manual-proxy').checked;
-      chrome.send('coreOptionsUserMetricsAction',
-                  ['Options_NetworkManualProxy_Disable']);
+      sendChromeMetricsAction('Options_NetworkManualProxy_Disable');
     },
 
     /**
@@ -559,8 +591,7 @@ cr.define('options.internet', function() {
       $('proxy-pac-url').disabled = true;
       $('auto-proxy-parms').hidden = !$('auto-proxy').checked;
       $('manual-proxy-parms').hidden = !$('manual-proxy').checked;
-      chrome.send('coreOptionsUserMetricsAction',
-                  ['Options_NetworkManualProxy_Enable']);
+      sendChromeMetricsAction('Options_NetworkManualProxy_Enable');
     },
 
     updateConnectionButtonVisibilty_: function() {
@@ -891,8 +922,7 @@ cr.define('options.internet', function() {
     updateHidden('#details-internet-page .action-area', true);
     detailsPage.updateControls();
     detailsPage.visible = true;
-    chrome.send('coreOptionsUserMetricsAction',
-                ['Options_NetworkShowProxyTab']);
+    sendChromeMetricsAction('Options_NetworkShowProxyTab');
   };
 
   /**
@@ -941,6 +971,10 @@ cr.define('options.internet', function() {
 
   DetailsInternetPage.loginFromDetails = function() {
     var detailsPage = DetailsInternetPage.getInstance();
+    if (detailsPage.type_ == 'WiFi')
+      sendChromeMetricsAction('Options_NetworkConnectToWifi');
+    else if (detailsPage.type_ == 'VPN')
+      sendChromeMetricsAction('Options_NetworkConnectToVPN');
     chrome.send('networkCommand',
                 [detailsPage.type_, detailsPage.servicePath_, 'connect']);
     PageManager.closeOverlay();
@@ -948,6 +982,10 @@ cr.define('options.internet', function() {
 
   DetailsInternetPage.disconnectNetwork = function() {
     var detailsPage = DetailsInternetPage.getInstance();
+    if (detailsPage.type_ == 'WiFi')
+      sendChromeMetricsAction('Options_NetworkDisconnectWifi');
+    else if (detailsPage.type_ == 'VPN')
+      sendChromeMetricsAction('Options_NetworkDisconnectVPN');
     chrome.send('networkCommand',
                 [detailsPage.type_, detailsPage.servicePath_, 'disconnect']);
     PageManager.closeOverlay();
@@ -974,22 +1012,31 @@ cr.define('options.internet', function() {
     var type = detailsPage.type_;
     var servicePath = detailsPage.servicePath_;
     if (type == 'WiFi') {
-      sendCheckedIfEnabled(servicePath, 'setPreferNetwork',
-                           $('prefer-network-wifi'));
-      sendCheckedIfEnabled(servicePath, 'setAutoConnect',
-                           $('auto-connect-network-wifi'));
+      sendCheckedIfEnabled(servicePath,
+                           'setPreferNetwork',
+                           $('prefer-network-wifi'),
+                           'Options_NetworkSetPrefer');
+      sendCheckedIfEnabled(servicePath,
+                           'setAutoConnect',
+                           $('auto-connect-network-wifi'),
+                           'Options_NetworkAutoConnect');
     } else if (type == 'Wimax') {
-      sendCheckedIfEnabled(servicePath, 'setAutoConnect',
-                           $('auto-connect-network-wimax'));
+      sendCheckedIfEnabled(servicePath,
+                           'setAutoConnect',
+                           $('auto-connect-network-wimax'),
+                           'Options_NetworkAutoConnect');
     } else if (type == 'Cellular') {
-      sendCheckedIfEnabled(servicePath, 'setAutoConnect',
-                           $('auto-connect-network-cellular'));
+      sendCheckedIfEnabled(servicePath,
+                           'setAutoConnect',
+                           $('auto-connect-network-cellular'),
+                           'Options_NetworkAutoConnect');
     } else if (type == 'VPN') {
       chrome.send('setServerHostname',
-                  [servicePath,
-                   $('inet-server-hostname').value]);
-      sendCheckedIfEnabled(servicePath, 'setAutoConnect',
-                           $('auto-connect-network-vpn'));
+                  [servicePath, $('inet-server-hostname').value]);
+      sendCheckedIfEnabled(servicePath,
+                           'setAutoConnect',
+                           $('auto-connect-network-vpn'),
+                           'Options_NetworkAutoConnect');
     }
 
     var nameServerTypes = ['automatic', 'google', 'user'];
@@ -1081,6 +1128,8 @@ cr.define('options.internet', function() {
     detailsPage.type_ = type;
     detailsPage.servicePath_ = data.servicePath;
 
+    sendMetrics(type, onc.getActiveValue('ConnectionState'));
+
     detailsPage.populateHeader_();
     detailsPage.updateConnectionButtonVisibilty_();
     detailsPage.updateDetails_(data);