Merge "Parse and use extra HTTP headers when downloading the payload." into nyc-dev
diff --git a/Android.mk b/Android.mk
index 6b24fd0..d529f70 100644
--- a/Android.mk
+++ b/Android.mk
@@ -702,7 +702,7 @@
$(eval LOCAL_MODULE_STEM := $(1)) \
$(eval my_gen := $(call local-intermediates-dir)/gen/$(1)) \
$(eval $(my_gen) : PRIVATE_CUSTOM_TOOL = \
- tar -jxf $$< -C $$(dir $$@) $$(notdir $$@)) \
+ tar -jxf $$< -C $$(dir $$@) $$(notdir $$@) && touch $$@) \
$(eval $(my_gen) : $(LOCAL_PATH)/sample_images/sample_images.tar.bz2 ; \
$$(transform-generated-source)) \
$(eval LOCAL_PREBUILT_MODULE_FILE := $(my_gen)) \
@@ -722,6 +722,7 @@
ifdef BRILLO
LOCAL_MODULE_TAGS := eng
endif
+LOCAL_MODULE_PATH := $(TARGET_OUT_DATA_NATIVE_TESTS)/update_engine_unittests
LOCAL_MODULE_CLASS := EXECUTABLES
LOCAL_CPP_EXTENSION := .cc
LOCAL_CLANG := true
@@ -733,7 +734,7 @@
LOCAL_SRC_FILES := \
common/http_common.cc \
test_http_server.cc
-include $(BUILD_NATIVE_TEST)
+include $(BUILD_EXECUTABLE)
# update_engine_unittests (type: executable)
# ========================================================
diff --git a/common/action.h b/common/action.h
index d8049ac..6c88216 100644
--- a/common/action.h
+++ b/common/action.h
@@ -124,6 +124,15 @@
// Only the ActionProcessor should call this.
virtual void TerminateProcessing() {}
+ // Called on asynchronous actions if the processing is suspended and resumed,
+ // respectively. These methods are called by the ActionProcessor and should
+ // not be explicitly called.
+ // The action may still call ActionCompleted() once the action is completed
+ // while the processing is suspended, for example if suspend/resume is not
+ // implemented for the given action.
+ virtual void SuspendAction() {}
+ virtual void ResumeAction() {}
+
// These methods are useful for debugging. TODO(adlr): consider using
// std::type_info for this?
// Type() returns a string of the Action type. I.e., for DownloadAction,
diff --git a/common/action_processor.cc b/common/action_processor.cc
index 7ccdfbd..3549e08 100644
--- a/common/action_processor.cc
+++ b/common/action_processor.cc
@@ -21,14 +21,12 @@
#include <base/logging.h>
#include "update_engine/common/action.h"
+#include "update_engine/common/error_code_utils.h"
using std::string;
namespace chromeos_update_engine {
-ActionProcessor::ActionProcessor()
- : current_action_(nullptr), delegate_(nullptr) {}
-
ActionProcessor::~ActionProcessor() {
if (IsRunning())
StopProcessing();
@@ -45,8 +43,7 @@
CHECK(!IsRunning());
if (!actions_.empty()) {
current_action_ = actions_.front();
- LOG(INFO) << "ActionProcessor::StartProcessing: "
- << current_action_->Type();
+ LOG(INFO) << "ActionProcessor: starting " << current_action_->Type();
actions_.pop_front();
current_action_->PerformAction();
}
@@ -54,17 +51,59 @@
void ActionProcessor::StopProcessing() {
CHECK(IsRunning());
- CHECK(current_action_);
- current_action_->TerminateProcessing();
- CHECK(current_action_);
- current_action_->SetProcessor(nullptr);
- LOG(INFO) << "ActionProcessor::StopProcessing: aborted "
- << current_action_->Type();
+ if (current_action_) {
+ current_action_->TerminateProcessing();
+ current_action_->SetProcessor(nullptr);
+ }
+ LOG(INFO) << "ActionProcessor: aborted "
+ << (current_action_ ? current_action_->Type() : "")
+ << (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);
}
+void ActionProcessor::SuspendProcessing() {
+ // No current_action_ when not suspended means that the action processor was
+ // never started or already finished.
+ if (suspended_ || !current_action_) {
+ LOG(WARNING) << "Called SuspendProcessing while not processing.";
+ return;
+ }
+ suspended_ = true;
+
+ // If there's a current action we should notify it that it should suspend, but
+ // the action can ignore that and terminate at any point.
+ LOG(INFO) << "ActionProcessor: suspending " << current_action_->Type();
+ current_action_->SuspendAction();
+}
+
+void ActionProcessor::ResumeProcessing() {
+ if (!suspended_) {
+ LOG(WARNING) << "Called ResumeProcessing while not suspended.";
+ return;
+ }
+ suspended_ = false;
+ if (current_action_) {
+ // The current_action_ did not call ActionComplete while suspended, so we
+ // should notify it of the resume operation.
+ LOG(INFO) << "ActionProcessor: resuming " << current_action_->Type();
+ current_action_->ResumeAction();
+ } else {
+ // The last action called ActionComplete while suspended, so there is
+ // already a log message with the type of the finished action. We simply
+ // state that we are resuming processing and the next function will log the
+ // start of the next action or processing completion.
+ LOG(INFO) << "ActionProcessor: resuming processing";
+ StartNextActionOrFinish(suspended_error_code_);
+ }
+}
+
void ActionProcessor::ActionComplete(AbstractAction* actionptr,
ErrorCode code) {
CHECK_EQ(actionptr, current_action_);
@@ -74,17 +113,26 @@
current_action_->ActionCompleted(code);
current_action_->SetProcessor(nullptr);
current_action_ = nullptr;
- if (actions_.empty()) {
- LOG(INFO) << "ActionProcessor::ActionComplete: finished last action of"
- " type " << old_type;
- } else if (code != ErrorCode::kSuccess) {
- LOG(INFO) << "ActionProcessor::ActionComplete: " << old_type
- << " action failed. Aborting processing.";
+ LOG(INFO) << "ActionProcessor: finished "
+ << (actions_.empty() ? "last action " : "") << old_type
+ << (suspended_ ? " while suspended" : "")
+ << " with code " << utils::ErrorCodeToString(code);
+ if (!actions_.empty() && code != ErrorCode::kSuccess) {
+ LOG(INFO) << "ActionProcessor: Aborting processing due to failure.";
actions_.clear();
}
+ if (suspended_) {
+ // If an action finished while suspended we don't start the next action (or
+ // terminate the processing) until the processor is resumed. This condition
+ // will be flagged by a nullptr current_action_ while suspended_ is true.
+ suspended_error_code_ = code;
+ return;
+ }
+ StartNextActionOrFinish(code);
+}
+
+void ActionProcessor::StartNextActionOrFinish(ErrorCode code) {
if (actions_.empty()) {
- LOG(INFO) << "ActionProcessor::ActionComplete: finished last action of"
- " type " << old_type;
if (delegate_) {
delegate_->ProcessingDone(this, code);
}
@@ -92,8 +140,7 @@
}
current_action_ = actions_.front();
actions_.pop_front();
- LOG(INFO) << "ActionProcessor::ActionComplete: finished " << old_type
- << ", starting " << current_action_->Type();
+ LOG(INFO) << "ActionProcessor: starting " << current_action_->Type();
current_action_->PerformAction();
}
diff --git a/common/action_processor.h b/common/action_processor.h
index d61e12d..c9c179e 100644
--- a/common/action_processor.h
+++ b/common/action_processor.h
@@ -20,6 +20,7 @@
#include <deque>
#include <base/macros.h>
+#include <brillo/errors/error.h>
#include "update_engine/common/error_code.h"
@@ -40,7 +41,7 @@
class ActionProcessor {
public:
- ActionProcessor();
+ ActionProcessor() = default;
virtual ~ActionProcessor();
@@ -50,12 +51,25 @@
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();
- // Returns true iff an Action is currently processing.
- bool IsRunning() const { return nullptr != current_action_; }
+ // Suspend the processing. If an Action is running, it will have the
+ // SuspendProcessing() called on it, and it should suspend operations until
+ // ResumeProcessing() is called on this class to continue. While suspended,
+ // no new actions will be started. Calling SuspendProcessing while the
+ // processing is suspended or not running this method performs no action.
+ void SuspendProcessing();
+
+ // Resume the suspended processing. If the ActionProcessor is not suspended
+ // 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
+ // stopped.
+ bool IsRunning() const { return current_action_ != nullptr || suspended_; }
// Adds another Action to the end of the queue.
virtual void EnqueueAction(AbstractAction* action);
@@ -75,15 +89,29 @@
void ActionComplete(AbstractAction* actionptr, ErrorCode code);
private:
+ // Continue processing actions (if any) after the last action terminated with
+ // the passed error code. If there are no more actions to process, the
+ // processing will terminate.
+ void StartNextActionOrFinish(ErrorCode code);
+
// Actions that have not yet begun processing, in the order in which
// they'll be processed.
std::deque<AbstractAction*> actions_;
// A pointer to the currently processing Action, if any.
- AbstractAction* current_action_;
+ AbstractAction* current_action_{nullptr};
+
+ // The ErrorCode reported by an action that was suspended but finished while
+ // being suspended. This error code is stored here to be reported back to the
+ // delegate once the processor is resumed.
+ ErrorCode suspended_error_code_{ErrorCode::kSuccess};
+
+ // Whether the action processor is or should be suspended.
+ bool suspended_{false};
// A pointer to the delegate, or null if none.
- ActionProcessorDelegate *delegate_;
+ ActionProcessorDelegate* delegate_{nullptr};
+
DISALLOW_COPY_AND_ASSIGN(ActionProcessor);
};
diff --git a/common/action_processor_unittest.cc b/common/action_processor_unittest.cc
index 8285470..631e42d 100644
--- a/common/action_processor_unittest.cc
+++ b/common/action_processor_unittest.cc
@@ -16,9 +16,12 @@
#include "update_engine/common/action_processor.h"
-#include <gtest/gtest.h>
#include <string>
+
+#include <gtest/gtest.h>
+
#include "update_engine/common/action.h"
+#include "update_engine/common/mock_action.h"
using std::string;
@@ -51,26 +54,6 @@
string Type() const { return "ActionProcessorTestAction"; }
};
-class ActionProcessorTest : public ::testing::Test { };
-
-// This test creates two simple Actions and sends a message via an ActionPipe
-// from one to the other.
-TEST(ActionProcessorTest, SimpleTest) {
- ActionProcessorTestAction action;
- ActionProcessor action_processor;
- EXPECT_FALSE(action_processor.IsRunning());
- action_processor.EnqueueAction(&action);
- EXPECT_FALSE(action_processor.IsRunning());
- EXPECT_FALSE(action.IsRunning());
- action_processor.StartProcessing();
- EXPECT_TRUE(action_processor.IsRunning());
- EXPECT_TRUE(action.IsRunning());
- EXPECT_EQ(action_processor.current_action(), &action);
- action.CompleteAction();
- EXPECT_FALSE(action_processor.IsRunning());
- EXPECT_FALSE(action.IsRunning());
-}
-
namespace {
class MyActionProcessorDelegate : public ActionProcessorDelegate {
public:
@@ -109,53 +92,79 @@
};
} // namespace
-TEST(ActionProcessorTest, DelegateTest) {
- ActionProcessorTestAction action;
- ActionProcessor action_processor;
- MyActionProcessorDelegate delegate(&action_processor);
- action_processor.set_delegate(&delegate);
+class ActionProcessorTest : public ::testing::Test {
+ void SetUp() override {
+ action_processor_.set_delegate(&delegate_);
+ // Silence Type() calls used for logging.
+ EXPECT_CALL(mock_action_, Type()).Times(testing::AnyNumber());
+ }
- action_processor.EnqueueAction(&action);
- action_processor.StartProcessing();
- action.CompleteAction();
- action_processor.set_delegate(nullptr);
- EXPECT_TRUE(delegate.processing_done_called_);
- EXPECT_TRUE(delegate.action_completed_called_);
+ void TearDown() override {
+ action_processor_.set_delegate(nullptr);
+ }
+
+ protected:
+ // The ActionProcessor under test.
+ ActionProcessor action_processor_;
+
+ MyActionProcessorDelegate delegate_{&action_processor_};
+
+ // Common actions used during most tests.
+ testing::StrictMock<MockAction> mock_action_;
+ ActionProcessorTestAction action_;
+};
+
+TEST_F(ActionProcessorTest, SimpleTest) {
+ EXPECT_FALSE(action_processor_.IsRunning());
+ action_processor_.EnqueueAction(&action_);
+ EXPECT_FALSE(action_processor_.IsRunning());
+ EXPECT_FALSE(action_.IsRunning());
+ action_processor_.StartProcessing();
+ EXPECT_TRUE(action_processor_.IsRunning());
+ EXPECT_TRUE(action_.IsRunning());
+ EXPECT_EQ(action_processor_.current_action(), &action_);
+ action_.CompleteAction();
+ EXPECT_FALSE(action_processor_.IsRunning());
+ EXPECT_FALSE(action_.IsRunning());
}
-TEST(ActionProcessorTest, StopProcessingTest) {
- ActionProcessorTestAction action;
- ActionProcessor action_processor;
- MyActionProcessorDelegate delegate(&action_processor);
- action_processor.set_delegate(&delegate);
-
- action_processor.EnqueueAction(&action);
- action_processor.StartProcessing();
- action_processor.StopProcessing();
- action_processor.set_delegate(nullptr);
- EXPECT_TRUE(delegate.processing_stopped_called_);
- EXPECT_FALSE(delegate.action_completed_called_);
- EXPECT_FALSE(action_processor.IsRunning());
- EXPECT_EQ(nullptr, action_processor.current_action());
+TEST_F(ActionProcessorTest, DelegateTest) {
+ action_processor_.EnqueueAction(&action_);
+ action_processor_.StartProcessing();
+ action_.CompleteAction();
+ EXPECT_TRUE(delegate_.processing_done_called_);
+ EXPECT_TRUE(delegate_.action_completed_called_);
}
-TEST(ActionProcessorTest, ChainActionsTest) {
+TEST_F(ActionProcessorTest, StopProcessingTest) {
+ action_processor_.EnqueueAction(&action_);
+ action_processor_.StartProcessing();
+ action_processor_.StopProcessing();
+ EXPECT_TRUE(delegate_.processing_stopped_called_);
+ EXPECT_FALSE(delegate_.action_completed_called_);
+ EXPECT_FALSE(action_processor_.IsRunning());
+ EXPECT_EQ(nullptr, action_processor_.current_action());
+}
+
+TEST_F(ActionProcessorTest, ChainActionsTest) {
+ // This test doesn't use a delegate since it terminates several actions.
+ action_processor_.set_delegate(nullptr);
+
ActionProcessorTestAction action1, action2;
- ActionProcessor action_processor;
- action_processor.EnqueueAction(&action1);
- action_processor.EnqueueAction(&action2);
- action_processor.StartProcessing();
- EXPECT_EQ(&action1, action_processor.current_action());
- EXPECT_TRUE(action_processor.IsRunning());
+ action_processor_.EnqueueAction(&action1);
+ action_processor_.EnqueueAction(&action2);
+ action_processor_.StartProcessing();
+ EXPECT_EQ(&action1, action_processor_.current_action());
+ EXPECT_TRUE(action_processor_.IsRunning());
action1.CompleteAction();
- EXPECT_EQ(&action2, action_processor.current_action());
- EXPECT_TRUE(action_processor.IsRunning());
+ EXPECT_EQ(&action2, action_processor_.current_action());
+ EXPECT_TRUE(action_processor_.IsRunning());
action2.CompleteAction();
- EXPECT_EQ(nullptr, action_processor.current_action());
- EXPECT_FALSE(action_processor.IsRunning());
+ EXPECT_EQ(nullptr, action_processor_.current_action());
+ EXPECT_FALSE(action_processor_.IsRunning());
}
-TEST(ActionProcessorTest, DtorTest) {
+TEST_F(ActionProcessorTest, DtorTest) {
ActionProcessorTestAction action1, action2;
{
ActionProcessor action_processor;
@@ -169,22 +178,87 @@
EXPECT_FALSE(action2.IsRunning());
}
-TEST(ActionProcessorTest, DefaultDelegateTest) {
+TEST_F(ActionProcessorTest, DefaultDelegateTest) {
// Just make sure it doesn't crash
- ActionProcessorTestAction action;
- ActionProcessor action_processor;
- ActionProcessorDelegate delegate;
- action_processor.set_delegate(&delegate);
+ action_processor_.EnqueueAction(&action_);
+ action_processor_.StartProcessing();
+ action_.CompleteAction();
- action_processor.EnqueueAction(&action);
- action_processor.StartProcessing();
- action.CompleteAction();
+ action_processor_.EnqueueAction(&action_);
+ action_processor_.StartProcessing();
+ action_processor_.StopProcessing();
+}
- action_processor.EnqueueAction(&action);
- action_processor.StartProcessing();
- action_processor.StopProcessing();
+// This test suspends and resume the action processor while running one action_.
+TEST_F(ActionProcessorTest, SuspendResumeTest) {
+ action_processor_.EnqueueAction(&mock_action_);
- action_processor.set_delegate(nullptr);
+ testing::InSequence s;
+ EXPECT_CALL(mock_action_, PerformAction());
+ action_processor_.StartProcessing();
+
+ EXPECT_CALL(mock_action_, SuspendAction());
+ action_processor_.SuspendProcessing();
+ // Suspending the processor twice should not suspend the action twice.
+ action_processor_.SuspendProcessing();
+
+ // IsRunning should return whether there's is an action doing some work, even
+ // if it is suspended.
+ EXPECT_TRUE(action_processor_.IsRunning());
+ EXPECT_EQ(&mock_action_, action_processor_.current_action());
+
+ EXPECT_CALL(mock_action_, ResumeAction());
+ action_processor_.ResumeProcessing();
+
+ // Calling ResumeProcessing twice should not affect the action_.
+ action_processor_.ResumeProcessing();
+
+ action_processor_.ActionComplete(&mock_action_, ErrorCode::kSuccess);
+}
+
+// This test suspends an action that presumably doesn't support suspend/resume
+// and it finished before being resumed.
+TEST_F(ActionProcessorTest, ActionCompletedWhileSuspendedTest) {
+ action_processor_.EnqueueAction(&mock_action_);
+
+ testing::InSequence s;
+ EXPECT_CALL(mock_action_, PerformAction());
+ action_processor_.StartProcessing();
+
+ EXPECT_CALL(mock_action_, SuspendAction());
+ action_processor_.SuspendProcessing();
+
+ // Simulate the action completion while suspended. No other call to
+ // |mock_action_| is expected at this point.
+ action_processor_.ActionComplete(&mock_action_, ErrorCode::kSuccess);
+
+ // The processing should not be done since the ActionProcessor is suspended
+ // and the processing is considered to be still running until resumed.
+ EXPECT_FALSE(delegate_.processing_done_called_);
+ EXPECT_TRUE(action_processor_.IsRunning());
+
+ action_processor_.ResumeProcessing();
+ EXPECT_TRUE(delegate_.processing_done_called_);
+ EXPECT_FALSE(delegate_.processing_stopped_called_);
+}
+
+TEST_F(ActionProcessorTest, StoppedWhileSuspendedTest) {
+ action_processor_.EnqueueAction(&mock_action_);
+
+ testing::InSequence s;
+ EXPECT_CALL(mock_action_, PerformAction());
+ action_processor_.StartProcessing();
+ EXPECT_CALL(mock_action_, SuspendAction());
+ action_processor_.SuspendProcessing();
+
+ EXPECT_CALL(mock_action_, TerminateProcessing());
+ action_processor_.StopProcessing();
+ // Stopping the processing should abort the current execution no matter what.
+ EXPECT_TRUE(delegate_.processing_stopped_called_);
+ EXPECT_FALSE(delegate_.processing_done_called_);
+ EXPECT_FALSE(delegate_.action_completed_called_);
+ EXPECT_FALSE(action_processor_.IsRunning());
+ EXPECT_EQ(nullptr, action_processor_.current_action());
}
} // namespace chromeos_update_engine
diff --git a/common/constants.cc b/common/constants.cc
index 3b7aa6e..f138ce3 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -29,6 +29,8 @@
const char kStatefulPartition[] = "/mnt/stateful_partition";
+const char kPostinstallDefaultScript[] = "postinst";
+
// Constants defining keys for the persisted state of update engine.
const char kPrefsAttemptInProgress[] = "attempt-in-progress";
const char kPrefsBackoffExpiryTime[] = "backoff-expiry-time";
@@ -45,6 +47,7 @@
const char kPrefsLastActivePingDay[] = "last-active-ping-day";
const char kPrefsLastRollCallPingDay[] = "last-roll-call-ping-day";
const char kPrefsManifestMetadataSize[] = "manifest-metadata-size";
+const char kPrefsManifestSignatureSize[] = "manifest-signature-size";
const char kPrefsMetricsAttemptLastReportingTime[] =
"metrics-attempt-last-reporting-time";
const char kPrefsMetricsCheckLastReportingTime[] =
diff --git a/common/constants.h b/common/constants.h
index d001329..f0d589d 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -32,6 +32,9 @@
// The location where we store the AU preferences (state etc).
extern const char kPrefsSubDirectory[];
+// Path to the post install command, relative to the partition.
+extern const char kPostinstallDefaultScript[];
+
// Path to the stateful partition on the root filesystem.
extern const char kStatefulPartition[];
@@ -50,6 +53,7 @@
extern const char kPrefsLastActivePingDay[];
extern const char kPrefsLastRollCallPingDay[];
extern const char kPrefsManifestMetadataSize[];
+extern const char kPrefsManifestSignatureSize[];
extern const char kPrefsMetricsAttemptLastReportingTime[];
extern const char kPrefsMetricsCheckLastReportingTime[];
extern const char kPrefsNumReboots[];
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc
index bd723d7..0d4b5da 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::string;
using std::unique_ptr;
using std::vector;
+using testing::DoAll;
+using testing::Return;
+using testing::SaveArg;
+using testing::_;
namespace {
@@ -193,14 +198,19 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
// 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; }
@@ -555,26 +548,51 @@
} // 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 725bdd4..b2dffc2 100644
--- a/common/libcurl_http_fetcher.cc
+++ b/common/libcurl_http_fetcher.cc
@@ -99,6 +99,7 @@
curl_handle_ = curl_easy_init();
CHECK(curl_handle_);
+ ignore_failure_ = false;
CHECK(HasProxy());
bool is_direct = (GetCurrentProxy() == kNoProxy);
@@ -299,6 +300,12 @@
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();
}
@@ -344,96 +351,110 @@
return;
}
}
- if (0 == running_handles) {
- GetHttpResponseCode();
- if (http_response_code_) {
- LOG(INFO) << "HTTP response code: " << http_response_code_;
- no_network_retry_count_ = 0;
+
+ // 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)));
} else {
- LOG(ERROR) << "Unable to get http response code.";
+ // 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_)) {
+ 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)";
- // 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_++;
+ 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(kNoNetworkRetrySeconds));
- LOG(INFO) << "No HTTP response, retry " << no_network_retry_count_;
- return;
- }
-
- 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_)) {
- 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_));
- }
- } else {
- LOG(INFO) << "Transfer completed (" << http_response_code_
- << "), " << bytes_downloaded_ << " bytes downloaded";
- if (delegate_) {
- bool success = IsHttpResponseSuccess();
- delegate_->TransferComplete(this, success);
- }
+ 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) {
@@ -468,15 +489,43 @@
}
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.
@@ -556,7 +605,7 @@
// 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,
@@ -566,6 +615,10 @@
}
void LibcurlHttpFetcher::RetryTimeoutCallback() {
+ if (transfer_paused_) {
+ restart_transfer_on_unpause_ = true;
+ return;
+ }
ResumeTransfer(url_);
CurlPerformOnce();
}
@@ -618,6 +671,8 @@
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 5a64236..d126171 100644
--- a/common/libcurl_http_fetcher.h
+++ b/common/libcurl_http_fetcher.h
@@ -136,9 +136,9 @@
// 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
@@ -199,6 +199,16 @@
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_action.h b/common/mock_action.h
similarity index 74%
rename from mock_action.h
rename to common/mock_action.h
index 0ba796d..06acad1 100644
--- a/mock_action.h
+++ b/common/mock_action.h
@@ -14,8 +14,8 @@
// limitations under the License.
//
-#ifndef UPDATE_ENGINE_MOCK_ACTION_H_
-#define UPDATE_ENGINE_MOCK_ACTION_H_
+#ifndef UPDATE_ENGINE_COMMON_MOCK_ACTION_H_
+#define UPDATE_ENGINE_COMMON_MOCK_ACTION_H_
#include <string>
@@ -27,7 +27,7 @@
class MockAction;
-template<>
+template <>
class ActionTraits<MockAction> {
public:
typedef NoneType OutputObjectType;
@@ -36,10 +36,17 @@
class MockAction : public Action<MockAction> {
public:
+ MockAction() {
+ ON_CALL(*this, Type()).WillByDefault(testing::Return("MockAction"));
+ }
+
MOCK_METHOD0(PerformAction, void());
+ MOCK_METHOD0(TerminateProcessing, void());
+ MOCK_METHOD0(SuspendAction, void());
+ MOCK_METHOD0(ResumeAction, void());
MOCK_CONST_METHOD0(Type, std::string());
};
} // namespace chromeos_update_engine
-#endif // UPDATE_ENGINE_MOCK_ACTION_H_
+#endif // UPDATE_ENGINE_COMMON_MOCK_ACTION_H_
diff --git a/mock_action_processor.h b/common/mock_action_processor.h
similarity index 84%
rename from mock_action_processor.h
rename to common/mock_action_processor.h
index c98ff3c..04275c1 100644
--- a/mock_action_processor.h
+++ b/common/mock_action_processor.h
@@ -14,8 +14,8 @@
// limitations under the License.
//
-#ifndef UPDATE_ENGINE_MOCK_ACTION_PROCESSOR_H_
-#define UPDATE_ENGINE_MOCK_ACTION_PROCESSOR_H_
+#ifndef UPDATE_ENGINE_COMMON_MOCK_ACTION_PROCESSOR_H_
+#define UPDATE_ENGINE_COMMON_MOCK_ACTION_PROCESSOR_H_
#include <gmock/gmock.h>
@@ -31,4 +31,4 @@
} // namespace chromeos_update_engine
-#endif // UPDATE_ENGINE_MOCK_ACTION_PROCESSOR_H_
+#endif // UPDATE_ENGINE_COMMON_MOCK_ACTION_PROCESSOR_H_
diff --git a/common/platform_constants.h b/common/platform_constants.h
index d1786ff..6eaa940 100644
--- a/common/platform_constants.h
+++ b/common/platform_constants.h
@@ -50,6 +50,10 @@
// The stateful directory used by update_engine.
extern const char kNonVolatileDirectory[];
+// Options passed to the filesystem when mounting the new partition during
+// postinstall.
+extern const char kPostinstallMountOptions[];
+
} // namespace constants
} // namespace chromeos_update_engine
diff --git a/common/platform_constants_android.cc b/common/platform_constants_android.cc
index 4f55106..371fe26 100644
--- a/common/platform_constants_android.cc
+++ b/common/platform_constants_android.cc
@@ -31,6 +31,8 @@
// No deadline file API support on Android.
const char kOmahaResponseDeadlineFile[] = "";
const char kNonVolatileDirectory[] = "/data/misc/update_engine";
+const char kPostinstallMountOptions[] =
+ "context=u:object_r:postinstall_file:s0";
} // namespace constants
} // namespace chromeos_update_engine
diff --git a/common/platform_constants_chromeos.cc b/common/platform_constants_chromeos.cc
index d8587ca..7c1d627 100644
--- a/common/platform_constants_chromeos.cc
+++ b/common/platform_constants_chromeos.cc
@@ -32,6 +32,7 @@
"/tmp/update-check-response-deadline";
// This directory is wiped during powerwash.
const char kNonVolatileDirectory[] = "/var/lib/update_engine";
+const char kPostinstallMountOptions[] = nullptr;
} // namespace constants
} // namespace chromeos_update_engine
diff --git a/common/test_utils.cc b/common/test_utils.cc
index c09096b..77a9141 100644
--- a/common/test_utils.cc
+++ b/common/test_utils.cc
@@ -260,7 +260,7 @@
string loop_dev;
loop_binder_.reset(new ScopedLoopbackDeviceBinder(file_path, &loop_dev));
- EXPECT_TRUE(utils::MountFilesystem(loop_dev, *mnt_path, flags));
+ EXPECT_TRUE(utils::MountFilesystem(loop_dev, *mnt_path, flags, "", nullptr));
unmounter_.reset(new ScopedFilesystemUnmounter(*mnt_path));
}
diff --git a/common/utils.cc b/common/utils.cc
index 91dcfc8..912bc96 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -613,19 +613,27 @@
bool MountFilesystem(const string& device,
const string& mountpoint,
- unsigned long mountflags) { // NOLINT(runtime/int)
- // TODO(sosa): Remove "ext3" once crbug.com/208022 is resolved.
- const vector<const char*> fstypes{"ext2", "ext3", "ext4", "squashfs"};
+ unsigned long mountflags, // NOLINT(runtime/int)
+ const string& type,
+ const string& fs_mount_options) {
+ vector<const char*> fstypes;
+ if (type.empty()) {
+ fstypes = {"ext2", "ext3", "ext4", "squashfs"};
+ } else {
+ fstypes = {type.c_str()};
+ }
for (const char* fstype : fstypes) {
int rc = mount(device.c_str(), mountpoint.c_str(), fstype, mountflags,
- nullptr);
+ fs_mount_options.c_str());
if (rc == 0)
return true;
PLOG(WARNING) << "Unable to mount destination device " << device
<< " on " << mountpoint << " as " << fstype;
}
- LOG(ERROR) << "Unable to mount " << device << " with any supported type";
+ if (!type.empty()) {
+ LOG(ERROR) << "Unable to mount " << device << " with any supported type";
+ }
return false;
}
diff --git a/common/utils.h b/common/utils.h
index 5bf1422..df06ef1 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -171,10 +171,14 @@
std::string MakePartitionNameForMount(const std::string& part_name);
// Synchronously mount or unmount a filesystem. Return true on success.
-// When mounting, it will attempt to mount the the device as "ext3", "ext2" and
-// "squashfs", with the passed |flags| options.
-bool MountFilesystem(const std::string& device, const std::string& mountpoint,
- unsigned long flags); // NOLINT(runtime/int)
+// When mounting, it will attempt to mount the device as the passed filesystem
+// type |type|, with the passed |flags| options. If |type| is empty, "ext2",
+// "ext3", "ext4" and "squashfs" will be tried.
+bool MountFilesystem(const std::string& device,
+ const std::string& mountpoint,
+ unsigned long flags, // NOLINT(runtime/int)
+ const std::string& type,
+ const std::string& fs_mount_options);
bool UnmountFilesystem(const std::string& mountpoint);
// Returns the block count and the block byte size of the file system on
diff --git a/mock_proxy_resolver.h b/mock_proxy_resolver.h
new file mode 100644
index 0000000..0595f5a
--- /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/omaha_request_action.cc b/omaha_request_action.cc
index 1b0c2fe..bc84c9f 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1074,18 +1074,23 @@
// safe-guards). See http://crbug.com/297170 for an example)
size_t minimum_size = 0;
int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
int64_t next_data_offset = 0;
int64_t next_data_length = 0;
if (system_state_ &&
system_state_->prefs()->GetInt64(kPrefsManifestMetadataSize,
&manifest_metadata_size) &&
manifest_metadata_size != -1 &&
+ system_state_->prefs()->GetInt64(kPrefsManifestSignatureSize,
+ &manifest_signature_size) &&
+ manifest_signature_size != -1 &&
system_state_->prefs()->GetInt64(kPrefsUpdateStateNextDataOffset,
&next_data_offset) &&
next_data_offset != -1 &&
system_state_->prefs()->GetInt64(kPrefsUpdateStateNextDataLength,
&next_data_length)) {
- minimum_size = manifest_metadata_size + next_data_offset + next_data_length;
+ minimum_size = manifest_metadata_size + manifest_signature_size +
+ next_data_offset + next_data_length;
}
string file_id = utils::CalculateP2PFileId(response.hash, response.size);
diff --git a/payload_consumer/bzip_extent_writer_unittest.cc b/payload_consumer/bzip_extent_writer_unittest.cc
index 0abf04e..8ac3e59 100644
--- a/payload_consumer/bzip_extent_writer_unittest.cc
+++ b/payload_consumer/bzip_extent_writer_unittest.cc
@@ -16,6 +16,8 @@
#include "update_engine/payload_consumer/bzip_extent_writer.h"
+#include <fcntl.h>
+
#include <algorithm>
#include <string>
#include <vector>
@@ -82,8 +84,8 @@
TEST_F(BzipExtentWriterTest, ChunkedTest) {
// Generated with:
- // yes "ABC" | head -c 819200 | bzip2 -9 | \
- // hexdump -v -e '" " 11/1 "0x%02x, " "\n"'
+ // yes "ABC" | head -c 819200 | bzip2 -9 |
+ // hexdump -v -e '" " 11/1 "0x%02x, " "\n"'
static const uint8_t kCompressedData[] = {
0x42, 0x5a, 0x68, 0x39, 0x31, 0x41, 0x59, 0x26, 0x53, 0x59, 0xbe,
0x1c, 0xda, 0xee, 0x03, 0x1f, 0xff, 0xc4, 0x00, 0x00, 0x10, 0x38,
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index cb3cdb5..f490c08 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -578,6 +578,9 @@
LOG_IF(WARNING, !prefs_->SetInt64(kPrefsManifestMetadataSize,
metadata_size_))
<< "Unable to save the manifest metadata size.";
+ LOG_IF(WARNING, !prefs_->SetInt64(kPrefsManifestSignatureSize,
+ metadata_signature_size_))
+ << "Unable to save the manifest signature size.";
if (!PrimeUpdateState()) {
*error = ErrorCode::kDownloadStateInitializationError;
@@ -686,8 +689,10 @@
}
// In major version 2, we don't add dummy operation to the payload.
+ // If we already extracted the signature we should skip this step.
if (major_payload_version_ == kBrilloMajorPayloadVersion &&
- manifest_.has_signatures_offset() && manifest_.has_signatures_size()) {
+ manifest_.has_signatures_offset() && manifest_.has_signatures_size() &&
+ signatures_message_data_.empty()) {
if (manifest_.signatures_offset() != buffer_offset_) {
LOG(ERROR) << "Payload signatures offset points to blob offset "
<< manifest_.signatures_offset()
@@ -706,6 +711,10 @@
return false;
}
DiscardBuffer(true, 0);
+ // Since we extracted the SignatureMessage we need to advance the
+ // checkpoint, otherwise we would reload the signature and try to extract
+ // it again.
+ CheckpointUpdateProgress();
}
return true;
@@ -781,6 +790,12 @@
install_part.name = partition.partition_name();
install_part.run_postinstall =
partition.has_run_postinstall() && partition.run_postinstall();
+ if (install_part.run_postinstall) {
+ install_part.postinstall_path =
+ (partition.has_postinstall_path() ? partition.postinstall_path()
+ : kPostinstallDefaultScript);
+ install_part.filesystem_type = partition.filesystem_type();
+ }
if (partition.has_old_partition_info()) {
const PartitionInfo& info = partition.old_partition_info();
@@ -1666,8 +1681,10 @@
return false;
int64_t resumed_update_failures;
- if (!(prefs->GetInt64(kPrefsResumedUpdateFailures, &resumed_update_failures)
- && resumed_update_failures > kMaxResumedUpdateFailures))
+ // Note that storing this value is optional, but if it is there it should not
+ // be more than the limit.
+ if (prefs->GetInt64(kPrefsResumedUpdateFailures, &resumed_update_failures) &&
+ resumed_update_failures > kMaxResumedUpdateFailures)
return false;
// Sanity check the rest.
@@ -1686,6 +1703,12 @@
manifest_metadata_size > 0))
return false;
+ int64_t manifest_signature_size = 0;
+ if (!(prefs->GetInt64(kPrefsManifestSignatureSize,
+ &manifest_signature_size) &&
+ manifest_signature_size >= 0))
+ return false;
+
return true;
}
@@ -1700,6 +1723,7 @@
prefs->SetString(kPrefsUpdateStateSignedSHA256Context, "");
prefs->SetString(kPrefsUpdateStateSignatureBlob, "");
prefs->SetInt64(kPrefsManifestMetadataSize, -1);
+ prefs->SetInt64(kPrefsManifestSignatureSize, -1);
prefs->SetInt64(kPrefsResumedUpdateFailures, 0);
}
return true;
@@ -1786,6 +1810,12 @@
manifest_metadata_size > 0);
metadata_size_ = manifest_metadata_size;
+ int64_t manifest_signature_size = 0;
+ TEST_AND_RETURN_FALSE(
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size) &&
+ manifest_signature_size >= 0);
+ metadata_signature_size_ = manifest_signature_size;
+
// Advance the download progress to reflect what doesn't need to be
// re-downloaded.
total_bytes_received_ += buffer_offset_;
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index 34ce0de..3d7f8fa 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -693,6 +693,8 @@
MockPrefs prefs;
EXPECT_CALL(prefs, SetInt64(kPrefsManifestMetadataSize,
state->metadata_size)).WillOnce(Return(true));
+ EXPECT_CALL(prefs, SetInt64(kPrefsManifestSignatureSize, 0))
+ .WillOnce(Return(true));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
.WillRepeatedly(Return(true));
EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index 6a71fdb..fdbbd72 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -232,6 +232,14 @@
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 d000c67..fc32068 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -68,6 +68,9 @@
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 @@
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 @@
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/payload_consumer/extent_writer_unittest.cc b/payload_consumer/extent_writer_unittest.cc
index 4d6b4d6..24d238e 100644
--- a/payload_consumer/extent_writer_unittest.cc
+++ b/payload_consumer/extent_writer_unittest.cc
@@ -16,6 +16,8 @@
#include "update_engine/payload_consumer/extent_writer.h"
+#include <fcntl.h>
+
#include <algorithm>
#include <string>
#include <vector>
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index 572ff41..51e85b3 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -59,9 +59,13 @@
void InstallPlan::Dump() const {
string partitions_str;
for (const auto& partition : partitions) {
- partitions_str += base::StringPrintf(
- ", part: %s (source_size: %" PRIu64 ", target_size %" PRIu64 ")",
- partition.name.c_str(), partition.source_size, partition.target_size);
+ partitions_str +=
+ base::StringPrintf(", part: %s (source_size: %" PRIu64
+ ", target_size %" PRIu64 ", postinst:%s)",
+ partition.name.c_str(),
+ partition.source_size,
+ partition.target_size,
+ utils::ToString(partition.run_postinstall).c_str());
}
LOG(INFO) << "InstallPlan: "
@@ -109,7 +113,9 @@
target_path == that.target_path &&
target_size == that.target_size &&
target_hash == that.target_hash &&
- run_postinstall == that.run_postinstall);
+ run_postinstall == that.run_postinstall &&
+ postinstall_path == that.postinstall_path &&
+ filesystem_type == that.filesystem_type);
}
} // namespace chromeos_update_engine
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index d2f15fa..454dd78 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -90,8 +90,11 @@
uint64_t target_size{0};
brillo::Blob target_hash;
- // Whether we should run the postinstall script from this partition.
+ // Whether we should run the postinstall script from this partition and the
+ // postinstall parameters.
bool run_postinstall{false};
+ std::string postinstall_path;
+ std::string filesystem_type;
};
std::vector<Partition> partitions;
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index 84ca398..d57ef4e 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -23,9 +23,11 @@
#include <base/bind.h>
#include <base/files/file_path.h>
#include <base/files/file_util.h>
+#include <base/strings/string_util.h>
#include "update_engine/common/action_processor.h"
#include "update_engine/common/boot_control_interface.h"
+#include "update_engine/common/platform_constants.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
@@ -34,16 +36,6 @@
using std::string;
using std::vector;
-namespace {
-// The absolute path to the post install command.
-const char kPostinstallScript[] = "/postinst";
-
-// Path to the binary file used by kPostinstallScript. Used to get and log the
-// file format of the binary to debug issues when the ELF format on the update
-// doesn't match the one on the current system. This path is not executed.
-const char kDebugPostinstallBinaryPath[] = "/usr/bin/cros_installer";
-}
-
void PostinstallRunnerAction::PerformAction() {
CHECK(HasInputObject());
install_plan_ = GetInputObject();
@@ -60,6 +52,11 @@
}
void PostinstallRunnerAction::PerformPartitionPostinstall() {
+ if (install_plan_.download_url.empty()) {
+ LOG(INFO) << "Skipping post-install during rollback";
+ return CompletePostinstall(ErrorCode::kSuccess);
+ }
+
// Skip all the partitions that don't have a post-install step.
while (current_partition_ < install_plan_.partitions.size() &&
!install_plan_.partitions[current_partition_].run_postinstall) {
@@ -83,38 +80,45 @@
// Perform post-install for the current_partition_ partition. At this point we
// need to call CompletePartitionPostinstall to complete the operation and
// cleanup.
+#ifdef __ANDROID__
+ fs_mount_dir_ = "/postinstall";
+#else // __ANDROID__
TEST_AND_RETURN(
- utils::MakeTempDirectory("au_postint_mount.XXXXXX", &temp_rootfs_dir_));
+ utils::MakeTempDirectory("au_postint_mount.XXXXXX", &fs_mount_dir_));
+#endif // __ANDROID__
- if (!utils::MountFilesystem(mountable_device, temp_rootfs_dir_, MS_RDONLY)) {
+ string abs_path = base::FilePath(fs_mount_dir_)
+ .AppendASCII(partition.postinstall_path)
+ .value();
+ if (!base::StartsWith(
+ abs_path, fs_mount_dir_, base::CompareCase::SENSITIVE)) {
+ LOG(ERROR) << "Invalid relative postinstall path: "
+ << partition.postinstall_path;
+ return CompletePostinstall(ErrorCode::kPostinstallRunnerError);
+ }
+
+ if (!utils::MountFilesystem(mountable_device,
+ fs_mount_dir_,
+ MS_RDONLY,
+ partition.filesystem_type,
+ constants::kPostinstallMountOptions)) {
return CompletePartitionPostinstall(
1, "Error mounting the device " + mountable_device);
}
- LOG(INFO) << "Performing postinst (" << kPostinstallScript
- << ") installed on device " << partition.target_path
+ LOG(INFO) << "Performing postinst (" << partition.postinstall_path << " at "
+ << abs_path << ") installed on device " << partition.target_path
<< " and mountable device " << mountable_device;
// Logs the file format of the postinstall script we are about to run. This
// will help debug when the postinstall script doesn't match the architecture
// of our build.
- LOG(INFO) << "Format file for new " << kPostinstallScript << " is: "
- << utils::GetFileFormat(temp_rootfs_dir_ + kPostinstallScript);
- LOG(INFO) << "Format file for new " << kDebugPostinstallBinaryPath << " is: "
- << utils::GetFileFormat(
- temp_rootfs_dir_ + kDebugPostinstallBinaryPath);
+ LOG(INFO) << "Format file for new " << partition.postinstall_path
+ << " is: " << utils::GetFileFormat(abs_path);
// Runs the postinstall script asynchronously to free up the main loop while
// it's running.
- vector<string> command;
- if (!install_plan_.download_url.empty()) {
- command.push_back(temp_rootfs_dir_ + kPostinstallScript);
- } else {
- // TODO(sosa): crbug.com/366207.
- // If we're doing a rollback, just run our own postinstall.
- command.push_back(kPostinstallScript);
- }
- command.push_back(partition.target_path);
+ vector<string> command = {abs_path, partition.target_path};
if (!Subprocess::Get().Exec(
command,
base::Bind(
@@ -127,11 +131,13 @@
void PostinstallRunnerAction::CompletePartitionPostinstall(
int return_code,
const string& output) {
- utils::UnmountFilesystem(temp_rootfs_dir_);
- if (!base::DeleteFile(base::FilePath(temp_rootfs_dir_), false)) {
- PLOG(WARNING) << "Not removing mountpoint " << temp_rootfs_dir_;
+ utils::UnmountFilesystem(fs_mount_dir_);
+#ifndef ANDROID
+ if (!base::DeleteFile(base::FilePath(fs_mount_dir_), false)) {
+ PLOG(WARNING) << "Not removing temporary mountpoint " << fs_mount_dir_;
}
- temp_rootfs_dir_.clear();
+#endif // !ANDROID
+ fs_mount_dir_.clear();
if (return_code != 0) {
LOG(ERROR) << "Postinst command failed with code: " << return_code;
diff --git a/payload_consumer/postinstall_runner_action.h b/payload_consumer/postinstall_runner_action.h
index ab267b8..b4defae 100644
--- a/payload_consumer/postinstall_runner_action.h
+++ b/payload_consumer/postinstall_runner_action.h
@@ -63,7 +63,9 @@
void CompletePostinstall(ErrorCode error_code);
InstallPlan install_plan_;
- std::string temp_rootfs_dir_;
+
+ // The path where the filesystem will be mounted during post-install.
+ std::string fs_mount_dir_;
// The partition being processed on the list of partitions specified in the
// InstallPlan.
diff --git a/payload_consumer/postinstall_runner_action_unittest.cc b/payload_consumer/postinstall_runner_action_unittest.cc
index c4c68b1..85535d7 100644
--- a/payload_consumer/postinstall_runner_action_unittest.cc
+++ b/payload_consumer/postinstall_runner_action_unittest.cc
@@ -191,6 +191,7 @@
part.name = "part";
part.target_path = dev;
part.run_postinstall = true;
+ part.postinstall_path = kPostinstallDefaultScript;
InstallPlan install_plan;
install_plan.partitions = {part};
install_plan.download_url = "http://devserver:8080/update";
@@ -215,7 +216,7 @@
EXPECT_TRUE(delegate.code_set_);
EXPECT_EQ(should_succeed, delegate.code_ == ErrorCode::kSuccess);
if (should_succeed)
- EXPECT_TRUE(install_plan == collector_action.object());
+ EXPECT_EQ(install_plan, collector_action.object());
const base::FilePath kPowerwashMarkerPath(powerwash_marker_file);
string actual_cmd;
diff --git a/update_attempter.cc b/update_attempter.cc
index b67fcb3..cfd2425 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -1380,14 +1380,17 @@
if (response_handler_action_->install_plan().is_resume) {
// Resuming an update so fetch the update manifest metadata first.
int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
- fetcher->AddRange(0, manifest_metadata_size);
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
+ fetcher->AddRange(0, manifest_metadata_size + manifest_signature_size);
// If there're remaining unprocessed data blobs, fetch them. Be careful not
// to request data beyond the end of the payload to avoid 416 HTTP response
// error codes.
int64_t next_data_offset = 0;
prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset = manifest_metadata_size + next_data_offset;
+ uint64_t resume_offset =
+ manifest_metadata_size + manifest_signature_size + next_data_offset;
if (resume_offset < response_handler_action_->install_plan().payload_size) {
fetcher->AddRange(resume_offset);
}
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index 8403dec..bc9a698 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -180,6 +180,7 @@
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.
@@ -188,23 +189,24 @@
}
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) {
@@ -419,14 +421,18 @@
if (install_plan_.is_resume) {
// Resuming an update so fetch the update manifest metadata first.
int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
- fetcher->AddRange(base_offset_, manifest_metadata_size);
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
+ fetcher->AddRange(base_offset_,
+ manifest_metadata_size + manifest_signature_size);
// If there're remaining unprocessed data blobs, fetch them. Be careful not
// to request data beyond the end of the payload to avoid 416 HTTP response
// error codes.
int64_t next_data_offset = 0;
prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset = manifest_metadata_size + next_data_offset;
+ uint64_t resume_offset =
+ manifest_metadata_size + manifest_signature_size + next_data_offset;
if (!install_plan_.payload_size) {
fetcher->AddRange(base_offset_ + resume_offset);
} else if (resume_offset < install_plan_.payload_size) {
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index a0977b1..35bd206 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -38,6 +38,8 @@
#include "libcros/dbus-proxy-mocks.h"
#include "update_engine/common/fake_clock.h"
#include "update_engine/common/fake_prefs.h"
+#include "update_engine/common/mock_action.h"
+#include "update_engine/common/mock_action_processor.h"
#include "update_engine/common/mock_http_fetcher.h"
#include "update_engine/common/mock_prefs.h"
#include "update_engine/common/platform_constants.h"
@@ -45,8 +47,6 @@
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
#include "update_engine/fake_system_state.h"
-#include "update_engine/mock_action.h"
-#include "update_engine/mock_action_processor.h"
#include "update_engine/mock_p2p_manager.h"
#include "update_engine/mock_payload_state.h"
#include "update_engine/payload_consumer/filesystem_verifier_action.h"