Add success field to FindHostCallback

This CL adds a bool success field to FindHostCallback to indicate if the
HostLocator was successful in querying for hosts.

Bug: chromium:757625
Change-Id: Ie1a2c4ee0c8392157234cc1f1eefdb0d3bc4d88f
Reviewed-on: https://chromium-review.googlesource.com/963757
Commit-Queue: Allen Vicencio <allenvic@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545155}

chrome/browser/chromeos/smb_client/discovery/host_locator.h

@@ -16,7 +16,11 @@ namespace smb_client {
 using Hostname = std::string;
 using Address = std::string;
 using HostMap = std::map<Hostname, Address>;
-using FindHostsCallback = base::OnceCallback<void(const HostMap& hosts)>;
+
+// |success| will be false if an error occurred when finding hosts. |success|
+// can be true even if |hosts| is empty.
+using FindHostsCallback =
+    base::OnceCallback<void(bool success, const HostMap& hosts)>;
 
 // Interface that abstracts the multiple methods of finding SMB hosts in a
 // network. (e.g. mDNS, NetBIOS over TCP, LMHosts, DNS)

chrome/browser/chromeos/smb_client/discovery/in_memory_host_locator.cc

@@ -29,7 +29,7 @@ void InMemoryHostLocator::RemoveHost(const Hostname& hostname) {
 }
 
 void InMemoryHostLocator::FindHosts(FindHostsCallback callback) {
-  std::move(callback).Run(host_map_);
+  std::move(callback).Run(true /* success */, host_map_);
 }
 
 }  // namespace smb_client

chrome/browser/chromeos/smb_client/discovery/in_memory_host_locator_unittest.cc

@@ -17,13 +17,17 @@ namespace {
 
 // Expects |actual_hosts| to not equal |hosts|.
 void ExpectMapEntriesNotEqual(const HostMap& hosts,
+                              bool success,
                               const HostMap& actual_hosts) {
+  EXPECT_TRUE(success);
   EXPECT_NE(hosts, actual_hosts);
 }
 
 // Expects |actual_hosts| to equal |expected_hosts|.
-void ExpectMapEntries(const HostMap& expected_hosts,
-                      const HostMap& actual_hosts) {
+void ExpectMapEntriesEqual(const HostMap& expected_hosts,
+                           bool success,
+                           const HostMap& actual_hosts) {
+  EXPECT_TRUE(success);
   EXPECT_EQ(expected_hosts, actual_hosts);
 }
 
@@ -36,7 +40,7 @@ class InMemoryHostLocatorTest : public testing::Test {
 
  protected:
   void ExpectHostMapEqual(const HostMap& hosts) {
-    locator_.FindHosts(base::BindOnce(&ExpectMapEntries, hosts));
+    locator_.FindHosts(base::BindOnce(&ExpectMapEntriesEqual, hosts));
   }
 
   void ExpectHostMapNotEqual(const HostMap& hosts) {

chrome/browser/chromeos/smb_client/discovery/network_scanner.cc

@@ -38,7 +38,7 @@ NetworkScanner::~NetworkScanner() = default;
 void NetworkScanner::FindHostsInNetwork(FindHostsCallback callback) {
   if (locators_.empty()) {
     // Fire the callback immediately if there are no registered HostLocators.
-    std::move(callback).Run(HostMap());
+    std::move(callback).Run(false /* success */, HostMap());
     return;
   }
 
@@ -54,10 +54,14 @@ void NetworkScanner::RegisterHostLocator(std::unique_ptr<HostLocator> locator) {
 }
 
 void NetworkScanner::OnHostsFound(uint32_t request_id,
+                                  bool success,
                                   const HostMap& host_map) {
   DCHECK_GT(requests_.count(request_id), 0u);
 
-  AddHostsToResults(request_id, host_map);
+  if (success) {
+    AddHostsToResults(request_id, host_map);
+  }
+
   FireCallbackIfFinished(request_id);
 }
 
@@ -98,7 +102,7 @@ void NetworkScanner::FireCallbackIfFinished(uint32_t request_id) {
     RequestInfo info = std::move(request_iter->second);
     requests_.erase(request_iter);
 
-    std::move(info.callback).Run(info.hosts_found);
+    std::move(info.callback).Run(true /* success */, info.hosts_found);
   }
 }
 

chrome/browser/chromeos/smb_client/discovery/network_scanner.h

@@ -40,7 +40,7 @@ class NetworkScanner : public base::SupportsWeakPtr<NetworkScanner> {
   // Query the registered HostLocators and return all the hosts found.
   // |callback| is called once all the HostLocators have responded with their
   // results. If there are no locators, the callback is fired immediately with
-  // an empty result.
+  // an empty result and success set to false.
   void FindHostsInNetwork(FindHostsCallback callback);
 
   // Registeres a |locator| to be queried when FindHostsInNetwork() is called.
@@ -48,7 +48,7 @@ class NetworkScanner : public base::SupportsWeakPtr<NetworkScanner> {
 
  private:
   // Callback handler for HostLocator::FindHosts().
-  void OnHostsFound(uint32_t request_id, const HostMap& host_map);
+  void OnHostsFound(uint32_t request_id, bool success, const HostMap& host_map);
 
   // Adds |host_map| hosts to current results. The host will not be added if the
   // hostname already exists in results, and if the IP address does not match,

chrome/browser/chromeos/smb_client/discovery/network_scanner_unittest.cc

@@ -19,10 +19,17 @@ namespace {
 
 // Expects |actual_hosts| to equal |expected_hosts|.
 void ExpectMapEntriesEqual(const HostMap& expected_hosts,
+                           bool success,
                            const HostMap& actual_hosts) {
+  EXPECT_TRUE(success);
   EXPECT_EQ(expected_hosts, actual_hosts);
 }
 
+void ExpectFailure(bool success, const HostMap& actual_hosts) {
+  EXPECT_FALSE(success);
+  EXPECT_TRUE(actual_hosts.empty());
+}
+
 }  // namespace
 
 class NetworkScannerTest : public testing::Test {
@@ -43,14 +50,18 @@ class NetworkScannerTest : public testing::Test {
         base::BindOnce(&ExpectMapEntriesEqual, expected_hosts));
   }
 
+  void ExpectCallFailure() {
+    scanner_.FindHostsInNetwork(base::BindOnce(&ExpectFailure));
+  }
+
  private:
   NetworkScanner scanner_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkScannerTest);
 };
 
-TEST_F(NetworkScannerTest, ShouldFindNoHostsWithNoLocator) {
-  ExpectHostMapEqual(HostMap());
+TEST_F(NetworkScannerTest, SuccessIsFalseAndHostsMapIsEmptyWithNoLocator) {
+  ExpectCallFailure();
 }
 
 TEST_F(NetworkScannerTest, ShouldFindNoHostsWithOneLocator) {