Allow null SystemState in the DownloadAction.
The SystemState is only defined in the libupdate_engine library, so
it may not be defined for other users of libpayload_consumer. This
patch allows to pass a nullptr for the SystemState while explicitly
passing the other classes defined in libpayload_consumer upon
construction.
Bug: None
TEST=FEATURES=test emerge-link update_engine
TEST=`mmma system/update_engine` on aosp_arm-eng and edison-eng
Change-Id: I535d0184a85e0a167ac65875f6e7c07832efbf40
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index cb404b8..6a71fdb 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -39,9 +39,13 @@
namespace chromeos_update_engine {
DownloadAction::DownloadAction(PrefsInterface* prefs,
+ BootControlInterface* boot_control,
+ HardwareInterface* hardware,
SystemState* system_state,
HttpFetcher* http_fetcher)
: prefs_(prefs),
+ boot_control_(boot_control),
+ hardware_(hardware),
system_state_(system_state),
http_fetcher_(http_fetcher),
writer_(nullptr),
@@ -49,7 +53,8 @@
delegate_(nullptr),
bytes_received_(0),
p2p_sharing_fd_(-1),
- p2p_visible_(true) {}
+ p2p_visible_(true) {
+}
DownloadAction::~DownloadAction() {}
@@ -171,8 +176,7 @@
install_plan_.Dump();
LOG(INFO) << "Marking new slot as unbootable";
- if (!system_state_->boot_control()->MarkSlotUnbootable(
- install_plan_.target_slot)) {
+ if (!boot_control_->MarkSlotUnbootable(install_plan_.target_slot)) {
LOG(WARNING) << "Unable to mark new slot "
<< BootControlInterface::SlotName(install_plan_.target_slot)
<< ". Proceeding with the update anyway.";
@@ -181,14 +185,11 @@
if (writer_) {
LOG(INFO) << "Using writer for test.";
} else {
- delta_performer_.reset(new DeltaPerformer(prefs_,
- system_state_->boot_control(),
- system_state_->hardware(),
- delegate_,
- &install_plan_));
+ delta_performer_.reset(new DeltaPerformer(
+ prefs_, boot_control_, hardware_, delegate_, &install_plan_));
writer_ = delta_performer_.get();
}
- download_active_= true;
+ download_active_ = true;
if (system_state_ != nullptr) {
const PayloadStateInterface* payload_state = system_state_->payload_state();
@@ -236,7 +237,7 @@
writer_->Close();
writer_ = nullptr;
}
- download_active_= false;
+ download_active_ = false;
CloseP2PSharingFd(false); // Keep p2p file.
// Terminates the transfer. The action is terminated, if necessary, when the
// TransferTerminated callback is received.
@@ -275,8 +276,8 @@
// Call p2p_manager_->FileMakeVisible() when we've successfully
// verified the manifest!
- if (!p2p_visible_ &&
- delta_performer_.get() && delta_performer_->IsManifestValid()) {
+ if (!p2p_visible_ && system_state_ && delta_performer_.get() &&
+ delta_performer_->IsManifestValid()) {
LOG(INFO) << "Manifest has been validated. Making p2p file visible.";
system_state_->p2p_manager()->FileMakeVisible(p2p_file_id_);
p2p_visible_ = true;
@@ -288,7 +289,7 @@
LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer.";
writer_ = nullptr;
}
- download_active_= false;
+ download_active_ = false;
ErrorCode code =
successful ? ErrorCode::kSuccess : ErrorCode::kDownloadTransferError;
if (code == ErrorCode::kSuccess && delta_performer_.get()) {
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index 4074fdd..d000c67 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -27,6 +27,7 @@
#include <curl/curl.h>
#include "update_engine/common/action.h"
+#include "update_engine/common/boot_control_interface.h"
#include "update_engine/common/http_fetcher.h"
#include "update_engine/payload_consumer/delta_performer.h"
#include "update_engine/payload_consumer/install_plan.h"
@@ -69,8 +70,11 @@
public:
// Takes ownership of the passed in HttpFetcher. Useful for testing.
// A good calling pattern is:
- // DownloadAction(prefs, system_state, new WhateverHttpFetcher);
+ // DownloadAction(prefs, boot_contol, hardware, system_state,
+ // new WhateverHttpFetcher);
DownloadAction(PrefsInterface* prefs,
+ BootControlInterface* boot_control,
+ HardwareInterface* hardware,
SystemState* system_state,
HttpFetcher* http_fetcher);
~DownloadAction() override;
@@ -128,8 +132,10 @@
// The InstallPlan passed in
InstallPlan install_plan_;
- // Update Engine preference store.
+ // SystemState required pointers.
PrefsInterface* prefs_;
+ BootControlInterface* boot_control_;
+ HardwareInterface* hardware_;
// Global context for the system.
SystemState* system_state_;
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index 08053bf..60cc6f2 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -166,7 +166,11 @@
data.size(),
nullptr);
// takes ownership of passed in HttpFetcher
- DownloadAction download_action(&prefs, &fake_system_state, http_fetcher);
+ DownloadAction download_action(&prefs,
+ fake_system_state.boot_control(),
+ fake_system_state.hardware(),
+ &fake_system_state,
+ http_fetcher);
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
MockDownloadActionDelegate download_delegate;
@@ -278,10 +282,12 @@
feeder_action.set_obj(install_plan);
FakeSystemState fake_system_state_;
MockPrefs prefs;
- DownloadAction download_action(&prefs, &fake_system_state_,
- new MockHttpFetcher(data.data(),
- data.size(),
- nullptr));
+ DownloadAction download_action(
+ &prefs,
+ fake_system_state_.boot_control(),
+ fake_system_state_.hardware(),
+ &fake_system_state_,
+ new MockHttpFetcher(data.data(), data.size(), nullptr));
download_action.SetTestFileWriter(&writer);
MockDownloadActionDelegate download_delegate;
if (use_download_delegate) {
@@ -381,7 +387,10 @@
feeder_action.set_obj(install_plan);
MockPrefs prefs;
FakeSystemState fake_system_state_;
- DownloadAction download_action(&prefs, &fake_system_state_,
+ DownloadAction download_action(&prefs,
+ fake_system_state_.boot_control(),
+ fake_system_state_.hardware(),
+ &fake_system_state_,
new MockHttpFetcher("x", 1, nullptr));
download_action.SetTestFileWriter(&writer);
@@ -469,7 +478,10 @@
data_.length(),
nullptr);
// Note that DownloadAction takes ownership of the passed in HttpFetcher.
- download_action_.reset(new DownloadAction(&prefs, &fake_system_state_,
+ download_action_.reset(new DownloadAction(&prefs,
+ fake_system_state_.boot_control(),
+ fake_system_state_.hardware(),
+ &fake_system_state_,
http_fetcher_));
download_action_->SetTestFileWriter(&writer);
BondActions(&feeder_action, download_action_.get());
diff --git a/update_attempter.cc b/update_attempter.cc
index b826121..aeb433b 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -617,11 +617,12 @@
LibcurlHttpFetcher* download_fetcher =
new LibcurlHttpFetcher(GetProxyResolver(), system_state_->hardware());
download_fetcher->set_server_to_check(ServerToCheck::kDownload);
- shared_ptr<DownloadAction> download_action(
- new DownloadAction(prefs_,
- system_state_,
- new MultiRangeHttpFetcher(
- download_fetcher))); // passes ownership
+ shared_ptr<DownloadAction> download_action(new DownloadAction(
+ prefs_,
+ system_state_->boot_control(),
+ system_state_->hardware(),
+ system_state_,
+ new MultiRangeHttpFetcher(download_fetcher))); // passes ownership
shared_ptr<OmahaRequestAction> download_finished_action(
new OmahaRequestAction(
system_state_,
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 6703a96..e77b571 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -56,7 +56,6 @@
using base::Time;
using base::TimeDelta;
-using brillo::MessageLoop;
using org::chromium::LibCrosServiceInterfaceProxyMock;
using org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxyMock;
using std::string;
@@ -223,7 +222,7 @@
TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) {
unique_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, nullptr));
fetcher->FailTransfer(503); // Sets the HTTP response code.
- DownloadAction action(prefs_, nullptr, fetcher.release());
+ DownloadAction action(prefs_, nullptr, nullptr, nullptr, fetcher.release());
EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(nullptr, &action, ErrorCode::kSuccess);
EXPECT_EQ(503, attempter_.http_response_code());