Fix favorite and auto_connect property usage (again)

The original CL was:
http://codereview.chromium.org/4818002

which missed a unittest fix, and the commit(r66253) was reverted (r66258).

Changed
chrome/browser/chromeos/options/wifi_config_view_browsertest.cc
and network_library.h.

BUG=chromium-os:8193
TEST=manually open chrome://settings/internet and see if remembered networks are listed properly. For DOMUI network menu, pass --enable-domui-menu for chrome to enable and check. Also check browser_test --gtest_filter="WifiConfigViewTest.ChangeAutoConnectSaveTest"

Review URL: http://codereview.chromium.org/5094002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66411 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 63 insertions, 54 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index feaa091..42cdff9 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -9346,9 +9346,6 @@ Keep your key file in a safe place. You will need it to create new versions of y
       <message name="IDS_NETWORK_RECONNECT_TITLE" desc="In network menu, title of the reconnect button that allows user to retry connection on error.">
         Reconnect
       </message>
-      <message name="IDS_NETWORK_REMEMBER_THIS_NETWORK_TITLE" desc="In network menu, title of checkbox that remembers the network when checked.">
-        Remember this network
-      </message>
       <message name="IDS_NETWORK_SSID_HINT" desc="In network menu, hint text for network name field.">
         network name
       </message>
diff --git a/chrome/browser/chromeos/cros/network_library.cc b/chrome/browser/chromeos/cros/network_library.cc
index c67a495..6f021bb 100644
--- a/chrome/browser/chromeos/cros/network_library.cc
+++ b/chrome/browser/chromeos/cros/network_library.cc
@@ -1377,16 +1377,16 @@ class NetworkLibraryImpl : public NetworkLibrary  {
     DVLOG(1) << "Remembered networks:";
     for (int i = 0; i < system->remembered_service_size; i++) {
       const ServiceInfo* service = system->GetRememberedServiceInfo(i);
-      // Only services marked as auto_connect are considered remembered
-      // networks.
+      // Only services marked as favorite are considered remembered networks.
       // TODO(chocobo): Don't add to remembered service if currently available.
-      if (service->auto_connect) {
+      if (service->favorite) {
         DVLOG(1) << "  (" << service->type << ") " << service->name
                  << " mode=" << service->mode
                  << " sec=" << service->security
                  << " pass=" << service->passphrase
                  << " id=" << service->identity
                  << " certpath=" << service->cert_path
+                 << " fav=" << service->favorite
                  << " auto=" << service->auto_connect;
         if (service->type == TYPE_WIFI) {
           remembered_wifi_networks->push_back(new WifiNetwork(service));
diff --git a/chrome/browser/chromeos/cros/network_library.h b/chrome/browser/chromeos/cros/network_library.h
index fb80d09..9c49a12 100644
--- a/chrome/browser/chromeos/cros/network_library.h
+++ b/chrome/browser/chromeos/cros/network_library.h
@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/gtest_prod_util.h"
 #include "base/observer_list.h"
 #include "base/platform_thread.h"
 #include "base/singleton.h"
@@ -138,7 +139,9 @@ class WirelessNetwork : public Network {
   bool favorite() const { return favorite_; }
 
   void set_auto_connect(bool auto_connect) { auto_connect_ = auto_connect; }
-  void set_favorite(bool favorite) { favorite_ = favorite; }
+  // We don't have a setter for |favorite_| because to unfavorite a network is
+  // equivalent to forget a network, so we call forget network on cros for
+  // that.  See ForgetWifiNetwork().
 
   // Network overrides.
   virtual void Clear();
@@ -158,6 +161,9 @@ class WirelessNetwork : public Network {
   bool favorite_;
 
  private:
+  // ChangeAutoConnectSaveTest accesses |favorite_|.
+  FRIEND_TEST_ALL_PREFIXES(WifiConfigViewTest, ChangeAutoConnectSaveTest);
+
   void set_name(const std::string& name) { name_ = name; }
   void set_strength(int strength) { strength_ = strength; }
 
diff --git a/chrome/browser/chromeos/dom_ui/internet_options_handler.cc b/chrome/browser/chromeos/dom_ui/internet_options_handler.cc
index 207cb879..cd66af9 100644
--- a/chrome/browser/chromeos/dom_ui/internet_options_handler.cc
+++ b/chrome/browser/chromeos/dom_ui/internet_options_handler.cc
@@ -158,7 +158,7 @@ void InternetOptionsHandler::GetLocalizedValues(
   localized_strings->SetString("inetPassProtected",
       l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_NET_PROTECTED));
-  localized_strings->SetString("inetRememberNetwork",
+  localized_strings->SetString("inetAutoConnectNetwork",
       l10n_util::GetStringUTF16(
           IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_AUTO_CONNECT));
   localized_strings->SetString("inetCertPkcs",
@@ -459,11 +459,11 @@ DictionaryValue* InternetOptionsHandler::CellularDataPlanToDictionary(
 
 void InternetOptionsHandler::SetDetailsCallback(const ListValue* args) {
   std::string service_path;
-  std::string remember;
+  std::string auto_connect_str;
 
   if (args->GetSize() < 2 ||
       !args->GetString(0, &service_path) ||
-      !args->GetString(1, &remember)) {
+      !args->GetString(1, &auto_connect_str)) {
     NOTREACHED();
     return;
   }
@@ -501,7 +501,7 @@ void InternetOptionsHandler::SetDetailsCallback(const ListValue* args) {
     }
   }
 
-  bool auto_connect = remember == "true";
+  bool auto_connect = auto_connect_str == "true";
   if (auto_connect != network->auto_connect()) {
     network->set_auto_connect(auto_connect);
     changed = true;
diff --git a/chrome/browser/chromeos/dom_ui/network_menu_ui.cc b/chrome/browser/chromeos/dom_ui/network_menu_ui.cc
index 30df58f..a9aa9c4 100644
--- a/chrome/browser/chromeos/dom_ui/network_menu_ui.cc
+++ b/chrome/browser/chromeos/dom_ui/network_menu_ui.cc
@@ -30,8 +30,9 @@ class NetworkMenuSourceDelegate : public chromeos::MenuSourceDelegate {
 
     localized_strings->SetString("reconnect", l10n_util::GetStringUTF16(
         IDS_NETWORK_RECONNECT_TITLE));
-    localized_strings->SetString("remeber_this_network",
-        l10n_util::GetStringUTF16(IDS_NETWORK_REMEMBER_THIS_NETWORK_TITLE));
+    localized_strings->SetString("auto_connect_this_network",
+        l10n_util::GetStringUTF16(
+            IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_AUTO_CONNECT));
     localized_strings->SetString("ssid_prompt",
         l10n_util::GetStringUTF16(IDS_NETWORK_SSID_HINT));
     localized_strings->SetString("pass_prompt",
@@ -149,14 +150,14 @@ bool NetworkMenuUI::ModelAction(const menus::MenuModel* model,
   values->GetString(2, &passphrase);  // Optional
   std::string ssid;
   values->GetString(3, &ssid);  // Optional
-  int remember = -1;  // -1 indicates not set
-  std::string remember_str;
-  if (values->GetString(4, &remember_str))  // Optional
-    base::StringToInt(remember_str, &remember);
+  int auto_connect = -1;  // -1 indicates default action (auto connect)
+  std::string auto_connect_str;
+  if (values->GetString(4, &auto_connect_str))  // Optional
+    base::StringToInt(auto_connect_str, &auto_connect);
 
   if (action == "connect" || action == "reconnect") {
     close_menu = network_menu->ConnectToNetworkAt(index, passphrase, ssid,
-                                                  remember);
+                                                  auto_connect);
   } else {
     LOG(WARNING) << "Unrecognized action: " << action
                  << " from: " << chrome::kChromeUINetworkMenu;
diff --git a/chrome/browser/chromeos/options/wifi_config_view_browsertest.cc b/chrome/browser/chromeos/options/wifi_config_view_browsertest.cc
index 98f4e1f..02d5066 100644
--- a/chrome/browser/chromeos/options/wifi_config_view_browsertest.cc
+++ b/chrome/browser/chromeos/options/wifi_config_view_browsertest.cc
@@ -41,7 +41,7 @@ IN_PROC_BROWSER_TEST_F(WifiConfigViewTest, NoChangeSaveTest) {
 IN_PROC_BROWSER_TEST_F(WifiConfigViewTest, ChangeAutoConnectSaveTest) {
   EXPECT_CALL(*mock_network_library_, SaveWifiNetwork(_)).Times(1);
   scoped_ptr<WifiNetwork> remembered_network(new WifiNetwork());
-  remembered_network->set_favorite(true);
+  remembered_network->favorite_ = true;
   WifiConfigView* view = new WifiConfigView(NULL, remembered_network.get());
   ASSERT_TRUE(view->autoconnect_checkbox_ != NULL);
   view->autoconnect_checkbox_->SetChecked(
diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index c30eabe..17686d8a 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -145,6 +145,7 @@ bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
       }
       info->ip_address = wifi->ip_address();
       info->remembered = wifi->favorite();
+      info->auto_connect = info->remembered ? wifi->auto_connect() : true;
     } else {
       res = false;  // Network not found, hide entry.
     }
@@ -200,7 +201,7 @@ bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
 bool NetworkMenu::ConnectToNetworkAt(int index,
                                      const std::string& passphrase,
                                      const std::string& ssid,
-                                     int remember) const {
+                                     int auto_connect) const {
   int flags = menu_items_[index].flags;
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   if (flags & FLAG_WIFI) {
@@ -208,8 +209,8 @@ bool NetworkMenu::ConnectToNetworkAt(int index,
         menu_items_[index].wireless_path);
     if (wifi) {
       // Connect or reconnect.
-      if (remember >= 0)
-        wifi->set_favorite(remember ? true : false);
+      if (auto_connect >= 0)
+        wifi->set_auto_connect(auto_connect ? true : false);
       if (cros->wifi_network() &&
           wifi->service_path() == cros->wifi_network()->service_path()) {
         // Show the config settings for the active network.
@@ -279,10 +280,11 @@ bool NetworkMenu::ConnectToNetworkAt(int index,
   } else if (flags & FLAG_OTHER_NETWORK) {
     bool connected = false;
     if (MenuUI::IsEnabled()) {
-      bool favorite = remember == 0 ? false : true;  // default is true
+      // default is true
+      bool auto_connect_bool = auto_connect == 0 ? false : true;
       connected = cros->ConnectToWifiNetwork(
           passphrase.empty() ? SECURITY_NONE : SECURITY_UNKNOWN,
-          ssid, passphrase, std::string(), std::string(), favorite);
+          ssid, passphrase, std::string(), std::string(), auto_connect_bool);
     }
     if (!connected) {
       ShowOther();
diff --git a/chrome/browser/chromeos/status/network_menu.h b/chrome/browser/chromeos/status/network_menu.h
index bffe26f..05b941e 100644
--- a/chrome/browser/chromeos/status/network_menu.h
+++ b/chrome/browser/chromeos/status/network_menu.h
@@ -52,7 +52,8 @@ class NetworkMenu : public views::ViewMenuDelegate,
                     public menus::MenuModel {
  public:
   struct NetworkInfo {
-    NetworkInfo() : need_passphrase(false), remembered(true) {}
+    NetworkInfo() :
+        need_passphrase(false), remembered(true), auto_connect(true) {}
     // "ethernet" | "wifi" | "cellular" | "other".
     std::string network_type;
     // "connected" | "connecting" | "disconnected" | "error".
@@ -67,6 +68,8 @@ class NetworkMenu : public views::ViewMenuDelegate,
     bool need_passphrase;
     // true if the network is currently remembered.
     bool remembered;
+    // true if the network is auto connect (meaningful for Wifi only).
+    bool auto_connect;
   };
 
   NetworkMenu();
diff --git a/chrome/browser/resources/network_menu.js b/chrome/browser/resources/network_menu.js
index 8413e99..19f40a6 100644
--- a/chrome/browser/resources/network_menu.js
+++ b/chrome/browser/resources/network_menu.js
@@ -31,13 +31,13 @@ const NetworkOther = 'other';
 /**
  * Sends "connect" using the 'action' DOMUI message.
  */
-function sendConnect(index, passphrase, identity, remember) {
+function sendConnect(index, passphrase, identity, auto_connect) {
   chrome.send('action',
       ['connect',
        String(index),
        passphrase,
        identity,
-       remember ? '1' : '0']);
+       auto_connect ? '1' : '0']);
 }
 
 var networkMenuItemProto = (function() {
@@ -62,7 +62,7 @@ NetworkMenuItem.prototype = {
 
   ssidEdit: null,
   passwordEdit: null,
-  rememberCheckbox: null,
+  autoConnectCheckbox: null,
 
   /**
    * The label element.
@@ -135,19 +135,18 @@ NetworkMenuItem.prototype = {
         sendConnect(index,
             this.passwordEdit.value,
             this.ssidEdit.value,
-            this.rememberCheckbox.checked);
+            this.autoConnectCheckbox.checked);
       }
     } else if (this.passwordEdit) {
       if (this.passwordEdit.value) {
         sendConnect(index,
-            this.passwordEdit.value, '', this.rememberCheckbox.checked);
+            this.passwordEdit.value, '', this.autoConnectCheckbox.checked);
       }
     } else {
       if (this.attrs.remembered) {
-        sendConnect(index, this.attrs.passphrase, '',
-                    this.rememberCheckbox.checked);
+        sendConnect(index, this.attrs.passphrase, '', this.attrs.auto_connect);
       } else {
-        sendConnect(index, '', '', this.rememberCheckbox.checked);
+        sendConnect(index, '', '', this.autoConnectCheckbox.checked);
       }
     }
   },
@@ -230,22 +229,23 @@ NetworkMenuItem.prototype = {
   },
 
   /**
-   * Add remember this network check box to action area.
+   * Add auto-connect this network check box to action area.
    * @private
    */
-  addRememberCheckbox_: function() {
-    this.rememberCheckbox = this.ownerDocument.createElement('input');
-    this.rememberCheckbox.type = 'checkbox';
-    this.rememberCheckbox.checked = this.attrs.remembered;
+  addAutoConnectCheckbox_: function() {
+    this.autoConnectCheckbox = this.ownerDocument.createElement('input');
+    this.autoConnectCheckbox.type = 'checkbox';
+    this.autoConnectCheckbox.checked = this.attrs.auto_connect;
 
-    var rememberSpan = this.ownerDocument.createElement('span');
-    rememberSpan.textContent = localStrings.getString('remeber_this_network');
+    var autoConnectSpan = this.ownerDocument.createElement('span');
+    autoConnectSpan.textContent =
+        localStrings.getString('auto_connect_this_network');
 
-    var rememberLabel = this.ownerDocument.createElement('label');
-    rememberLabel.appendChild(this.rememberCheckbox);
-    rememberLabel.appendChild(rememberSpan);
+    var autoConnectLabel = this.ownerDocument.createElement('label');
+    autoConnectLabel.appendChild(this.autoConnectCheckbox);
+    autoConnectLabel.appendChild(autoConnectSpan);
 
-    this.action_.appendChild(rememberLabel);
+    this.action_.appendChild(autoConnectLabel);
   },
 
   /**
@@ -266,7 +266,7 @@ NetworkMenuItem.prototype = {
     if (attrs.network_type == NetworkOther) {
       this.addSsidEdit_();
       this.addPasswordEdit_();
-      this.addRememberCheckbox_();
+      this.addAutoConnectCheckbox_();
     } else if (attrs.status && attrs.status != 'unknown') {
       if (attrs.status == StatusConnected) {
         this.setStatus_(attrs.ip_address);
@@ -293,7 +293,7 @@ NetworkMenuItem.prototype = {
         this.addPasswordEdit_();
       }
 
-      this.addRememberCheckbox_();
+      this.addAutoConnectCheckbox_();
     }
     //////// End NetworkMenuItem specifi code
 
@@ -346,10 +346,10 @@ NetworkMenu.prototype = {
   /** @inheritDoc */
   onClick_: function(event, item) {
     // If item is a NetworkMenuItem, it must have at least one of the following.
-    if (item.rememberCheckbox || item.ssidEdit || item.passwordEdit) {
+    if (item.autoConnectCheckbox || item.ssidEdit || item.passwordEdit) {
       // Ignore clicks other than on the NetworkMenuItem itself.
-      if (event.target == item.rememberCheckbox ||
-          event.target == item.rememberCheckbox.nextElementSibling ||
+      if (event.target == item.autoConnectCheckbox ||
+          event.target == item.autoConnectCheckbox.nextElementSibling ||
           event.target == item.ssidEdit ||
           event.target == item.passwordEdit) {
         return;
diff --git a/chrome/browser/resources/options/chromeos_internet_detail.html b/chrome/browser/resources/options/chromeos_internet_detail.html
index f5fe513..a6ddd4c 100644
--- a/chrome/browser/resources/options/chromeos_internet_detail.html
+++ b/chrome/browser/resources/options/chromeos_internet_detail.html
@@ -42,8 +42,8 @@
         <tr>
           <td colspan="2">
             <label class="checkbox">
-              <input id="rememberNetwork" type="checkbox">
-              <span i18n-content="inetRememberNetwork"></span>
+              <input id="autoConnectNetwork" type="checkbox">
+              <span i18n-content="inetAutoConnectNetwork"></span>
             </label>
           </td>
         </tr>
diff --git a/chrome/browser/resources/options/chromeos_internet_options.js b/chrome/browser/resources/options/chromeos_internet_options.js
index af9adcb..eafd72f 100644
--- a/chrome/browser/resources/options/chromeos_internet_options.js
+++ b/chrome/browser/resources/options/chromeos_internet_options.js
@@ -116,7 +116,7 @@ cr.define('options', function() {
     if (data.type == 2) {
       var newinfo = [];
       newinfo.push(data.servicePath);
-      newinfo.push($('rememberNetwork').checked ? "true" : "false");
+      newinfo.push($('autoConnectNetwork').checked ? "true" : "false");
       if (data.encrypted && data.certNeeded) {
         newinfo.push($('inetIdent').value);
         newinfo.push($('inetCert').value);
@@ -258,10 +258,10 @@ cr.define('options', function() {
       page.removeAttribute('cellular');
       page.removeAttribute('gsm');
       $('inetSsid').textContent = data.ssid;
-      $('rememberNetwork').checked = data.autoConnect;
+      $('autoConnectNetwork').checked = data.autoConnect;
       if (!AccountsOptions.currentUserIsOwner()) {
         // Disable this for guest non-Owners.
-        $('rememberNetwork').disabled = true;
+        $('autoConnectNetwork').disabled = true;
       }
       page.removeAttribute('password');
       page.removeAttribute('cert');