Elim parent_window parameter from network_connect

We were passing a 'parent_window' parameter to the network_connect::ConnectToNetwork() in an effort to open network configuration dialogs from the desktop they were triggered from. This was never a good idea since gfx::NativeWindow is actually a pointer which may become invalid before the callback is triggered.

In practice this is unnecessary since we decided that all network configuration applies to the primary user, so we should always use the currently active desktop to host the dialog (which is what GetNativeWindow() does in ash_system_tray_delegate.cc).

BUG=413925

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

Cr-Commit-Position: refs/heads/master@{#296969}

ash/system/chromeos/network/network_connect.cc

@@ -65,8 +65,7 @@ void ShowErrorNotification(const std::string& error_name,
       ShowNetworkConnectError(error_name, service_path);
 }
 
-void HandleUnconfiguredNetwork(const std::string& service_path,
-                               gfx::NativeWindow parent_window) {
+void HandleUnconfiguredNetwork(const std::string& service_path) {
   const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
       GetNetworkState(service_path);
   if (!network) {
@@ -78,7 +77,7 @@ void HandleUnconfiguredNetwork(const std::string& service_path,
     // Only show the config view for secure networks, otherwise do nothing.
     if (network->security() != shill::kSecurityNone) {
       ash::Shell::GetInstance()->system_tray_delegate()->
-          ShowNetworkConfigure(service_path, parent_window);
+          ShowNetworkConfigure(service_path);
     }
     return;
   }
@@ -86,7 +85,7 @@ void HandleUnconfiguredNetwork(const std::string& service_path,
   if (network->type() == shill::kTypeWimax ||
       network->type() == shill::kTypeVPN) {
     ash::Shell::GetInstance()->system_tray_delegate()->
-        ShowNetworkConfigure(service_path, parent_window);
+        ShowNetworkConfigure(service_path);
     return;
   }
 
@@ -136,7 +135,6 @@ bool GetNetworkProfilePath(bool shared, std::string* profile_path) {
 }
 
 void OnConnectFailed(const std::string& service_path,
-                     gfx::NativeWindow parent_window,
                      const std::string& error_name,
                      scoped_ptr<base::DictionaryValue> error_data) {
   NET_LOG_ERROR("Connect Failed: " + error_name, service_path);
@@ -152,14 +150,14 @@ void OnConnectFailed(const std::string& service_path,
       error_name == NetworkConnectionHandler::kErrorPassphraseRequired ||
       error_name == NetworkConnectionHandler::kErrorConfigurationRequired ||
       error_name == NetworkConnectionHandler::kErrorAuthenticationRequired) {
-    HandleUnconfiguredNetwork(service_path, parent_window);
+    HandleUnconfiguredNetwork(service_path);
     return;
   }
 
   if (error_name == NetworkConnectionHandler::kErrorCertificateRequired) {
     if (!ash::Shell::GetInstance()->system_tray_delegate()->EnrollNetwork(
-            service_path, parent_window)) {
-      HandleUnconfiguredNetwork(service_path, parent_window);
+            service_path)) {
+      HandleUnconfiguredNetwork(service_path);
     }
     return;
   }
@@ -191,7 +189,7 @@ void OnConnectFailed(const std::string& service_path,
   if (dbus_error_name == kErrorInProgress)
     return;
 
-  HandleUnconfiguredNetwork(service_path, parent_window);
+  HandleUnconfiguredNetwork(service_path);
 }
 
 void OnConnectSucceeded(const std::string& service_path) {
@@ -204,12 +202,9 @@ void OnConnectSucceeded(const std::string& service_path) {
 
 // If |check_error_state| is true, error state for the network is checked,
 // otherwise any current error state is ignored (e.g. for recently configured
-// networks or repeat connect attempts). |parent_window| will be used to parent
-// any configuration UI on failure and may be NULL (in which case the default
-// window will be used).
+// networks or repeat connect attempts).
 void CallConnectToNetwork(const std::string& service_path,
-                          bool check_error_state,
-                          gfx::NativeWindow parent_window) {
+                          bool check_error_state) {
   if (!ash::Shell::HasInstance())
     return;
   message_center::MessageCenter::Get()->RemoveNotification(
@@ -218,7 +213,7 @@ void CallConnectToNetwork(const std::string& service_path,
   NetworkHandler::Get()->network_connection_handler()->ConnectToNetwork(
       service_path,
       base::Bind(&OnConnectSucceeded, service_path),
-      base::Bind(&OnConnectFailed, service_path, parent_window),
+      base::Bind(&OnConnectFailed, service_path),
       check_error_state);
 }
 
@@ -246,8 +241,7 @@ void OnConfigureSucceeded(bool connect_on_configure,
     return;
   // After configuring a network, ignore any (possibly stale) error state.
   const bool check_error_state = false;
-  const gfx::NativeWindow parent_window = NULL;
-  CallConnectToNetwork(service_path, check_error_state, parent_window);
+  CallConnectToNetwork(service_path, check_error_state);
 }
 
 void CallCreateConfiguration(base::DictionaryValue* properties,
@@ -297,13 +291,11 @@ void ClearPropertiesAndConnect(
   NET_LOG_USER("ClearPropertiesAndConnect", service_path);
   // After configuring a network, ignore any (possibly stale) error state.
   const bool check_error_state = false;
-  const gfx::NativeWindow parent_window = NULL;
   NetworkHandler::Get()->network_configuration_handler()->ClearProperties(
       service_path,
       properties_to_clear,
       base::Bind(&CallConnectToNetwork,
-                 service_path, check_error_state,
-                 parent_window),
+                 service_path, check_error_state),
       base::Bind(&SetPropertiesFailed, "ClearProperties", service_path));
 }
 
@@ -337,8 +329,7 @@ const char kNetworkActivateNotificationId[] =
 
 const char kErrorActivateFailed[] = "activate-failed";
 
-void ConnectToNetwork(const std::string& service_path,
-                      gfx::NativeWindow parent_window) {
+void ConnectToNetwork(const std::string& service_path) {
   NET_LOG_USER("ConnectToNetwork", service_path);
   const NetworkState* network = GetNetworkState(service_path);
   if (network) {
@@ -346,7 +337,7 @@ void ConnectToNetwork(const std::string& service_path,
       NET_LOG_USER("Configure: " + network->error(), service_path);
       // If the network is in an error state, show the configuration UI directly
       // to avoid a spurious notification.
-      HandleUnconfiguredNetwork(service_path, parent_window);
+      HandleUnconfiguredNetwork(service_path);
       return;
     } else if (network->RequiresActivation()) {
       ActivateCellular(service_path);
@@ -354,7 +345,7 @@ void ConnectToNetwork(const std::string& service_path,
     }
   }
   const bool check_error_state = true;
-  CallConnectToNetwork(service_path, check_error_state, parent_window);
+  CallConnectToNetwork(service_path, check_error_state);
 }
 
 void SetTechnologyEnabled(const NetworkTypePattern& technology,

ash/system/chromeos/network/network_connect.h

@@ -9,7 +9,6 @@
 
 #include "ash/ash_export.h"
 #include "base/strings/string16.h"
-#include "ui/gfx/native_widget_types.h"  // gfx::NativeWindow
 
 namespace base {
 class DictionaryValue;
@@ -28,10 +27,7 @@ ASH_EXPORT extern const char kNetworkActivateNotificationId[];
 ASH_EXPORT extern const char kErrorActivateFailed[];
 
 // Requests a network connection and handles any errors and notifications.
-// |parent_window| is used to parent any UI on failure (e.g. for certificate
-// enrollment). If NULL, the default window will be used.
-ASH_EXPORT void ConnectToNetwork(const std::string& service_path,
-                                 gfx::NativeWindow parent_window);
+ASH_EXPORT void ConnectToNetwork(const std::string& service_path);
 
 // Enables or disables a network technology. If |technology| refers to cellular
 // and the device cannot be enabled due to a SIM lock, this function will

ash/system/chromeos/network/network_state_list_detailed_view.cc

@@ -271,7 +271,7 @@ void NetworkStateListDetailedView::OnViewClicked(views::View* sender) {
         list_type_ == LIST_TYPE_VPN ?
         ash::UMA_STATUS_AREA_CONNECT_TO_VPN :
         ash::UMA_STATUS_AREA_CONNECT_TO_CONFIGURED_NETWORK);
-    ash::network_connect::ConnectToNetwork(service_path, NULL);
+    ash::network_connect::ConnectToNetwork(service_path);
   }
 }
 

ash/system/chromeos/network/network_state_notifier_unittest.cc

@@ -93,7 +93,7 @@ class NetworkStateNotifierTest : public AshTestBase {
 
 TEST_F(NetworkStateNotifierTest, ConnectionFailure) {
   EXPECT_FALSE(GetSystemTray()->HasNotificationBubble());
-  ash::network_connect::ConnectToNetwork("wifi1", NULL /* owning_window */);
+  ash::network_connect::ConnectToNetwork("wifi1");
   RunAllPendingInMessageLoop();
   // Failure should spawn a notification.
   message_center::MessageCenter* message_center =

ash/system/tray/default_system_tray_delegate.cc

@@ -207,12 +207,10 @@ void DefaultSystemTrayDelegate::ActivateIMEProperty(const std::string& key) {
 }
 
 void DefaultSystemTrayDelegate::ShowNetworkConfigure(
-    const std::string& network_id,
-    gfx::NativeWindow parent_window) {
+    const std::string& network_id) {
 }
 
-bool DefaultSystemTrayDelegate::EnrollNetwork(const std::string& network_id,
-                                              gfx::NativeWindow parent_window) {
+bool DefaultSystemTrayDelegate::EnrollNetwork(const std::string& network_id) {
   return true;
 }
 

ash/system/tray/default_system_tray_delegate.h

@@ -65,10 +65,8 @@ class ASH_EXPORT DefaultSystemTrayDelegate : public SystemTrayDelegate {
   virtual void GetCurrentIMEProperties(IMEPropertyInfoList* list) OVERRIDE;
   virtual void SwitchIME(const std::string& ime_id) OVERRIDE;
   virtual void ActivateIMEProperty(const std::string& key) OVERRIDE;
-  virtual void ShowNetworkConfigure(const std::string& network_id,
-                                    gfx::NativeWindow parent_window) OVERRIDE;
-  virtual bool EnrollNetwork(const std::string& network_id,
-                             gfx::NativeWindow parent_window) OVERRIDE;
+  virtual void ShowNetworkConfigure(const std::string& network_id) OVERRIDE;
+  virtual bool EnrollNetwork(const std::string& network_id) OVERRIDE;
   virtual void ManageBluetoothDevices() OVERRIDE;
   virtual void ToggleBluetooth() OVERRIDE;
   virtual bool IsBluetoothDiscovering() OVERRIDE;

ash/system/tray/system_tray_delegate.h

@@ -15,7 +15,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
 #include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/native_widget_types.h"
 
 namespace base {
 class TimeDelta;
@@ -235,16 +234,11 @@ class ASH_EXPORT SystemTrayDelegate {
 
   // Shows UI to configure or activate the network specified by |network_id|,
   // which may include showing Payment or Portal UI when appropriate.
-  // |parent_window| is used to parent any configuration UI. If NULL a default
-  // window will be used.
-  virtual void ShowNetworkConfigure(const std::string& network_id,
-                                    gfx::NativeWindow parent_window) = 0;
+  virtual void ShowNetworkConfigure(const std::string& network_id) = 0;
 
   // Shows UI to enroll the network specified by |network_id| if appropriate
-  // and returns true, otherwise returns false. |parent_window| is used
-  // to parent any configuration UI. If NULL a default window will be used.
-  virtual bool EnrollNetwork(const std::string& network_id,
-                             gfx::NativeWindow parent_window) = 0;
+  // and returns true, otherwise returns false.
+  virtual bool EnrollNetwork(const std::string& network_id) = 0;
 
   // Shows UI to manage bluetooth devices.
   virtual void ManageBluetoothDevices() = 0;

chrome/browser/chromeos/mobile/mobile_activator.cc

@@ -624,8 +624,7 @@ void MobileActivator::ContinueConnecting() {
     LOG(WARNING) << "Connect failed, will try again in a little bit.";
     if (network) {
       VLOG(1) << "Connecting to: " << network->path();
-      ash::network_connect::ConnectToNetwork(
-          network->path(), NULL /* no parent window */);
+      ash::network_connect::ConnectToNetwork(network->path());
     }
   }
 }

chrome/browser/chromeos/status/network_menu.cc

@@ -217,8 +217,7 @@ class MainMenuModel : public NetworkMenuModel {
 
 void NetworkMenuModel::ConnectToNetworkAt(int index) {
   const std::string& service_path = menu_items_[index].service_path;
-  gfx::NativeWindow native_window = owner_->delegate()->GetNativeWindow();
-  ash::network_connect::ConnectToNetwork(service_path, native_window);
+  ash::network_connect::ConnectToNetwork(service_path);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

chrome/browser/ui/ash/system_tray_delegate_chromeos.cc

@@ -746,15 +746,13 @@ void SystemTrayDelegateChromeOS::ActivateIMEProperty(const std::string& key) {
 }
 
 void SystemTrayDelegateChromeOS::ShowNetworkConfigure(
-    const std::string& network_id,
-    gfx::NativeWindow parent_window) {
-  NetworkConfigView::Show(network_id, parent_window);
+    const std::string& network_id) {
+  NetworkConfigView::Show(network_id, GetNativeWindow());
 }
 
 bool SystemTrayDelegateChromeOS::EnrollNetwork(
-    const std::string& network_id,
-    gfx::NativeWindow parent_window) {
-  return enrollment::CreateDialog(network_id, parent_window);
+    const std::string& network_id) {
+  return enrollment::CreateDialog(network_id, GetNativeWindow());
 }
 
 void SystemTrayDelegateChromeOS::ManageBluetoothDevices() {

chrome/browser/ui/ash/system_tray_delegate_chromeos.h

@@ -103,10 +103,8 @@ class SystemTrayDelegateChromeOS
   virtual void GetCurrentIMEProperties(ash::IMEPropertyInfoList* list) OVERRIDE;
   virtual void SwitchIME(const std::string& ime_id) OVERRIDE;
   virtual void ActivateIMEProperty(const std::string& key) OVERRIDE;
-  virtual void ShowNetworkConfigure(const std::string& network_id,
-                                    gfx::NativeWindow parent_window) OVERRIDE;
-  virtual bool EnrollNetwork(const std::string& network_id,
-                             gfx::NativeWindow parent_window) OVERRIDE;
+  virtual void ShowNetworkConfigure(const std::string& network_id) OVERRIDE;
+  virtual bool EnrollNetwork(const std::string& network_id) OVERRIDE;
   virtual void ManageBluetoothDevices() OVERRIDE;
   virtual void ToggleBluetooth() OVERRIDE;
   virtual void ShowMobileSimDialog() OVERRIDE;

chrome/browser/ui/ash/system_tray_delegate_linux.cc

@@ -213,12 +213,10 @@ class SystemTrayDelegateLinux : public ash::SystemTrayDelegate,
   virtual void ActivateIMEProperty(const std::string& key) OVERRIDE {
   }
 
-  virtual void ShowNetworkConfigure(const std::string& network_id,
-                                    gfx::NativeWindow parent_window) OVERRIDE {
+  virtual void ShowNetworkConfigure(const std::string& network_id) OVERRIDE {
   }
 
-  virtual bool EnrollNetwork(const std::string& network_id,
-                             gfx::NativeWindow parent_window) OVERRIDE {
+  virtual bool EnrollNetwork(const std::string& network_id) OVERRIDE {
     return true;
   }
 

chrome/browser/ui/ash/system_tray_delegate_win.cc

@@ -210,12 +210,10 @@ class SystemTrayDelegateWin : public ash::SystemTrayDelegate,
   virtual void ActivateIMEProperty(const std::string& key) OVERRIDE {
   }
 
-  virtual void ShowNetworkConfigure(const std::string& network_id,
-                                    gfx::NativeWindow parent_window) OVERRIDE {
+  virtual void ShowNetworkConfigure(const std::string& network_id) OVERRIDE {
   }
 
-  virtual bool EnrollNetwork(const std::string& network_id,
-                             gfx::NativeWindow parent_window) OVERRIDE {
+  virtual bool EnrollNetwork(const std::string& network_id) OVERRIDE {
     return true;
   }
 

chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc

@@ -290,12 +290,10 @@ bool AddIntegerPropertyIfChanged(const std::string& key,
   return false;
 }
 
-void RequestReconnect(const std::string& service_path,
-                      gfx::NativeWindow owning_window) {
+void RequestReconnect(const std::string& service_path) {
   NetworkHandler::Get()->network_connection_handler()->DisconnectNetwork(
       service_path,
-      base::Bind(&ash::network_connect::ConnectToNetwork,
-                 service_path, owning_window),
+      base::Bind(&ash::network_connect::ConnectToNetwork, service_path),
       base::Bind(&ShillError, "RequestReconnect"));
 }
 
@@ -595,7 +593,7 @@ void InternetOptionsHandler::StartConnectCallback(const base::ListValue* args) {
     NOTREACHED();
     return;
   }
-  ash::network_connect::ConnectToNetwork(service_path, GetNativeWindow());
+  ash::network_connect::ConnectToNetwork(service_path);
 }
 
 void InternetOptionsHandler::StartDisconnectCallback(
@@ -833,7 +831,7 @@ void InternetOptionsHandler::SetIPConfigProperties(
     // If auto config or a static IP property changed, we need to reconnect
     // to the network.
     if (request_reconnect)
-      callback = base::Bind(&RequestReconnect, service_path, GetNativeWindow());
+      callback = base::Bind(&RequestReconnect, service_path);
     NetworkHandler::Get()->network_device_handler()->RequestRefreshIPConfigs(
         device_path,
         callback,