NetworkPortalDetector/NetworkStateInformer: Switch over to use NetworkStateHandler

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

Index: chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
index 633c458bff04150cde797a5409ac745ba0900129..f6fbc4650f138e1bd6dcca559ed6ab81b3f8d373 100644
--- a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
@@ -18,8 +18,6 @@
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
-#include "chrome/browser/chromeos/cros/cros_library.h"
-#include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/input_method/input_method_configuration.h"
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h"
 #include "chrome/browser/chromeos/login/hwid_checker.h"
@@ -47,6 +45,8 @@
 #include "chromeos/dbus/power_manager_client.h"
 #include "chromeos/ime/input_method_manager.h"
 #include "chromeos/ime/xkeyboard.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "google_apis/gaia/gaia_auth_util.h"
@@ -54,6 +54,7 @@
 #include "google_apis/gaia/gaia_urls.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(USE_AURA)
@@ -174,21 +175,10 @@ bool IsSigninScreenError(ErrorScreen::ErrorState error_state) {
       error_state == ErrorScreen::ERROR_STATE_AUTH_EXT_TIMEOUT;
 }
 
-// Returns a pointer to a Network instance by service path or NULL if
-// network can not be found.
-Network* FindNetworkByPath(const std::string& service_path) {
-  CrosLibrary* cros = CrosLibrary::Get();
-  if (!cros)
-    return NULL;
-  NetworkLibrary* network_library = cros->GetNetworkLibrary();
-  if (!network_library)
-    return NULL;
-  return network_library->FindNetworkByPath(service_path);
-}
-
 // Returns network name by service path.
 std::string GetNetworkName(const std::string& service_path) {
-  Network* network = FindNetworkByPath(service_path);
+  const NetworkState* network =
+      NetworkStateHandler::Get()->GetNetworkState(service_path);
   if (!network)
     return std::string();
   return network->name();
@@ -196,31 +186,33 @@ std::string GetNetworkName(const std::string& service_path) {
 
 // Returns network unique id by service path.
 std::string GetNetworkUniqueId(const std::string& service_path) {
-  Network* network = FindNetworkByPath(service_path);
+  const NetworkState* network =
+      NetworkStateHandler::Get()->GetNetworkState(service_path);
   if (!network)
     return std::string();
-  return network->unique_id();
+  return network->guid();
 }
 
 // Returns captive portal state for a network by its service path.
 NetworkPortalDetector::CaptivePortalState GetCaptivePortalState(
     const std::string& service_path) {
   NetworkPortalDetector* detector = NetworkPortalDetector::GetInstance();
-  Network* network = FindNetworkByPath(service_path);
+  const NetworkState* network =
+      NetworkStateHandler::Get()->GetNetworkState(service_path);
   if (!detector || !network)
     return NetworkPortalDetector::CaptivePortalState();
   return detector->GetCaptivePortalState(network);
 }
 
 void RecordDiscrepancyWithShill(
-    const Network* network,
+    const NetworkState* network,
     const NetworkPortalDetector::CaptivePortalStatus status) {
-  if (network->online()) {
+  if (network->connection_state() == flimflam::kStateOnline) {
     UMA_HISTOGRAM_ENUMERATION(
         "CaptivePortal.OOBE.DiscrepancyWithShill_Online",
         status,
         NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_COUNT);
-  } else if (network->restricted_pool()) {
+  } else if (network->connection_state() == flimflam::kStatePortal) {
     UMA_HISTOGRAM_ENUMERATION(
         "CaptivePortal.OOBE.DiscrepancyWithShill_RestrictedPool",
         status,
@@ -237,7 +229,8 @@ void RecordDiscrepancyWithShill(
 // network is online but NetworkPortalDetector claims that it's behind
 // portal) for the network identified by |service_path|.
 void RecordNetworkPortalDetectorStats(const std::string& service_path) {
-  const Network* network = FindNetworkByPath(service_path);
+  const NetworkState* network =
+      NetworkStateHandler::Get()->GetNetworkState(service_path);
   if (!network)
     return;
   NetworkPortalDetector::CaptivePortalState state =
@@ -254,19 +247,20 @@ void RecordNetworkPortalDetectorStats(const std::string& service_path) {
       NOTREACHED();
       break;
     case NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_OFFLINE:
-      if (network->online() || network->restricted_pool())
+      if (network->connection_state() == flimflam::kStateOnline ||
+          network->connection_state() == flimflam::kStatePortal)
         RecordDiscrepancyWithShill(network, state.status);
       break;
     case NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE:
-      if (!network->online())
+      if (network->connection_state() != flimflam::kStateOnline)
         RecordDiscrepancyWithShill(network, state.status);
       break;
     case NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL:
-      if (!network->restricted_pool())
+      if (network->connection_state() != flimflam::kStatePortal)
         RecordDiscrepancyWithShill(network, state.status);
       break;
     case NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PROXY_AUTH_REQUIRED:
-      if (!network->online())
+      if (network->connection_state() != flimflam::kStateOnline)
         RecordDiscrepancyWithShill(network, state.status);
       break;
     case NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_COUNT:
@@ -448,7 +442,7 @@ void SigninScreenHandler::OnNetworkReady() {
 
 void SigninScreenHandler::UpdateState(NetworkStateInformer::State state,
                                       const std::string& service_path,
-                                      ConnectionType connection_type,
+                                      const std::string& connection_type,
                                       const std::string& reason) {
   UpdateStateInternal(state, service_path, connection_type, reason, false);
 }
@@ -479,9 +473,9 @@ void SigninScreenHandler::UpdateUIState(UIState ui_state,
 // TODO (ygorshenin@): split this method into small parts.
 void SigninScreenHandler::UpdateStateInternal(
     NetworkStateInformer::State state,
-    const std::string service_path,
-    ConnectionType connection_type,
-    std::string reason,
+    const std::string& service_path,
+    const std::string& connection_type,
+    const std::string& reason,
     bool force_update) {
   // Skip "update" notification about OFFLINE state from
   // NetworkStateInformer if previous notification already was
@@ -600,7 +594,7 @@ void SigninScreenHandler::UpdateStateInternal(
 void SigninScreenHandler::SetupAndShowOfflineMessage(
     NetworkStateInformer:: State state,
     const std::string& service_path,
-    ConnectionType connection_type,
+    const std::string& connection_type,
     const std::string& reason,
     bool is_proxy_error,
     bool is_under_captive_portal,
@@ -652,7 +646,7 @@ void SigninScreenHandler::SetupAndShowOfflineMessage(
 
   if (GetCurrentScreen() != OobeUI::SCREEN_ERROR_MESSAGE) {
     DictionaryValue params;
-    params.SetInteger("lastNetworkType", static_cast<int>(connection_type));
+    params.SetString("lastNetworkType", connection_type);

[ygorshenin1, 2013/05/06 09:14:39]

As you're passing connection_type as string instead of integer, please, fix 

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

and

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[gauravsh, 2013/05/07 23:38:10]

Actually, NetworkDropdown doesn't implement SetLastNetworkType anymore:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

So I went ahead and got rid of lastNetworkType as without SetLastNetworkType
implementation, there's no use for it in the JS. Let me know if I may have
missed something.

[ygorshenin1, 2013/05/08 08:03:24]

Yes, lastNetworkType is not used, but we have incorrect comment for the JS code:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

and problems during casting from StringValue to double:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

Could you please fix comments and data types?

On 2013/05/07 23:38:10, gauravsh wrote:

     error_screen_actor_->SetUIState(ErrorScreen::UI_STATE_SIGNIN);
     error_screen_actor_->Show(OobeUI::SCREEN_GAIA_SIGNIN, &params);
   }