Allow to cancel a proxy resolution request.
After calling GetProxiesForUrl(), there was no way to prevent the
proxy resolver from calling the passed callback once the response is
ready. This implies that the object passed in the callback (normally
as the "data" pointer) must be kept alive until the callback comes
back.
This patch allows to cancel an ongoing request and converts the passed
callback to a base::Callback instead of using a raw pointer.
Bug: 34178297
Test: Added unittests.
Change-Id: Ie544d0230fd0c2dc85c6b9eaca9b5b13702516fa
diff --git a/chrome_browser_proxy_resolver_unittest.cc b/chrome_browser_proxy_resolver_unittest.cc
index bb5193e..24f8a2e 100644
--- a/chrome_browser_proxy_resolver_unittest.cc
+++ b/chrome_browser_proxy_resolver_unittest.cc
@@ -77,7 +77,6 @@
void RunTest(bool chrome_replies, bool chrome_alive);
- private:
brillo::FakeMessageLoop loop_{nullptr};
// Local pointers to the mocks. The instances are owned by the
@@ -107,15 +106,14 @@
}
namespace {
-void CheckResponseResolved(const deque<string>& proxies,
- void* /* pirv_data */) {
+void CheckResponseResolved(const deque<string>& proxies) {
EXPECT_EQ(2U, proxies.size());
EXPECT_EQ("socks5://192.168.52.83:5555", proxies[0]);
EXPECT_EQ(kNoProxy, proxies[1]);
MessageLoop::current()->BreakLoop();
}
-void CheckResponseNoReply(const deque<string>& proxies, void* /* pirv_data */) {
+void CheckResponseNoReply(const deque<string>& proxies) {
EXPECT_EQ(1U, proxies.size());
EXPECT_EQ(kNoProxy, proxies[0]);
MessageLoop::current()->BreakLoop();
@@ -138,9 +136,9 @@
_))
.WillOnce(Return(chrome_alive));
- ProxiesResolvedFn get_proxies_response = &CheckResponseNoReply;
+ ProxiesResolvedFn get_proxies_response = base::Bind(&CheckResponseNoReply);
if (chrome_replies) {
- get_proxies_response = &CheckResponseResolved;
+ get_proxies_response = base::Bind(&CheckResponseResolved);
MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&ChromeBrowserProxyResolverTest::SendReplySignal,
@@ -151,7 +149,8 @@
base::TimeDelta::FromSeconds(1));
}
- EXPECT_TRUE(resolver_.GetProxiesForUrl(kUrl, get_proxies_response, nullptr));
+ EXPECT_NE(kProxyRequestIdNull,
+ resolver_.GetProxiesForUrl(kUrl, get_proxies_response));
MessageLoop::current()->Run();
}
@@ -208,4 +207,32 @@
RunTest(false, false);
}
+TEST_F(ChromeBrowserProxyResolverTest, CancelCallbackTest) {
+ int called = 0;
+ auto callback = base::Bind(
+ [](int* called, const deque<string>& proxies) { (*called)++; }, &called);
+
+ EXPECT_CALL(*service_interface_mock_, ResolveNetworkProxy(_, _, _, _, _))
+ .Times(4)
+ .WillRepeatedly(Return(true));
+
+ EXPECT_NE(kProxyRequestIdNull,
+ resolver_.GetProxiesForUrl("http://urlA", callback));
+ ProxyRequestId req_b = resolver_.GetProxiesForUrl("http://urlB", callback);
+ // Note that we add twice the same url.
+ ProxyRequestId req_c = resolver_.GetProxiesForUrl("http://urlC", callback);
+ EXPECT_NE(kProxyRequestIdNull,
+ resolver_.GetProxiesForUrl("http://urlC", callback));
+
+ EXPECT_EQ(0, called);
+ EXPECT_TRUE(resolver_.CancelProxyRequest(req_b));
+ EXPECT_TRUE(resolver_.CancelProxyRequest(req_c));
+ // Canceling the same request twice should fail even if there's another
+ // request for the same URL.
+ EXPECT_FALSE(resolver_.CancelProxyRequest(req_c));
+
+ loop_.Run();
+ EXPECT_EQ(2, called);
+}
+
} // namespace chromeos_update_engine