diff options
| -rw-r--r-- | common/action_processor.cc | 4 | ||||
| -rw-r--r-- | common/action_processor.h | 7 | ||||
| -rw-r--r-- | common/http_fetcher_unittest.cc | 120 | ||||
| -rw-r--r-- | common/libcurl_http_fetcher.cc | 223 | ||||
| -rw-r--r-- | common/libcurl_http_fetcher.h | 14 | ||||
| -rw-r--r-- | mock_proxy_resolver.h | 38 | ||||
| -rw-r--r-- | payload_consumer/download_action.cc | 8 | ||||
| -rw-r--r-- | payload_consumer/download_action.h | 12 | ||||
| -rw-r--r-- | update_attempter_android.cc | 22 |
9 files changed, 294 insertions, 154 deletions
diff --git a/common/action_processor.cc b/common/action_processor.cc index 2618e4e7..3549e080 100644 --- a/common/action_processor.cc +++ b/common/action_processor.cc @@ -60,6 +60,10 @@ void ActionProcessor::StopProcessing() { << (suspended_ ? " while suspended" : ""); current_action_ = nullptr; suspended_ = false; + // Delete all the actions before calling the delegate. + for (auto action : actions_) + action->SetProcessor(nullptr); + actions_.clear(); if (delegate_) delegate_->ProcessingStopped(this); } diff --git a/common/action_processor.h b/common/action_processor.h index 050eee97..c9c179ea 100644 --- a/common/action_processor.h +++ b/common/action_processor.h @@ -51,8 +51,9 @@ class ActionProcessor { virtual void StartProcessing(); // Aborts processing. If an Action is running, it will have - // TerminateProcessing() called on it. The Action that was running - // will be lost and must be re-enqueued if this Processor is to use it. + // TerminateProcessing() called on it. The Action that was running and all the + // remaining actions will be lost and must be re-enqueued if this Processor is + // to use it. void StopProcessing(); // Suspend the processing. If an Action is running, it will have the @@ -63,7 +64,7 @@ class ActionProcessor { void SuspendProcessing(); // Resume the suspended processing. If the ActionProcessor is not suspended - // or not running on the first place this method performs no action. + // or not running in the first place this method performs no action. void ResumeProcessing(); // Returns true iff the processing was started but not yet completed nor diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc index aaa538c0..54509583 100644 --- a/common/http_fetcher_unittest.cc +++ b/common/http_fetcher_unittest.cc @@ -48,6 +48,7 @@ #include "update_engine/common/multi_range_http_fetcher.h" #include "update_engine/common/test_utils.h" #include "update_engine/common/utils.h" +#include "update_engine/mock_proxy_resolver.h" #include "update_engine/proxy_resolver.h" using brillo::MessageLoop; @@ -56,6 +57,10 @@ using std::pair; using std::string; using std::unique_ptr; using std::vector; +using testing::DoAll; +using testing::Return; +using testing::SaveArg; +using testing::_; namespace { @@ -193,14 +198,19 @@ class AnyHttpFetcherTest { AnyHttpFetcherTest() {} virtual ~AnyHttpFetcherTest() {} - virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0; + virtual HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) = 0; + HttpFetcher* NewLargeFetcher(size_t num_proxies) { + proxy_resolver_.set_num_proxies(num_proxies); + return NewLargeFetcher(&proxy_resolver_); + } HttpFetcher* NewLargeFetcher() { return NewLargeFetcher(1); } - virtual HttpFetcher* NewSmallFetcher(size_t num_proxies) = 0; + virtual HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) = 0; HttpFetcher* NewSmallFetcher() { - return NewSmallFetcher(1); + proxy_resolver_.set_num_proxies(1); + return NewSmallFetcher(&proxy_resolver_); } virtual string BigUrl(in_port_t port) const { return kUnusedUrl; } @@ -227,25 +237,16 @@ class MockHttpFetcherTest : public AnyHttpFetcherTest { public: // Necessary to unhide the definition in the base class. using AnyHttpFetcherTest::NewLargeFetcher; - HttpFetcher* NewLargeFetcher(size_t num_proxies) override { + HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override { brillo::Blob big_data(1000000); - CHECK_GT(num_proxies, 0u); - proxy_resolver_.set_num_proxies(num_proxies); return new MockHttpFetcher( - big_data.data(), - big_data.size(), - reinterpret_cast<ProxyResolver*>(&proxy_resolver_)); + big_data.data(), big_data.size(), proxy_resolver); } // Necessary to unhide the definition in the base class. using AnyHttpFetcherTest::NewSmallFetcher; - HttpFetcher* NewSmallFetcher(size_t num_proxies) override { - CHECK_GT(num_proxies, 0u); - proxy_resolver_.set_num_proxies(num_proxies); - return new MockHttpFetcher( - "x", - 1, - reinterpret_cast<ProxyResolver*>(&proxy_resolver_)); + HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { + return new MockHttpFetcher("x", 1, proxy_resolver); } bool IsMock() const override { return true; } @@ -260,12 +261,9 @@ class LibcurlHttpFetcherTest : public AnyHttpFetcherTest { public: // Necessary to unhide the definition in the base class. using AnyHttpFetcherTest::NewLargeFetcher; - HttpFetcher* NewLargeFetcher(size_t num_proxies) override { - CHECK_GT(num_proxies, 0u); - proxy_resolver_.set_num_proxies(num_proxies); - LibcurlHttpFetcher *ret = new - LibcurlHttpFetcher(reinterpret_cast<ProxyResolver*>(&proxy_resolver_), - &fake_hardware_); + HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override { + LibcurlHttpFetcher* ret = + new LibcurlHttpFetcher(proxy_resolver, &fake_hardware_); // Speed up test execution. ret->set_idle_seconds(1); ret->set_retry_seconds(1); @@ -275,8 +273,8 @@ class LibcurlHttpFetcherTest : public AnyHttpFetcherTest { // Necessary to unhide the definition in the base class. using AnyHttpFetcherTest::NewSmallFetcher; - HttpFetcher* NewSmallFetcher(size_t num_proxies) override { - return NewLargeFetcher(num_proxies); + HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { + return NewLargeFetcher(proxy_resolver); } string BigUrl(in_port_t port) const override { @@ -307,14 +305,9 @@ class MultiRangeHttpFetcherTest : public LibcurlHttpFetcherTest { public: // Necessary to unhide the definition in the base class. using AnyHttpFetcherTest::NewLargeFetcher; - HttpFetcher* NewLargeFetcher(size_t num_proxies) override { - CHECK_GT(num_proxies, 0u); - proxy_resolver_.set_num_proxies(num_proxies); - ProxyResolver* resolver = - reinterpret_cast<ProxyResolver*>(&proxy_resolver_); - MultiRangeHttpFetcher *ret = - new MultiRangeHttpFetcher( - new LibcurlHttpFetcher(resolver, &fake_hardware_)); + HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override { + MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher( + new LibcurlHttpFetcher(proxy_resolver, &fake_hardware_)); ret->ClearRanges(); ret->AddRange(0); // Speed up test execution. @@ -326,8 +319,8 @@ class MultiRangeHttpFetcherTest : public LibcurlHttpFetcherTest { // Necessary to unhide the definition in the base class. using AnyHttpFetcherTest::NewSmallFetcher; - HttpFetcher* NewSmallFetcher(size_t num_proxies) override { - return NewLargeFetcher(num_proxies); + HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { + return NewLargeFetcher(proxy_resolver); } bool IsMulti() const override { return true; } @@ -517,26 +510,51 @@ void UnpausingTimeoutCallback(PausingHttpFetcherTestDelegate* delegate, } // namespace TYPED_TEST(HttpFetcherTest, PauseTest) { - { - PausingHttpFetcherTestDelegate delegate; - unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher()); - delegate.paused_ = false; - delegate.fetcher_ = fetcher.get(); - fetcher->set_delegate(&delegate); + PausingHttpFetcherTestDelegate delegate; + unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher()); + delegate.paused_ = false; + delegate.fetcher_ = fetcher.get(); + fetcher->set_delegate(&delegate); - unique_ptr<HttpServer> server(this->test_.CreateServer()); - ASSERT_TRUE(server->started_); + unique_ptr<HttpServer> server(this->test_.CreateServer()); + ASSERT_TRUE(server->started_); - MessageLoop::TaskId callback_id; - callback_id = this->loop_.PostDelayedTask( - FROM_HERE, - base::Bind(&UnpausingTimeoutCallback, &delegate, &callback_id), - base::TimeDelta::FromMilliseconds(200)); - fetcher->BeginTransfer(this->test_.BigUrl(server->GetPort())); + MessageLoop::TaskId callback_id; + callback_id = this->loop_.PostDelayedTask( + FROM_HERE, + base::Bind(&UnpausingTimeoutCallback, &delegate, &callback_id), + base::TimeDelta::FromMilliseconds(200)); + fetcher->BeginTransfer(this->test_.BigUrl(server->GetPort())); - this->loop_.Run(); - EXPECT_TRUE(this->loop_.CancelTask(callback_id)); - } + this->loop_.Run(); + EXPECT_TRUE(this->loop_.CancelTask(callback_id)); +} + +// This test will pause the fetcher while the download is not yet started +// because it is waiting for the proxy to be resolved. +TYPED_TEST(HttpFetcherTest, PauseWhileResolvingProxyTest) { + if (this->test_.IsMock()) + return; + MockProxyResolver mock_resolver; + unique_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher(&mock_resolver)); + + // Saved arguments from the proxy call. + ProxiesResolvedFn proxy_callback = nullptr; + void* proxy_data = nullptr; + + EXPECT_CALL(mock_resolver, GetProxiesForUrl("http://fake_url", _, _)) + .WillOnce(DoAll( + SaveArg<1>(&proxy_callback), SaveArg<2>(&proxy_data), 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_TRUE(proxy_callback); + (*proxy_callback)({1, kNoProxy}, proxy_data); } namespace { diff --git a/common/libcurl_http_fetcher.cc b/common/libcurl_http_fetcher.cc index 761b74e1..789f46ef 100644 --- a/common/libcurl_http_fetcher.cc +++ b/common/libcurl_http_fetcher.cc @@ -99,6 +99,7 @@ void LibcurlHttpFetcher::ResumeTransfer(const string& url) { curl_handle_ = curl_easy_init(); CHECK(curl_handle_); + ignore_failure_ = false; CHECK(HasProxy()); bool is_direct = (GetCurrentProxy() == kNoProxy); @@ -291,6 +292,12 @@ void LibcurlHttpFetcher::ProxiesResolved() { http_response_code_ = 0; terminate_requested_ = false; sent_byte_ = false; + + // If we are paused, we delay these two operations until Unpause is called. + if (transfer_paused_) { + restart_transfer_on_unpause_ = true; + return; + } ResumeTransfer(url_); CurlPerformOnce(); } @@ -325,96 +332,110 @@ void LibcurlHttpFetcher::CurlPerformOnce() { return; } } - if (0 == running_handles) { - GetHttpResponseCode(); - if (http_response_code_) { - LOG(INFO) << "HTTP response code: " << http_response_code_; - no_network_retry_count_ = 0; - } else { - LOG(ERROR) << "Unable to get http response code."; - } - // we're done! - CleanUp(); - - // TODO(petkov): This temporary code tries to deal with the case where the - // update engine performs an update check while the network is not ready - // (e.g., right after resume). Longer term, we should check if the network - // is online/offline and return an appropriate error code. - if (!sent_byte_ && - http_response_code_ == 0 && - no_network_retry_count_ < no_network_max_retries_) { - no_network_retry_count_++; - MessageLoop::current()->PostDelayedTask( + // If the transfer completes while paused, we should ignore the failure once + // the fetcher is unpaused. + if (running_handles == 0 && transfer_paused_ && !ignore_failure_) { + LOG(INFO) << "Connection closed while paused, ignoring failure."; + ignore_failure_ = true; + } + + if (running_handles != 0 || transfer_paused_) { + // There's either more work to do or we are paused, so we just keep the + // file descriptors to watch up to date and exit, until we are done with the + // work and we are not paused. + SetupMessageLoopSources(); + return; + } + + // At this point, the transfer was completed in some way (error, connection + // closed or download finished). + + GetHttpResponseCode(); + if (http_response_code_) { + LOG(INFO) << "HTTP response code: " << http_response_code_; + no_network_retry_count_ = 0; + } else { + LOG(ERROR) << "Unable to get http response code."; + } + + // we're done! + CleanUp(); + + // TODO(petkov): This temporary code tries to deal with the case where the + // update engine performs an update check while the network is not ready + // (e.g., right after resume). Longer term, we should check if the network + // is online/offline and return an appropriate error code. + if (!sent_byte_ && + http_response_code_ == 0 && + no_network_retry_count_ < no_network_max_retries_) { + no_network_retry_count_++; + MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&LibcurlHttpFetcher::RetryTimeoutCallback, + base::Unretained(this)), + TimeDelta::FromSeconds(kNoNetworkRetrySeconds)); + LOG(INFO) << "No HTTP response, retry " << no_network_retry_count_; + } else if ((!sent_byte_ && !IsHttpResponseSuccess()) || + IsHttpResponseError()) { + // The transfer completed w/ error and we didn't get any bytes. + // If we have another proxy to try, try that. + // + // TODO(garnold) in fact there are two separate cases here: one case is an + // other-than-success return code (including no return code) and no + // received bytes, which is necessary due to the way callbacks are + // currently processing error conditions; the second is an explicit HTTP + // error code, where some data may have been received (as in the case of a + // semi-successful multi-chunk fetch). This is a confusing behavior and + // should be unified into a complete, coherent interface. + LOG(INFO) << "Transfer resulted in an error (" << http_response_code_ + << "), " << bytes_downloaded_ << " bytes downloaded"; + + PopProxy(); // Delete the proxy we just gave up on. + + if (HasProxy()) { + // We have another proxy. Retry immediately. + LOG(INFO) << "Retrying with next proxy setting"; + MessageLoop::current()->PostTask( FROM_HERE, base::Bind(&LibcurlHttpFetcher::RetryTimeoutCallback, - base::Unretained(this)), - TimeDelta::FromSeconds(kNoNetworkRetrySeconds)); - LOG(INFO) << "No HTTP response, retry " << no_network_retry_count_; - return; + base::Unretained(this))); + } else { + // Out of proxies. Give up. + LOG(INFO) << "No further proxies, indicating transfer complete"; + if (delegate_) + delegate_->TransferComplete(this, false); // signal fail } - - if ((!sent_byte_ && !IsHttpResponseSuccess()) || IsHttpResponseError()) { - // The transfer completed w/ error and we didn't get any bytes. - // If we have another proxy to try, try that. - // - // TODO(garnold) in fact there are two separate cases here: one case is an - // other-than-success return code (including no return code) and no - // received bytes, which is necessary due to the way callbacks are - // currently processing error conditions; the second is an explicit HTTP - // error code, where some data may have been received (as in the case of a - // semi-successful multi-chunk fetch). This is a confusing behavior and - // should be unified into a complete, coherent interface. - LOG(INFO) << "Transfer resulted in an error (" << http_response_code_ - << "), " << bytes_downloaded_ << " bytes downloaded"; - - PopProxy(); // Delete the proxy we just gave up on. - - if (HasProxy()) { - // We have another proxy. Retry immediately. - LOG(INFO) << "Retrying with next proxy setting"; - MessageLoop::current()->PostTask( - FROM_HERE, - base::Bind(&LibcurlHttpFetcher::RetryTimeoutCallback, - base::Unretained(this))); - } else { - // Out of proxies. Give up. - LOG(INFO) << "No further proxies, indicating transfer complete"; - if (delegate_) - delegate_->TransferComplete(this, false); // signal fail - } - } else if ((transfer_size_ >= 0) && (bytes_downloaded_ < transfer_size_)) { + } else if ((transfer_size_ >= 0) && (bytes_downloaded_ < transfer_size_)) { + if (!ignore_failure_) retry_count_++; - LOG(INFO) << "Transfer interrupted after downloading " - << bytes_downloaded_ << " of " << transfer_size_ << " bytes. " - << transfer_size_ - bytes_downloaded_ << " bytes remaining " - << "after " << retry_count_ << " attempt(s)"; - - if (retry_count_ > max_retry_count_) { - LOG(INFO) << "Reached max attempts (" << retry_count_ << ")"; - if (delegate_) - delegate_->TransferComplete(this, false); // signal fail - } else { - // Need to restart transfer - LOG(INFO) << "Restarting transfer to download the remaining bytes"; - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&LibcurlHttpFetcher::RetryTimeoutCallback, - base::Unretained(this)), - TimeDelta::FromSeconds(retry_seconds_)); - } + LOG(INFO) << "Transfer interrupted after downloading " + << bytes_downloaded_ << " of " << transfer_size_ << " bytes. " + << transfer_size_ - bytes_downloaded_ << " bytes remaining " + << "after " << retry_count_ << " attempt(s)"; + + if (retry_count_ > max_retry_count_) { + LOG(INFO) << "Reached max attempts (" << retry_count_ << ")"; + if (delegate_) + delegate_->TransferComplete(this, false); // signal fail } else { - LOG(INFO) << "Transfer completed (" << http_response_code_ - << "), " << bytes_downloaded_ << " bytes downloaded"; - if (delegate_) { - bool success = IsHttpResponseSuccess(); - delegate_->TransferComplete(this, success); - } + // Need to restart transfer + LOG(INFO) << "Restarting transfer to download the remaining bytes"; + MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&LibcurlHttpFetcher::RetryTimeoutCallback, + base::Unretained(this)), + TimeDelta::FromSeconds(retry_seconds_)); } } else { - // set up callback - SetupMessageLoopSources(); + LOG(INFO) << "Transfer completed (" << http_response_code_ + << "), " << bytes_downloaded_ << " bytes downloaded"; + if (delegate_) { + bool success = IsHttpResponseSuccess(); + delegate_->TransferComplete(this, success); + } } + ignore_failure_ = false; } size_t LibcurlHttpFetcher::LibcurlWrite(void *ptr, size_t size, size_t nmemb) { @@ -449,15 +470,43 @@ size_t LibcurlHttpFetcher::LibcurlWrite(void *ptr, size_t size, size_t nmemb) { } void LibcurlHttpFetcher::Pause() { + if (transfer_paused_) { + LOG(ERROR) << "Fetcher already paused."; + return; + } + transfer_paused_ = true; + if (!transfer_in_progress_) { + // If pause before we started a connection, we don't need to notify curl + // about that, we will simply not start the connection later. + return; + } CHECK(curl_handle_); - CHECK(transfer_in_progress_); CHECK_EQ(curl_easy_pause(curl_handle_, CURLPAUSE_ALL), CURLE_OK); } void LibcurlHttpFetcher::Unpause() { + if (!transfer_paused_) { + LOG(ERROR) << "Resume attempted when fetcher not paused."; + return; + } + transfer_paused_ = false; + if (restart_transfer_on_unpause_) { + restart_transfer_on_unpause_ = false; + ResumeTransfer(url_); + CurlPerformOnce(); + return; + } + if (!transfer_in_progress_) { + // If resumed before starting the connection, there's no need to notify + // anybody. We will simply start the connection once it is time. + return; + } CHECK(curl_handle_); - CHECK(transfer_in_progress_); CHECK_EQ(curl_easy_pause(curl_handle_, CURLPAUSE_CONT), CURLE_OK); + // Since the transfer is in progress, we need to dispatch a CurlPerformOnce() + // now to let the connection continue, otherwise it would be called by the + // TimeoutCallback but with a delay. + CurlPerformOnce(); } // This method sets up callbacks with the MessageLoop. @@ -537,7 +586,7 @@ void LibcurlHttpFetcher::SetupMessageLoopSources() { // Set up a timeout callback for libcurl. if (timeout_id_ == MessageLoop::kTaskIdNull) { - LOG(INFO) << "Setting up timeout source: " << idle_seconds_ << " seconds."; + VLOG(1) << "Setting up timeout source: " << idle_seconds_ << " seconds."; timeout_id_ = MessageLoop::current()->PostDelayedTask( FROM_HERE, base::Bind(&LibcurlHttpFetcher::TimeoutCallback, @@ -547,6 +596,10 @@ void LibcurlHttpFetcher::SetupMessageLoopSources() { } void LibcurlHttpFetcher::RetryTimeoutCallback() { + if (transfer_paused_) { + restart_transfer_on_unpause_ = true; + return; + } ResumeTransfer(url_); CurlPerformOnce(); } @@ -599,6 +652,8 @@ void LibcurlHttpFetcher::CleanUp() { curl_multi_handle_ = nullptr; } transfer_in_progress_ = false; + transfer_paused_ = false; + restart_transfer_on_unpause_ = false; } void LibcurlHttpFetcher::GetHttpResponseCode() { diff --git a/common/libcurl_http_fetcher.h b/common/libcurl_http_fetcher.h index 66dbb18c..218e6cba 100644 --- a/common/libcurl_http_fetcher.h +++ b/common/libcurl_http_fetcher.h @@ -132,9 +132,9 @@ class LibcurlHttpFetcher : public HttpFetcher { // Calls into curl_multi_perform to let libcurl do its work. Returns after // curl_multi_perform is finished, which may actually be after more than // one call to curl_multi_perform. This method will set up the message - // loop with sources for future work that libcurl will do. + // loop with sources for future work that libcurl will do, if any, or complete + // the transfer and finish the action if no work left to do. // This method will not block. - // Returns true if we should resume immediately after this call. void CurlPerformOnce(); // Sets up message loop sources as needed by libcurl. This is generally @@ -192,6 +192,16 @@ class LibcurlHttpFetcher : public HttpFetcher { brillo::MessageLoop::TaskId timeout_id_{brillo::MessageLoop::kTaskIdNull}; bool transfer_in_progress_{false}; + bool transfer_paused_{false}; + + // Whether it should ignore transfer failures for the purpose of retrying the + // connection. + bool ignore_failure_{false}; + + // Whether we should restart the transfer once Unpause() is called. This can + // be caused because either the connection dropped while pause or the proxy + // was resolved and we never started the transfer in the first place. + bool restart_transfer_on_unpause_{false}; // The transfer size. -1 if not known. off_t transfer_size_{0}; diff --git a/mock_proxy_resolver.h b/mock_proxy_resolver.h new file mode 100644 index 00000000..0595f5ab --- /dev/null +++ b/mock_proxy_resolver.h @@ -0,0 +1,38 @@ +// +// 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_MOCK_PROXY_RESOLVER_H_ +#define UPDATE_ENGINE_MOCK_PROXY_RESOLVER_H_ + +#include <string> + +#include <gmock/gmock.h> + +#include "update_engine/proxy_resolver.h" + +namespace chromeos_update_engine { + +class MockProxyResolver : public ProxyResolver { + public: + MOCK_METHOD3(GetProxiesForUrl, + bool(const std::string& url, + ProxiesResolvedFn callback, + void* data)); +}; + +} // namespace chromeos_update_engine + +#endif // UPDATE_ENGINE_MOCK_PROXY_RESOLVER_H_ diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc index 6a71fdbe..fdbbd72e 100644 --- a/payload_consumer/download_action.cc +++ b/payload_consumer/download_action.cc @@ -232,6 +232,14 @@ void DownloadAction::PerformAction() { http_fetcher_->BeginTransfer(install_plan_.download_url); } +void DownloadAction::SuspendAction() { + http_fetcher_->Pause(); +} + +void DownloadAction::ResumeAction() { + http_fetcher_->Unpause(); +} + void DownloadAction::TerminateProcessing() { if (writer_) { writer_->Close(); diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h index d000c670..fc320688 100644 --- a/payload_consumer/download_action.h +++ b/payload_consumer/download_action.h @@ -68,6 +68,9 @@ class PrefsInterface; class DownloadAction : public InstallPlanAction, public HttpFetcherDelegate { public: + // Debugging/logging + static std::string StaticType() { return "DownloadAction"; } + // Takes ownership of the passed in HttpFetcher. Useful for testing. // A good calling pattern is: // DownloadAction(prefs, boot_contol, hardware, system_state, @@ -78,8 +81,13 @@ class DownloadAction : public InstallPlanAction, SystemState* system_state, HttpFetcher* http_fetcher); ~DownloadAction() override; + + // InstallPlanAction overrides. void PerformAction() override; + void SuspendAction() override; + void ResumeAction() override; void TerminateProcessing() override; + std::string Type() const override { return StaticType(); } // Testing void SetTestFileWriter(FileWriter* writer) { @@ -88,10 +96,6 @@ class DownloadAction : public InstallPlanAction, int GetHTTPResponseCode() { return http_fetcher_->http_response_code(); } - // Debugging/logging - static std::string StaticType() { return "DownloadAction"; } - std::string Type() const override { return StaticType(); } - // HttpFetcherDelegate methods (see http_fetcher.h) void ReceivedBytes(HttpFetcher* fetcher, const void* bytes, size_t length) override; diff --git a/update_attempter_android.cc b/update_attempter_android.cc index c10b0803..6f88ee75 100644 --- a/update_attempter_android.cc +++ b/update_attempter_android.cc @@ -173,6 +173,7 @@ bool UpdateAttempterAndroid::ApplyPayload( SetupDownload(); cpu_limiter_.StartLimiter(); SetStatusAndNotify(UpdateStatus::UPDATE_AVAILABLE); + ongoing_update_ = true; // Just in case we didn't update boot flags yet, make sure they're updated // before any update processing starts. This will start the update process. @@ -181,23 +182,24 @@ bool UpdateAttempterAndroid::ApplyPayload( } bool UpdateAttempterAndroid::SuspendUpdate(brillo::ErrorPtr* error) { - // TODO(deymo): Implement suspend/resume. - return LogAndSetError(error, FROM_HERE, "Suspend/resume not implemented"); + if (!ongoing_update_) + return LogAndSetError(error, FROM_HERE, "No ongoing update to suspend."); + processor_->SuspendProcessing(); + return true; } bool UpdateAttempterAndroid::ResumeUpdate(brillo::ErrorPtr* error) { - // TODO(deymo): Implement suspend/resume. - return LogAndSetError(error, FROM_HERE, "Suspend/resume not implemented"); + if (!ongoing_update_) + return LogAndSetError(error, FROM_HERE, "No ongoing update to resume."); + processor_->ResumeProcessing(); + return true; } bool UpdateAttempterAndroid::CancelUpdate(brillo::ErrorPtr* error) { - if (status_ == UpdateStatus::IDLE || - status_ == UpdateStatus::UPDATED_NEED_REBOOT) { + if (!ongoing_update_) return LogAndSetError(error, FROM_HERE, "No ongoing update to cancel."); - } - - // TODO(deymo): Implement cancel. - return LogAndSetError(error, FROM_HERE, "Cancel not implemented"); + processor_->StopProcessing(); + return true; } bool UpdateAttempterAndroid::ResetStatus(brillo::ErrorPtr* error) { |