Fix so that login errors are shown from login/oobe screens.

Split functionality of check for login failures to NetworkLoginObserver from NetworkMessageObserver.
BUG=chromium-os:8251
TEST=manually make sure the login error dialog pops up when you enter bad password.
Review URL: http://codereview.chromium.org/6027012

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

chrome/browser/chromeos/cros/network_library.cc

@@ -15,6 +15,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_thread.h"
+#include "chrome/browser/chromeos/network_login_observer.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/common/time_format.h"
@@ -846,6 +847,7 @@ class NetworkLibraryImpl : public NetworkLibrary  {
                                 this);
       data_plan_monitor_ = MonitorCellularDataPlan(&DataPlanUpdateHandler,
                                                    this);
+      network_login_observer_.reset(new NetworkLoginObserver(this));
     } else {
       InitTestData();
     }
@@ -1860,6 +1862,9 @@ class NetworkLibraryImpl : public NetworkLibrary  {
   // For monitoring data plan changes to the connected cellular network.
   DataPlanUpdateMonitor data_plan_monitor_;
 
+  // Network login observer.
+  scoped_ptr<NetworkLoginObserver> network_login_observer_;
+
   // The ethernet network.
   EthernetNetwork* ethernet_;
 

chrome/browser/chromeos/login/background_view.cc

@@ -15,6 +15,7 @@
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/login/helper.h"
+#include "chrome/browser/chromeos/login/login_utils.h"
 #include "chrome/browser/chromeos/login/oobe_progress_bar.h"
 #include "chrome/browser/chromeos/login/proxy_settings_dialog.h"
 #include "chrome/browser/chromeos/login/rounded_rect_painter.h"
@@ -27,6 +28,7 @@
 #include "chrome/browser/chromeos/wm_ipc.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/views/dom_view.h"
+#include "chrome/browser/ui/views/window.h"
 #include "gfx/gtk_util.h"
 #include "googleurl/src/gurl.h"
 #include "grit/chromium_strings.h"
@@ -37,6 +39,7 @@
 #include "views/controls/label.h"
 #include "views/screen.h"
 #include "views/widget/widget_gtk.h"
+#include "views/window/window.h"
 
 // X Windows headers have "#define Status int". That interferes with
 // NetworkLibrary header which defines enum "Status".
@@ -179,9 +182,18 @@ views::Widget* BackgroundView::CreateWindowContainingView(
   GdkWindow* gdk_window = window->GetNativeView()->window;
   gdk_window_set_back_pixmap(gdk_window, NULL, false);
 
+  LoginUtils::Get()->SetBackgroundView(*view);
+
   return window;
 }
 
+void BackgroundView::CreateModalPopup(views::WindowDelegate* view) {
+  views::Window* window = browser::CreateViewsWindow(
+      GetNativeWindow(), gfx::Rect(), view);
+  window->SetIsAlwaysOnTop(true);
+  window->Show();
+}
+
 void BackgroundView::SetStatusAreaVisible(bool visible) {
   status_area_->SetVisible(visible);
 }

chrome/browser/chromeos/login/background_view.h

@@ -17,6 +17,7 @@ namespace views {
 class Label;
 class TextButton;
 class Widget;
+class WindowDelegate;
 }
 
 class DOMView;
@@ -64,6 +65,9 @@ class BackgroundView : public views::View,
       const GURL& background_url,
       BackgroundView** view);
 
+  // Create a modal popup view.
+  void CreateModalPopup(views::WindowDelegate* view);
+
   // Toggles status area visibility.
   void SetStatusAreaVisible(bool visible);
 

chrome/browser/chromeos/login/login_utils.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/chromeos/cros/login_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/input_method/input_method_util.h"
+#include "chrome/browser/chromeos/login/background_view.h"
 #include "chrome/browser/chromeos/login/cookie_fetcher.h"
 #include "chrome/browser/chromeos/login/google_authenticator.h"
 #include "chrome/browser/chromeos/login/language_switch_menu.h"
@@ -99,7 +100,8 @@ class ResetDefaultProxyConfigServiceTask : public Task {
 class LoginUtilsImpl : public LoginUtils {
  public:
   LoginUtilsImpl()
-      : browser_launch_enabled_(true) {
+      : browser_launch_enabled_(true),
+        background_view_(NULL) {
   }
 
   // Invoked after the user has successfully logged in. This launches a browser
@@ -143,6 +145,12 @@ class LoginUtilsImpl : public LoginUtils {
       Profile* profile,
       const GaiaAuthConsumer::ClientLoginResult& credentials);
 
+  // Sets the current background view.
+  virtual void SetBackgroundView(chromeos::BackgroundView* background_view);
+
+  // Gets the current background view.
+  virtual chromeos::BackgroundView* GetBackgroundView();
+
  private:
   // Check user's profile for kApplicationLocale setting.
   void RespectLocalePreference(PrefService* pref);
@@ -150,6 +158,9 @@ class LoginUtilsImpl : public LoginUtils {
   // Indicates if DoBrowserLaunch will actually launch the browser or not.
   bool browser_launch_enabled_;
 
+  // The current background view.
+  chromeos::BackgroundView* background_view_;
+
   DISALLOW_COPY_AND_ASSIGN(LoginUtilsImpl);
 };
 
@@ -533,6 +544,14 @@ void LoginUtilsImpl::PrewarmAuthentication() {
   }
 }
 
+void LoginUtilsImpl::SetBackgroundView(BackgroundView* background_view) {
+  background_view_ = background_view;
+}
+
+BackgroundView* LoginUtilsImpl::GetBackgroundView() {
+  return background_view_;
+}
+
 LoginUtils* LoginUtils::Get() {
   return LoginUtilsWrapper::GetInstance()->get();
 }

chrome/browser/chromeos/login/login_utils.h

@@ -17,6 +17,7 @@ class PrefService;
 namespace chromeos {
 
 class Authenticator;
+class BackgroundView;
 class LoginStatusConsumer;
 
 class LoginUtils {
@@ -77,6 +78,11 @@ class LoginUtils {
       Profile* profile,
       const GaiaAuthConsumer::ClientLoginResult& credentials) = 0;
 
+  // Sets the current background view.
+  virtual void SetBackgroundView(BackgroundView* background_view) = 0;
+
+  // Gets the current background view.
+  virtual BackgroundView* GetBackgroundView() = 0;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/login/mock_authenticator.h

@@ -10,6 +10,7 @@
 
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/chromeos/login/authenticator.h"
+#include "chrome/browser/chromeos/login/background_view.h"
 #include "chrome/browser/chromeos/login/login_utils.h"
 #include "chrome/common/net/gaia/google_service_auth_error.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -149,10 +150,19 @@ class MockLoginUtils : public LoginUtils {
       const GaiaAuthConsumer::ClientLoginResult& credentials) {
   }
 
+  void SetBackgroundView(BackgroundView* background_view) {
+    background_view_ = background_view;
+  }
+
+  BackgroundView* GetBackgroundView() {
+    return background_view_;
+  }
+
  private:
   std::string expected_username_;
   std::string expected_password_;
   std::string auth_token_;
+  chromeos::BackgroundView* background_view_;
 
   DISALLOW_COPY_AND_ASSIGN(MockLoginUtils);
 };

chrome/browser/chromeos/network_login_observer.cc

+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/network_login_observer.h"
+
+#include "chrome/browser/browser_list.h"
+#include "chrome/browser/browser_window.h"
+#include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/browser/chromeos/cros/network_library.h"
+#include "chrome/browser/chromeos/login/background_view.h"
+#include "chrome/browser/chromeos/login/login_utils.h"
+#include "chrome/browser/chromeos/options/network_config_view.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/views/window.h"
+#include "views/window/dialog_delegate.h"
+#include "views/window/window.h"
+
+namespace chromeos {
+
+NetworkLoginObserver::NetworkLoginObserver(NetworkLibrary* netlib) {
+  RefreshStoredNetworks(netlib->wifi_networks());
+  netlib->AddNetworkManagerObserver(this);
+}
+
+NetworkLoginObserver::~NetworkLoginObserver() {
+  CrosLibrary::Get()->GetNetworkLibrary()->RemoveNetworkManagerObserver(this);
+}
+
+void NetworkLoginObserver::CreateModalPopup(views::WindowDelegate* view) {
+  Browser* browser = BrowserList::GetLastActive();
+  if (browser && browser->type() != Browser::TYPE_NORMAL) {
+    browser = BrowserList::FindBrowserWithType(browser->profile(),
+                                               Browser::TYPE_NORMAL,
+                                               true);
+  }
+  if (browser) {
+    views::Window* window = browser::CreateViewsWindow(
+        browser->window()->GetNativeHandle(), gfx::Rect(), view);
+    window->SetIsAlwaysOnTop(true);
+    window->Show();
+  } else {
+    // Browser not found, so we should be in login/oobe screen.
+    BackgroundView* background_view = LoginUtils::Get()->GetBackgroundView();
+    if (background_view) {
+      background_view->CreateModalPopup(view);
+    }
+  }
+}
+
+void NetworkLoginObserver::RefreshStoredNetworks(
+    const WifiNetworkVector& wifi_networks) {
+  wifi_network_failures_.clear();
+  for (WifiNetworkVector::const_iterator it = wifi_networks.begin();
+       it < wifi_networks.end(); it++) {
+    const WifiNetwork* wifi = *it;
+    wifi_network_failures_[wifi->service_path()] = wifi->failed();
+  }
+}
+
+void NetworkLoginObserver::OnNetworkManagerChanged(NetworkLibrary* obj) {
+  const WifiNetworkVector& wifi_networks = obj->wifi_networks();
+
+  NetworkConfigView* view = NULL;
+  // Check to see if we have any newly failed wifi network.
+  for (WifiNetworkVector::const_iterator it = wifi_networks.begin();
+       it < wifi_networks.end(); it++) {
+    const WifiNetwork* wifi = *it;
+    if (wifi->failed()) {
+      WifiFailureMap::iterator iter =
+          wifi_network_failures_.find(wifi->service_path());
+      // If the network did not previously exist, then don't do anything.
+      // For example, if the user travels to a location and finds a service
+      // that has previously failed, we don't want to show an error.
+      if (iter == wifi_network_failures_.end())
+        continue;
+
+      // If this network was in a failed state previously, then it's not new.
+      if (iter->second)
+        continue;
+
+      // Display login box again for bad_passphrase and bad_wepkey errors.
+      if (wifi->error() == ERROR_BAD_PASSPHRASE ||
+          wifi->error() == ERROR_BAD_WEPKEY) {
+        // The NetworkConfigView will show the appropriate error message.
+        view = new NetworkConfigView(wifi);
+        // There should only be one wifi network that failed to connect.
+        // If for some reason, we have more than one failure,
+        // we only display the first one. So we break here.
+        break;
+      }
+    }
+  }
+
+  RefreshStoredNetworks(wifi_networks);
+
+  // Show login box if necessary.
+  if (view)
+    CreateModalPopup(view);
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/network_login_observer.h

+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_NETWORK_LOGIN_OBSERVER_H_
+#define CHROME_BROWSER_CHROMEOS_NETWORK_LOGIN_OBSERVER_H_
+#pragma once
+
+#include <map>
+#include <string>
+
+#include "base/basictypes.h"
+#include "chrome/browser/chromeos/cros/network_library.h"
+
+namespace views {
+class WindowDelegate;
+}
+
+namespace chromeos {
+
+// The network login observer reshows a login dialog if there was an error.
+
+class NetworkLoginObserver : public NetworkLibrary::NetworkManagerObserver {
+ public:
+  explicit NetworkLoginObserver(NetworkLibrary* netlib);
+  virtual ~NetworkLoginObserver();
+
+  typedef std::map<std::string, bool> WifiFailureMap;
+ private:
+  virtual void CreateModalPopup(views::WindowDelegate* view);
+
+  virtual void RefreshStoredNetworks(const WifiNetworkVector& wifi_networks);
+
+  // NetworkLibrary::NetworkManagerObserver implementation.
+  virtual void OnNetworkManagerChanged(NetworkLibrary* obj);
+
+  // Wifi networks by service path mapped to if it failed previously.
+  WifiFailureMap wifi_network_failures_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkLoginObserver);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_NETWORK_LOGIN_OBSERVER_H_

chrome/browser/chromeos/network_message_observer.cc

@@ -10,21 +10,16 @@
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_list.h"
-#include "chrome/browser/browser_window.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/notifications/balloon_view_host.h"
-#include "chrome/browser/chromeos/options/network_config_view.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/views/window.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/time_format.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
-#include "views/window/dialog_delegate.h"
-#include "views/window/window.h"
 
 namespace {
 
@@ -44,8 +39,7 @@ bool ShouldShowMobilePlanNotifications() {
 namespace chromeos {
 
 NetworkMessageObserver::NetworkMessageObserver(Profile* profile)
-    : initialized_(false),
-      notification_connection_error_(profile, "network_connection.chromeos",
+    : notification_connection_error_(profile, "network_connection.chromeos",
           IDR_NOTIFICATION_NETWORK_FAILED,
           l10n_util::GetStringUTF16(IDS_NETWORK_CONNECTION_ERROR_TITLE)),
       notification_low_data_(profile, "network_low_data.chromeos",
@@ -58,7 +52,6 @@ NetworkMessageObserver::NetworkMessageObserver(Profile* profile)
   OnNetworkManagerChanged(netlib);
   // Note that this gets added as a NetworkManagerObserver and a
   // CellularDataPlanObserver in browser_init.cc
-  initialized_ = true;
 }
 
 NetworkMessageObserver::~NetworkMessageObserver() {
@@ -72,20 +65,6 @@ NetworkMessageObserver::~NetworkMessageObserver() {
   STLDeleteValues(&wifi_networks_);
 }
 
-void NetworkMessageObserver::CreateModalPopup(views::WindowDelegate* view) {
-  Browser* browser = BrowserList::GetLastActive();
-  if (browser && browser->type() != Browser::TYPE_NORMAL) {
-    browser = BrowserList::FindBrowserWithType(browser->profile(),
-                                               Browser::TYPE_NORMAL,
-                                               true);
-  }
-  DCHECK(browser);
-  views::Window* window = browser::CreateViewsWindow(
-      browser->window()->GetNativeHandle(), gfx::Rect(), view);
-  window->SetIsAlwaysOnTop(true);
-  window->Show();
-}
-
 void NetworkMessageObserver::OpenMobileSetupPage(const ListValue* args) {
   Browser* browser = BrowserList::GetLastActive();
   if (browser)
@@ -108,7 +87,6 @@ void NetworkMessageObserver::OnNetworkManagerChanged(NetworkLibrary* obj) {
   const WifiNetworkVector& wifi_networks = obj->wifi_networks();
   const CellularNetworkVector& cellular_networks = obj->cellular_networks();
 
-  NetworkConfigView* view = NULL;
   std::string new_failed_network;
   // Check to see if we have any newly failed wifi network.
   for (WifiNetworkVector::const_iterator it = wifi_networks.begin();
@@ -124,21 +102,6 @@ void NetworkMessageObserver::OnNetworkManagerChanged(NetworkLibrary* obj) {
         continue;
 
       const WifiNetwork* wifi_old = iter->second;
-      // If this network was in a failed state previously, then it's not new.
-      if (wifi_old->failed())
-        continue;
-
-      // Display login box again for bad_passphrase and bad_wepkey errors.
-      if (wifi->error() == ERROR_BAD_PASSPHRASE ||
-          wifi->error() == ERROR_BAD_WEPKEY) {
-        // The NetworkConfigView will show the appropriate error message.
-        view = new NetworkConfigView(wifi);
-        // There should only be one wifi network that failed to connect.
-        // If for some reason, we have more than one failure,
-        // we only display the first one. So we break here.
-        break;
-      }
-
       // If network connection failed, display a notification.
       // We only do this if we were trying to make a new connection.
       // So if a previously connected network got disconnected for any reason,
@@ -183,10 +146,6 @@ void NetworkMessageObserver::OnNetworkManagerChanged(NetworkLibrary* obj) {
         IDS_NETWORK_CONNECTION_ERROR_MESSAGE,
         ASCIIToUTF16(new_failed_network)), false, false);
   }
-
-  // Show login box if necessary.
-  if (view && initialized_)
-    CreateModalPopup(view);
 }
 
 void NetworkMessageObserver::OnCellularDataPlanChanged(NetworkLibrary* obj) {

chrome/browser/chromeos/network_message_observer.h

@@ -14,9 +14,6 @@
 #include "chrome/browser/chromeos/notifications/system_notification.h"
 
 class Profile;
-namespace views {
-class WindowDelegate;
-}
 
 namespace chromeos {
 
@@ -32,7 +29,6 @@ class NetworkMessageObserver : public NetworkLibrary::NetworkManagerObserver,
   typedef std::map<std::string, WifiNetwork*> ServicePathWifiMap;
   typedef std::map<std::string, CellularNetwork*> ServicePathCellularMap;
  private:
-  virtual void CreateModalPopup(views::WindowDelegate* view);
   virtual void OpenMobileSetupPage(const ListValue* args);
   virtual void OpenMoreInfoPage(const ListValue* args);
 
@@ -41,7 +37,6 @@ class NetworkMessageObserver : public NetworkLibrary::NetworkManagerObserver,
   // NetworkLibrary::CellularDataPlanObserver implementation.
   virtual void OnCellularDataPlanChanged(NetworkLibrary* obj);
 
-  bool initialized_;
   // Wifi networks by service path.
   ServicePathWifiMap wifi_networks_;
   // Cellular networks by service path.

chrome/chrome_browser.gypi

@@ -685,6 +685,8 @@
         'browser/chromeos/network_list.h',
         'browser/chromeos/network_state_notifier.cc',
         'browser/chromeos/network_state_notifier.h',
+        'browser/chromeos/network_login_observer.cc',
+        'browser/chromeos/network_login_observer.h',
         'browser/chromeos/network_message_observer.cc',
         'browser/chromeos/network_message_observer.h',
         'browser/chromeos/notifications/balloon_collection_impl.cc',