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

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/views/window.h"
+#include "views/window/dialog_delegate.h"
+#include "views/window/window.h"
+
+namespace chromeos {
+
+NetworkLoginObserver::NetworkLoginObserver(NetworkLibrary* netlib) {
+  OnNetworkManagerChanged(netlib);
+  initialized_ = true;

[stevenjb, 2011/01/11 20:53:52]

nit: Rather than tracking an initiailized_ state, the "Refresh stored networks"
section of OnNetworkManagerChanged() could be moved to a separate private method
and called instead of OnNetworkManagerChanged() here.

[Charlie Lee, 2011/01/11 22:52:06]

Done.

+  netlib->AddNetworkManagerObserver(this);
+}
+
+NetworkLoginObserver::~NetworkLoginObserver() {
+  CrosLibrary::Get()->GetNetworkLibrary()->RemoveNetworkManagerObserver(this);
+  STLDeleteValues(&wifi_networks_);
+}
+
+void NetworkLoginObserver::CreateModalPopup(views::WindowDelegate* view) {
+  Browser* browser = BrowserList::GetLastActive();
+  if (browser && browser->type() != Browser::TYPE_NORMAL) {

[stevenjb, 2011/01/11 20:53:52]

FindBrowserWithType() searches in order of last activation, so we should be able
to skip the GetLastActive() call and always use FindBrowserWithType().

[Charlie Lee, 2011/01/11 22:52:06]

FindBrowserWithType takes in a profile which we get from the browser object. So
we can't call it without getting an initial browser.

I copied this code from elsewhere, so i think it's the right way to do it.

On 2011/01/11 20:53:52, Steven Bennetts wrote:

[stevenjb, 2011/01/12 01:34:52]

I forgot to add that we would need to use ProfileManager::GetDefaultProfile()
instead of b->profile().

I searched through the code and it looks like this is something we ought to
clean up consistently throughout, so I am fine with leaving it as-is for now.


On 2011/01/11 22:52:06, Charlie Lee wrote:

+    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();
+    return;

[stevenjb, 2011/01/11 20:53:52]

nit: return statement is unnecessary.

[Charlie Lee, 2011/01/11 22:52:06]

Done.

+  } 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::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()) {
+      ServicePathWifiMap::iterator iter =
+          wifi_networks_.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_networks_.end())
+        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;
+      }
+    }
+  }
+
+  // Refresh stored networks.
+  STLDeleteValues(&wifi_networks_);
+  wifi_networks_.clear();
+  for (WifiNetworkVector::const_iterator it = wifi_networks.begin();
+       it < wifi_networks.end(); it++) {
+    const WifiNetwork* wifi = *it;
+    wifi_networks_[wifi->service_path()] = new WifiNetwork(*wifi);

[stevenjb, 2011/01/11 20:53:52]

We are doing a lot of unnecessary copying of WifiNetwork in both
NetworkLoginObserver and NetworkMessageObserver when we only need to save error_
and state_. It's a small optimization, but given how often this could happen, it
might be worth the effort. I will leave it up to you.

[Charlie Lee, 2011/01/11 22:52:06]

Done.

+  }
+
+  // Show login box if necessary.
+  if (view && initialized_)
+    CreateModalPopup(view);
+}
+
+}  // namespace chromeos