Remove ProxyResolver from update_engine
Current ProxyResolver code only supports no-proxy. Therefore it is
barely any use. Remove current implementation for a better one.
Test: th
Bug: 235816007
Change-Id: Ieb46dedf6c6ea86b11c81d7691c2adb578d3d97d
diff --git a/Android.bp b/Android.bp
index 7049acd..a3b45de 100644
--- a/Android.bp
+++ b/Android.bp
@@ -222,7 +222,6 @@
"common/hwid_override.cc",
"common/multi_range_http_fetcher.cc",
"common/prefs.cc",
- "common/proxy_resolver.cc",
"common/subprocess.cc",
"common/terminator.cc",
"common/utils.cc",
@@ -968,17 +967,14 @@
"common/subprocess.cc",
"common/test_utils.cc",
"common/utils.cc",
- "common/proxy_resolver.cc",
"libcurl_http_fetcher.cc",
"payload_consumer/certificate_parser_android.cc",
"payload_consumer/payload_verifier.cc",
"payload_generator/payload_signer.cc",
"update_status_utils.cc",
-
"certificate_checker_unittest.cc",
"common/http_fetcher_unittest.cc",
"common/mock_http_fetcher.cc",
- "common/proxy_resolver_unittest.cc",
"common/subprocess_unittest.cc",
"libcurl_http_fetcher_unittest.cc",
"payload_consumer/certificate_parser_android_unittest.cc",
diff --git a/aosp/update_attempter_android.cc b/aosp/update_attempter_android.cc
index 1d35b5e..9c16329 100644
--- a/aosp/update_attempter_android.cc
+++ b/aosp/update_attempter_android.cc
@@ -35,6 +35,7 @@
#include "update_engine/aosp/cleanup_previous_update_action.h"
#include "update_engine/common/constants.h"
#include "update_engine/common/daemon_state_interface.h"
+#include "update_engine/common/clock.h"
#include "update_engine/common/download_action.h"
#include "update_engine/common/error_code_utils.h"
#include "update_engine/common/file_fetcher.h"
@@ -42,7 +43,6 @@
#include "update_engine/common/network_selector.h"
#include "update_engine/common/utils.h"
#include "update_engine/metrics_utils.h"
-#include "update_engine/payload_consumer/certificate_parser_interface.h"
#include "update_engine/payload_consumer/delta_performer.h"
#include "update_engine/payload_consumer/file_descriptor.h"
#include "update_engine/payload_consumer/file_descriptor_utils.h"
@@ -328,8 +328,7 @@
#ifdef _UE_SIDELOAD
LOG(FATAL) << "Unsupported sideload URI: " << payload_url;
#else
- LibcurlHttpFetcher* libcurl_fetcher =
- new LibcurlHttpFetcher(&proxy_resolver_, hardware_);
+ LibcurlHttpFetcher* libcurl_fetcher = new LibcurlHttpFetcher(hardware_);
libcurl_fetcher->set_server_to_check(ServerToCheck::kDownload);
fetcher = libcurl_fetcher;
#endif // _UE_SIDELOAD
diff --git a/aosp/update_attempter_android.h b/aosp/update_attempter_android.h
index 5d832e0..f3feed0 100644
--- a/aosp/update_attempter_android.h
+++ b/aosp/update_attempter_android.h
@@ -31,15 +31,13 @@
#include "update_engine/client_library/include/update_engine/update_status.h"
#include "update_engine/common/action_processor.h"
#include "update_engine/common/boot_control_interface.h"
-#include "update_engine/common/clock.h"
+#include "update_engine/common/clock_interface.h"
#include "update_engine/common/daemon_state_interface.h"
#include "update_engine/common/download_action.h"
#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/metrics_reporter_interface.h"
#include "update_engine/common/network_selector_interface.h"
#include "update_engine/common/prefs_interface.h"
-#include "update_engine/common/service_observer_interface.h"
-#include "update_engine/metrics_utils.h"
#include "update_engine/payload_consumer/filesystem_verifier_action.h"
#include "update_engine/payload_consumer/postinstall_runner_action.h"
@@ -248,9 +246,6 @@
// set back in the middle of an update.
base::TimeTicks last_notify_time_;
- // Only direct proxy supported.
- DirectProxyResolver proxy_resolver_;
-
// The processor for running Actions.
std::unique_ptr<ActionProcessor> processor_;
diff --git a/aosp/update_attempter_android_unittest.cc b/aosp/update_attempter_android_unittest.cc
index 458c224..b97ae21 100644
--- a/aosp/update_attempter_android_unittest.cc
+++ b/aosp/update_attempter_android_unittest.cc
@@ -33,28 +33,18 @@
#include <fs_mgr.h>
#include <liblp/liblp.h>
-#include "update_engine/aosp/boot_control_android.h"
#include "update_engine/aosp/daemon_state_android.h"
#include "update_engine/common/constants.h"
#include "update_engine/common/fake_boot_control.h"
#include "update_engine/common/fake_clock.h"
#include "update_engine/common/fake_hardware.h"
#include "update_engine/common/fake_prefs.h"
-#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/mock_action_processor.h"
#include "update_engine/common/mock_metrics_reporter.h"
-#include "update_engine/common/prefs.h"
-#include "update_engine/common/test_utils.h"
-#include "update_engine/common/testing_constants.h"
#include "update_engine/common/utils.h"
+#include "update_engine/metrics_utils.h"
#include "update_engine/payload_consumer/install_plan.h"
-#include "update_engine/payload_consumer/payload_constants.h"
-#include "update_engine/payload_generator/delta_diff_generator.h"
-#include "update_engine/payload_generator/extent_ranges.h"
-#include "update_engine/payload_generator/payload_file.h"
-#include "update_engine/payload_generator/payload_signer.h"
#include "update_engine/update_metadata.pb.h"
-#include "update_engine/update_status_utils.h"
using base::Time;
using base::TimeDelta;
diff --git a/common/constants.h b/common/constants.h
index 8c07fcf..53c5bba 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -191,6 +191,9 @@
static constexpr const auto& kXGoogleUpdateUpdater = "X-Goog-Update-Updater";
static constexpr const auto& kXGoogleUpdateSessionId = "X-Goog-SessionId";
+// Proxy URL for direction connection
+static constexpr const auto& kNoProxy = "direct://";
+
// A download source is any combination of protocol and server (that's of
// interest to us when looking at UMA metrics) using which we may download
// the payload.
diff --git a/common/download_action.h b/common/download_action.h
index ee6c8be..dd73a9d 100644
--- a/common/download_action.h
+++ b/common/download_action.h
@@ -25,7 +25,6 @@
#include <string>
#include <utility>
-#include "update_engine/common/action.h"
#include "update_engine/common/boot_control_interface.h"
#include "update_engine/common/http_fetcher.h"
#include "update_engine/common/multi_range_http_fetcher.h"
diff --git a/common/file_fetcher.h b/common/file_fetcher.h
index 0f034e3..cc0e880 100644
--- a/common/file_fetcher.h
+++ b/common/file_fetcher.h
@@ -37,7 +37,7 @@
// Returns whether the passed url is supported.
static bool SupportedUrl(const std::string& url);
- FileFetcher() : HttpFetcher(nullptr) {}
+ FileFetcher() : HttpFetcher() {}
// Cleans up all internal state. Does not notify delegate.
~FileFetcher() override;
diff --git a/common/http_fetcher.cc b/common/http_fetcher.cc
index 5a98dfc..34caba4 100644
--- a/common/http_fetcher.cc
+++ b/common/http_fetcher.cc
@@ -25,9 +25,7 @@
namespace chromeos_update_engine {
-HttpFetcher::~HttpFetcher() {
- CancelProxyResolution();
-}
+HttpFetcher::~HttpFetcher() {}
void HttpFetcher::SetPostData(const void* data,
size_t size,
@@ -43,51 +41,4 @@
SetPostData(data, size, kHttpContentTypeUnspecified);
}
-// Proxy methods to set the proxies, then to pop them off.
-void HttpFetcher::ResolveProxiesForUrl(const string& url,
- const Closure& callback) {
- CHECK_EQ(static_cast<Closure*>(nullptr), callback_.get());
- callback_.reset(new Closure(callback));
-
- if (!proxy_resolver_) {
- LOG(INFO) << "Not resolving proxies (no proxy resolver).";
- no_resolver_idle_id_ = MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&HttpFetcher::NoProxyResolverCallback,
- base::Unretained(this)));
- return;
- }
- proxy_request_ = proxy_resolver_->GetProxiesForUrl(
- url, base::Bind(&HttpFetcher::ProxiesResolved, base::Unretained(this)));
-}
-
-void HttpFetcher::NoProxyResolverCallback() {
- no_resolver_idle_id_ = MessageLoop::kTaskIdNull;
- ProxiesResolved(deque<string>());
-}
-
-void HttpFetcher::ProxiesResolved(const deque<string>& proxies) {
- proxy_request_ = kProxyRequestIdNull;
- if (!proxies.empty())
- SetProxies(proxies);
- CHECK(callback_.get()) << "ProxiesResolved but none pending.";
- Closure* callback = callback_.release();
- // This may indirectly call back into ResolveProxiesForUrl():
- callback->Run();
- delete callback;
-}
-
-bool HttpFetcher::CancelProxyResolution() {
- bool ret = false;
- if (no_resolver_idle_id_ != MessageLoop::kTaskIdNull) {
- ret = MessageLoop::current()->CancelTask(no_resolver_idle_id_);
- no_resolver_idle_id_ = MessageLoop::kTaskIdNull;
- }
- if (proxy_request_ && proxy_resolver_) {
- ret = proxy_resolver_->CancelProxyRequest(proxy_request_) || ret;
- proxy_request_ = kProxyRequestIdNull;
- }
- return ret;
-}
-
} // namespace chromeos_update_engine
diff --git a/common/http_fetcher.h b/common/http_fetcher.h
index 80985af..f32c01d 100644
--- a/common/http_fetcher.h
+++ b/common/http_fetcher.h
@@ -26,10 +26,11 @@
#include <base/logging.h>
#include <base/macros.h>
#include <brillo/message_loops/message_loop.h>
+#include <brillo/secure_blob.h>
+#include "update_engine/common/constants.h"
+#include "update_engine/common/error_code.h"
#include "update_engine/common/http_common.h"
-#include "update_engine/common/metrics_constants.h"
-#include "update_engine/common/proxy_resolver.h"
// This class is a simple wrapper around an HTTP library (libcurl). We can
// easily mock out this interface for testing.
@@ -46,12 +47,11 @@
// |proxy_resolver| is the resolver that will be consulted for proxy
// settings. It may be null, in which case direct connections will
// be used. Does not take ownership of the resolver.
- explicit HttpFetcher(ProxyResolver* proxy_resolver)
+ explicit HttpFetcher()
: post_data_set_(false),
http_response_code_(0),
delegate_(nullptr),
proxies_(1, kNoProxy),
- proxy_resolver_(proxy_resolver),
callback_(nullptr) {}
virtual ~HttpFetcher();
@@ -73,11 +73,7 @@
// Same without a specified Content-Type.
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,
- const base::Closure& callback);
-
- void SetProxies(const std::deque<std::string>& proxies) {
+ virtual void SetProxies(const std::deque<std::string>& proxies) {
proxies_ = proxies;
}
const std::string& GetCurrentProxy() const { return proxies_.front(); }
@@ -144,21 +140,14 @@
// Get the total number of bytes downloaded by fetcher.
virtual size_t GetBytesDownloaded() = 0;
- ProxyResolver* proxy_resolver() const { return proxy_resolver_; }
-
protected:
- // Cancels a proxy resolution in progress. The callback passed to
- // ResolveProxiesForUrl() will not be called. Returns whether there was a
- // pending proxy resolution to be canceled.
- bool CancelProxyResolution();
-
// The URL we're actively fetching from
std::string url_;
// POST data for the transfer, and whether or not it was ever set
bool post_data_set_;
brillo::Blob post_data_;
- HttpContentType post_content_type_;
+ HttpContentType post_content_type_{};
// The server's HTTP response code from the last transfer. This
// field should be set to 0 when a new transfer is initiated, and
@@ -175,12 +164,6 @@
// Proxy servers
std::deque<std::string> proxies_;
- ProxyResolver* const proxy_resolver_;
-
- // The ID of the idle callback, used when we have no proxy resolver.
- brillo::MessageLoop::TaskId no_resolver_idle_id_{
- brillo::MessageLoop::kTaskIdNull};
-
// Callback for when we are resolving proxies
std::unique_ptr<base::Closure> callback_;
@@ -192,10 +175,6 @@
// |proxy_resolver_|.
void NoProxyResolverCallback();
- // Stores the ongoing proxy request id if there is one, otherwise
- // kProxyRequestIdNull.
- ProxyRequestId proxy_request_{kProxyRequestIdNull};
-
DISALLOW_COPY_AND_ASSIGN(HttpFetcher);
};
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc
index bc8a325..c6f6c48 100644
--- a/common/http_fetcher_unittest.cc
+++ b/common/http_fetcher_unittest.cc
@@ -24,6 +24,7 @@
#include <string>
#include <utility>
#include <vector>
+#include <deque>
#include <base/bind.h>
#include <base/location.h>
@@ -50,14 +51,13 @@
#include <brillo/streams/file_stream.h>
#include <brillo/streams/stream.h>
#include <gtest/gtest.h>
+#include <gmock/gmock.h>
#include "update_engine/common/fake_hardware.h"
#include "update_engine/common/file_fetcher.h"
#include "update_engine/common/http_common.h"
#include "update_engine/common/mock_http_fetcher.h"
-#include "update_engine/common/mock_proxy_resolver.h"
#include "update_engine/common/multi_range_http_fetcher.h"
-#include "update_engine/common/proxy_resolver.h"
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
#include "update_engine/libcurl_http_fetcher.h"
@@ -200,53 +200,48 @@
AnyHttpFetcherFactory() {}
virtual ~AnyHttpFetcherFactory() {}
- virtual HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) = 0;
+ virtual HttpFetcher* NewLargeFetcher() = 0;
HttpFetcher* NewLargeFetcher(size_t num_proxies) {
- proxy_resolver_.set_num_proxies(num_proxies);
- return NewLargeFetcher(&proxy_resolver_);
- }
- HttpFetcher* NewLargeFetcher() { return NewLargeFetcher(1); }
+ auto res = NewLargeFetcher();
- virtual HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) = 0;
- HttpFetcher* NewSmallFetcher() {
- proxy_resolver_.set_num_proxies(1);
- return NewSmallFetcher(&proxy_resolver_);
+ res->SetProxies(std::deque<std::string>(num_proxies, kNoProxy));
+ return res;
}
- virtual string BigUrl(in_port_t port) const { return kUnusedUrl; }
- virtual string SmallUrl(in_port_t port) const { return kUnusedUrl; }
- virtual string ErrorUrl(in_port_t port) const { return kUnusedUrl; }
+ virtual HttpFetcher* NewSmallFetcher() = 0;
- virtual bool IsMock() const = 0;
- virtual bool IsMulti() const = 0;
- virtual bool IsHttpSupported() const = 0;
- virtual bool IsFileFetcher() const = 0;
+ virtual string BigUrl(in_port_t port) const { return kUnusedUrl; }
+ virtual string SmallUrl(in_port_t port) const { return kUnusedUrl; }
+ virtual string ErrorUrl(in_port_t port) const { return kUnusedUrl; }
- virtual void IgnoreServerAborting(HttpServer* server) const {}
+ virtual bool IsMock() const = 0;
+ virtual bool IsMulti() const = 0;
+ virtual bool IsHttpSupported() const = 0;
+ virtual bool IsFileFetcher() const = 0;
- virtual HttpServer* CreateServer() = 0;
+ virtual void IgnoreServerAborting(HttpServer* server) const {}
- FakeHardware* fake_hardware() { return &fake_hardware_; }
+ virtual HttpServer* CreateServer() = 0;
- protected:
- DirectProxyResolver proxy_resolver_;
- FakeHardware fake_hardware_;
+ FakeHardware* fake_hardware() { return &fake_hardware_; }
+
+ protected:
+ FakeHardware fake_hardware_;
};
class MockHttpFetcherFactory : public AnyHttpFetcherFactory {
public:
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewLargeFetcher;
- HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override {
+ HttpFetcher* NewLargeFetcher() override {
brillo::Blob big_data(1000000);
- return new MockHttpFetcher(
- big_data.data(), big_data.size(), proxy_resolver);
+ return new MockHttpFetcher(big_data.data(), big_data.size());
}
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewSmallFetcher;
- HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
- return new MockHttpFetcher("x", 1, proxy_resolver);
+ HttpFetcher* NewSmallFetcher() override {
+ return new MockHttpFetcher("x", 1);
}
bool IsMock() const override { return true; }
@@ -261,9 +256,8 @@
public:
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewLargeFetcher;
- HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override {
- LibcurlHttpFetcher* ret =
- new LibcurlHttpFetcher(proxy_resolver, &fake_hardware_);
+ HttpFetcher* NewLargeFetcher() override {
+ LibcurlHttpFetcher* ret = new LibcurlHttpFetcher(&fake_hardware_);
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
@@ -273,9 +267,8 @@
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewSmallFetcher;
- HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
- return NewLargeFetcher(proxy_resolver);
- }
+
+ HttpFetcher* NewSmallFetcher() override { return NewLargeFetcher(); }
string BigUrl(in_port_t port) const override {
return LocalServerUrlForPath(
@@ -304,9 +297,9 @@
public:
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewLargeFetcher;
- HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override {
- MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher(
- new LibcurlHttpFetcher(proxy_resolver, &fake_hardware_));
+ HttpFetcher* NewLargeFetcher() override {
+ MultiRangeHttpFetcher* ret =
+ new MultiRangeHttpFetcher(new LibcurlHttpFetcher(&fake_hardware_));
ret->ClearRanges();
ret->AddRange(0);
// Speed up test execution.
@@ -318,9 +311,7 @@
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewSmallFetcher;
- HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
- return NewLargeFetcher(proxy_resolver);
- }
+ HttpFetcher* NewSmallFetcher() override { return NewLargeFetcher(); }
bool IsMulti() const override { return true; }
};
@@ -329,15 +320,11 @@
public:
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewLargeFetcher;
- HttpFetcher* NewLargeFetcher(ProxyResolver* /* proxy_resolver */) override {
- return new FileFetcher();
- }
+ HttpFetcher* NewLargeFetcher() override { return new FileFetcher(); }
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewSmallFetcher;
- HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
- return NewLargeFetcher(proxy_resolver);
- }
+ HttpFetcher* NewSmallFetcher() override { return NewLargeFetcher(); }
string BigUrl(in_port_t port) const override {
static string big_contents = []() {
@@ -378,7 +365,7 @@
public:
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewLargeFetcher;
- HttpFetcher* NewLargeFetcher(ProxyResolver* /* proxy_resolver */) override {
+ HttpFetcher* NewLargeFetcher() override {
MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher(new FileFetcher());
ret->ClearRanges();
// FileFetcher doesn't support range with unspecified length.
@@ -392,9 +379,7 @@
// Necessary to unhide the definition in the base class.
using AnyHttpFetcherFactory::NewSmallFetcher;
- HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
- return NewLargeFetcher(proxy_resolver);
- }
+ HttpFetcher* NewSmallFetcher() override { return NewLargeFetcher(); }
bool IsMulti() const override { return true; }
};
@@ -652,23 +637,16 @@
TYPED_TEST(HttpFetcherTest, PauseWhileResolvingProxyTest) {
if (this->test_.IsMock() || !this->test_.IsHttpSupported())
return;
- MockProxyResolver mock_resolver;
- unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher(&mock_resolver));
+ unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
// Saved arguments from the proxy call.
- ProxiesResolvedFn proxy_callback;
- EXPECT_CALL(mock_resolver, GetProxiesForUrl("http://fake_url", _))
- .WillOnce(DoAll(SaveArg<1>(&proxy_callback), Return(true)));
fetcher->BeginTransfer("http://fake_url");
- testing::Mock::VerifyAndClearExpectations(&mock_resolver);
// Pausing and unpausing while resolving the proxy should not affect anything.
fetcher->Pause();
fetcher->Unpause();
fetcher->Pause();
// Proxy resolver comes back after we paused the fetcher.
- ASSERT_FALSE(proxy_callback.is_null());
- proxy_callback.Run({1, kNoProxy});
}
class AbortingHttpFetcherTestDelegate : public HttpFetcherDelegate {
@@ -740,21 +718,16 @@
TYPED_TEST(HttpFetcherTest, TerminateTransferWhileResolvingProxyTest) {
if (this->test_.IsMock() || !this->test_.IsHttpSupported())
return;
- MockProxyResolver mock_resolver;
- unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher(&mock_resolver));
+ unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
HttpFetcherTestDelegate delegate;
fetcher->set_delegate(&delegate);
- EXPECT_CALL(mock_resolver, GetProxiesForUrl(_, _)).WillOnce(Return(123));
fetcher->BeginTransfer("http://fake_url");
// Run the message loop until idle. This must call the MockProxyResolver with
// the request.
while (this->loop_.RunOnce(false)) {
}
- testing::Mock::VerifyAndClearExpectations(&mock_resolver);
-
- EXPECT_CALL(mock_resolver, CancelProxyRequest(123)).WillOnce(Return(true));
// Terminate the transfer right before the proxy resolution response.
fetcher->TerminateTransfer();
@@ -1300,7 +1273,7 @@
vector<pair<off_t, off_t>> ranges;
ranges.push_back(make_pair(0, 25));
ranges.push_back(make_pair(99, 0));
- MultiTest(this->test_.NewLargeFetcher(2),
+ MultiTest(this->test_.NewLargeFetcher(),
this->test_.fake_hardware(),
LocalServerUrlForPath(
server->GetPort(),
diff --git a/common/mock_http_fetcher.h b/common/mock_http_fetcher.h
index 3d7859b..0d4b130 100644
--- a/common/mock_http_fetcher.h
+++ b/common/mock_http_fetcher.h
@@ -42,10 +42,8 @@
public:
// The data passed in here is copied and then passed to the delegate after
// the transfer begins.
- MockHttpFetcher(const uint8_t* data,
- size_t size,
- ProxyResolver* proxy_resolver)
- : HttpFetcher(proxy_resolver),
+ MockHttpFetcher(const uint8_t* data, size_t size)
+ : HttpFetcher(),
sent_offset_(0),
timeout_id_(brillo::MessageLoop::kTaskIdNull),
paused_(false),
@@ -55,9 +53,8 @@
}
// Constructor overload for string data.
- MockHttpFetcher(const char* data, size_t size, ProxyResolver* proxy_resolver)
- : MockHttpFetcher(
- reinterpret_cast<const uint8_t*>(data), size, proxy_resolver) {}
+ MockHttpFetcher(const char* data, size_t size)
+ : MockHttpFetcher(reinterpret_cast<const uint8_t*>(data), size) {}
// Cleans up all internal state. Does not notify delegate
~MockHttpFetcher() override;
diff --git a/common/mock_proxy_resolver.h b/common/mock_proxy_resolver.h
deleted file mode 100644
index 67de68f..0000000
--- a/common/mock_proxy_resolver.h
+++ /dev/null
@@ -1,38 +0,0 @@
-//
-// Copyright (C) 2016 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#ifndef UPDATE_ENGINE_COMMON_MOCK_PROXY_RESOLVER_H_
-#define UPDATE_ENGINE_COMMON_MOCK_PROXY_RESOLVER_H_
-
-#include <string>
-
-#include <gmock/gmock.h>
-
-#include "update_engine/common/proxy_resolver.h"
-
-namespace chromeos_update_engine {
-
-class MockProxyResolver : public ProxyResolver {
- public:
- MOCK_METHOD2(GetProxiesForUrl,
- ProxyRequestId(const std::string& url,
- const ProxiesResolvedFn& callback));
- MOCK_METHOD1(CancelProxyRequest, bool(ProxyRequestId request));
-};
-
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_COMMON_MOCK_PROXY_RESOLVER_H_
diff --git a/common/multi_range_http_fetcher.h b/common/multi_range_http_fetcher.h
index ef32f0d..849ed32 100644
--- a/common/multi_range_http_fetcher.h
+++ b/common/multi_range_http_fetcher.h
@@ -46,7 +46,7 @@
public:
// Takes ownership of the passed in fetcher.
explicit MultiRangeHttpFetcher(HttpFetcher* base_fetcher)
- : HttpFetcher(base_fetcher->proxy_resolver()),
+ : HttpFetcher(),
base_fetcher_(base_fetcher),
base_fetcher_active_(false),
pending_transfer_ended_(false),
@@ -101,7 +101,8 @@
}
// TODO(deymo): Determine if this method should be virtual in HttpFetcher so
// this call is sent to the base_fetcher_.
- virtual void SetProxies(const std::deque<std::string>& proxies) {
+ void SetProxies(const std::deque<std::string>& proxies) override {
+ HttpFetcher::SetProxies(proxies);
base_fetcher_->SetProxies(proxies);
}
diff --git a/common/proxy_resolver.cc b/common/proxy_resolver.cc
deleted file mode 100644
index 0591c3e..0000000
--- a/common/proxy_resolver.cc
+++ /dev/null
@@ -1,66 +0,0 @@
-//
-// Copyright (C) 2010 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#include "update_engine/common/proxy_resolver.h"
-
-#include <base/bind.h>
-#include <base/location.h>
-
-using brillo::MessageLoop;
-using std::deque;
-using std::string;
-
-namespace chromeos_update_engine {
-
-const char kNoProxy[] = "direct://";
-const ProxyRequestId kProxyRequestIdNull = brillo::MessageLoop::kTaskIdNull;
-
-DirectProxyResolver::~DirectProxyResolver() {
- if (idle_callback_id_ != MessageLoop::kTaskIdNull) {
- // The DirectProxyResolver is instantiated as part of the UpdateAttempter
- // which is also instantiated by default by the FakeSystemState, even when
- // it is not used. We check the manage_shares_id_ before calling the
- // MessageLoop::current() since the unit test using a FakeSystemState may
- // have not define a MessageLoop for the current thread.
- MessageLoop::current()->CancelTask(idle_callback_id_);
- idle_callback_id_ = MessageLoop::kTaskIdNull;
- }
-}
-
-ProxyRequestId DirectProxyResolver::GetProxiesForUrl(
- const string& url, const ProxiesResolvedFn& callback) {
- idle_callback_id_ = MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&DirectProxyResolver::ReturnCallback,
- base::Unretained(this),
- callback));
- return idle_callback_id_;
-}
-
-bool DirectProxyResolver::CancelProxyRequest(ProxyRequestId request) {
- return MessageLoop::current()->CancelTask(request);
-}
-
-void DirectProxyResolver::ReturnCallback(const ProxiesResolvedFn& callback) {
- idle_callback_id_ = MessageLoop::kTaskIdNull;
-
- // Initialize proxy pool with as many proxies as indicated (all identical).
- deque<string> proxies(num_proxies_, kNoProxy);
-
- callback.Run(proxies);
-}
-
-} // namespace chromeos_update_engine
diff --git a/common/proxy_resolver.h b/common/proxy_resolver.h
deleted file mode 100644
index 9bd51fc..0000000
--- a/common/proxy_resolver.h
+++ /dev/null
@@ -1,98 +0,0 @@
-//
-// Copyright (C) 2010 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#ifndef UPDATE_ENGINE_COMMON_PROXY_RESOLVER_H_
-#define UPDATE_ENGINE_COMMON_PROXY_RESOLVER_H_
-
-#include <deque>
-#include <string>
-
-#include <base/logging.h>
-#include <brillo/message_loops/message_loop.h>
-
-#include "update_engine/common/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 base::Callback<void(const std::deque<std::string>& proxies)>
- ProxiesResolvedFn;
-
-// An id that identifies a proxy request. Used to cancel an ongoing request
-// before the callback is called.
-typedef brillo::MessageLoop::TaskId ProxyRequestId;
-
-// A constant identifying an invalid ProxyRequestId.
-extern const ProxyRequestId kProxyRequestIdNull;
-
-class ProxyResolver {
- public:
- ProxyResolver() {}
- virtual ~ProxyResolver() {}
-
- // Finds proxies for the given URL and returns them via the callback.
- // Returns the id of the pending request on success or kProxyRequestIdNull
- // otherwise.
- virtual ProxyRequestId GetProxiesForUrl(
- const std::string& url, const ProxiesResolvedFn& callback) = 0;
-
- // Cancel the proxy resolution request initiated by GetProxiesForUrl(). The
- // |request| value must be the one provided by GetProxiesForUrl().
- virtual bool CancelProxyRequest(ProxyRequestId request) = 0;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(ProxyResolver);
-};
-
-// Always says to not use a proxy
-class DirectProxyResolver : public ProxyResolver {
- public:
- DirectProxyResolver() = default;
- ~DirectProxyResolver() override;
- ProxyRequestId GetProxiesForUrl(const std::string& url,
- const ProxiesResolvedFn& callback) override;
- bool CancelProxyRequest(ProxyRequestId request) override;
-
- // Set the number of direct (non-) proxies to be returned by resolver.
- // The default value is 1; higher numbers are currently used in testing.
- inline void set_num_proxies(size_t num_proxies) {
- num_proxies_ = num_proxies;
- }
-
- private:
- // The ID of the main loop callback.
- brillo::MessageLoop::TaskId idle_callback_id_{
- brillo::MessageLoop::kTaskIdNull};
-
- // Number of direct proxies to return on resolved list; currently used for
- // testing.
- size_t num_proxies_{1};
-
- // The MainLoop callback, from here we return to the client.
- void ReturnCallback(const ProxiesResolvedFn& callback);
- DISALLOW_COPY_AND_ASSIGN(DirectProxyResolver);
-};
-
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_COMMON_PROXY_RESOLVER_H_
diff --git a/common/proxy_resolver_unittest.cc b/common/proxy_resolver_unittest.cc
deleted file mode 100644
index 101bf6b..0000000
--- a/common/proxy_resolver_unittest.cc
+++ /dev/null
@@ -1,91 +0,0 @@
-//
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-#include "update_engine/common/proxy_resolver.h"
-
-#include <deque>
-#include <string>
-
-#include <gtest/gtest.h>
-
-#include <base/bind.h>
-#include <brillo/message_loops/fake_message_loop.h>
-
-using std::deque;
-using std::string;
-
-namespace chromeos_update_engine {
-
-class ProxyResolverTest : public ::testing::Test {
- protected:
- virtual ~ProxyResolverTest() = default;
-
- void SetUp() override { loop_.SetAsCurrent(); }
-
- void TearDown() override { EXPECT_FALSE(loop_.PendingTasks()); }
-
- brillo::FakeMessageLoop loop_{nullptr};
- DirectProxyResolver resolver_;
-};
-
-TEST_F(ProxyResolverTest, DirectProxyResolverCallbackTest) {
- bool called = false;
- deque<string> callback_proxies;
- auto callback = base::Bind(
- [](bool* called,
- deque<string>* callback_proxies,
- const deque<string>& proxies) {
- *called = true;
- *callback_proxies = proxies;
- },
- &called,
- &callback_proxies);
-
- EXPECT_NE(kProxyRequestIdNull,
- resolver_.GetProxiesForUrl("http://foo", callback));
- // Check the callback is not called until the message loop runs.
- EXPECT_FALSE(called);
- loop_.Run();
- EXPECT_TRUE(called);
- EXPECT_EQ(kNoProxy, callback_proxies.front());
-}
-
-TEST_F(ProxyResolverTest, DirectProxyResolverCancelCallbackTest) {
- bool called = false;
- auto callback = base::Bind(
- [](bool* called, const deque<string>& proxies) { *called = true; },
- &called);
-
- ProxyRequestId request = resolver_.GetProxiesForUrl("http://foo", callback);
- EXPECT_FALSE(called);
- EXPECT_TRUE(resolver_.CancelProxyRequest(request));
- loop_.Run();
- EXPECT_FALSE(called);
-}
-
-TEST_F(ProxyResolverTest, DirectProxyResolverSimultaneousCallbacksTest) {
- int called = 0;
- auto callback = base::Bind(
- [](int* called, const deque<string>& proxies) { (*called)++; }, &called);
-
- resolver_.GetProxiesForUrl("http://foo", callback);
- resolver_.GetProxiesForUrl("http://bar", callback);
- EXPECT_EQ(0, called);
- loop_.Run();
- EXPECT_EQ(2, called);
-}
-
-} // namespace chromeos_update_engine
diff --git a/download_action_android_unittest.cc b/download_action_android_unittest.cc
index bef4342..968f875 100644
--- a/download_action_android_unittest.cc
+++ b/download_action_android_unittest.cc
@@ -37,7 +37,6 @@
#include "update_engine/common/utils.h"
#include "update_engine/payload_consumer/install_plan.h"
#include "update_engine/payload_consumer/payload_constants.h"
-#include "update_engine/payload_generator/annotated_operation.h"
#include "update_engine/payload_generator/payload_file.h"
#include "update_engine/payload_generator/payload_signer.h"
@@ -70,8 +69,7 @@
.WillRepeatedly(DoAll(SetArgPointee<1>(data), Return(true)));
BootControlStub boot_control;
- MockHttpFetcher* http_fetcher =
- new MockHttpFetcher(data.data(), data.size(), nullptr);
+ MockHttpFetcher* http_fetcher = new MockHttpFetcher(data.data(), data.size());
http_fetcher->set_delay(false);
InstallPlan install_plan;
auto& payload = install_plan.payloads.emplace_back();
@@ -139,8 +137,7 @@
.WillRepeatedly(DoAll(SetArgPointee<1>(0), Return(true)));
BootControlStub boot_control;
- MockHttpFetcher* http_fetcher =
- new MockHttpFetcher(data.data(), data.size(), nullptr);
+ MockHttpFetcher* http_fetcher = new MockHttpFetcher(data.data(), data.size());
http_fetcher->set_delay(false);
InstallPlan install_plan;
auto& payload = install_plan.payloads.emplace_back();
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 1599aac..4737b46 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -41,6 +41,7 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/common/hardware_interface.h"
#include "update_engine/common/platform_constants.h"
+#include "update_engine/common/utils.h"
using base::TimeDelta;
using brillo::MessageLoop;
@@ -92,9 +93,8 @@
return 1;
}
-LibcurlHttpFetcher::LibcurlHttpFetcher(ProxyResolver* proxy_resolver,
- HardwareInterface* hardware)
- : HttpFetcher(proxy_resolver), hardware_(hardware) {
+LibcurlHttpFetcher::LibcurlHttpFetcher(HardwareInterface* hardware)
+ : hardware_(hardware) {
// Dev users want a longer timeout (180 seconds) because they may
// be waiting on the dev server to build an image.
if (!hardware_->IsOfficialBuild())
@@ -106,7 +106,6 @@
LibcurlHttpFetcher::~LibcurlHttpFetcher() {
LOG_IF(ERROR, transfer_in_progress_)
<< "Destroying the fetcher while a transfer is in progress.";
- CancelProxyResolution();
CleanUp();
}
@@ -338,12 +337,7 @@
void LibcurlHttpFetcher::BeginTransfer(const string& url) {
CHECK(!transfer_in_progress_);
url_ = url;
- auto closure =
- base::Bind(&LibcurlHttpFetcher::ProxiesResolved, base::Unretained(this));
- ResolveProxiesForUrl(url_, closure);
-}
-void LibcurlHttpFetcher::ProxiesResolved() {
transfer_size_ = -1;
resume_offset_ = 0;
retry_count_ = 0;
@@ -362,7 +356,6 @@
}
void LibcurlHttpFetcher::ForceTransferTermination() {
- CancelProxyResolution();
CleanUp();
if (delegate_) {
// Note that after the callback returns this object may be destroyed.
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index 4e91b69..c101d90 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -82,8 +82,7 @@
class LibcurlHttpFetcher : public HttpFetcher {
public:
- LibcurlHttpFetcher(ProxyResolver* proxy_resolver,
- HardwareInterface* hardware);
+ LibcurlHttpFetcher(HardwareInterface* hardware);
// Cleans up all internal state. Does not notify delegate
~LibcurlHttpFetcher() override;
@@ -165,10 +164,6 @@
// closing a socket created with the CURLOPT_OPENSOCKETFUNCTION callback.
static int LibcurlCloseSocketCallback(void* clientp, curl_socket_t item);
- // 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.
virtual void GetHttpResponseCode();
diff --git a/libcurl_http_fetcher_unittest.cc b/libcurl_http_fetcher_unittest.cc
index 3543870..a944f37 100644
--- a/libcurl_http_fetcher_unittest.cc
+++ b/libcurl_http_fetcher_unittest.cc
@@ -23,7 +23,6 @@
#include <gtest/gtest.h>
#include "update_engine/common/fake_hardware.h"
-#include "update_engine/common/mock_proxy_resolver.h"
#include "update_engine/mock_libcurl_http_fetcher.h"
using std::string;
@@ -44,7 +43,7 @@
brillo::FakeMessageLoop loop_{nullptr};
FakeHardware fake_hardware_;
- MockLibcurlHttpFetcher libcurl_fetcher_{nullptr, &fake_hardware_};
+ MockLibcurlHttpFetcher libcurl_fetcher_{&fake_hardware_};
UnresolvedHostStateMachine state_machine_;
};
diff --git a/mock_libcurl_http_fetcher.h b/mock_libcurl_http_fetcher.h
index a14f953..054d878 100644
--- a/mock_libcurl_http_fetcher.h
+++ b/mock_libcurl_http_fetcher.h
@@ -25,9 +25,8 @@
class MockLibcurlHttpFetcher : public LibcurlHttpFetcher {
public:
- MockLibcurlHttpFetcher(ProxyResolver* proxy_resolver,
- HardwareInterface* hardware)
- : LibcurlHttpFetcher(proxy_resolver, hardware) {}
+ MockLibcurlHttpFetcher(HardwareInterface* hardware)
+ : LibcurlHttpFetcher(hardware) {}
MOCK_METHOD0(GetHttpResponseCode, void());
};
