Split share discovery into two steps
This change modifies the current GetSharesInNetwork function so that a
callback is fired as soon a discovery is done to indicate that name
resolution is possible. Additionally, a new ShareFinder::DiscoveryHosts
method enables performing host discovery without querying each host for
its shares. This change will make it possible to perform name resolution
on reboot.
Bug: chromium:757625
Change-Id: I437ca8ad6400824a3fbc9b870b5fb0307cee2ac4
Reviewed-on: https://chromium-review.googlesource.com/1199685
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589711}diff --git a/chrome/browser/chromeos/smb_client/smb_service.cc b/chrome/browser/chromeos/smb_client/smb_service.cc
index ad07a030..52d463b 100644
--- a/chrome/browser/chromeos/smb_client/smb_service.cc
+++ b/chrome/browser/chromeos/smb_client/smb_service.cc
@@ -113,8 +113,15 @@
CallMount(options, share_path, username, password, std::move(callback));
}
-void SmbService::GatherSharesInNetwork(GatherSharesResponse callback) {
- share_finder_->GatherSharesInNetwork(std::move(callback));
+void SmbService::GatherSharesInNetwork(GatherSharesResponse shares_callback) {
+ share_finder_->GatherSharesInNetwork(base::DoNothing(),
+ std::move(shares_callback));
+}
+
+void SmbService::GatherSharesInNetwork(HostDiscoveryResponse discovery_callback,
+ GatherSharesResponse shares_callback) {
+ share_finder_->GatherSharesInNetwork(std::move(discovery_callback),
+ std::move(shares_callback));
}
void SmbService::CallMount(const file_system_provider::MountOptions& options,
diff --git a/chrome/browser/chromeos/smb_client/smb_service.h b/chrome/browser/chromeos/smb_client/smb_service.h
index eab3d9b..a4523dd 100644
--- a/chrome/browser/chromeos/smb_client/smb_service.h
+++ b/chrome/browser/chromeos/smb_client/smb_service.h
@@ -72,9 +72,12 @@
int32_t mount_id);
// Gathers the hosts in the network using |share_finder_| and gets the shares
- // for each of the hosts found. |callback| will be called once per host and
- // will contain the URLs to the shares found.
- void GatherSharesInNetwork(GatherSharesResponse callback);
+ // for each of the hosts found. |discovery_callback| is called as soon as host
+ // discovery is complete. |shares_callback| is called once per host and will
+ // contain the URLs to the shares found.
+ void GatherSharesInNetwork(GatherSharesResponse shares_callback);
+ void GatherSharesInNetwork(HostDiscoveryResponse discovery_callback,
+ GatherSharesResponse shares_callback);
private:
// Calls SmbProviderClient::Mount(). |temp_file_manager_| must be initialized
diff --git a/chrome/browser/chromeos/smb_client/smb_share_finder.cc b/chrome/browser/chromeos/smb_client/smb_share_finder.cc
index 6a3a4e6..8b2196d 100644
--- a/chrome/browser/chromeos/smb_client/smb_share_finder.cc
+++ b/chrome/browser/chromeos/smb_client/smb_share_finder.cc
@@ -16,9 +16,19 @@
SmbShareFinder::SmbShareFinder(SmbProviderClient* client) : client_(client) {}
SmbShareFinder::~SmbShareFinder() = default;
-void SmbShareFinder::GatherSharesInNetwork(GatherSharesResponse callback) {
- scanner_.FindHostsInNetwork(base::BindOnce(&SmbShareFinder::OnHostsFound,
- AsWeakPtr(), std::move(callback)));
+void SmbShareFinder::GatherSharesInNetwork(
+ HostDiscoveryResponse discovery_callback,
+ GatherSharesResponse shares_callback) {
+ scanner_.FindHostsInNetwork(base::BindOnce(
+ &SmbShareFinder::OnHostsFound, AsWeakPtr(), std::move(discovery_callback),
+ std::move(shares_callback)));
+}
+
+void SmbShareFinder::DiscoverHostsInNetwork(
+ HostDiscoveryResponse discovery_callback) {
+ scanner_.FindHostsInNetwork(base::BindOnce(&SmbShareFinder::OnHostsDiscovered,
+ AsWeakPtr(),
+ std::move(discovery_callback)));
}
void SmbShareFinder::RegisterHostLocator(std::unique_ptr<HostLocator> locator) {
@@ -36,17 +46,25 @@
return url.ReplaceHost(ip_address);
}
-void SmbShareFinder::OnHostsFound(GatherSharesResponse callback,
+void SmbShareFinder::OnHostsDiscovered(HostDiscoveryResponse discovery_callback,
+ bool success,
+ const HostMap& hosts) {
+ std::move(discovery_callback).Run();
+}
+
+void SmbShareFinder::OnHostsFound(HostDiscoveryResponse discovery_callback,
+ GatherSharesResponse shares_callback,
bool success,
const HostMap& hosts) {
+ std::move(discovery_callback).Run();
if (!success) {
LOG(ERROR) << "SmbShareFinder failed to find hosts";
- callback.Run(std::vector<SmbUrl>());
+ shares_callback.Run(std::vector<SmbUrl>());
return;
}
if (hosts.empty()) {
- callback.Run(std::vector<SmbUrl>());
+ shares_callback.Run(std::vector<SmbUrl>());
return;
}
@@ -57,18 +75,18 @@
client_->GetShares(
server_url, base::BindOnce(&SmbShareFinder::OnSharesFound, AsWeakPtr(),
- host_name, callback));
+ host_name, shares_callback));
}
}
void SmbShareFinder::OnSharesFound(
const std::string& host_name,
- GatherSharesResponse callback,
+ GatherSharesResponse shares_callback,
smbprovider::ErrorType error,
const smbprovider::DirectoryEntryListProto& entries) {
if (error != smbprovider::ErrorType::ERROR_OK) {
LOG(ERROR) << "Error finding shares: " << error;
- callback.Run(std::vector<SmbUrl>());
+ shares_callback.Run(std::vector<SmbUrl>());
return;
}
@@ -82,7 +100,7 @@
}
}
- callback.Run(shares);
+ shares_callback.Run(shares);
}
} // namespace smb_client
diff --git a/chrome/browser/chromeos/smb_client/smb_share_finder.h b/chrome/browser/chromeos/smb_client/smb_share_finder.h
index 3cb8aa5..321be7e 100644
--- a/chrome/browser/chromeos/smb_client/smb_share_finder.h
+++ b/chrome/browser/chromeos/smb_client/smb_share_finder.h
@@ -23,6 +23,9 @@
using GatherSharesResponse =
base::RepeatingCallback<void(const std::vector<SmbUrl>& shares_gathered)>;
+// The callback run to indicate the scan for hosts on the network is complete.
+using HostDiscoveryResponse = base::OnceClosure;
+
// This class is responsible for finding hosts in a network and getting the
// available shares for each host found.
class SmbShareFinder : public base::SupportsWeakPtr<SmbShareFinder> {
@@ -31,9 +34,16 @@
~SmbShareFinder();
// Gathers the hosts in the network using |scanner_| and gets the shares for
- // each of the hosts found. |callback| will be called once per host and will
- // contain the paths to the shares found (e.g. "smb://host/share").
- void GatherSharesInNetwork(GatherSharesResponse callback);
+ // each of the hosts found. |discovery_callback| runs once when host
+ // disovery is complete. |shares_callback| runs once per host and will contain
+ // the paths to the shares found (e.g. "smb://host/share").
+ void GatherSharesInNetwork(HostDiscoveryResponse discovery_callback,
+ GatherSharesResponse shares_callback);
+
+ // Gathers the hosts in the network using |scanner_|. Runs
+ // |discovery_callback| upon completion. No data is returned to the caller,
+ // but hosts are cached in |scanner_| and can be used for name resolution.
+ void DiscoverHostsInNetwork(HostDiscoveryResponse discovery_callback);
// Registers HostLocator |locator| to |scanner_|.
void RegisterHostLocator(std::unique_ptr<HostLocator> locator);
@@ -43,14 +53,20 @@
std::string GetResolvedUrl(const SmbUrl& url) const;
private:
+ // Handles the response from discovering hosts in the network.
+ void OnHostsDiscovered(HostDiscoveryResponse discovery_callback,
+ bool success,
+ const HostMap& hosts);
+
// Handles the response from finding hosts in the network.
- void OnHostsFound(GatherSharesResponse callback,
+ void OnHostsFound(HostDiscoveryResponse discovery_callback,
+ GatherSharesResponse shares_callback,
bool success,
const HostMap& hosts);
// Handles the response from getting shares for a given host.
void OnSharesFound(const std::string& host_name,
- GatherSharesResponse callback,
+ GatherSharesResponse shares_callback,
smbprovider::ErrorType error,
const smbprovider::DirectoryEntryListProto& entries);
diff --git a/chrome/browser/chromeos/smb_client/smb_share_finder_unittest.cc b/chrome/browser/chromeos/smb_client/smb_share_finder_unittest.cc
index 90a7f30..6cf9205 100644
--- a/chrome/browser/chromeos/smb_client/smb_share_finder_unittest.cc
+++ b/chrome/browser/chromeos/smb_client/smb_share_finder_unittest.cc
@@ -7,7 +7,8 @@
#include <algorithm>
#include <string>
-#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/smb_client/discovery/in_memory_host_locator.h"
#include "chrome/browser/chromeos/smb_client/smb_constants.h"
@@ -67,14 +68,22 @@
// Helper function when expecting shares to be found in the network.
void ExpectSharesFound() {
- share_finder_->GatherSharesInNetwork(base::BindRepeating(
- &SmbShareFinderTest::SharesFoundCallback, base::Unretained(this)));
+ share_finder_->GatherSharesInNetwork(
+ base::BindOnce(&SmbShareFinderTest::HostsDiscoveredCallback,
+ base::Unretained(this)),
+ base::BindRepeating(&SmbShareFinderTest::SharesFoundCallback,
+ base::Unretained(this)));
+ EXPECT_TRUE(discovery_callback_called_);
}
// Helper function when expecting no shares to be found in the network.
void ExpectNoSharesFound() {
- share_finder_->GatherSharesInNetwork(base::BindRepeating(
- &SmbShareFinderTest::EmptySharesCallback, base::Unretained(this)));
+ share_finder_->GatherSharesInNetwork(
+ base::BindOnce(&SmbShareFinderTest::HostsDiscoveredCallback,
+ base::Unretained(this)),
+ base::BindRepeating(&SmbShareFinderTest::EmptySharesCallback,
+ base::Unretained(this)));
+ EXPECT_TRUE(discovery_callback_called_);
}
// Helper function that expects expected_shares_ to be empty.
@@ -86,7 +95,9 @@
}
private:
- // Removes shares discovered from expected_shares_.
+ void HostsDiscoveredCallback() { discovery_callback_called_ = true; }
+
+ // Removes shares discovered from |expected_shares_|.
void SharesFoundCallback(const std::vector<SmbUrl>& shares_found) {
EXPECT_GE(shares_found.size(), 0u);
@@ -99,6 +110,8 @@
EXPECT_EQ(0u, shares_found.size());
}
+ bool discovery_callback_called_ = false;
+
// Keeps track of expected shares across multiple hosts.
std::set<std::string> expected_shares_;
