summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--download_action.cc26
-rw-r--r--download_action_unittest.cc12
-rw-r--r--mock_payload_state.h6
-rw-r--r--omaha_request_action.cc21
-rw-r--r--omaha_request_action_unittest.cc54
-rw-r--r--omaha_request_params.h45
-rw-r--r--omaha_response_handler_action.cc16
-rw-r--r--omaha_response_handler_action_unittest.cc7
-rw-r--r--payload_state.cc5
-rw-r--r--payload_state.h37
-rw-r--r--payload_state_interface.h17
-rw-r--r--payload_state_unittest.cc6
-rw-r--r--update_attempter.cc18
-rw-r--r--update_attempter_unittest.cc81
14 files changed, 192 insertions, 159 deletions
diff --git a/download_action.cc b/download_action.cc
index e51549e3..f37828b0 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -15,6 +15,7 @@
#include "update_engine/action_pipe.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
+#include "update_engine/payload_state_interface.h"
#include "update_engine/subprocess.h"
#include "update_engine/utils.h"
@@ -187,9 +188,10 @@ void DownloadAction::PerformAction() {
}
if (system_state_ != nullptr) {
+ const PayloadStateInterface* payload_state = system_state_->payload_state();
string file_id = utils::CalculateP2PFileId(install_plan_.payload_hash,
install_plan_.payload_size);
- if (system_state_->request_params()->use_p2p_for_sharing()) {
+ if (payload_state->GetUsingP2PForSharing()) {
// If we're sharing the update, store the file_id to convey
// that we should write to the file.
p2p_file_id_ = file_id;
@@ -210,19 +212,17 @@ void DownloadAction::PerformAction() {
}
}
}
- }
- // Tweak timeouts on the HTTP fetcher if we're downloading from a
- // local peer.
- if (system_state_ != nullptr &&
- system_state_->request_params()->use_p2p_for_downloading() &&
- system_state_->request_params()->p2p_url() ==
- install_plan_.download_url) {
- LOG(INFO) << "Tweaking HTTP fetcher since we're downloading via p2p";
- http_fetcher_->set_low_speed_limit(kDownloadP2PLowSpeedLimitBps,
- kDownloadP2PLowSpeedTimeSeconds);
- http_fetcher_->set_max_retry_count(kDownloadP2PMaxRetryCount);
- http_fetcher_->set_connect_timeout(kDownloadP2PConnectTimeoutSeconds);
+ // Tweak timeouts on the HTTP fetcher if we're downloading from a
+ // local peer.
+ if (payload_state->GetUsingP2PForDownloading() &&
+ payload_state->GetP2PUrl() == install_plan_.download_url) {
+ LOG(INFO) << "Tweaking HTTP fetcher since we're downloading via p2p";
+ http_fetcher_->set_low_speed_limit(kDownloadP2PLowSpeedLimitBps,
+ kDownloadP2PLowSpeedTimeSeconds);
+ http_fetcher_->set_max_retry_count(kDownloadP2PMaxRetryCount);
+ http_fetcher_->set_connect_timeout(kDownloadP2PConnectTimeoutSeconds);
+ }
}
http_fetcher_->BeginTransfer(install_plan_.download_url);
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 705d7590..93a72aed 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -35,6 +35,7 @@ using std::unique_ptr;
using std::vector;
using testing::AtLeast;
using testing::InSequence;
+using testing::Return;
using testing::_;
class DownloadActionTest : public ::testing::Test { };
@@ -476,8 +477,9 @@ class P2PDownloadActionTest : public testing::Test {
// |use_p2p_to_share| parameter is used to indicate whether the
// payload should be shared via p2p.
void StartDownload(bool use_p2p_to_share) {
- fake_system_state_.request_params()->set_use_p2p_for_sharing(
- use_p2p_to_share);
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(),
+ GetUsingP2PForSharing())
+ .WillRepeatedly(Return(use_p2p_to_share));
ScopedTempFile output_temp_file;
TestDirectFileWriter writer;
@@ -562,9 +564,9 @@ TEST_F(P2PDownloadActionTest, IsWrittenTo) {
// Check the p2p file and its content matches what was sent.
string file_id = download_action_->p2p_file_id();
- EXPECT_NE(file_id, "");
- EXPECT_EQ(p2p_manager_->FileGetSize(file_id), data_.length());
- EXPECT_EQ(p2p_manager_->FileGetExpectedSize(file_id), data_.length());
+ EXPECT_NE("", file_id);
+ EXPECT_EQ(data_.length(), p2p_manager_->FileGetSize(file_id));
+ EXPECT_EQ(data_.length(), p2p_manager_->FileGetExpectedSize(file_id));
string p2p_file_contents;
EXPECT_TRUE(ReadFileToString(p2p_manager_->FileGetPath(file_id),
&p2p_file_contents));
diff --git a/mock_payload_state.h b/mock_payload_state.h
index c3175434..2d40a196 100644
--- a/mock_payload_state.h
+++ b/mock_payload_state.h
@@ -36,7 +36,9 @@ class MockPayloadState: public PayloadStateInterface {
MOCK_METHOD0(P2PNewAttempt, void());
MOCK_METHOD0(P2PAttemptAllowed, bool());
MOCK_METHOD1(SetUsingP2PForDownloading, void(bool value));
+ MOCK_METHOD1(SetUsingP2PForSharing, void(bool value));
MOCK_METHOD1(SetScatteringWaitPeriod, void(base::TimeDelta));
+ MOCK_METHOD1(SetP2PUrl, void(const std::string&));
// Getters.
MOCK_METHOD0(GetResponseSignature, std::string());
@@ -55,8 +57,10 @@ class MockPayloadState: public PayloadStateInterface {
MOCK_METHOD0(GetRollbackVersion, std::string());
MOCK_METHOD0(GetP2PNumAttempts, int());
MOCK_METHOD0(GetP2PFirstAttemptTimestamp, base::Time());
- MOCK_METHOD0(GetUsingP2PForDownloading, bool());
+ MOCK_CONST_METHOD0(GetUsingP2PForDownloading, bool());
+ MOCK_CONST_METHOD0(GetUsingP2PForSharing, bool());
MOCK_METHOD0(GetScatteringWaitPeriod, base::TimeDelta());
+ MOCK_CONST_METHOD0(GetP2PUrl, std::string());
};
} // namespace chromeos_update_engine
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index fad9ad11..7d7af81a 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -766,6 +766,8 @@ void OmahaRequestAction::TransferComplete(HttpFetcher *fetcher,
string current_response(response_buffer_.begin(), response_buffer_.end());
LOG(INFO) << "Omaha request response: " << current_response;
+ PayloadStateInterface* const payload_state = system_state_->payload_state();
+
// Events are best effort transactions -- assume they always succeed.
if (IsEvent()) {
CHECK(!HasOutputPipe()) << "No output pipe allowed for event requests.";
@@ -840,12 +842,12 @@ void OmahaRequestAction::TransferComplete(HttpFetcher *fetcher,
if (output_object.disable_p2p_for_downloading) {
LOG(INFO) << "Forcibly disabling use of p2p for downloading as "
<< "requested by Omaha.";
- params_->set_use_p2p_for_downloading(false);
+ payload_state->SetUsingP2PForDownloading(false);
}
if (output_object.disable_p2p_for_sharing) {
LOG(INFO) << "Forcibly disabling use of p2p for sharing as "
<< "requested by Omaha.";
- params_->set_use_p2p_for_sharing(false);
+ payload_state->SetUsingP2PForSharing(false);
}
// Update the payload state with the current response. The payload state
@@ -854,17 +856,16 @@ void OmahaRequestAction::TransferComplete(HttpFetcher *fetcher,
// as possible in this method so that if a new release gets pushed and then
// got pulled back due to some issues, we don't want to clear our internal
// state unnecessarily.
- PayloadStateInterface* payload_state = system_state_->payload_state();
payload_state->SetResponse(output_object);
// It could be we've already exceeded the deadline for when p2p is
// allowed or that we've tried too many times with p2p. Check that.
- if (params_->use_p2p_for_downloading()) {
+ if (payload_state->GetUsingP2PForDownloading()) {
payload_state->P2PNewAttempt();
if (!payload_state->P2PAttemptAllowed()) {
LOG(INFO) << "Forcibly disabling use of p2p for downloading because "
<< "of previous failures when using p2p.";
- params_->set_use_p2p_for_downloading(false);
+ payload_state->SetUsingP2PForDownloading(false);
}
}
@@ -877,7 +878,7 @@ void OmahaRequestAction::TransferComplete(HttpFetcher *fetcher,
// attention to wall-clock-based waiting if the URL is indeed
// available via p2p. Therefore, check if the file is available via
// p2p before deferring...
- if (params_->use_p2p_for_downloading()) {
+ if (payload_state->GetUsingP2PForDownloading()) {
LookupPayloadViaP2P(output_object);
} else {
CompleteProcessing();
@@ -909,11 +910,11 @@ void OmahaRequestAction::CompleteProcessing() {
void OmahaRequestAction::OnLookupPayloadViaP2PCompleted(const string& url) {
LOG(INFO) << "Lookup complete, p2p-client returned URL '" << url << "'";
if (!url.empty()) {
- params_->set_p2p_url(url);
+ system_state_->payload_state()->SetP2PUrl(url);
} else {
LOG(INFO) << "Forcibly disabling use of p2p for downloading "
<< "because no suitable peer could be found.";
- params_->set_use_p2p_for_downloading(false);
+ system_state_->payload_state()->SetUsingP2PForDownloading(false);
}
CompleteProcessing();
}
@@ -971,7 +972,9 @@ bool OmahaRequestAction::ShouldDeferDownload(OmahaResponse* output_object) {
// defer the download. This is because the download will always
// happen from a peer on the LAN and we've been waiting in line for
// our turn.
- if (params_->use_p2p_for_downloading() && !params_->p2p_url().empty()) {
+ const PayloadStateInterface* payload_state = system_state_->payload_state();
+ if (payload_state->GetUsingP2PForDownloading() &&
+ !payload_state->GetP2PUrl().empty()) {
LOG(INFO) << "Download not deferred because download "
<< "will happen from a local peer (via p2p).";
return false;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 775f074b..4adf9987 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -19,6 +19,7 @@
#include "update_engine/fake_prefs.h"
#include "update_engine/mock_connection_manager.h"
#include "update_engine/mock_http_fetcher.h"
+#include "update_engine/mock_payload_state.h"
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/omaha_request_action.h"
#include "update_engine/omaha_request_params.h"
@@ -37,6 +38,8 @@ using testing::Ge;
using testing::Le;
using testing::NiceMock;
using testing::Return;
+using testing::ReturnPointee;
+using testing::SaveArg;
using testing::SetArgumentPointee;
using testing::_;
@@ -116,9 +119,7 @@ class OmahaRequestActionTest : public ::testing::Test {
false, // delta okay
false, // interactive
"http://url",
- "", // target_version_prefix
- false, // use_p2p_for_downloading
- false}; // use_p2p_for_sharing
+ ""}; // target_version_prefix
FakePrefs fake_prefs_;
};
@@ -1049,9 +1050,7 @@ TEST_F(OmahaRequestActionTest, XmlEncodeTest) {
false, // delta okay
false, // interactive
"http://url",
- "", // target_version_prefix
- false, // use_p2p_for_downloading
- false); // use_p2p_for_sharing
+ ""); // target_version_prefix
OmahaResponse response;
ASSERT_FALSE(
TestUpdateCheck(&params,
@@ -1247,9 +1246,7 @@ TEST_F(OmahaRequestActionTest, FormatDeltaOkayOutputTest) {
delta_okay,
false, // interactive
"http://url",
- "", // target_version_prefix
- false, // use_p2p_for_downloading
- false); // use_p2p_for_sharing
+ ""); // target_version_prefix
ASSERT_FALSE(TestUpdateCheck(&params,
"invalid xml>",
-1,
@@ -1290,9 +1287,7 @@ TEST_F(OmahaRequestActionTest, FormatInteractiveOutputTest) {
true, // delta_okay
interactive,
"http://url",
- "", // target_version_prefix
- false, // use_p2p_for_downloading
- false); // use_p2p_for_sharing
+ ""); // target_version_prefix
ASSERT_FALSE(TestUpdateCheck(&params,
"invalid xml>",
-1,
@@ -1924,13 +1919,25 @@ void OmahaRequestActionTest::P2PTest(
const string& expected_p2p_url) {
OmahaResponse response;
OmahaRequestParams request_params = request_params_;
- request_params.set_use_p2p_for_downloading(initial_allow_p2p_for_downloading);
- request_params.set_use_p2p_for_sharing(initial_allow_p2p_for_sharing);
+ bool actual_allow_p2p_for_downloading = initial_allow_p2p_for_downloading;
+ bool actual_allow_p2p_for_sharing = initial_allow_p2p_for_sharing;
+ string actual_p2p_url;
MockPayloadState mock_payload_state;
fake_system_state_.set_payload_state(&mock_payload_state);
EXPECT_CALL(mock_payload_state, P2PAttemptAllowed())
.WillRepeatedly(Return(payload_state_allow_p2p_attempt));
+ EXPECT_CALL(mock_payload_state, GetUsingP2PForDownloading())
+ .WillRepeatedly(ReturnPointee(&actual_allow_p2p_for_downloading));
+ EXPECT_CALL(mock_payload_state, GetUsingP2PForSharing())
+ .WillRepeatedly(ReturnPointee(&actual_allow_p2p_for_sharing));
+ EXPECT_CALL(mock_payload_state, SetUsingP2PForDownloading(_))
+ .WillRepeatedly(SaveArg<0>(&actual_allow_p2p_for_downloading));
+ EXPECT_CALL(mock_payload_state, SetUsingP2PForSharing(_))
+ .WillRepeatedly(SaveArg<0>(&actual_allow_p2p_for_sharing));
+ EXPECT_CALL(mock_payload_state, SetP2PUrl(_))
+ .WillRepeatedly(SaveArg<0>(&actual_p2p_url));
+
MockP2PManager mock_p2p_manager;
fake_system_state_.set_p2p_manager(&mock_p2p_manager);
mock_p2p_manager.fake().SetLookupUrlForFileResult(p2p_client_result_url);
@@ -1965,18 +1972,15 @@ void OmahaRequestActionTest::P2PTest(
nullptr));
EXPECT_TRUE(response.update_exists);
- EXPECT_EQ(response.disable_p2p_for_downloading,
- omaha_disable_p2p_for_downloading);
- EXPECT_EQ(response.disable_p2p_for_sharing,
- omaha_disable_p2p_for_sharing);
-
- EXPECT_EQ(request_params.use_p2p_for_downloading(),
- expected_allow_p2p_for_downloading);
-
- EXPECT_EQ(request_params.use_p2p_for_sharing(),
- expected_allow_p2p_for_sharing);
+ EXPECT_EQ(omaha_disable_p2p_for_downloading,
+ response.disable_p2p_for_downloading);
+ EXPECT_EQ(omaha_disable_p2p_for_sharing,
+ response.disable_p2p_for_sharing);
- EXPECT_EQ(request_params.p2p_url(), expected_p2p_url);
+ EXPECT_EQ(expected_allow_p2p_for_downloading,
+ actual_allow_p2p_for_downloading);
+ EXPECT_EQ(expected_allow_p2p_for_sharing, actual_allow_p2p_for_sharing);
+ EXPECT_EQ(expected_p2p_url, actual_p2p_url);
}
TEST_F(OmahaRequestActionTest, P2PWithPeer) {
diff --git a/omaha_request_params.h b/omaha_request_params.h
index 62903223..94db4755 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -45,9 +45,7 @@ class OmahaRequestParams {
max_update_checks_allowed_(kDefaultMaxUpdateChecks),
is_powerwash_allowed_(false),
force_lock_down_(false),
- forced_lock_down_(false),
- use_p2p_for_downloading_(false),
- use_p2p_for_sharing_(false) {
+ forced_lock_down_(false) {
InitFromLsbValue();
}
@@ -66,9 +64,7 @@ class OmahaRequestParams {
bool in_delta_okay,
bool in_interactive,
const std::string& in_update_url,
- const std::string& in_target_version_prefix,
- bool in_use_p2p_for_downloading,
- bool in_use_p2p_for_sharing)
+ const std::string& in_target_version_prefix)
: system_state_(system_state),
os_platform_(in_os_platform),
os_version_(in_os_version),
@@ -93,9 +89,7 @@ class OmahaRequestParams {
max_update_checks_allowed_(kDefaultMaxUpdateChecks),
is_powerwash_allowed_(false),
force_lock_down_(false),
- forced_lock_down_(false),
- use_p2p_for_downloading_(in_use_p2p_for_downloading),
- use_p2p_for_sharing_(in_use_p2p_for_sharing) {}
+ forced_lock_down_(false) {}
// Setters and getters for the various properties.
inline std::string os_platform() const { return os_platform_; }
@@ -171,27 +165,6 @@ class OmahaRequestParams {
return max_update_checks_allowed_;
}
- inline void set_use_p2p_for_downloading(bool value) {
- use_p2p_for_downloading_ = value;
- }
- inline bool use_p2p_for_downloading() const {
- return use_p2p_for_downloading_;
- }
-
- inline void set_use_p2p_for_sharing(bool value) {
- use_p2p_for_sharing_ = value;
- }
- inline bool use_p2p_for_sharing() const {
- return use_p2p_for_sharing_;
- }
-
- inline void set_p2p_url(const std::string& value) {
- p2p_url_ = value;
- }
- inline std::string p2p_url() const {
- return p2p_url_;
- }
-
// True if we're trying to update to a more stable channel.
// i.e. index(target_channel) > index(current_channel).
virtual bool to_more_stable_channel() const;
@@ -377,18 +350,6 @@ class OmahaRequestParams {
bool force_lock_down_;
bool forced_lock_down_;
- // True if we may use p2p to download. This is based on owner
- // preferences and policy.
- bool use_p2p_for_downloading_;
-
- // True if we may use p2p to share. This is based on owner
- // preferences and policy.
- bool use_p2p_for_sharing_;
-
- // An URL to a local peer serving the requested payload or "" if no
- // such peer is available.
- std::string p2p_url_;
-
// TODO(jaysri): Uncomment this after fixing unit tests, as part of
// chromium-os:39752
// DISALLOW_COPY_AND_ASSIGN(OmahaRequestParams);
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 4846a000..fcb312a1 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -64,15 +64,17 @@ void OmahaResponseHandlerAction::PerformAction() {
install_plan_.download_url = current_url;
install_plan_.version = response.version;
- OmahaRequestParams* params = system_state_->request_params();
+ OmahaRequestParams* const params = system_state_->request_params();
+ PayloadStateInterface* const payload_state = system_state_->payload_state();
// If we're using p2p to download and there is a local peer, use it.
- if (params->use_p2p_for_downloading() && !params->p2p_url().empty()) {
+ if (payload_state->GetUsingP2PForDownloading() &&
+ !payload_state->GetP2PUrl().empty()) {
LOG(INFO) << "Replacing URL " << install_plan_.download_url
- << " with local URL " << params->p2p_url()
+ << " with local URL " << payload_state->GetP2PUrl()
<< " since p2p is enabled.";
- install_plan_.download_url = params->p2p_url();
- system_state_->payload_state()->SetUsingP2PForDownloading(true);
+ install_plan_.download_url = payload_state->GetP2PUrl();
+ payload_state->SetUsingP2PForDownloading(true);
}
// Fill up the other properties based on the response.
@@ -85,9 +87,9 @@ void OmahaResponseHandlerAction::PerformAction() {
install_plan_.is_resume =
DeltaPerformer::CanResumeUpdate(system_state_->prefs(), response.hash);
if (install_plan_.is_resume) {
- system_state_->payload_state()->UpdateResumed();
+ payload_state->UpdateResumed();
} else {
- system_state_->payload_state()->UpdateRestarted();
+ payload_state->UpdateRestarted();
LOG_IF(WARNING, !DeltaPerformer::ResetUpdateProgress(
system_state_->prefs(), false))
<< "Unable to reset the update progress.";
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 242cf616..49b5c138 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -8,6 +8,7 @@
#include "update_engine/constants.h"
#include "update_engine/fake_system_state.h"
+#include "update_engine/mock_payload_state.h"
#include "update_engine/omaha_response_handler_action.h"
#include "update_engine/test_utils.h"
#include "update_engine/utils.h"
@@ -349,8 +350,10 @@ TEST_F(OmahaResponseHandlerActionTest, P2PUrlIsUsedAndHashChecksMandatory) {
SetUsingP2PForDownloading(true));
string p2p_url = "http://9.8.7.6/p2p";
- params.set_p2p_url(p2p_url);
- params.set_use_p2p_for_downloading(true);
+ EXPECT_CALL(*fake_system_state.mock_payload_state(), GetP2PUrl())
+ .WillRepeatedly(Return(p2p_url));
+ EXPECT_CALL(*fake_system_state.mock_payload_state(),
+ GetUsingP2PForDownloading()).WillRepeatedly(Return(true));
InstallPlan install_plan;
EXPECT_TRUE(DoTestCommon(&fake_system_state, in, "/dev/sda5", "",
diff --git a/payload_state.cc b/payload_state.cc
index 4ce3d673..87965d7f 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -41,12 +41,12 @@ static const uint32_t kMaxBackoffFuzzMinutes = 12 * 60;
PayloadState::PayloadState()
: prefs_(nullptr),
using_p2p_for_downloading_(false),
+ p2p_num_attempts_(0),
payload_attempt_number_(0),
full_payload_attempt_number_(0),
url_index_(0),
url_failure_count_(0),
url_switch_count_(0),
- p2p_num_attempts_(0),
attempt_num_bytes_downloaded_(0),
attempt_connection_type_(metrics::ConnectionType::kUnknown),
attempt_type_(AttemptType::kUpdate) {
@@ -357,8 +357,7 @@ bool PayloadState::ShouldBackoffDownload() {
"Can proceed with the download";
return false;
}
- if (system_state_->request_params()->use_p2p_for_downloading() &&
- !system_state_->request_params()->p2p_url().empty()) {
+ if (GetUsingP2PForDownloading() && !GetP2PUrl().empty()) {
LOG(INFO) << "Payload backoff logic is disabled because download "
<< "will happen from local peer (via p2p).";
return false;
diff --git a/payload_state.h b/payload_state.h
index 8eb541cd..a301cebe 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -50,6 +50,10 @@ class PayloadState : public PayloadStateInterface {
virtual void ExpectRebootInNewVersion(const std::string& target_version_uid);
virtual void SetUsingP2PForDownloading(bool value);
+ void SetUsingP2PForSharing(bool value) override {
+ using_p2p_for_sharing_ = value;
+ }
+
virtual inline std::string GetResponseSignature() {
return response_signature_;
}
@@ -109,16 +113,28 @@ class PayloadState : public PayloadStateInterface {
virtual void P2PNewAttempt();
virtual bool P2PAttemptAllowed();
- virtual bool GetUsingP2PForDownloading() {
+ bool GetUsingP2PForDownloading() const override {
return using_p2p_for_downloading_;
}
+ bool GetUsingP2PForSharing() const override {
+ return using_p2p_for_sharing_;
+ }
+
base::TimeDelta GetScatteringWaitPeriod() override {
return scattering_wait_period_;
}
void SetScatteringWaitPeriod(base::TimeDelta wait_period) override;
+ void SetP2PUrl(const std::string& url) override {
+ p2p_url_ = url;
+ }
+
+ std::string GetP2PUrl() const override {
+ return p2p_url_;
+ }
+
private:
enum class AttemptType {
kUpdate,
@@ -410,9 +426,18 @@ class PayloadState : public PayloadStateInterface {
// This is the current response object from Omaha.
OmahaResponse response_;
- // Whether p2p is being used for downloading as set with the
- // SetUsingP2PForDownloading() method.
+ // Whether P2P is being used for downloading and sharing.
bool using_p2p_for_downloading_;
+ bool using_p2p_for_sharing_;
+
+ // Stores the P2P download URL, if one is used.
+ std::string p2p_url_;
+
+ // The cached value of |kPrefsP2PFirstAttemptTimestamp|.
+ base::Time p2p_first_attempt_timestamp_;
+
+ // The cached value of |kPrefsP2PNumAttempts|.
+ int p2p_num_attempts_;
// This stores a "signature" of the current response. The signature here
// refers to a subset of the current response from Omaha. Each update to
@@ -517,12 +542,6 @@ class PayloadState : public PayloadStateInterface {
// reboot.
std::string rollback_version_;
- // The cached value of |kPrefsP2PFirstAttemptTimestamp|.
- base::Time p2p_first_attempt_timestamp_;
-
- // The cached value of |kPrefsP2PNumAttempts|.
- int p2p_num_attempts_;
-
// The number of bytes downloaded per attempt.
int64_t attempt_num_bytes_downloaded_;
diff --git a/payload_state_interface.h b/payload_state_interface.h
index c706e984..1825a40d 100644
--- a/payload_state_interface.h
+++ b/payload_state_interface.h
@@ -74,11 +74,14 @@ class PayloadStateInterface {
virtual void ExpectRebootInNewVersion(
const std::string& target_version_uid) = 0;
- // Sets whether p2p is being used to download the update payload. This
- // is used to keep track download sources being used and should be called
+ // Sets whether P2P is being used to download the update payload. This
+ // is used to keep track of download sources being used and should be called
// before the transfer begins.
virtual void SetUsingP2PForDownloading(bool value) = 0;
+ // Sets whether P2P is being used for sharing the update payloads.
+ virtual void SetUsingP2PForSharing(bool value) = 0;
+
// Returns true if we should backoff the current download attempt.
// False otherwise.
virtual bool ShouldBackoffDownload() = 0;
@@ -161,14 +164,20 @@ class PayloadStateInterface {
// as when the first attempt was.
virtual bool P2PAttemptAllowed() = 0;
- // Gets the value previously set with SetUsingP2PForDownloading().
- virtual bool GetUsingP2PForDownloading() = 0;
+ // Gets the values previously set with SetUsingP2PForDownloading() and
+ // SetUsingP2PForSharing().
+ virtual bool GetUsingP2PForDownloading() const = 0;
+ virtual bool GetUsingP2PForSharing() const = 0;
// Returns the current (persisted) scattering wallclock-based wait period.
virtual base::TimeDelta GetScatteringWaitPeriod() = 0;
// Sets and persists the scattering wallclock-based wait period.
virtual void SetScatteringWaitPeriod(base::TimeDelta wait_period) = 0;
+
+ // Sets/gets the P2P download URL, if one is to be used.
+ virtual void SetP2PUrl(const std::string& url) = 0;
+ virtual std::string GetP2PUrl() const = 0;
};
} // namespace chromeos_update_engine
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index cec69f1e..9d732009 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -695,12 +695,12 @@ TEST(PayloadStateTest, NoBackoffForP2PUpdates) {
EXPECT_EQ(1, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(1, payload_state.GetFullPayloadAttemptNumber());
// Set p2p url.
- params.set_use_p2p_for_downloading(true);
- params.set_p2p_url("http://mypeer:52909/path/to/file");
+ payload_state.SetUsingP2PForDownloading(true);
+ payload_state.SetP2PUrl("http://mypeer:52909/path/to/file");
// Should not backoff for p2p updates.
EXPECT_FALSE(payload_state.ShouldBackoffDownload());
- params.set_p2p_url("");
+ payload_state.SetP2PUrl("");
// No actual p2p update if no url is provided.
EXPECT_TRUE(payload_state.ShouldBackoffDownload());
}
diff --git a/update_attempter.cc b/update_attempter.cc
index 29115402..200493d2 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -352,8 +352,9 @@ void UpdateAttempter::CalculateP2PParams(bool interactive) {
}
}
- omaha_request_params_->set_use_p2p_for_downloading(use_p2p_for_downloading);
- omaha_request_params_->set_use_p2p_for_sharing(use_p2p_for_sharing);
+ PayloadStateInterface* const payload_state = system_state_->payload_state();
+ payload_state->SetUsingP2PForDownloading(use_p2p_for_downloading);
+ payload_state->SetUsingP2PForSharing(use_p2p_for_sharing);
}
bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
@@ -363,6 +364,7 @@ bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
bool obey_proxies,
bool interactive) {
http_response_code_ = 0;
+ PayloadStateInterface* const payload_state = system_state_->payload_state();
// Refresh the policy before computing all the update parameters.
RefreshDevicePolicy();
@@ -374,15 +376,15 @@ bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
CalculateScatteringParams(interactive);
CalculateP2PParams(interactive);
- if (omaha_request_params_->use_p2p_for_downloading() ||
- omaha_request_params_->use_p2p_for_sharing()) {
+ if (payload_state->GetUsingP2PForDownloading() ||
+ payload_state->GetUsingP2PForSharing()) {
// OK, p2p is to be used - start it and perform housekeeping.
if (!StartP2PAndPerformHousekeeping()) {
// If this fails, disable p2p for this attempt
LOG(INFO) << "Forcibly disabling use of p2p since starting p2p or "
<< "performing housekeeping failed.";
- omaha_request_params_->set_use_p2p_for_downloading(false);
- omaha_request_params_->set_use_p2p_for_sharing(false);
+ payload_state->SetUsingP2PForDownloading(false);
+ payload_state->SetUsingP2PForSharing(false);
}
}
@@ -423,9 +425,9 @@ bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
omaha_request_params_->waiting_period().InSeconds());
LOG(INFO) << "Use p2p For Downloading = "
- << omaha_request_params_->use_p2p_for_downloading()
+ << payload_state->GetUsingP2PForDownloading()
<< ", Use p2p For Sharing = "
- << omaha_request_params_->use_p2p_for_sharing();
+ << payload_state->GetUsingP2PForSharing();
obeying_proxies_ = true;
if (obey_proxies || proxy_manual_checks_ == 0) {
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 9098180c..6dc921bc 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -37,7 +37,9 @@ using testing::InSequence;
using testing::Ne;
using testing::NiceMock;
using testing::Property;
+using testing::SaveArg;
using testing::Return;
+using testing::ReturnPointee;
using testing::SetArgumentPointee;
using testing::_;
@@ -118,6 +120,22 @@ class UpdateAttempterTest : public ::testing::Test {
processor_ = new NiceMock<ActionProcessorMock>();
attempter_.processor_.reset(processor_); // Transfers ownership.
prefs_ = fake_system_state_.mock_prefs();
+
+ // Set up store/load semantics of P2P properties via the mock PayloadState.
+ actual_using_p2p_for_downloading_ = false;
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(),
+ SetUsingP2PForDownloading(_))
+ .WillRepeatedly(SaveArg<0>(&actual_using_p2p_for_downloading_));
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(),
+ GetUsingP2PForDownloading())
+ .WillRepeatedly(ReturnPointee(&actual_using_p2p_for_downloading_));
+ actual_using_p2p_for_sharing_ = false;
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(),
+ SetUsingP2PForSharing(_))
+ .WillRepeatedly(SaveArg<0>(&actual_using_p2p_for_sharing_));
+ EXPECT_CALL(*fake_system_state_.mock_payload_state(),
+ GetUsingP2PForDownloading())
+ .WillRepeatedly(ReturnPointee(&actual_using_p2p_for_sharing_));
}
virtual void TearDown() {
@@ -169,6 +187,13 @@ class UpdateAttempterTest : public ::testing::Test {
void P2PEnabledHousekeepingFailsStart();
static gboolean StaticP2PEnabledHousekeepingFails(gpointer data);
+ bool actual_using_p2p_for_downloading() {
+ return actual_using_p2p_for_downloading_;
+ }
+ bool actual_using_p2p_for_sharing() {
+ return actual_using_p2p_for_sharing_;
+ }
+
FakeSystemState fake_system_state_;
NiceMock<MockDBusWrapper> dbus_;
UpdateAttempterUnderTest attempter_;
@@ -178,6 +203,9 @@ class UpdateAttempterTest : public ::testing::Test {
GMainLoop* loop_;
string test_dir_;
+
+ bool actual_using_p2p_for_downloading_;
+ bool actual_using_p2p_for_sharing_;
};
TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) {
@@ -257,7 +285,7 @@ TEST_F(UpdateAttempterTest, GetErrorCodeForActionTest) {
GetErrorCodeForAction(&postinstall_runner_action,
ErrorCode::kError));
ActionMock action_mock;
- EXPECT_CALL(action_mock, Type()).Times(1).WillOnce(Return("ActionMock"));
+ EXPECT_CALL(action_mock, Type()).WillOnce(Return("ActionMock"));
EXPECT_EQ(ErrorCode::kError,
GetErrorCodeForAction(&action_mock, ErrorCode::kError));
}
@@ -296,10 +324,9 @@ TEST_F(UpdateAttempterTest, MarkDeltaUpdateFailureTest) {
EXPECT_CALL(*prefs_, SetInt64(Ne(kPrefsDeltaUpdateFailures), _))
.WillRepeatedly(Return(true));
EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 1)).Times(2);
- EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 2)).Times(1);
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 2));
EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures,
- UpdateAttempter::kMaxDeltaUpdateFailures + 1))
- .Times(1);
+ UpdateAttempter::kMaxDeltaUpdateFailures + 1));
for (int i = 0; i < 4; i ++)
attempter_.MarkDeltaUpdateFailure();
}
@@ -323,9 +350,8 @@ TEST_F(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest) {
TEST_F(UpdateAttempterTest, ScheduleErrorEventActionTest) {
EXPECT_CALL(*processor_,
EnqueueAction(Property(&AbstractAction::Type,
- OmahaRequestAction::StaticType())))
- .Times(1);
- EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ OmahaRequestAction::StaticType())));
+ EXPECT_CALL(*processor_, StartProcessing());
ErrorCode err = ErrorCode::kError;
EXPECT_CALL(*fake_system_state_.mock_payload_state(), UpdateFailed(err));
attempter_.error_event_.reset(new OmahaEvent(OmahaEvent::kTypeUpdateComplete,
@@ -443,9 +469,9 @@ void UpdateAttempterTest::UpdateTestStart() {
for (size_t i = 0; i < arraysize(kUpdateActionTypes); ++i) {
EXPECT_CALL(*processor_,
EnqueueAction(Property(&AbstractAction::Type,
- kUpdateActionTypes[i]))).Times(1);
+ kUpdateActionTypes[i])));
}
- EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ EXPECT_CALL(*processor_, StartProcessing());
}
attempter_.Update("", "", "", "", false, false);
@@ -511,9 +537,9 @@ void UpdateAttempterTest::RollbackTestStart(
for (size_t i = 0; i < arraysize(kRollbackActionTypes); ++i) {
EXPECT_CALL(*processor_,
EnqueueAction(Property(&AbstractAction::Type,
- kRollbackActionTypes[i]))).Times(1);
+ kRollbackActionTypes[i])));
}
- EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ EXPECT_CALL(*processor_, StartProcessing());
EXPECT_TRUE(attempter_.Rollback(true));
g_idle_add(&StaticRollbackTestVerify, this);
@@ -577,9 +603,8 @@ TEST_F(UpdateAttempterTest, EnterpriseRollbackTest) {
void UpdateAttempterTest::PingOmahaTestStart() {
EXPECT_CALL(*processor_,
EnqueueAction(Property(&AbstractAction::Type,
- OmahaRequestAction::StaticType())))
- .Times(1);
- EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ OmahaRequestAction::StaticType())));
+ EXPECT_CALL(*processor_, StartProcessing());
attempter_.PingOmaha();
g_idle_add(&StaticQuitMainLoop, this);
}
@@ -652,7 +677,7 @@ TEST_F(UpdateAttempterTest, P2PStartedAtStartupWhenEnabledAndSharing) {
fake_system_state_.set_p2p_manager(&mock_p2p_manager);
mock_p2p_manager.fake().SetP2PEnabled(true);
mock_p2p_manager.fake().SetCountSharedFilesResult(1);
- EXPECT_CALL(mock_p2p_manager, EnsureP2PRunning()).Times(1);
+ EXPECT_CALL(mock_p2p_manager, EnsureP2PRunning());
attempter_.UpdateEngineStarted();
}
@@ -676,8 +701,8 @@ void UpdateAttempterTest::P2PNotEnabledStart() {
mock_p2p_manager.fake().SetP2PEnabled(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
attempter_.Update("", "", "", "", false, false);
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_sharing());
+ EXPECT_FALSE(actual_using_p2p_for_downloading());
+ EXPECT_FALSE(actual_using_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
}
@@ -704,8 +729,8 @@ void UpdateAttempterTest::P2PEnabledStartingFailsStart() {
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(0);
attempter_.Update("", "", "", "", false, false);
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_sharing());
+ EXPECT_FALSE(actual_using_p2p_for_downloading());
+ EXPECT_FALSE(actual_using_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
}
@@ -730,10 +755,10 @@ void UpdateAttempterTest::P2PEnabledHousekeepingFailsStart() {
mock_p2p_manager.fake().SetP2PEnabled(true);
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(false);
- EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(1);
+ EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
attempter_.Update("", "", "", "", false, false);
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_sharing());
+ EXPECT_FALSE(actual_using_p2p_for_downloading());
+ EXPECT_FALSE(actual_using_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
}
@@ -757,10 +782,10 @@ void UpdateAttempterTest::P2PEnabledStart() {
mock_p2p_manager.fake().SetP2PEnabled(true);
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
- EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(1);
+ EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
attempter_.Update("", "", "", "", false, false);
- EXPECT_TRUE(attempter_.omaha_request_params_->use_p2p_for_downloading());
- EXPECT_TRUE(attempter_.omaha_request_params_->use_p2p_for_sharing());
+ EXPECT_TRUE(actual_using_p2p_for_downloading());
+ EXPECT_TRUE(actual_using_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
}
@@ -785,10 +810,10 @@ void UpdateAttempterTest::P2PEnabledInteractiveStart() {
mock_p2p_manager.fake().SetP2PEnabled(true);
mock_p2p_manager.fake().SetEnsureP2PRunningResult(true);
mock_p2p_manager.fake().SetPerformHousekeepingResult(true);
- EXPECT_CALL(mock_p2p_manager, PerformHousekeeping()).Times(1);
+ EXPECT_CALL(mock_p2p_manager, PerformHousekeeping());
attempter_.Update("", "", "", "", false, true /* interactive */);
- EXPECT_FALSE(attempter_.omaha_request_params_->use_p2p_for_downloading());
- EXPECT_TRUE(attempter_.omaha_request_params_->use_p2p_for_sharing());
+ EXPECT_FALSE(actual_using_p2p_for_downloading());
+ EXPECT_TRUE(actual_using_p2p_for_sharing());
g_idle_add(&StaticQuitMainLoop, this);
}