NetworkStateHandler: Add hostname property and observer event

BUG=1103272

Change-Id: I9e07c2e8b1457d41a5934b60f9eaf5273e67e6da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2319341
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798327}

chromeos/network/network_state_handler.cc

@@ -1568,6 +1568,12 @@ void NetworkStateHandler::CheckPortalListChanged(
   check_portal_list_ = check_portal_list;
 }
 
+void NetworkStateHandler::HostnameChanged(const std::string& hostname) {
+  hostname_ = hostname;
+  for (auto& observer : observers_)
+    observer.HostnameChanged(hostname);
+}
+
 void NetworkStateHandler::TechnologyListChanged() {
   // Eventually we would like to replace Technology state with Device state.
   // For now, treat technology state changes as device list changes.

chromeos/network/network_state_handler.h

@@ -350,10 +350,16 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandler
   // only set it.
   void SetWakeOnLanEnabled(bool enabled);
 
-  // Sets the HostName property. Note: we do not track this property, we
-  // only set it.
+  // Sets the DHCP HostName property. Note: This does not directly set
+  // |hostname_|, it sets the Shill property and relies on Shill emitting the
+  // change which updates the cached |hostname_|. This ensures that Chrome and
+  // Shill are in sync.
   void SetHostname(const std::string& hostname);
 
+  // Returns the cached DHCP HostName property provided by Shill. Initialized
+  // to an empty string and set once the Manager properties are received.
+  const std::string& hostname() const { return hostname_; }
+
   // Enable or disable network bandwidth throttling, on all interfaces on the
   // system. If |enabled| is true, |upload_rate_kbits| and |download_rate_kbits|
   // are the desired rates (in kbits/s) to throttle to. If |enabled| is false,
@@ -452,11 +458,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandler
                                 const std::string& ip_config_path,
                                 const base::Value& properties) override;
 
-  // Called by ShillPropertyHandler when the portal check list manager property
-  // changes.
   void CheckPortalListChanged(const std::string& check_portal_list) override;
-
-  // Called by ShillPropertyHandler when a technology list changes.
+  void HostnameChanged(const std::string& hostname) override;
   void TechnologyListChanged() override;
 
   // Called by |shill_property_handler_| when the service or device list has
@@ -668,6 +671,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandler
   // List of interfaces on which portal check is enabled.
   std::string check_portal_list_;
 
+  // DHCP Hostname.
+  std::string hostname_;
+
   // Map of network specifiers to guids. Contains an entry for each
   // NetworkState that is not saved in a profile.
   SpecifierGuidMap specifier_guid_map_;

chromeos/network/network_state_handler_observer.cc

@@ -34,6 +34,9 @@ void NetworkStateHandlerObserver::ScanRequested(
 
 void NetworkStateHandlerObserver::ScanCompleted(const DeviceState* device) {}
 
+void NetworkStateHandlerObserver::HostnameChanged(const std::string& hostname) {
+}
+
 void NetworkStateHandlerObserver::OnShuttingDown() {}
 
 }  // namespace chromeos

chromeos/network/network_state_handler_observer.h

@@ -63,6 +63,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandlerObserver {
   // A scan for |device| completed.
   virtual void ScanCompleted(const DeviceState* device);
 
+  // The DHCP Hostname changed.
+  virtual void HostnameChanged(const std::string& hostname);
+
   // Called just before NetworkStateHandler is destroyed so that observers
   // can safely stop observing.
   virtual void OnShuttingDown();

chromeos/network/network_state_handler_unittest.cc

@@ -149,6 +149,10 @@ class TestObserver final : public chromeos::NetworkStateHandlerObserver {
     scan_completed_count_++;
   }
 
+  void HostnameChanged(const std::string& hostname) override {
+    hostname_ = hostname;
+  }
+
   size_t active_network_change_count() { return active_network_change_count_; }
   size_t default_network_change_count() {
     return default_network_change_count_;
@@ -162,6 +166,7 @@ class TestObserver final : public chromeos::NetworkStateHandlerObserver {
     return scan_requests_;
   }
   size_t scan_completed_count() { return scan_completed_count_; }
+  const std::string& hostname() { return hostname_; }
   void reset_change_counts() {
     VLOG(1) << "=== RESET CHANGE COUNTS ===";
     active_network_change_count_ = 0;
@@ -211,6 +216,7 @@ class TestObserver final : public chromeos::NetworkStateHandlerObserver {
   size_t network_count_ = 0;
   std::vector<NetworkTypePattern> scan_requests_;
   size_t scan_completed_count_ = 0;
+  std::string hostname_;
   std::vector<std::string> active_network_paths_;
   std::string default_network_;
   std::string default_network_connection_state_;
@@ -2260,4 +2266,17 @@ TEST_F(NetworkStateHandlerTest, SetNetworkChromePortalDetected) {
             test_observer_->ConnectionStateChangesForService(network->path()));
 }
 
+TEST_F(NetworkStateHandlerTest, Hostname) {
+  const std::string kTestHostname = "Test Hostname";
+  network_state_handler_->SetHostname(kTestHostname);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(network_state_handler_->hostname(), kTestHostname);
+  EXPECT_EQ(test_observer_->hostname(), kTestHostname);
+
+  network_state_handler_->SetHostname(std::string());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(network_state_handler_->hostname().empty());
+  EXPECT_TRUE(test_observer_->hostname().empty());
+}
+
 }  // namespace chromeos

chromeos/network/shill_property_handler.cc

@@ -383,6 +383,10 @@ void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
     std::string check_portal_list;
     if (value.GetAsString(&check_portal_list))
       listener_->CheckPortalListChanged(check_portal_list);
+  } else if (key == shill::kDhcpPropertyHostnameProperty) {
+    std::string hostname;
+    if (value.GetAsString(&hostname))
+      listener_->HostnameChanged(hostname);
   } else {
     VLOG(2) << "Ignored Manager Property: " << key;
   }

chromeos/network/shill_property_handler.h

@@ -80,6 +80,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ShillPropertyHandler
     virtual void CheckPortalListChanged(
          const std::string& check_portal_list) = 0;
 
+    // Called when the DHCP Hostname property changes.
+    virtual void HostnameChanged(const std::string& hostname) = 0;
+
     // Called when a technology list changes.
     virtual void TechnologyListChanged() = 0;
 

chromeos/network/shill_property_handler_unittest.cc

@@ -76,13 +76,17 @@ class TestListener : public internal::ShillPropertyHandler::Listener {
     AddPropertyUpdate(shill::kIPConfigsProperty, ip_config_path);
   }
 
+  void CheckPortalListChanged(const std::string& check_portal_list) override {}
+
+  void HostnameChanged(const std::string& hostname) override {
+    hostname_ = hostname;
+  }
+
   void TechnologyListChanged() override {
     VLOG(1) << "TechnologyListChanged.";
     ++technology_list_updates_;
   }
 
-  void CheckPortalListChanged(const std::string& check_portal_list) override {}
-
   void ManagedStateListChanged(ManagedState::ManagedType type) override {
     VLOG(1) << "ManagedStateListChanged: " << GetTypeString(type);
     AddStateListUpdate(GetTypeString(type));
@@ -107,6 +111,7 @@ class TestListener : public internal::ShillPropertyHandler::Listener {
     list_updates_.clear();
     technology_list_updates_ = 0;
   }
+  std::string hostname() { return hostname_; }
   int errors() { return errors_; }
 
  private:
@@ -152,6 +157,7 @@ class TestListener : public internal::ShillPropertyHandler::Listener {
   // Map of list-type -> list update counts
   std::map<std::string, int> list_updates_;
   int technology_list_updates_;
+  std::string hostname_;
   int errors_;
 };
 
@@ -286,6 +292,14 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerStub) {
   EXPECT_EQ(0, listener_->errors());
 }
 
+TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerHostnameChanged) {
+  EXPECT_TRUE(listener_->hostname().empty());
+  const char kTestHostname[] = "Test Hostname";
+  shill_property_handler_->SetHostname(kTestHostname);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(listener_->hostname(), kTestHostname);
+}
+
 TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerTechnologyChanged) {
   const int initial_technology_updates = 2;  // Available and Enabled lists
   EXPECT_EQ(initial_technology_updates, listener_->technology_list_updates());