AU: Make proxy resolution asynchronous.
This doesn't change proxy resolution overall (we still use settings
stored in the session manager), but it changes the implementation in
the updater to be asynchronous. The clients of the proxy resolver now
give a callback to be called when the proxies are known.
This is anticipation of a switch to using Chrome to resolve proxies,
which will need to be asynchronous.
BUG=chromium-os:12079
TEST=unittests; tested update on device w/ and w/o proxy settings
Review URL: http://codereview.chromium.org/6516026
Change-Id: Icc5c08e3abf4381be55d8d555020d4c630a07fd6
diff --git a/chrome_proxy_resolver.cc b/chrome_proxy_resolver.cc
index 66bc5dc..1deab11 100644
--- a/chrome_proxy_resolver.cc
+++ b/chrome_proxy_resolver.cc
@@ -10,6 +10,9 @@
#include "update_engine/utils.h"
+using google::protobuf::Closure;
+using google::protobuf::NewCallback;
+using std::deque;
using std::string;
using std::vector;
@@ -24,16 +27,32 @@
bool ChromeProxyResolver::GetProxiesForUrl(
const std::string& url,
- std::deque<std::string>* out_proxies) {
+ ProxiesResolvedFn callback,
+ void* data) {
+ ChromeProxyResolverClosureArgs args;
+ args.url = url;
+ args.callback = callback;
+ args.data = data;
+ Closure* closure = NewCallback(this,
+ &ChromeProxyResolver::GetProxiesForUrlCallback,
+ args);
+ g_idle_add(utils::GlibRunClosure, closure);
+ return true;
+}
+
+void ChromeProxyResolver::GetProxiesForUrlCallback(
+ ChromeProxyResolverClosureArgs args) {
+ deque<string> proxies;
// First, query dbus for the currently stored settings
DBusGProxy* proxy = DbusProxy();
- TEST_AND_RETURN_FALSE(proxy);
- string json_settings;
- TEST_AND_RETURN_FALSE(GetJsonProxySettings(proxy, &json_settings));
- LOG(INFO) << "got settings:" << json_settings;
- TEST_AND_RETURN_FALSE(
- GetProxiesForUrlWithSettings(url, json_settings, out_proxies));
- return true;
+ if (proxy) {
+ string json_settings;
+ if (GetJsonProxySettings(proxy, &json_settings)) {
+ LOG(INFO) << "got settings:" << json_settings;
+ GetProxiesForUrlWithSettings(args.url, json_settings, &proxies);
+ }
+ }
+ (*args.callback)(proxies, args.data);
}
bool ChromeProxyResolver::GetJsonProxySettings(DBusGProxy* proxy,
diff --git a/chrome_proxy_resolver.h b/chrome_proxy_resolver.h
index 28d96cd..2df751e 100644
--- a/chrome_proxy_resolver.h
+++ b/chrome_proxy_resolver.h
@@ -27,13 +27,20 @@
// Currently only supports manual settings, not PAC files or autodetected
// settings.
+struct ChromeProxyResolverClosureArgs {
+ std::string url;
+ ProxiesResolvedFn callback;
+ void* data;
+};
+
class ChromeProxyResolver : public ProxyResolver {
public:
explicit ChromeProxyResolver(DbusGlibInterface* dbus) : dbus_(dbus) {}
virtual ~ChromeProxyResolver() {}
virtual bool GetProxiesForUrl(const std::string& url,
- std::deque<std::string>* out_proxies);
+ ProxiesResolvedFn callback,
+ void* data);
// Get the curl proxy type for a given proxy url. Returns true on success.
// Note: if proxy is kNoProxy, this will return false.
@@ -42,6 +49,10 @@
private:
FRIEND_TEST(ChromeProxyResolverTest, GetProxiesForUrlWithSettingsTest);
+ // Closure callback, so we can pretend we need to wait on the main loop
+ // before returing proxies to the client.
+ void GetProxiesForUrlCallback(ChromeProxyResolverClosureArgs args);
+
// Fetches a dbus proxy to session manager. Returns NULL on failure.
DBusGProxy* DbusProxy();
diff --git a/chrome_proxy_resolver_unittest.cc b/chrome_proxy_resolver_unittest.cc
index 1cdd5bf..cab2d33 100644
--- a/chrome_proxy_resolver_unittest.cc
+++ b/chrome_proxy_resolver_unittest.cc
@@ -81,6 +81,16 @@
EXPECT_EQ(kNoProxy, out[0]);
}
+namespace {
+void DbusInterfaceTestResolved(const std::deque<std::string>& proxies,
+ void* data) {
+ EXPECT_EQ(2, proxies.size());
+ EXPECT_EQ("socks5://192.168.52.83:5555", proxies[0]);
+ EXPECT_EQ(kNoProxy, proxies[1]);
+ g_main_loop_quit(reinterpret_cast<GMainLoop*>(data));
+}
+}
+
TEST(ChromeProxyResolverTest, DbusInterfaceTest) {
long number = 1;
DBusGConnection* kMockSystemBus =
@@ -119,12 +129,13 @@
SetArgumentPointee<9>(ret_array),
Return(TRUE)));
- deque<string> proxies;
+ GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
+
EXPECT_TRUE(resolver.GetProxiesForUrl("http://user:pass@foo.com:22",
- &proxies));
- EXPECT_EQ(2, proxies.size());
- EXPECT_EQ("socks5://192.168.52.83:5555", proxies[0]);
- EXPECT_EQ(kNoProxy, proxies[1]);
+ &DbusInterfaceTestResolved,
+ loop));
+ g_main_loop_run(loop);
+ g_main_loop_unref(loop);
}
TEST(ChromeProxyResolverTest, GetProxyTypeTest) {
diff --git a/http_fetcher.cc b/http_fetcher.cc
index 5d00660..7d9297a 100644
--- a/http_fetcher.cc
+++ b/http_fetcher.cc
@@ -4,11 +4,19 @@
#include "update_engine/http_fetcher.h"
+using google::protobuf::Closure;
using std::deque;
using std::string;
namespace chromeos_update_engine {
+HttpFetcher::~HttpFetcher() {
+ if (no_resolver_idle_id_) {
+ g_source_remove(no_resolver_idle_id_);
+ no_resolver_idle_id_ = 0;
+ }
+}
+
void HttpFetcher::SetPostData(const void* data, size_t size) {
post_data_set_ = true;
post_data_.clear();
@@ -17,15 +25,24 @@
}
// Proxy methods to set the proxies, then to pop them off.
-void HttpFetcher::ResolveProxiesForUrl(const string& url) {
+bool HttpFetcher::ResolveProxiesForUrl(const string& url, Closure* callback) {
if (!proxy_resolver_) {
LOG(INFO) << "Not resolving proxies (no proxy resolver).";
- return;
+ no_resolver_idle_id_ = g_idle_add(utils::GlibRunClosure, callback);
+ return true;
}
- deque<string> proxies;
- if (proxy_resolver_->GetProxiesForUrl(url, &proxies)) {
+ callback_ = callback;
+ return proxy_resolver_->GetProxiesForUrl(url,
+ &HttpFetcher::StaticProxiesResolved,
+ this);
+}
+
+void HttpFetcher::ProxiesResolved(const std::deque<std::string>& proxies) {
+ no_resolver_idle_id_ = 0;
+ if (!proxies.empty())
SetProxies(proxies);
- }
+ callback_->Run();
+ callback_ = NULL;
}
} // namespace chromeos_update_engine
diff --git a/http_fetcher.h b/http_fetcher.h
old mode 100644
new mode 100755
index e4f791a..f4ee9b7
--- a/http_fetcher.h
+++ b/http_fetcher.h
@@ -12,6 +12,7 @@
#include <base/basictypes.h>
#include <base/logging.h>
#include <glib.h>
+#include <google/protobuf/stubs/common.h>
#include "update_engine/proxy_resolver.h"
@@ -36,8 +37,10 @@
http_response_code_(0),
delegate_(NULL),
proxies_(1, kNoProxy),
- proxy_resolver_(proxy_resolver) {}
- virtual ~HttpFetcher() {}
+ proxy_resolver_(proxy_resolver),
+ no_resolver_idle_id_(0),
+ callback_(NULL) {}
+ virtual ~HttpFetcher();
void set_delegate(HttpFetcherDelegate* delegate) { delegate_ = delegate; }
HttpFetcherDelegate* delegate() const { return delegate_; }
@@ -48,7 +51,9 @@
void SetPostData(const void* data, size_t size);
// Proxy methods to set the proxies, then to pop them off.
- void ResolveProxiesForUrl(const std::string& url);
+ // Returns true on success.
+ bool ResolveProxiesForUrl(const std::string& url,
+ google::protobuf::Closure* callback);
void SetProxies(const std::deque<std::string>& proxies) {
proxies_ = proxies;
@@ -110,10 +115,23 @@
// Proxy servers
std::deque<std::string> proxies_;
-
+
ProxyResolver* const proxy_resolver_;
+ // The ID of the idle callback, used when we have no proxy resolver.
+ guint no_resolver_idle_id_;
+
+ // Callback for when we are resolving proxies
+ google::protobuf::Closure* callback_;
+
private:
+ // Callback from the proxy resolver
+ void ProxiesResolved(const std::deque<std::string>& proxies);
+ static void StaticProxiesResolved(const std::deque<std::string>& proxies,
+ void* data) {
+ reinterpret_cast<HttpFetcher*>(data)->ProxiesResolved(proxies);
+ }
+
DISALLOW_COPY_AND_ASSIGN(HttpFetcher);
};
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index ccda0b6..8170b7b 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -316,15 +316,12 @@
typename TestFixture::HttpServer server;
ASSERT_TRUE(server.started_);
- GSource* timeout_source_;
- timeout_source_ = g_timeout_source_new(0); // ms
- g_source_set_callback(timeout_source_, UnpausingTimeoutCallback, &delegate,
- NULL);
- g_source_attach(timeout_source_, NULL);
+
+ guint callback_id = g_timeout_add(500, UnpausingTimeoutCallback, &delegate);
fetcher->BeginTransfer(this->BigUrl());
g_main_loop_run(loop);
- g_source_destroy(timeout_source_);
+ g_source_remove(callback_id);
}
g_main_loop_unref(loop);
}
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 16d0239..e46bf8e 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -14,6 +14,7 @@
#include "update_engine/flimflam_proxy.h"
#include "update_engine/utils.h"
+using google::protobuf::NewCallback;
using std::max;
using std::make_pair;
using std::string;
@@ -151,14 +152,25 @@
// Begins the transfer, which must not have already been started.
void LibcurlHttpFetcher::BeginTransfer(const std::string& url) {
+ CHECK(!transfer_in_progress_);
+ url_ = url;
+ if (!ResolveProxiesForUrl(
+ url_,
+ NewCallback(this, &LibcurlHttpFetcher::ProxiesResolved))) {
+ LOG(ERROR) << "Couldn't resolve proxies";
+ if (delegate_)
+ delegate_->TransferComplete(this, false);
+ }
+}
+
+void LibcurlHttpFetcher::ProxiesResolved() {
transfer_size_ = -1;
resume_offset_ = 0;
retry_count_ = 0;
no_network_retry_count_ = 0;
http_response_code_ = 0;
terminate_requested_ = false;
- ResolveProxiesForUrl(url);
- ResumeTransfer(url);
+ ResumeTransfer(url_);
CurlPerformOnce();
}
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index c5f784b..177664b 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -91,6 +91,10 @@
}
private:
+ // Callback for when proxy resolution has completed. This begins the
+ // transfer.
+ void ProxiesResolved();
+
// Asks libcurl for the http response code and stores it in the object.
void GetHttpResponseCode();
diff --git a/proxy_resolver.cc b/proxy_resolver.cc
index 4ec8d1e..26dff7b 100644
--- a/proxy_resolver.cc
+++ b/proxy_resolver.cc
@@ -11,11 +11,32 @@
const char kNoProxy[] = "direct://";
-bool DirectProxyResolver::GetProxiesForUrl(const string& url,
- deque<string>* out_proxies) {
- out_proxies->clear();
- out_proxies->push_back(kNoProxy);
+DirectProxyResolver::~DirectProxyResolver() {
+ if (idle_callback_id_) {
+ g_source_remove(idle_callback_id_);
+ idle_callback_id_ = 0;
+ }
+}
+
+bool DirectProxyResolver::GetProxiesForUrl(const std::string& url,
+ ProxiesResolvedFn callback,
+ void* data) {
+ google::protobuf::Closure* closure =
+ google::protobuf::NewCallback(this,
+ &DirectProxyResolver::ReturnCallback,
+ callback,
+ data);
+ idle_callback_id_ = g_idle_add(utils::GlibRunClosure, closure);
return true;
}
+void DirectProxyResolver::ReturnCallback(ProxiesResolvedFn callback,
+ void* data) {
+ idle_callback_id_ = 0;
+ std::deque<std::string> proxies;
+ proxies.push_back(kNoProxy);
+ (*callback)(proxies, data);
+}
+
+
} // namespace chromeos_update_engine
diff --git a/proxy_resolver.h b/proxy_resolver.h
index af228d2..53fd920 100644
--- a/proxy_resolver.h
+++ b/proxy_resolver.h
@@ -5,28 +5,39 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_PROXY_RESOLVER_H__
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_PROXY_RESOLVER_H__
-#include <base/logging.h>
#include <deque>
#include <string>
+#include <base/logging.h>
+#include <google/protobuf/stubs/common.h>
+
+#include "update_engine/utils.h"
+
namespace chromeos_update_engine {
extern const char kNoProxy[];
+// Callback for a call to GetProxiesForUrl().
+// Resultant proxies are in |out_proxy|. Each will be in one of the
+// following forms:
+// http://<host>[:<port>] - HTTP proxy
+// socks{4,5}://<host>[:<port>] - SOCKS4/5 proxy
+// kNoProxy - no proxy
+typedef void (*ProxiesResolvedFn)(const std::deque<std::string>& proxies,
+ void* data);
+
class ProxyResolver {
public:
ProxyResolver() {}
virtual ~ProxyResolver() {}
- // Stores a list of proxies for a given |url| in |out_proxy|.
- // Returns true on success. The resultant proxy will be in one of the
- // following forms:
- // http://<host>[:<port>] - HTTP proxy
- // socks{4,5}://<host>[:<port>] - SOCKS4/5 proxy
- // kNoProxy - no proxy
+ // Finds proxies for the given URL and returns them via the callback.
+ // |data| will be passed to the callback.
+ // Returns true on success.
virtual bool GetProxiesForUrl(const std::string& url,
- std::deque<std::string>* out_proxies) = 0;
+ ProxiesResolvedFn callback,
+ void* data) = 0;
private:
DISALLOW_COPY_AND_ASSIGN(ProxyResolver);
@@ -35,11 +46,18 @@
// Always says to not use a proxy
class DirectProxyResolver : public ProxyResolver {
public:
- DirectProxyResolver() {}
+ DirectProxyResolver() : idle_callback_id_(0) {}
+ virtual ~DirectProxyResolver();
virtual bool GetProxiesForUrl(const std::string& url,
- std::deque<std::string>* out_proxies);
+ ProxiesResolvedFn callback,
+ void* data);
private:
+ // The ID of the idle main loop callback
+ guint idle_callback_id_;
+
+ // The MainLoop callback, from here we return to the client.
+ void ReturnCallback(ProxiesResolvedFn callback, void* data);
DISALLOW_COPY_AND_ASSIGN(DirectProxyResolver);
};
diff --git a/utils.cc b/utils.cc
index 7206171..502b0d1 100644
--- a/utils.cc
+++ b/utils.cc
@@ -26,6 +26,7 @@
#include <base/string_util.h>
#include <base/logging.h>
#include <cros_boot_mode/boot_mode.h>
+#include <google/protobuf/stubs/common.h>
#include <rootdev/rootdev.h>
#include "update_engine/file_writer.h"
@@ -551,6 +552,13 @@
return base::RandInt(min, max);
}
+gboolean GlibRunClosure(gpointer data) {
+ google::protobuf::Closure* callback =
+ reinterpret_cast<google::protobuf::Closure*>(data);
+ callback->Run();
+ return FALSE;
+}
+
const char* const kStatefulPartition = "/mnt/stateful_partition";
} // namespace utils
diff --git a/utils.h b/utils.h
index 74711a8..a17f241 100644
--- a/utils.h
+++ b/utils.h
@@ -243,6 +243,9 @@
// success, false otherwise.
bool SetProcessPriority(ProcessPriority priority);
+// Assumes data points to a Closure. Runs it and returns FALSE;
+gboolean GlibRunClosure(gpointer data);
+
} // namespace utils
// Class to unmount FS when object goes out of scope