Clean up NetworkState members

ash/system/chromeos/network/network_connect.cc

 // Copyright (c) 2013 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 "ash/system/chromeos/network/network_connect.h"
 
 #include "ash/shell.h"
 #include "ash/system/chromeos/network/network_state_notifier.h"
 #include "ash/system/system_notifier.h"
 #include "ash/system/tray/system_tray_delegate.h"
@@ skipping 332 matching lines @@
   }
   const DeviceState* cellular_device =
       NetworkHandler::Get()->network_state_handler()->
       GetDeviceState(cellular->device_path());
   if (!cellular_device) {
     NET_LOG_ERROR("ActivateCellular with no Device", service_path);
     return;
   }
   if (!IsDirectActivatedCarrier(cellular_device->carrier())) {
     // For non direct activation, show the mobile setup dialog which can be
-    // used to activate the network. Only show the dialog, if an account
-    // management URL is available.
-    if (!cellular->payment_url().empty())

[stevenjb, 2013/09/03 23:23:07]

This check shouldn't be necessary; MobileSetupUI should handle that case after
requesting properties.

[armansito, 2013/09/03 23:52:06]

I think the main reason for doing this check here is to prevent an empty
activation dialog from temporarily popping up. Is this something that could be
handled in MobileSetupUI without displaying the dialog?

[stevenjb, 2013/09/04 00:04:50]

Is this a scenario that would ever actually happen outside of an error
condition? I see that MobileSetupUI checks for either payment_url or usage_url,
so this check seemed somewhat arbitrary and inconsistent. If this produces an
actual bug then we should fix it in MobileSetupUI instead of here.

[armansito, 2013/09/04 00:18:14]

Yes, it is actually possible for shill not to show the payment URL on occasion.
This is rare, but we should make sure that this is handled correctly by
MobileSetupDialog to prevent any regressions, as such an issue has been seen in
the past.

We can address that for 31 if the test seem runs into anything.

[stevenjb, 2013/09/04 01:04:11]

I'm not really sure that I understand. It doesn't seem like having a button
click do nothing (which is what would happen if we return here) is any better
than showing an empty dialog, or briefly showing and closing the dialog.

However, if we would prefer that, we could probably have
MobileSetupDialogDelegate::ShowDialog call GetProperties() and check
payment_url_ before calling chrome::ShowWebDialog? I really would like to avoid
caching this value for this one scenario.

[armansito, 2013/09/04 01:35:31]

Yes, I agree that this shouldn't be cached. We can talk about a work around for
the case when the payment URL hasn't loaded yet, but I think the current change
is OK as far as this CL is concerned.

-      ShowMobileSetup(service_path);
+    // used to activate the network.
+    ShowMobileSetup(service_path);
     return;
   }
   if (cellular->activation_state() == flimflam::kActivationStateActivated) {
     NET_LOG_ERROR("ActivateCellular for activated service", service_path);
     return;
   }
 
   NetworkHandler::Get()->network_activation_handler()->Activate(
       service_path,
       "",  // carrier
@@ skipping 136 matching lines @@
 
 void ShowNetworkSettings(const std::string& service_path) {
   if (!ash::Shell::HasInstance())
     return;
   ash::Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings(
       service_path);
 }
 
 }  // network_connect
 }  // ash