Merge 216482 "Simplified NetworkStateInformer. Now it doesn't co..."

> Simplified NetworkStateInformer. Now it doesn't contain any
> logic related to delay in notifications.
> 
> BUG=244541
> TEST=manual tests on Lumpy
> 
> Review URL: https://chromiumcodereview.appspot.com/22408002

TBR=ygorshenin@chromium.org

Review URL: https://codereview.chromium.org/22959003

git-svn-id: svn://svn.chromium.org/chrome/branches/1547/src@217280 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 52 insertions, 140 deletions

diff --git a/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
index f296b0c..728c443 100644
--- a/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc
@@ -52,6 +52,7 @@ void ErrorScreenHandler::Show(OobeDisplay::Screen parent_screen,
   ShowScreen(OobeUI::kScreenErrorMessage, params);
   NetworkErrorShown();
   EnableLazyDetection();
+  LOG(WARNING) << "Offline message is displayed";
 }
 
 void ErrorScreenHandler::Hide() {
@@ -61,6 +62,7 @@ void ErrorScreenHandler::Hide() {
   if (GetScreenName(parent_screen_, &screen_name))
     ShowScreen(screen_name.c_str(), NULL);
   DisableLazyDetection();
+  LOG(WARNING) << "Offline message is hidden";
 }
 
 void ErrorScreenHandler::FixCaptivePortal() {
diff --git a/chrome/browser/ui/webui/chromeos/login/network_state_informer.cc b/chrome/browser/ui/webui/chromeos/login/network_state_informer.cc
index 3f78058..62ce265 100644
--- a/chrome/browser/ui/webui/chromeos/login/network_state_informer.cc
+++ b/chrome/browser/ui/webui/chromeos/login/network_state_informer.cc
@@ -16,13 +16,6 @@
 #include "net/proxy/proxy_config.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
-namespace {
-
-// Timeout to smooth temporary network state transitions for flaky networks.
-const int kNetworkStateCheckDelaySec = 3;
-
-}  // namespace
-
 namespace chromeos {
 
 NetworkStateInformer::NetworkStateInformer()
@@ -70,50 +63,17 @@ void NetworkStateInformer::RemoveObserver(
 }
 
 void NetworkStateInformer::NetworkManagerChanged() {
-  const NetworkState* default_network =
-      NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
-  State new_state = OFFLINE;
-  std::string new_network_service_path;
-  if (default_network) {
-    new_state = GetNetworkState(default_network);
-    new_network_service_path = default_network->path();
-  }
-  if ((state_ != ONLINE && (new_state == ONLINE || new_state == CONNECTING)) ||
-      (state_ == ONLINE && (new_state == ONLINE || new_state == CONNECTING) &&
-       new_network_service_path != last_online_service_path_) ||
-      (new_state == CAPTIVE_PORTAL &&
-       new_network_service_path == last_network_service_path_)) {
-    last_network_service_path_ = new_network_service_path;
-    if (new_state == ONLINE)
-      last_online_service_path_ = new_network_service_path;
-    // Transitions {OFFLINE, PORTAL} -> ONLINE and connections to
-    // different network are processed without delay.
-    // Transitions {OFFLINE, ONLINE} -> PORTAL in the same network are
-    // also processed without delay.
-    UpdateStateAndNotify();
-  } else {
-    check_state_.Cancel();
-    check_state_.Reset(
-        base::Bind(&NetworkStateInformer::UpdateStateAndNotify,
-                   weak_ptr_factory_.GetWeakPtr()));
-    base::MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        check_state_.callback(),
-        base::TimeDelta::FromSeconds(kNetworkStateCheckDelaySec));
-  }
+  UpdateStateAndNotify();
 }
 
 void NetworkStateInformer::DefaultNetworkChanged(const NetworkState* network) {
-  NetworkManagerChanged();
+  UpdateStateAndNotify();
 }
 
 void NetworkStateInformer::OnPortalDetectionCompleted(
     const NetworkState* network,
     const NetworkPortalDetector::CaptivePortalState& state) {
-  if (NetworkHandler::IsInitialized() &&
-      NetworkHandler::Get()->network_state_handler()->DefaultNetwork() ==
-      network)
-    NetworkManagerChanged();
+  UpdateStateAndNotify();
 }
 
 void NetworkStateInformer::Observe(
@@ -129,26 +89,27 @@ void NetworkStateInformer::Observe(
 }
 
 void NetworkStateInformer::OnPortalDetected() {
-  SendStateToObservers(ErrorScreenActor::ERROR_REASON_PORTAL_DETECTED);
+  UpdateStateAndNotify();
 }
 
 bool NetworkStateInformer::UpdateState() {
-  State new_state = OFFLINE;
-
   const NetworkState* default_network =
       NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
+  State new_state = OFFLINE;
+  std::string new_network_path;
+  std::string new_network_type;
   if (default_network) {
     new_state = GetNetworkState(default_network);
-    last_network_service_path_ = default_network->path();
-    last_network_type_ = default_network->type();
+    new_network_path = default_network->path();
+    new_network_type = default_network->type();
   }
 
   bool updated = (new_state != state_) ||
-      (new_state != OFFLINE &&
-       last_network_service_path_ != last_connected_service_path_);
+      (new_network_path != network_path_) ||
+      (new_network_type != network_type_);
   state_ = new_state;
-  if (state_ != OFFLINE)
-    last_connected_service_path_ = last_network_service_path_;
+  network_path_ = new_network_path;
+  network_type_ = new_network_type;
 
   if (updated && state_ == ONLINE) {
     FOR_EACH_OBSERVER(NetworkStateInformerObserver, observers_,
@@ -159,9 +120,6 @@ bool NetworkStateInformer::UpdateState() {
 }
 
 void NetworkStateInformer::UpdateStateAndNotify() {
-  // Cancel pending update request if any.
-  check_state_.Cancel();
-
   if (UpdateState())
     SendStateToObservers(ErrorScreenActor::ERROR_REASON_NETWORK_STATE_CHANGED);
   else
@@ -171,7 +129,7 @@ void NetworkStateInformer::UpdateStateAndNotify() {
 void NetworkStateInformer::SendStateToObservers(
     ErrorScreenActor::ErrorReason reason) {
   FOR_EACH_OBSERVER(NetworkStateInformerObserver, observers_,
-      UpdateState(state_, reason));
+      UpdateState(reason));
 }
 
 NetworkStateInformer::State NetworkStateInformer::GetNetworkState(
diff --git a/chrome/browser/ui/webui/chromeos/login/network_state_informer.h b/chrome/browser/ui/webui/chromeos/login/network_state_informer.h
index 85710d8..69b2766 100644
--- a/chrome/browser/ui/webui/chromeos/login/network_state_informer.h
+++ b/chrome/browser/ui/webui/chromeos/login/network_state_informer.h
@@ -49,8 +49,7 @@ class NetworkStateInformer
     NetworkStateInformerObserver() {}
     virtual ~NetworkStateInformerObserver() {}
 
-    virtual void UpdateState(State state,
-                             ErrorScreenActor::ErrorReason reason) = 0;
+    virtual void UpdateState(ErrorScreenActor::ErrorReason reason) = 0;
     virtual void OnNetworkReady() {}
   };
 
@@ -81,18 +80,9 @@ class NetworkStateInformer
   // CaptivePortalWindowProxyDelegate implementation:
   virtual void OnPortalDetected() OVERRIDE;
 
-  // Returns active network's service path. It can be used to uniquely
-  // identify the network.
-  std::string active_network_service_path() {
-    return last_online_service_path_;
-  }
-
-  bool is_online() { return state_ == ONLINE; }
   State state() const { return state_; }
-  std::string last_network_service_path() const {
-    return last_network_service_path_;
-  }
-  std::string last_network_type() const { return last_network_type_; }
+  std::string network_path() const { return network_path_; }
+  std::string network_type() const { return network_type_; }
 
  private:
   friend class base::RefCounted<NetworkStateInformer>;
@@ -108,14 +98,12 @@ class NetworkStateInformer
   State GetNetworkState(const NetworkState* network);
   bool IsProxyConfigured(const NetworkState* network);
 
-  content::NotificationRegistrar registrar_;
   State state_;
+  std::string network_path_;
+  std::string network_type_;
+
   ObserverList<NetworkStateInformerObserver> observers_;
-  std::string last_online_service_path_;
-  std::string last_connected_service_path_;
-  std::string last_network_service_path_;
-  std::string last_network_type_;
-  base::CancelableClosure check_state_;
+  content::NotificationRegistrar registrar_;
 
   base::WeakPtrFactory<NetworkStateInformer> weak_ptr_factory_;
 };
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 7417efc..3375c1d 100644
--- a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
@@ -227,15 +227,6 @@ std::string GetNetworkName(const std::string& service_path) {
   return network->name();
 }
 
-// Returns network unique id by service path.
-std::string GetNetworkUniqueId(const std::string& service_path) {
-  const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
-      GetNetworkState(service_path);
-  if (!network)
-    return std::string();
-  return network->guid();
-}
-
 // Returns captive portal state for a network by its service path.
 NetworkPortalDetector::CaptivePortalState GetCaptivePortalState(
     const std::string& service_path) {
@@ -493,9 +484,8 @@ void SigninScreenHandler::OnNetworkReady() {
   MaybePreloadAuthExtension();
 }
 
-void SigninScreenHandler::UpdateState(NetworkStateInformer::State state,
-                                      ErrorScreenActor::ErrorReason reason) {
-  UpdateStateInternal(state, reason, false);
+void SigninScreenHandler::UpdateState(ErrorScreenActor::ErrorReason reason) {
+  UpdateStateInternal(reason, false);
 }
 
 // SigninScreenHandler, private: -----------------------------------------------
@@ -519,7 +509,6 @@ void SigninScreenHandler::UpdateUIState(UIState ui_state,
 
 // TODO (ygorshenin@): split this method into small parts.
 void SigninScreenHandler::UpdateStateInternal(
-    NetworkStateInformer::State state,
     ErrorScreenActor::ErrorReason reason,
     bool force_update) {
   // Do nothing once user has signed in or sign in is in progress.
@@ -531,16 +520,15 @@ void SigninScreenHandler::UpdateStateInternal(
     return;
   }
 
-  const std::string service_path =
-      network_state_informer_->last_network_service_path();
-  const std::string network_id =
-      GetNetworkUniqueId(service_path);
+  NetworkStateInformer::State state = network_state_informer_->state();
+  const std::string network_path = network_state_informer_->network_path();
+  const std::string network_name = GetNetworkName(network_path);
 
   // Skip "update" notification about OFFLINE state from
   // NetworkStateInformer if previous notification already was
   // delayed.
-  if (state == NetworkStateInformer::OFFLINE &&
-      reason == ErrorScreenActor::ERROR_REASON_UPDATE &&
+  if ((state == NetworkStateInformer::OFFLINE || has_pending_auth_ui_) &&
+      !force_update &&
       !update_state_closure_.IsCancelled()) {
     return;
   }
@@ -549,31 +537,23 @@ void SigninScreenHandler::UpdateStateInternal(
   // will be tested well.
   LOG(WARNING) << "SigninScreenHandler::UpdateStateInternal(): "
                << "state=" << NetworkStateStatusString(state) << ", "
-               << "network_id=" << network_id << ", "
+               << "network_name=" << network_name << ", "
                << "reason=" << ErrorReasonString(reason) << ", "
                << "force_update=" << force_update;
   update_state_closure_.Cancel();
 
-  // Delay UpdateStateInternal() call in the following cases:
-  // 1. this is the first call and it's about offline state of the
-  //    current network. This can happen because device is just powered
-  //    up and network stack isn't ready now.
-  // 2. proxy auth ui is displayed. UpdateStateCall() is delayed to
-  //    prevent screen change behind proxy auth ui.
-  if ((state == NetworkStateInformer::OFFLINE && is_first_update_state_call_) ||
+  if ((state == NetworkStateInformer::OFFLINE && !force_update) ||
       has_pending_auth_ui_) {
-    is_first_update_state_call_ = false;
     update_state_closure_.Reset(
         base::Bind(
             &SigninScreenHandler::UpdateStateInternal,
-            weak_factory_.GetWeakPtr(), state, reason, force_update));
+            weak_factory_.GetWeakPtr(), reason, true));
     base::MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
         update_state_closure_.callback(),
         base::TimeDelta::FromSeconds(kOfflineTimeoutSec));
     return;
   }
-  is_first_update_state_call_ = false;
 
   // Don't show or hide error screen if we're in connecting state.
   if (state == NetworkStateInformer::CONNECTING && !force_update) {
@@ -581,7 +561,7 @@ void SigninScreenHandler::UpdateStateInternal(
       // First notification about CONNECTING state.
       connecting_closure_.Reset(
           base::Bind(&SigninScreenHandler::UpdateStateInternal,
-                     weak_factory_.GetWeakPtr(), state, reason, true));
+                     weak_factory_.GetWeakPtr(), reason, true));
       base::MessageLoop::current()->PostDelayedTask(
           FROM_HERE,
           connecting_closure_.callback(),
@@ -648,19 +628,15 @@ void SigninScreenHandler::UpdateStateInternal(
 void SigninScreenHandler::SetupAndShowOfflineMessage(
     NetworkStateInformer:: State state,
     ErrorScreenActor::ErrorReason reason) {
-  const std::string service_path =
-      network_state_informer_->last_network_service_path();
-  const std::string network_id = GetNetworkUniqueId(service_path);
+  const std::string network_path = network_state_informer_->network_path();
   const bool is_under_captive_portal = IsUnderCaptivePortal(state, reason);
   const bool is_proxy_error = IsProxyError(state, reason, frame_error_);
   const bool is_gaia_loading_timeout =
       (reason == ErrorScreenActor::ERROR_REASON_LOADING_TIMEOUT);
 
-  LOG(WARNING) << "Offline message is displayed";
-
   // Record portal detection stats only if we're going to show or
   // change state of the error screen.
-  RecordNetworkPortalDetectorStats(service_path);
+  RecordNetworkPortalDetectorStats(network_path);
 
   if (is_proxy_error) {
     error_screen_actor_->SetErrorState(ErrorScreen::ERROR_STATE_PROXY,
@@ -675,7 +651,7 @@ void SigninScreenHandler::SetupAndShowOfflineMessage(
          ErrorScreen::ERROR_STATE_PORTAL)) {
       error_screen_actor_->FixCaptivePortal();
     }
-    const std::string network_name = GetNetworkName(service_path);
+    const std::string network_name = GetNetworkName(network_path);
     error_screen_actor_->SetErrorState(ErrorScreen::ERROR_STATE_PORTAL,
                                        network_name);
   } else if (is_gaia_loading_timeout) {
@@ -698,9 +674,8 @@ void SigninScreenHandler::SetupAndShowOfflineMessage(
 
   if (GetCurrentScreen() != OobeUI::SCREEN_ERROR_MESSAGE) {
     DictionaryValue params;
-    const std::string connection_type =
-        network_state_informer_->last_network_type();
-    params.SetString("lastNetworkType", connection_type);
+    const std::string network_type = network_state_informer_->network_type();
+    params.SetString("lastNetworkType", network_type);
     error_screen_actor_->SetUIState(ErrorScreen::UI_STATE_SIGNIN);
     error_screen_actor_->Show(OobeUI::SCREEN_GAIA_SIGNIN, &params);
   }
@@ -712,7 +687,6 @@ void SigninScreenHandler::HideOfflineMessage(
   if (!IsSigninScreenHiddenByError())
     return;
 
-  LOG(WARNING) << "Offline message is hidden";
   error_screen_actor_->Hide();
 
   // Forces a reload for Gaia screen on hiding error message.
@@ -965,11 +939,10 @@ void SigninScreenHandler::ShowSigninScreenIfReady() {
   if (!dns_cleared_ || !cookies_cleared_ || !delegate_)
     return;
 
-  std::string active_network =
-      network_state_informer_->active_network_service_path();
+  std::string active_network_path = network_state_informer_->network_path();
   if (gaia_silent_load_ &&
-      (!network_state_informer_->is_online() ||
-       gaia_silent_load_network_ != active_network)) {
+      (network_state_informer_->state() != NetworkStateInformer::ONLINE ||
+       gaia_silent_load_network_ != active_network_path)) {
     // Network has changed. Force Gaia reload.
     gaia_silent_load_ = false;
     // Gaia page will be realoded, so focus isn't stolen anymore.
@@ -993,8 +966,7 @@ void SigninScreenHandler::ShowSigninScreenIfReady() {
       HandleLoginWebuiReady();
   }
 
-  UpdateState(network_state_informer_->state(),
-              ErrorScreenActor::ERROR_REASON_UPDATE);
+  UpdateState(ErrorScreenActor::ERROR_REASON_UPDATE);
 }
 
 
@@ -1511,18 +1483,14 @@ void SigninScreenHandler::HandleFrameLoadingCompleted(int status) {
 
   if (network_state_informer_->state() != NetworkStateInformer::ONLINE)
     return;
-  if (frame_state_ == FRAME_STATE_LOADED) {
-    UpdateState(network_state_informer_->state(),
-                ErrorScreenActor::ERROR_REASON_UPDATE);
-  } else if (frame_state_ == FRAME_STATE_ERROR) {
-    UpdateState(network_state_informer_->state(),
-                ErrorScreenActor::ERROR_REASON_FRAME_ERROR);
-  }
+  if (frame_state_ == FRAME_STATE_LOADED)
+    UpdateState(ErrorScreenActor::ERROR_REASON_UPDATE);
+  else if (frame_state_ == FRAME_STATE_ERROR)
+    UpdateState(ErrorScreenActor::ERROR_REASON_FRAME_ERROR);
 }
 
 void SigninScreenHandler::HandleShowLoadingTimeoutError() {
-  UpdateState(network_state_informer_->state(),
-              ErrorScreenActor::ERROR_REASON_LOADING_TIMEOUT);
+  UpdateState(ErrorScreenActor::ERROR_REASON_LOADING_TIMEOUT);
 }
 
 void SigninScreenHandler::HandleUpdateOfflineLogin(bool offline_login_active) {
@@ -1565,10 +1533,9 @@ void SigninScreenHandler::MaybePreloadAuthExtension() {
       !ScreenLocker::default_screen_locker() &&
       !cookies_cleared_ &&
       !dns_clear_task_running_ &&
-      network_state_informer_->is_online()) {
+      network_state_informer_->state() == NetworkStateInformer::ONLINE) {
     gaia_silent_load_ = true;
-    gaia_silent_load_network_ =
-        network_state_informer_->active_network_service_path();
+    gaia_silent_load_network_ = network_state_informer_->network_path();
     LoadAuthExtension(true, true, false);
   }
 }
diff --git a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h
index 29e25cc..c7fad81 100644
--- a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h
+++ b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h
@@ -189,9 +189,7 @@ class SigninScreenHandler
 
   // NetworkStateInformer::NetworkStateInformerObserver implementation:
   virtual void OnNetworkReady() OVERRIDE;
-
-  virtual void UpdateState(NetworkStateInformer::State state,
-                           ErrorScreenActor::ErrorReason reason) OVERRIDE;
+  virtual void UpdateState(ErrorScreenActor::ErrorReason reason) OVERRIDE;
 
  private:
   enum UIState {
@@ -217,8 +215,7 @@ class SigninScreenHandler
   // |params| argument.
   void UpdateUIState(UIState ui_state, DictionaryValue* params);
 
-  void UpdateStateInternal(NetworkStateInformer::State state,
-                           ErrorScreenActor::ErrorReason reason,
+  void UpdateStateInternal(ErrorScreenActor::ErrorReason reason,
                            bool force_update);
   void SetupAndShowOfflineMessage(NetworkStateInformer::State state,
                                   ErrorScreenActor::ErrorReason reason);