NetworkPortalDetector/NetworkStateInformer: Switch over to use NetworkStateHandler

chrome/browser/chromeos/net/network_portal_detector_impl.cc

Index: chrome/browser/chromeos/net/network_portal_detector_impl.cc
diff --git a/chrome/browser/chromeos/net/network_portal_detector_impl.cc b/chrome/browser/chromeos/net/network_portal_detector_impl.cc
index 77e3edc5c9e25e60a32072dfe2a1b91092530cec..f71c12ece1f1dbe4ac0ca59e9c3dd6b0ac37d96a 100644
--- a/chrome/browser/chromeos/net/network_portal_detector_impl.cc
+++ b/chrome/browser/chromeos/net/network_portal_detector_impl.cc
@@ -12,9 +12,12 @@
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "content/public/browser/notification_service.h"
 #include "grit/generated_resources.h"
 #include "net/http/http_status_code.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using captive_portal::CaptivePortalDetector;
@@ -23,7 +26,7 @@ namespace chromeos {
 
 namespace {
 
-// Maximum number of portal detections for the same active network
+// Maximum number of portal detections for the same default network
 // after network change.
 const int kMaxRequestAttempts = 3;
 
@@ -65,31 +68,11 @@ std::string CaptivePortalStatusString(
       IDS_CHROMEOS_CAPTIVE_PORTAL_STATUS_UNRECOGNIZED);
 }
 
-NetworkLibrary* GetNetworkLibrary() {
-  CHECK(CrosLibrary::Get());
-  return CrosLibrary::Get()->GetNetworkLibrary();
-}
-
-const Network* GetActiveNetwork() {
-  NetworkLibrary* cros = GetNetworkLibrary();
-  if (!cros)
-    return NULL;
-  return cros->active_network();
-}
-
-const Network* FindNetworkByPath(const std::string& service_path) {
-  NetworkLibrary* cros = GetNetworkLibrary();
-  if (!cros)
-    return NULL;
-  return cros->FindNetworkByPath(service_path);
-}
-
 }  // namespace
 
 NetworkPortalDetectorImpl::NetworkPortalDetectorImpl(
     const scoped_refptr<net::URLRequestContextGetter>& request_context)
-    : active_connection_state_(STATE_UNKNOWN),
-      test_url_(CaptivePortalDetector::kDefaultURL),
+    : test_url_(CaptivePortalDetector::kDefaultURL),
       enabled_(false),
       weak_ptr_factory_(this),
       attempt_count_(0),
@@ -118,10 +101,8 @@ void NetworkPortalDetectorImpl::Init() {
   DCHECK(CalledOnValidThread());
 
   state_ = STATE_IDLE;
-  chromeos::NetworkLibrary* network_library = GetNetworkLibrary();
-  DCHECK(network_library);
-  network_library->AddNetworkManagerObserver(this);
-  network_library->RemoveObserverForAllNetworks(this);
+  DCHECK(NetworkStateHandler::Get());

[stevenjb, 2013/05/14 15:46:16]

nit: This is redundant. The line below will CHECK that NetworkStateHandler has
been Initialized.

[gauravsh, 2013/05/14 21:51:15]

Done.

+  NetworkStateHandler::Get()->AddObserver(this);
 }
 
 void NetworkPortalDetectorImpl::Shutdown() {
@@ -133,9 +114,8 @@ void NetworkPortalDetectorImpl::Shutdown() {
   captive_portal_detector_->Cancel();
   captive_portal_detector_.reset();
   observers_.Clear();
-  chromeos::NetworkLibrary* network_library = GetNetworkLibrary();
-  if (network_library)
-    network_library->RemoveNetworkManagerObserver(this);
+  if (NetworkStateHandler::Get())

[stevenjb, 2013/05/14 15:46:16]

Use NetworkStateHandler::IsInitialized(). Get() will CHECK if it has been
Shutdown.

[gauravsh, 2013/05/14 21:51:15]

Done.

+    NetworkStateHandler::Get()->RemoveObserver(this);
 }
 
 void NetworkPortalDetectorImpl::AddObserver(Observer* observer) {
@@ -150,7 +130,7 @@ void NetworkPortalDetectorImpl::AddAndFireObserver(Observer* observer) {
   if (!observer)
     return;
   AddObserver(observer);
-  const Network* network = GetActiveNetwork();
+  const NetworkState* network = NetworkStateHandler::Get()->DefaultNetwork();
   observer->OnPortalDetectionCompleted(network, GetCaptivePortalState(network));
 }
 
@@ -176,20 +156,21 @@ void NetworkPortalDetectorImpl::Enable(bool start_detection) {
     return;
   state_ = STATE_IDLE;
   attempt_count_ = 0;
-  const Network* active_network = GetActiveNetwork();
-  if (!active_network)
+  const NetworkState* default_network =
+      NetworkStateHandler::Get()->DefaultNetwork();
+  if (!default_network)
     return;
-  portal_state_map_.erase(active_network->service_path());
+  portal_state_map_.erase(default_network->path());
   DetectCaptivePortal(base::TimeDelta());
 }
 
 NetworkPortalDetectorImpl::CaptivePortalState
-NetworkPortalDetectorImpl::GetCaptivePortalState(const Network* network) {
+NetworkPortalDetectorImpl::GetCaptivePortalState(const NetworkState* network) {
   DCHECK(CalledOnValidThread());
   if (!network)
     return CaptivePortalState();
   CaptivePortalStateMap::const_iterator it =
-      portal_state_map_.find(network->service_path());
+      portal_state_map_.find(network->path());
   if (it == portal_state_map_.end())
     return CaptivePortalState();
   return it->second;
@@ -211,27 +192,22 @@ void NetworkPortalDetectorImpl::DisableLazyDetection() {
   lazy_detection_enabled_ = false;
 }
 
-void NetworkPortalDetectorImpl::OnNetworkManagerChanged(NetworkLibrary* cros) {
+void NetworkPortalDetectorImpl::NetworkManagerChanged() {
   DCHECK(CalledOnValidThread());
-  CHECK(cros);
-  const Network* active_network = cros->active_network();
-  if (!active_network)
+  const NetworkState* default_network =
+      NetworkStateHandler::Get()->DefaultNetwork();
+  if (!default_network)
     return;
 
-  active_network_id_ = active_network->unique_id();
+  default_network_id_ = default_network->guid();
 
-  bool network_changed =
-      (active_service_path_ != active_network->service_path());
-  if (network_changed) {
-    if (!active_service_path_.empty())
-      cros->RemoveNetworkObserver(active_service_path_, this);
-    active_service_path_ = active_network->service_path();
-    cros->AddNetworkObserver(active_service_path_, this);
-  }
+  bool network_changed = (default_service_path_ != default_network->path());
+  if (network_changed)
+    default_service_path_ = default_network->path();

[stevenjb, 2013/05/14 15:46:16]

nit: Skip if() here (or add if() above line 210 for consistency)

[gauravsh, 2013/05/14 21:51:15]

Done.

 
-  bool connection_state_changed =
-      (active_connection_state_ != active_network->connection_state());
-  active_connection_state_ = active_network->connection_state();
+  bool connection_state_changed = (default_connection_state_ !=
+                                   default_network->connection_state());
+  default_connection_state_ = default_network->connection_state();
 
   if (network_changed || connection_state_changed) {
     attempt_count_ = 0;
@@ -239,14 +215,14 @@ void NetworkPortalDetectorImpl::OnNetworkManagerChanged(NetworkLibrary* cros) {
   }
 
   if (!IsCheckingForPortal() && !IsPortalCheckPending() &&
-      Network::IsConnectedState(active_connection_state_) &&
+      NetworkState::StateIsConnected(default_connection_state_) &&
       (attempt_count_ < kMaxRequestAttempts || lazy_detection_enabled_)) {
-    DCHECK(active_network);
+    DCHECK(default_network);

[stevenjb, 2013/05/14 15:46:16]

This DCHECK is redundant

[gauravsh, 2013/05/14 21:51:15]

Done.

 
     // Initiate Captive Portal detection if network's captive
     // portal state is unknown (e.g. for freshly created networks),
     // offline or if network connection state was changed.
-    CaptivePortalState state = GetCaptivePortalState(active_network);
+    CaptivePortalState state = GetCaptivePortalState(default_network);
     if (state.status == CAPTIVE_PORTAL_STATUS_UNKNOWN ||
         state.status == CAPTIVE_PORTAL_STATUS_OFFLINE ||
         (!network_changed && connection_state_changed)) {
@@ -255,11 +231,9 @@ void NetworkPortalDetectorImpl::OnNetworkManagerChanged(NetworkLibrary* cros) {
   }
 }
 
-void NetworkPortalDetectorImpl::OnNetworkChanged(
-    chromeos::NetworkLibrary* cros,
-    const chromeos::Network* network) {
-  DCHECK(CalledOnValidThread());
-  OnNetworkManagerChanged(cros);
+void NetworkPortalDetectorImpl::DefaultNetworkChanged(
+    const NetworkState* network) {
+  NetworkManagerChanged();
 }
 
 void NetworkPortalDetectorImpl::DetectCaptivePortal(
@@ -311,7 +285,7 @@ void NetworkPortalDetectorImpl::DetectCaptivePortalTask() {
   if (attempt_count_ < kMaxRequestAttempts) {
     ++attempt_count_;
     VLOG(1) << "Portal detection started: "
-            << "network=" << active_network_id_ << ", "
+            << "network=" << default_network_id_ << ", "
             << "attempt=" << attempt_count_ << " of " << kMaxRequestAttempts;
   } else {
     DCHECK(lazy_detection_enabled_);
@@ -334,7 +308,7 @@ void NetworkPortalDetectorImpl::PortalDetectionTimeout() {
   DCHECK(CalledOnValidThread());
   DCHECK(IsCheckingForPortal());
 
-  VLOG(1) << "Portal detection timeout: network=" << active_network_id_;
+  VLOG(1) << "Portal detection timeout: network=" << default_network_id_;
 
   captive_portal_detector_->Cancel();
   CaptivePortalDetector::Results results;
@@ -357,7 +331,7 @@ void NetworkPortalDetectorImpl::OnPortalDetectionCompleted(
   DCHECK(IsCheckingForPortal());
 
   VLOG(1) << "Portal detection completed: "
-          << "network=" << active_network_id_ << ", "
+          << "network=" << default_network_id_ << ", "
           << "result=" << CaptivePortalDetector::CaptivePortalResultToString(
               results.result) << ", "
           << "response_code=" << results.response_code;
@@ -365,9 +339,9 @@ void NetworkPortalDetectorImpl::OnPortalDetectionCompleted(
   state_ = STATE_IDLE;
   detection_timeout_.Cancel();
 
-  NetworkLibrary* cros = GetNetworkLibrary();
-  const Network* active_network = cros->active_network();
-  if (!active_network) {
+  const NetworkState* default_network =
+      NetworkStateHandler::Get()->DefaultNetwork();
+  if (!default_network) {
     TryLazyDetection();
     return;
   }
@@ -379,10 +353,11 @@ void NetworkPortalDetectorImpl::OnPortalDetectionCompleted(
       if (attempt_count_ >= kMaxRequestAttempts) {
         if (state.response_code == net::HTTP_PROXY_AUTHENTICATION_REQUIRED) {
           state.status = CAPTIVE_PORTAL_STATUS_PROXY_AUTH_REQUIRED;
-        } else if (active_network->restricted_pool()) {
+        } else if (default_network->connection_state() ==
+                   flimflam::kStatePortal) {
           // Take into account shill's detection results.
           state.status = CAPTIVE_PORTAL_STATUS_PORTAL;
-          LOG(WARNING) << "Network " << active_network->unique_id() << " "
+          LOG(WARNING) << "Network " << default_network->guid() << " "
                        << "is marked as "
                        << CaptivePortalStatusString(state.status) << " "
                        << "despite the fact that CaptivePortalDetector "
@@ -390,18 +365,18 @@ void NetworkPortalDetectorImpl::OnPortalDetectionCompleted(
         } else {
           state.status = CAPTIVE_PORTAL_STATUS_OFFLINE;
         }
-        SetCaptivePortalState(active_network, state);
+        SetCaptivePortalState(default_network, state);
       } else {
         DetectCaptivePortal(results.retry_after_delta);
       }
       break;
     case captive_portal::RESULT_INTERNET_CONNECTED:
       state.status = CAPTIVE_PORTAL_STATUS_ONLINE;
-      SetCaptivePortalState(active_network, state);
+      SetCaptivePortalState(default_network, state);
       break;
     case captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL:
       state.status = CAPTIVE_PORTAL_STATUS_PORTAL;
-      SetCaptivePortalState(active_network, state);
+      SetCaptivePortalState(default_network, state);
       break;
     default:
       break;
@@ -442,7 +417,7 @@ bool NetworkPortalDetectorImpl::IsCheckingForPortal() const {
 }
 
 void NetworkPortalDetectorImpl::SetCaptivePortalState(
-    const Network* network,
+    const NetworkState* network,
     const CaptivePortalState& state) {
   DCHECK(network);
 
@@ -452,21 +427,21 @@ void NetworkPortalDetectorImpl::SetCaptivePortalState(
   }
 
   CaptivePortalStateMap::const_iterator it =
-      portal_state_map_.find(network->service_path());
+      portal_state_map_.find(network->path());
   if (it == portal_state_map_.end() ||
       it->second.status != state.status ||
       it->second.response_code != state.response_code) {
     VLOG(1) << "Updating Chrome Captive Portal state: "
-            << "network=" << network->unique_id() << ", "
+            << "network=" << network->guid() << ", "
             << "status=" << CaptivePortalStatusString(state.status) << ", "
             << "response_code=" << state.response_code;
-    portal_state_map_[network->service_path()] = state;
+    portal_state_map_[network->path()] = state;
   }
   NotifyPortalDetectionCompleted(network, state);
 }
 
 void NetworkPortalDetectorImpl::NotifyPortalDetectionCompleted(
-    const Network* network,
+    const NetworkState* network,
     const CaptivePortalState& state) {
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnPortalDetectionCompleted(network, state));