More changes to make network settings better match extension API

chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc

Index: chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
index e1dae71e6d1cfce40155c3584bd9ad9b60ee08d8..9d044c5308b520557aca256e3080ca586bc05d91 100644
--- a/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
+++ b/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
@@ -63,6 +63,11 @@ namespace options {
 
 namespace {
 
+// Keys for the initial "localized" dictionary values.
+const char kLoggedInAsOwnerKey[] = "loggedInAsOwner";
+const char kShowCarrierSelectKey[] = "showCarrierSelect";
+const char kNetworkDataKey[] = "networkData";
+
 // Keys for the network description dictionary passed to the web ui. Make sure
 // to keep the strings in sync with what the JavaScript side uses.
 const char kNetworkInfoKeyIconURL[] = "iconURL";
@@ -80,17 +85,15 @@ const char kRefreshNetworkDataFunction[] =
     "options.network.NetworkList.refreshNetworkData";
 const char kSetDefaultNetworkIconsFunction[] =
     "options.network.NetworkList.setDefaultNetworkIcons";
-const char kSendNetworkDetailsFunction[] =
-    "options.internet.DetailsInternetPage.sendNetworkDetails";
-const char kShowDetailedInfoFunction[] =
-    "options.internet.DetailsInternetPage.showDetailedInfo";
+const char kGetManagedPropertiesResultFunction[] =
+    "options.internet.DetailsInternetPage.getManagedPropertiesResult";
 const char kUpdateConnectionDataFunction[] =
     "options.internet.DetailsInternetPage.updateConnectionData";
 const char kUpdateCarrierFunction[] =
     "options.internet.DetailsInternetPage.updateCarrier";
 
-// These are used to register message handlers with JavaScript.
-const char kNetworkCommandMessage[] = "networkCommand";
+// Setter menthods called from JS that still need to be converted to match
+// networkingPrivate methods.
 const char kSetApnMessage[] = "setApn";
 const char kSetAutoConnectMessage[] = "setAutoConnect";
 const char kSetCarrierMessage[] = "setCarrier";
@@ -109,26 +112,26 @@ const char kRequestNetworkScanMessage[] = "requestNetworkScan";
 const char kStartConnectMessage[] = "startConnect";
 const char kStartDisconnectMessage[] = "startDisconnect";
 
+// TODO(stevenjb): Add these to networkingPrivate.
+const char kRemoveNetworkMessage[] = "removeNetwork";
+
+// TODO(stevenjb): Deprecate these and integrate with settings Web UI.
+const char kAddConnectionMessage[] = "addConnection";
+const char kConfigureNetworkMessage[] = "configureNetwork";
+const char kActivateNetworkMessage[] = "activateNetwork";
+
 // These are strings used to communicate with JavaScript.
-const char kTagActivate[] = "activate";
-const char kTagAddConnection[] = "add";
-const char kTagCarrierSelectFlag[] = "showCarrierSelect";
 const char kTagCellularAvailable[] = "cellularAvailable";
 const char kTagCellularEnabled[] = "cellularEnabled";
 const char kTagCellularSimAbsent[] = "cellularSimAbsent";
 const char kTagCellularSimLockType[] = "cellularSimLockType";
 const char kTagCellularSupportsScan[] = "cellularSupportsScan";
-const char kTagConfigure[] = "configure";
-const char kTagForget[] = "forget";
 const char kTagRememberedList[] = "rememberedList";
-const char kTagShowDetails[] = "showDetails";
-const char kTagShowViewAccountButton[] = "showViewAccountButton";
 const char kTagSimOpChangePin[] = "changePin";
 const char kTagSimOpConfigure[] = "configure";
 const char kTagSimOpSetLocked[] = "setLocked";
 const char kTagSimOpSetUnlocked[] = "setUnlocked";
 const char kTagSimOpUnlock[] = "unlock";
-const char kTagTrue[] = "true";
 const char kTagVpnList[] = "vpnList";
 const char kTagWifiAvailable[] = "wifiAvailable";
 const char kTagWifiEnabled[] = "wifiEnabled";
@@ -138,9 +141,9 @@ const char kTagWiredList[] = "wiredList";
 const char kTagWirelessList[] = "wirelessList";
 
 // Pseudo-ONC chrome specific properties appended to the ONC dictionary.
-const char kTagErrorMessage[] = "errorMessage";
 const char kNetworkInfoKeyServicePath[] = "servicePath";
-const char kNetworkInfoKeyGUID[] = "GUID";
+const char kTagErrorMessage[] = "errorMessage";
+const char kTagShowViewAccountButton[] = "showViewAccountButton";
 
 const int kPreferredPriority = 1;
 
@@ -238,21 +241,6 @@ bool ShowViewAccountButton(const NetworkState* cellular) {
   return true;
 }
 
-scoped_ptr<base::DictionaryValue> PopulateConnectionDetails(
-    const NetworkState* network,
-    const base::DictionaryValue& onc_properties) {
-  scoped_ptr<base::DictionaryValue> dictionary(onc_properties.DeepCopy());
-
-  // Append Service Path for now.
-  dictionary->SetString(kNetworkInfoKeyServicePath, network->path());
-  // Append a Chrome specific translated error message.
-  dictionary->SetString(
-      kTagErrorMessage,
-      ash::network_connect::ErrorString(network->error(), network->path()));
-
-  return dictionary.Pass();
-}
-
 // Helper methods for SetIPConfigProperties
 bool AppendPropertyKeyIfPresent(const std::string& key,
                                 const base::DictionaryValue& old_properties,
@@ -313,14 +301,16 @@ void InternetOptionsHandler::GetLocalizedValues(
   // InitializePage() gets called.
   std::string owner;
   chromeos::CrosSettings::Get()->GetString(chromeos::kDeviceOwner, &owner);
-  localized_strings->SetString("ownerUserId", base::UTF8ToUTF16(owner));
   bool logged_in_as_owner = LoginState::Get()->GetLoggedInUserType() ==
                             LoginState::LOGGED_IN_USER_OWNER;
-  localized_strings->SetBoolean("loggedInAsOwner", logged_in_as_owner);
+  localized_strings->SetBoolean(kLoggedInAsOwnerKey, logged_in_as_owner);
+  localized_strings->SetBoolean(
+      kShowCarrierSelectKey, CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kEnableCarrierSwitching));
 
   base::DictionaryValue* network_dictionary = new base::DictionaryValue;
   FillNetworkInfo(network_dictionary);
-  localized_strings->Set("networkData", network_dictionary);
+  localized_strings->Set(kNetworkDataKey, network_dictionary);
 }
 
 void InternetOptionsHandler::InitializePage() {
@@ -338,9 +328,17 @@ void InternetOptionsHandler::InitializePage() {
 }
 
 void InternetOptionsHandler::RegisterMessages() {
-  // Setup handlers specific to this panel.
-  web_ui()->RegisterMessageCallback(kNetworkCommandMessage,
-      base::Bind(&InternetOptionsHandler::NetworkCommandCallback,
+  web_ui()->RegisterMessageCallback(kAddConnectionMessage,
+      base::Bind(&InternetOptionsHandler::AddConnection,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(kRemoveNetworkMessage,
+      base::Bind(&InternetOptionsHandler::RemoveNetwork,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(kConfigureNetworkMessage,
+      base::Bind(&InternetOptionsHandler::ConfigureNetwork,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(kActivateNetworkMessage,
+      base::Bind(&InternetOptionsHandler::ActivateNetwork,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(kSetPreferNetworkMessage,
       base::Bind(&InternetOptionsHandler::SetPreferNetworkCallback,
@@ -565,13 +563,16 @@ void InternetOptionsHandler::GetManagedPropertiesCallback(
     NOTREACHED();
     return;
   }
-  NetworkHandler::Get()->managed_network_configuration_handler()
+  // This is only ever called to provided properties for the details page, so

[pneubeck (no reviews), 2014/10/27 16:37:20]

typo: provided -> provide

[stevenjb, 2014/10/27 17:32:33]

Done.

+  // set |details_path_| (used by the NetworkState observers) here.
+  details_path_ = service_path;
+  NetworkHandler::Get()
+      ->managed_network_configuration_handler()
       ->GetManagedProperties(
-          LoginState::Get()->primary_user_hash(),
-          service_path,
-          base::Bind(
-              &InternetOptionsHandler::PopulateDictionaryDetailsCallback,
-              weak_factory_.GetWeakPtr()),
+          LoginState::Get()->primary_user_hash(), service_path,
+          base::Bind(&InternetOptionsHandler::GetManagedPropertiesResult,
+                     weak_factory_.GetWeakPtr(),
+                     kGetManagedPropertiesResultFunction),
           base::Bind(&ShillError, "GetManagedProperties"));
 }
 
@@ -623,22 +624,36 @@ void InternetOptionsHandler::UpdateConnectionData(
   NetworkHandler::Get()
       ->managed_network_configuration_handler()
       ->GetManagedProperties(
-          LoginState::Get()->primary_user_hash(),
-          service_path,
-          base::Bind(&InternetOptionsHandler::UpdateConnectionDataCallback,
-                     weak_factory_.GetWeakPtr()),
+          LoginState::Get()->primary_user_hash(), service_path,
+          base::Bind(&InternetOptionsHandler::GetManagedPropertiesResult,
+                     weak_factory_.GetWeakPtr(), kUpdateConnectionDataFunction),
           base::Bind(&ShillError, "UpdateConnectionData"));
 }
 
-void InternetOptionsHandler::UpdateConnectionDataCallback(
+void InternetOptionsHandler::GetManagedPropertiesResult(
+    const std::string& js_callback_function,
     const std::string& service_path,
     const base::DictionaryValue& onc_properties) {
+  scoped_ptr<base::DictionaryValue> dictionary(onc_properties.DeepCopy());
+  // Append Service Path for now.
+  dictionary->SetString(kNetworkInfoKeyServicePath, service_path);
+
   const NetworkState* network = GetNetworkState(service_path);
-  if (!network)
-    return;
-  scoped_ptr<base::DictionaryValue> dictionary =
-      PopulateConnectionDetails(network, onc_properties);
-  web_ui()->CallJavascriptFunction(kUpdateConnectionDataFunction, *dictionary);
+  if (network) {
+    // Append a Chrome specific translated error message. TODO(stevenjb): Figure
+    // out a more robust way to track errors. Service.Error is transient so we
+    // use NetworkState.error() which accurately tracks the "last" error.
+    dictionary->SetString(
+        kTagErrorMessage,
+        ash::network_connect::ErrorString(network->error(), service_path));
+    // Append additional cellular properties to inform the UI, not available
+    // in ONC.
+    if (network->type() == shill::kTypeCellular) {
+      dictionary->SetBoolean(kTagShowViewAccountButton,
+                             ShowViewAccountButton(network));
+    }
+  }
+  web_ui()->CallJavascriptFunction(js_callback_function, *dictionary);
 }
 
 void InternetOptionsHandler::UpdateCarrier() {
@@ -703,14 +718,15 @@ void InternetOptionsHandler::SetServerHostnameCallback(
 
 void InternetOptionsHandler::SetPreferNetworkCallback(
     const base::ListValue* args) {
-  std::string service_path, prefer_network_str;
+  std::string service_path;
+  bool prefer_network;
   if (args->GetSize() < 2 ||
       !args->GetString(0, &service_path) ||
-      !args->GetString(1, &prefer_network_str)) {
+      !args->GetBoolean(1, &prefer_network)) {
     NOTREACHED();
     return;
   }
-  int priority = (prefer_network_str == kTagTrue) ? kPreferredPriority : 0;
+  int priority = prefer_network ? kPreferredPriority : 0;
   SetNetworkProperty(service_path,
                      shill::kPriorityProperty,
                      new base::FundamentalValue(priority));
@@ -718,14 +734,14 @@ void InternetOptionsHandler::SetPreferNetworkCallback(
 
 void InternetOptionsHandler::SetAutoConnectCallback(
     const base::ListValue* args) {
-  std::string service_path, auto_connect_str;
+  std::string service_path;
+  bool auto_connect;
   if (args->GetSize() < 2 ||
       !args->GetString(0, &service_path) ||
-      !args->GetString(1, &auto_connect_str)) {
+      !args->GetBoolean(1, &auto_connect)) {
     NOTREACHED();
     return;
   }
-  bool auto_connect = auto_connect_str == kTagTrue;
   SetNetworkProperty(service_path,
                      shill::kAutoConnectProperty,
                      new base::FundamentalValue(auto_connect));
@@ -833,21 +849,6 @@ void InternetOptionsHandler::SetIPConfigProperties(
   }
 }
 
-void InternetOptionsHandler::PopulateDictionaryDetailsCallback(
-    const std::string& service_path,
-    const base::DictionaryValue& onc_properties) {
-  const NetworkState* network = GetNetworkState(service_path);
-  if (!network) {
-    LOG(ERROR) << "Network properties not found: " << service_path;
-    return;
-  }
-  scoped_ptr<base::DictionaryValue> dictionary =
-      PopulateConnectionDetails(network, onc_properties);
-
-  // Show details dialog
-  web_ui()->CallJavascriptFunction(kSendNetworkDetailsFunction, *dictionary);
-}
-
 gfx::NativeWindow InternetOptionsHandler::GetNativeWindow() const {
   return web_ui()->GetWebContents()->GetTopLevelNativeWindow();
 }
@@ -860,79 +861,51 @@ const PrefService* InternetOptionsHandler::GetPrefs() const {
   return Profile::FromWebUI(web_ui())->GetPrefs();
 }
 
-void InternetOptionsHandler::NetworkCommandCallback(
-    const base::ListValue* args) {
+void InternetOptionsHandler::AddConnection(const base::ListValue* args) {
   std::string onc_type;
-  std::string service_path;
-  std::string command;
-  if (args->GetSize() != 3 ||
-      !args->GetString(0, &onc_type) ||
-      !args->GetString(1, &service_path) ||
-      !args->GetString(2, &command)) {
+  if (args->GetSize() != 1 || !args->GetString(0, &onc_type)) {
     NOTREACHED();
     return;
   }
-  std::string type;  // Shill type
-  if (!onc_type.empty()) {
-    type = network_util::TranslateONCTypeToShill(onc_type);
-    if (type.empty())
-      LOG(ERROR) << "Unable to translate ONC type: " << onc_type;
-  }
-  // Process commands that do not require an existing network.
-  if (command == kTagAddConnection) {
-    AddConnection(type);
-  } else if (command == kTagForget) {
-    NetworkHandler::Get()->network_configuration_handler()->
-        RemoveConfiguration(
-            service_path,
-            base::Bind(&base::DoNothing),
-            base::Bind(&ShillError, "NetworkCommand: " + command));
-  } else if (command == kTagShowDetails) {
-    SendShowDetailedInfo(service_path);
-  } else if (command == kTagConfigure) {
-    NetworkConfigView::Show(service_path, GetNativeWindow());
-  } else if (command == kTagActivate && type == shill::kTypeCellular) {
-    ash::network_connect::ActivateCellular(service_path);
-    // Activation may update network properties (e.g. ActivationState), so
-    // request them here in case they change.
-    UpdateConnectionData(service_path);

[pneubeck (no reviews), 2014/10/27 16:37:19]

I don't see how this is replaced in the new code.
Or is it not required anymore?

[stevenjb, 2014/10/27 17:32:33]

Not required any more. If there are state changes, the observers will trigger.

-  } else {
-    LOG(ERROR) << "Unknown internet options command: " << command;
-    NOTREACHED();
-  }
-}
-
-void InternetOptionsHandler::AddConnection(const std::string& type) {
-  if (type == shill::kTypeWifi) {
+  if (onc_type == ::onc::network_type::kWiFi) {
     NetworkConfigView::ShowForType(shill::kTypeWifi, GetNativeWindow());
-  } else if (type == shill::kTypeVPN) {
+  } else if (onc_type == ::onc::network_type::kVPN) {
     NetworkConfigView::ShowForType(shill::kTypeVPN, GetNativeWindow());
-  } else if (type == shill::kTypeCellular) {
+  } else if (onc_type == ::onc::network_type::kCellular) {
     ChooseMobileNetworkDialog::ShowDialog(GetNativeWindow());
   } else {
     LOG(ERROR) << "Unsupported type for AddConnection";
   }
 }
 
-void InternetOptionsHandler::SendShowDetailedInfo(
-    const std::string& service_path) {
-  details_path_ = service_path;
+void InternetOptionsHandler::ConfigureNetwork(const base::ListValue* args) {
+  std::string service_path;
+  if (args->GetSize() != 1 || !args->GetString(0, &service_path)) {
+    NOTREACHED();
+    return;
+  }
+  NetworkConfigView::Show(service_path, GetNativeWindow());
+}
 
-  scoped_ptr<base::DictionaryValue> dictionary(new base::DictionaryValue);
-  const NetworkState* network = GetNetworkState(service_path);
-  if (network) {
-    dictionary->SetString(kNetworkInfoKeyServicePath, service_path);
-    dictionary->SetString(kNetworkInfoKeyGUID, network->guid());
-    if (network->type() == shill::kTypeCellular) {
-      dictionary->SetBoolean(
-          kTagCarrierSelectFlag,
-          CommandLine::ForCurrentProcess()
-          ->HasSwitch(chromeos::switches::kEnableCarrierSwitching));
-      dictionary->SetBoolean(kTagShowViewAccountButton,
-                             ShowViewAccountButton(network));
-    }
+void InternetOptionsHandler::ActivateNetwork(const base::ListValue* args) {
+  std::string service_path;
+  if (args->GetSize() != 1 || !args->GetString(0, &service_path)) {
+    NOTREACHED();
+    return;
   }
-  web_ui()->CallJavascriptFunction(kShowDetailedInfoFunction, *dictionary);
+  ash::network_connect::ActivateCellular(service_path);
+}
+
+void InternetOptionsHandler::RemoveNetwork(const base::ListValue* args) {
+  std::string service_path;
+  if (args->GetSize() != 1 || !args->GetString(0, &service_path)) {
+    NOTREACHED();
+    return;
+  }
+  NetworkHandler::Get()
+      ->managed_network_configuration_handler()
+      ->RemoveConfiguration(service_path, base::Bind(&base::DoNothing),
+                            base::Bind(&ShillError, "RemoveNetwork"));
 }
 
 base::ListValue* InternetOptionsHandler::GetWiredList() {