Add NetworkStateHandler::RequestUpdateForNetwork

This also changes the code to signal observers after update requests,
even if nothing changed so that NetworkPropertiesUpdated() will get
signaled after a request.

BUG=240728
R=pneubeck@chromium.org

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

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

11 files changed, 99 insertions, 3 deletions

diff --git a/chrome/browser/chromeos/cros/network_library.cc b/chrome/browser/chromeos/cros/network_library.cc
index c453eb7..fcc60d8 100644
--- a/chrome/browser/chromeos/cros/network_library.cc
+++ b/chrome/browser/chromeos/cros/network_library.cc
@@ -376,6 +376,9 @@ void Network::SetValueProperty(const char* prop, const base::Value& value) {
   if (!EnsureCrosLoaded())
     return;
   CrosSetNetworkServiceProperty(service_path_, prop, value);
+  // Ensure NetworkStateHandler properties are up-to-date.
+  if (NetworkStateHandler::IsInitialized())
+    NetworkStateHandler::Get()->RequestUpdateForNetwork(service_path());
 }
 
 void Network::ClearProperty(const char* prop) {
@@ -383,6 +386,9 @@ void Network::ClearProperty(const char* prop) {
   if (!EnsureCrosLoaded())
     return;
   CrosClearNetworkServiceProperty(service_path_, prop);
+  // Ensure NetworkStateHandler properties are up-to-date.
+  if (NetworkStateHandler::IsInitialized())
+    NetworkStateHandler::Get()->RequestUpdateForNetwork(service_path());
 }
 
 void Network::SetStringProperty(
diff --git a/chrome/browser/chromeos/cros/network_library_impl_base.cc b/chrome/browser/chromeos/cros/network_library_impl_base.cc
index 7baeddb..0158976 100644
--- a/chrome/browser/chromeos/cros/network_library_impl_base.cc
+++ b/chrome/browser/chromeos/cros/network_library_impl_base.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/net/onc_utils.h"
 #include "chrome/browser/chromeos/network_login_observer.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_ui_data.h"
 #include "chromeos/network/onc/onc_constants.h"
 #include "chromeos/network/onc/onc_normalizer.h"
@@ -1202,6 +1203,9 @@ void NetworkLibraryImplBase::LoadOncNetworks(
   } else if (source == onc::ONC_SOURCE_USER_IMPORT && !removal_ids.empty()) {
     ForgetNetworksById(source, removal_ids, true);
   }
+  // Ensure NetworkStateHandler properties are up-to-date.
+  if (NetworkStateHandler::IsInitialized())
+    NetworkStateHandler::Get()->RequestUpdateForAllNetworks();
 }
 
 ////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/chromeos/cros/network_library_impl_cros.cc b/chrome/browser/chromeos/cros/network_library_impl_cros.cc
index 41d1b73..8691af6 100644
--- a/chrome/browser/chromeos/cros/network_library_impl_cros.cc
+++ b/chrome/browser/chromeos/cros/network_library_impl_cros.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/common/chrome_switches.h"
 #include "chromeos/network/cros_network_functions.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ -1353,6 +1354,10 @@ void NetworkLibraryImplCros::SetIPParametersCallback(
   if (!something_changed)
     return;
 
+  // Ensure NetworkStateHandler properties are up-to-date.
+  if (NetworkStateHandler::IsInitialized())
+    NetworkStateHandler::Get()->RequestUpdateForNetwork(service_path);
+
   // Attempt to refresh its IP parameters, so that the changes to the service
   // properties can take effect.
   if (network->connecting_or_connected())
diff --git a/chromeos/network/managed_state.cc b/chromeos/network/managed_state.cc
index b91d4a0..d7e6a15 100644
--- a/chromeos/network/managed_state.cc
+++ b/chromeos/network/managed_state.cc
@@ -16,7 +16,8 @@ namespace chromeos {
 ManagedState::ManagedState(ManagedType type, const std::string& path)
     : managed_type_(type),
       path_(path),
-      is_observed_(false) {
+      is_observed_(false),
+      update_requested_(false) {
 }
 
 ManagedState::~ManagedState() {
diff --git a/chromeos/network/managed_state.h b/chromeos/network/managed_state.h
index 5dcaff2..8fc198d 100644
--- a/chromeos/network/managed_state.h
+++ b/chromeos/network/managed_state.h
@@ -57,6 +57,10 @@ class ManagedState {
   const std::string& type() const { return type_; }
   bool is_observed() const { return is_observed_; }
   void set_is_observed(bool is_observed) { is_observed_ = is_observed; }
+  bool update_requested() const { return update_requested_; }
+  void set_update_requested(bool update_requested) {
+    update_requested_ = update_requested;
+  }
 
  protected:
   ManagedState(ManagedType type, const std::string& path);
@@ -94,6 +98,9 @@ class ManagedState {
   // Tracks when the state is being observed.
   bool is_observed_;
 
+  // Tracks when an update has been requested.
+  bool update_requested_;
+
   DISALLOW_COPY_AND_ASSIGN(ManagedState);
 };
 
diff --git a/chromeos/network/network_configuration_handler.cc b/chromeos/network/network_configuration_handler.cc
index 22a8998b..91c87a0 100644
--- a/chromeos/network/network_configuration_handler.cc
+++ b/chromeos/network/network_configuration_handler.cc
@@ -16,6 +16,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_service_client.h"
+#include "chromeos/network/network_state_handler.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -96,6 +97,11 @@ void RunCreateNetworkCallback(
     const network_handler::StringResultCallback& callback,
     const dbus::ObjectPath& service_path) {
   callback.Run(service_path.value());
+  // This may also get called when CreateConfiguration is used to update an
+  // existing configuration, so request a service update just in case.
+  // TODO(pneubeck): Separate 'Create' and 'Update' calls and only trigger
+  // this on an update.
+  NetworkStateHandler::Get()->RequestUpdateForNetwork(service_path.value());
 }
 
 void IgnoreObjectPathCallback(const base::Closure& callback,
@@ -147,6 +153,7 @@ void NetworkConfigurationHandler::SetProperties(
       base::Bind(&IgnoreObjectPathCallback, callback),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
                  service_path, error_callback));
+  NetworkStateHandler::Get()->RequestUpdateForNetwork(service_path);
 }
 
 void NetworkConfigurationHandler::ClearProperties(
diff --git a/chromeos/network/network_configuration_handler.h b/chromeos/network/network_configuration_handler.h
index 1707ca0..802798a 100644
--- a/chromeos/network/network_configuration_handler.h
+++ b/chromeos/network/network_configuration_handler.h
@@ -83,6 +83,8 @@ class CHROMEOS_EXPORT NetworkConfigurationHandler {
   // Creates a network with the given properties in the active Shill profile,
   // and returns the new service_path to |callback| if successful. See note on
   // |callback| and |error_callback|, in class description above.
+  // This may also be used to update an existing matching configuration, see
+  // Shill documentation for Manager.ConfigureService and Manger.GetService.
   void CreateConfiguration(
       const base::DictionaryValue& properties,
       const network_handler::StringResultCallback& callback,
diff --git a/chromeos/network/network_configuration_handler_unittest.cc b/chromeos/network/network_configuration_handler_unittest.cc
index c5f4f3a7..0dad618 100644
--- a/chromeos/network/network_configuration_handler_unittest.cc
+++ b/chromeos/network/network_configuration_handler_unittest.cc
@@ -12,6 +12,7 @@
 #include "chromeos/dbus/mock_shill_manager_client.h"
 #include "chromeos/dbus/mock_shill_service_client.h"
 #include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_state_handler.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -87,13 +88,14 @@ class NetworkConfigurationHandlerTest : public testing::Test {
     mock_service_client_ =
         mock_dbus_thread_manager->mock_shill_service_client();
 
-    // Initialize DBusThreadManager with a stub implementation.
+    NetworkStateHandler::Initialize();
     NetworkConfigurationHandler::Initialize();
     message_loop_.RunUntilIdle();
   }
 
   virtual void TearDown() OVERRIDE {
     NetworkConfigurationHandler::Shutdown();
+    NetworkStateHandler::Shutdown();
     DBusThreadManager::Shutdown();
   }
 
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index 8ef955f..db73348 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -294,6 +294,29 @@ void NetworkStateHandler::ConnectToBestWifiNetwork() {
                          shill_property_handler_->AsWeakPtr()));
 }
 
+bool NetworkStateHandler::RequestUpdateForNetwork(
+    const std::string& service_path) {
+  NetworkState* network = GetModifiableNetworkState(service_path);
+  if (!network)
+    return false;  // Only request an update for known networks.
+  network->set_update_requested(true);
+  NET_LOG_EVENT("RequestUpdate", service_path);
+  shill_property_handler_->RequestProperties(
+      ManagedState::MANAGED_TYPE_NETWORK, service_path);
+  return true;
+}
+
+void NetworkStateHandler::RequestUpdateForAllNetworks() {
+  NET_LOG_EVENT("RequestUpdateForAllNetworks", "");
+  for (ManagedStateList::iterator iter = network_list_.begin();
+       iter != network_list_.end(); ++iter) {
+    ManagedState* network = *iter;
+    network->set_update_requested(true);
+    shill_property_handler_->RequestProperties(
+        ManagedState::MANAGED_TYPE_NETWORK, network->path());
+  }
+}
+
 void NetworkStateHandler::SetConnectingNetwork(
     const std::string& service_path) {
   connecting_network_ = service_path;
@@ -395,7 +418,7 @@ void NetworkStateHandler::UpdateManagedStateProperties(
   managed->InitialPropertiesReceived();
   NET_LOG_DEBUG("PropertiesReceived", GetManagedStateLogName(managed));
   // Notify observers.
-  if (network_property_updated) {
+  if (network_property_updated || managed->update_requested()) {
     NetworkState* network = managed->AsNetworkState();
     DCHECK(network);
     // Signal connection state changed after all properties have been updated.
@@ -403,6 +426,7 @@ void NetworkStateHandler::UpdateManagedStateProperties(
       OnNetworkConnectionStateChanged(network);
     NetworkPropertiesUpdated(network);
   }
+  managed->set_update_requested(false);
 }
 
 void NetworkStateHandler::UpdateNetworkServiceProperty(
diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h
index b7619a2..072258d 100644
--- a/chromeos/network/network_state_handler.h
+++ b/chromeos/network/network_state_handler.h
@@ -157,6 +157,19 @@ class CHROMEOS_EXPORT NetworkStateHandler
   // networks when completed.
   void ConnectToBestWifiNetwork();
 
+  // Request an update for an existing NetworkState, e.g. after configuring
+  // a network. This is a no-op if an update request is already pending.
+  // Returns true if the network exists and an update is requested or pending.
+  // When the properties are received, NetworkPropertiesUpdated will be
+  // signaled for each member of |observers_|, regardless of whether any
+  // properties actually changed.
+  bool RequestUpdateForNetwork(const std::string& service_path);
+
+  // Request an update for all existing NetworkState entries, e.g. after
+  // loading an ONC configuration file that may have updated one or more
+  // existing networks.
+  void RequestUpdateForAllNetworks();
+
   // Set the user initiated connecting network.
   void SetConnectingNetwork(const std::string& service_path);
 
diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc
index f21d802..0cd0585 100644
--- a/chromeos/network/network_state_handler_unittest.cc
+++ b/chromeos/network/network_state_handler_unittest.cc
@@ -346,4 +346,29 @@ TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) {
   EXPECT_EQ(3u, test_observer_->default_network_change_count());
 }
 
+TEST_F(NetworkStateHandlerTest, RequestUpdate) {
+  // Request an update for kShillManagerClientStubDefaultWireless.
+  EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(
+      kShillManagerClientStubDefaultWireless));
+  EXPECT_TRUE(network_state_handler_->RequestUpdateForNetwork(
+      kShillManagerClientStubDefaultWireless));
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(
+      kShillManagerClientStubDefaultWireless));
+
+  // Request an update for all networks.
+  network_state_handler_->RequestUpdateForAllNetworks();
+  message_loop_.RunUntilIdle();
+  // kShillManagerClientStubDefaultWireless should now have 3 updates
+  EXPECT_EQ(3, test_observer_->PropertyUpdatesForService(
+      kShillManagerClientStubDefaultWireless));
+  // Other networks should have 2 updates (inital + request).
+  EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(
+      kShillManagerClientStubDefaultService));
+  EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(
+      kShillManagerClientStubWireless2));
+  EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(
+      kShillManagerClientStubCellular));
+}
+
 }  // namespace chromeos