Support for processing multiple URLs in update_engine.
Main changes:
1. Added a new PayloadState class which encapsulates all the persisted
state we use for multiple URLs, back-off (TBD), etc.
2. Added support for handling multiple URLs stored in the OmahaResponse in
OmahaRequestAction and OmahaResponseHandlerAction code.
3. Added support for picking the right URL in OmahaResponseHandlerAction
and putting it in the install_plan. This way, the rest of the code that
uses the install_plan is oblivious to the presence of multiple URLs :-)
4. Added support for advancing to next URL when an update fails. The full
error classification is a new work item (chromium-os:37206). Right now,
it's a basic round-robin on every error.
5. Updated the conditions for determining when hash checks are mandatory.
Previously since there was only one URL, if it was HTTPS, the checks were
waived. Now, even if there's one HTTP URL, we make hash checks mandatory
even if other HTTPS URLs are present.
6. Added new unit tests for PayloadState and the new logic added to other
places.
Noisy changes:
1. Instead of passing PrefsInterface to OmahaRequestAction and
OmahaResponseHandlerAction, we're now passing SystemState which will now
contain PrefsInterface and the newly added PayloadState object that these
actions need to do their work.
2. Renamed a bunch of setters/getters to set_x() and x() instead of SetX()
and GetX() methods - this was pending from Gilad's old CR. As I'm
adding new methods in the correct style, I went ahead and fixed it to
avoid the confusing styles.
3. Updated all existing unit tests to reflect these changes.
BUG=chromium-os:36807
TEST=All Single/Multiple URL scenarios work fine on my ZGB as expected.
TEST=Old and new unit tests run fine.
Change-Id: Id31f9ccb220471f3ec3a475f624dc03c16119144
Reviewed-on: https://gerrit.chromium.org/gerrit/39638
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/SConstruct b/SConstruct
index 43d4392..47df390 100644
--- a/SConstruct
+++ b/SConstruct
@@ -272,6 +272,7 @@
omaha_request_params.cc
omaha_response_handler_action.cc
payload_signer.cc
+ payload_state.cc
postinstall_runner_action.cc
prefs.cc
proxy_resolver.cc
@@ -318,6 +319,7 @@
omaha_request_params_unittest.cc
omaha_response_handler_action_unittest.cc
payload_signer_unittest.cc
+ payload_state_unittest.cc
postinstall_runner_action_unittest.cc
prefs_unittest.cc
simple_key_value_store_unittest.cc
diff --git a/certificate_checker.cc b/certificate_checker.cc
index 1f19baf..7432c21 100644
--- a/certificate_checker.cc
+++ b/certificate_checker.cc
@@ -51,10 +51,7 @@
}
// static
-MetricsLibraryInterface* CertificateChecker::metrics_lib_ = NULL;
-
-// static
-PrefsInterface* CertificateChecker::prefs_ = NULL;
+SystemState* CertificateChecker::system_state_ = NULL;
// static
OpenSSLWrapper* CertificateChecker::openssl_wrapper_ = NULL;
@@ -105,13 +102,15 @@
static const char kUMAActionCertChanged[] =
"Updater.ServerCertificateChanged";
static const char kUMAActionCertFailed[] = "Updater.ServerCertificateFailed";
+ TEST_AND_RETURN_FALSE(system_state_ != NULL);
+ TEST_AND_RETURN_FALSE(system_state_->prefs() != NULL);
TEST_AND_RETURN_FALSE(server_to_check != kNone);
// If pre-verification failed, we are not interested in the current
// certificate. We store a report to UMA and just propagate the fail result.
if (!preverify_ok) {
- LOG_IF(WARNING, !prefs_->SetString(kReportToSendKey[server_to_check],
- kUMAActionCertFailed))
+ LOG_IF(WARNING, !system_state_->prefs()->SetString(
+ kReportToSendKey[server_to_check], kUMAActionCertFailed))
<< "Failed to store UMA report on a failure to validate "
<< "certificate from update server.";
return false;
@@ -140,8 +139,9 @@
depth);
string stored_digest;
// If there's no stored certificate, we just store the current one and return.
- if (!prefs_->GetString(storage_key, &stored_digest)) {
- LOG_IF(WARNING, !prefs_->SetString(storage_key, digest_string))
+ if (!system_state_->prefs()->GetString(storage_key, &stored_digest)) {
+ LOG_IF(WARNING, !system_state_->prefs()->SetString(storage_key,
+ digest_string))
<< "Failed to store server certificate on storage key " << storage_key;
return true;
}
@@ -149,11 +149,12 @@
// Certificate changed, we store a report to UMA and store the most recent
// certificate.
if (stored_digest != digest_string) {
- LOG_IF(WARNING, !prefs_->SetString(kReportToSendKey[server_to_check],
- kUMAActionCertChanged))
+ LOG_IF(WARNING, !system_state_->prefs()->SetString(
+ kReportToSendKey[server_to_check], kUMAActionCertChanged))
<< "Failed to store UMA report on a change on the "
<< "certificate from update server.";
- LOG_IF(WARNING, !prefs_->SetString(storage_key, digest_string))
+ LOG_IF(WARNING, !system_state_->prefs()->SetString(storage_key,
+ digest_string))
<< "Failed to store server certificate on storage key " << storage_key;
}
@@ -164,20 +165,22 @@
// static
void CertificateChecker::FlushReport() {
// This check shouldn't be needed, but it is useful for testing.
- TEST_AND_RETURN(metrics_lib_ && prefs_);
+ TEST_AND_RETURN(system_state_);
+ TEST_AND_RETURN(system_state_->metrics_lib());
+ TEST_AND_RETURN(system_state_->prefs());
// We flush reports for both servers.
for (size_t i = 0; i < arraysize(kReportToSendKey); i++) {
string report_to_send;
- if (prefs_->GetString(kReportToSendKey[i], &report_to_send) &&
- !report_to_send.empty()) {
+ if (system_state_->prefs()->GetString(kReportToSendKey[i], &report_to_send)
+ && !report_to_send.empty()) {
// There is a report to be sent. We send it and erase it.
- LOG_IF(WARNING, !metrics_lib_->SendUserActionToUMA(report_to_send))
+ LOG(INFO) << "Found report #" << i << ". Sending it";
+ LOG_IF(WARNING, !system_state_->metrics_lib()->SendUserActionToUMA(
+ report_to_send))
<< "Failed to send server certificate report to UMA: "
<< report_to_send;
- // Since prefs doesn't provide deletion, we just set it as an empty
- // string.
- LOG_IF(WARNING, !prefs_->SetString(kReportToSendKey[i], ""))
+ LOG_IF(WARNING, !system_state_->prefs()->Delete(kReportToSendKey[i]))
<< "Failed to erase server certificate report to be sent to UMA";
}
}
diff --git a/certificate_checker.h b/certificate_checker.h
index f04179b..7925937 100644
--- a/certificate_checker.h
+++ b/certificate_checker.h
@@ -12,8 +12,7 @@
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include <openssl/ssl.h>
-class MetricsLibraryInterface;
-namespace chromeos_update_engine { class PrefsInterface; }
+#include "update_engine/system_state.h"
namespace chromeos_update_engine {
@@ -71,12 +70,8 @@
static void FlushReport();
// Setters.
- static void set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
- metrics_lib_ = metrics_lib;
- }
-
- static void set_prefs(PrefsInterface* prefs) {
- prefs_ = prefs;
+ static void set_system_state(SystemState* system_state) {
+ system_state_ = system_state;
}
static void set_openssl_wrapper(OpenSSLWrapper* openssl_wrapper) {
@@ -113,11 +108,8 @@
int preverify_ok,
X509_STORE_CTX* x509_ctx);
- // Metrics library used to report to UMA.
- static MetricsLibraryInterface* metrics_lib_;
-
- // Prefs used to store certificates and UMA reports.
- static PrefsInterface* prefs_;
+ // Global system context.
+ static SystemState* system_state_;
// The wrapper for openssl operations.
static OpenSSLWrapper* openssl_wrapper_;
diff --git a/certificate_checker_unittest.cc b/certificate_checker_unittest.cc
index 55168ba..4b33e02 100644
--- a/certificate_checker_unittest.cc
+++ b/certificate_checker_unittest.cc
@@ -12,6 +12,7 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/certificate_checker_mock.h"
+#include "update_engine/mock_system_state.h"
#include "update_engine/prefs_mock.h"
using ::testing::_;
@@ -45,15 +46,15 @@
depth_);
kCertChanged = "Updater.ServerCertificateChanged";
kCertFailed = "Updater.ServerCertificateFailed";
- CertificateChecker::set_metrics_lib(&metrics_lib_);
- CertificateChecker::set_prefs(&prefs_);
+ CertificateChecker::set_system_state(&mock_system_state_);
CertificateChecker::set_openssl_wrapper(&openssl_wrapper_);
+ prefs_ = mock_system_state_.mock_prefs();
}
virtual void TearDown() {}
- MetricsLibraryMock metrics_lib_;
- PrefsMock prefs_;
+ MockSystemState mock_system_state_;
+ PrefsMock* prefs_; // shortcut to mock_system_state_.mock_prefs()
OpenSSLWrapperMock openssl_wrapper_;
// Parameters of our mock certificate digest.
int depth_;
@@ -76,9 +77,9 @@
SetArgumentPointee<2>(length_),
SetArrayArgument<3>(digest_, digest_ + 4),
Return(true)));
- EXPECT_CALL(prefs_, GetString(cert_key_, _))
+ EXPECT_CALL(*prefs_, GetString(cert_key_, _))
.WillOnce(Return(false));
- EXPECT_CALL(prefs_, SetString(cert_key_, digest_hex_))
+ EXPECT_CALL(*prefs_, SetString(cert_key_, digest_hex_))
.WillOnce(Return(true));
ASSERT_TRUE(CertificateChecker::CheckCertificateChange(
server_to_check_, 1, NULL));
@@ -92,11 +93,11 @@
SetArgumentPointee<2>(length_),
SetArrayArgument<3>(digest_, digest_ + 4),
Return(true)));
- EXPECT_CALL(prefs_, GetString(cert_key_, _))
+ EXPECT_CALL(*prefs_, GetString(cert_key_, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(digest_hex_),
Return(true)));
- EXPECT_CALL(prefs_, SetString(_, _)).Times(0);
+ EXPECT_CALL(*prefs_, SetString(_, _)).Times(0);
ASSERT_TRUE(CertificateChecker::CheckCertificateChange(
server_to_check_, 1, NULL));
}
@@ -109,14 +110,14 @@
SetArgumentPointee<2>(length_),
SetArrayArgument<3>(digest_, digest_ + 4),
Return(true)));
- EXPECT_CALL(prefs_, GetString(cert_key_, _))
+ EXPECT_CALL(*prefs_, GetString(cert_key_, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(diff_digest_hex_),
Return(true)));
- EXPECT_CALL(prefs_, SetString(kPrefsCertificateReportToSendUpdate,
+ EXPECT_CALL(*prefs_, SetString(kPrefsCertificateReportToSendUpdate,
kCertChanged))
.WillOnce(Return(true));
- EXPECT_CALL(prefs_, SetString(cert_key_, digest_hex_))
+ EXPECT_CALL(*prefs_, SetString(cert_key_, digest_hex_))
.WillOnce(Return(true));
ASSERT_TRUE(CertificateChecker::CheckCertificateChange(
server_to_check_, 1, NULL));
@@ -124,10 +125,10 @@
// check certificate change, failed
TEST_F(CertificateCheckerTest, FailedCertificate) {
- EXPECT_CALL(prefs_, SetString(kPrefsCertificateReportToSendUpdate,
+ EXPECT_CALL(*prefs_, SetString(kPrefsCertificateReportToSendUpdate,
kCertFailed))
.WillOnce(Return(true));
- EXPECT_CALL(prefs_, GetString(_,_)).Times(0);
+ EXPECT_CALL(*prefs_, GetString(_,_)).Times(0);
EXPECT_CALL(openssl_wrapper_, GetCertificateDigest(_,_,_,_)).Times(0);
ASSERT_FALSE(CertificateChecker::CheckCertificateChange(
server_to_check_, 0, NULL));
@@ -135,17 +136,18 @@
// flush send report
TEST_F(CertificateCheckerTest, FlushReport) {
- EXPECT_CALL(prefs_, GetString(kPrefsCertificateReportToSendUpdate, _))
+ EXPECT_CALL(*prefs_, GetString(kPrefsCertificateReportToSendUpdate, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(kCertChanged),
Return(true)));
- EXPECT_CALL(prefs_, GetString(kPrefsCertificateReportToSendDownload, _))
+ EXPECT_CALL(*prefs_, GetString(kPrefsCertificateReportToSendDownload, _))
.WillOnce(Return(false));
- EXPECT_CALL(metrics_lib_, SendUserActionToUMA(kCertChanged))
+ EXPECT_CALL(*mock_system_state_.mock_metrics_lib(),
+ SendUserActionToUMA(kCertChanged))
.WillOnce(Return(true));
- EXPECT_CALL(prefs_, SetString(kPrefsCertificateReportToSendUpdate, ""))
+ EXPECT_CALL(*prefs_, Delete(kPrefsCertificateReportToSendUpdate))
.WillOnce(Return(true));
- EXPECT_CALL(prefs_, SetString(kPrefsCertificateReportToSendDownload, _))
+ EXPECT_CALL(*prefs_, SetString(kPrefsCertificateReportToSendDownload, _))
.Times(0);
CertificateChecker::FlushReport();
}
@@ -153,14 +155,15 @@
// flush nothing to report
TEST_F(CertificateCheckerTest, FlushNothingToReport) {
string empty = "";
- EXPECT_CALL(prefs_, GetString(kPrefsCertificateReportToSendUpdate, _))
+ EXPECT_CALL(*prefs_, GetString(kPrefsCertificateReportToSendUpdate, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(empty),
Return(true)));
- EXPECT_CALL(prefs_, GetString(kPrefsCertificateReportToSendDownload, _))
+ EXPECT_CALL(*prefs_, GetString(kPrefsCertificateReportToSendDownload, _))
.WillOnce(Return(false));
- EXPECT_CALL(metrics_lib_, SendUserActionToUMA(_)).Times(0);
- EXPECT_CALL(prefs_, SetString(_,_)).Times(0);
+ EXPECT_CALL(*mock_system_state_.mock_metrics_lib(),
+ SendUserActionToUMA(_)).Times(0);
+ EXPECT_CALL(*prefs_, SetString(_,_)).Times(0);
CertificateChecker::FlushReport();
}
diff --git a/connection_manager.cc b/connection_manager.cc
index 438ed41..3044130 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -157,7 +157,7 @@
case kNetCellular: {
set<string> allowed_types;
const policy::DevicePolicy* device_policy =
- system_state_->GetDevicePolicy();
+ system_state_->device_policy();
if (!device_policy) {
LOG(INFO) << "Disabling updates over cellular connection as there's no "
"device policy object present";
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index 5499d2f..e6a2ef4 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -28,7 +28,7 @@
kMockFlimFlamServiceProxy_(NULL),
kServicePath_(NULL),
cmut_(&mock_system_state_) {
- mock_system_state_.SetConnectionManager(&cmut_);
+ mock_system_state_.set_connection_manager(&cmut_);
}
protected:
@@ -173,31 +173,31 @@
}
TEST_F(ConnectionManagerTest, AllowUpdatesOverEthernetTest) {
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
// Updates over Ethernet are allowed even if there's no policy.
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetEthernet));
}
TEST_F(ConnectionManagerTest, AllowUpdatesOverWifiTest) {
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWifi));
}
TEST_F(ConnectionManagerTest, AllowUpdatesOverWimaxTest) {
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWimax));
}
TEST_F(ConnectionManagerTest, BlockUpdatesOverBluetoothTest) {
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_CALL(mock_system_state_, device_policy()).Times(0);
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetBluetooth));
}
TEST_F(ConnectionManagerTest, AllowUpdatesOnlyOver3GPerPolicyTest) {
policy::MockDevicePolicy allow_3g_policy;
- EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ EXPECT_CALL(mock_system_state_, device_policy())
.Times(1)
.WillOnce(Return(&allow_3g_policy));
@@ -215,7 +215,7 @@
TEST_F(ConnectionManagerTest, AllowUpdatesOver3GAndOtherTypesPerPolicyTest) {
policy::MockDevicePolicy allow_3g_policy;
- EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ EXPECT_CALL(mock_system_state_, device_policy())
.Times(1)
.WillOnce(Return(&allow_3g_policy));
@@ -234,14 +234,14 @@
}
TEST_F(ConnectionManagerTest, BlockUpdatesOver3GByDefaultTest) {
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(1);
+ EXPECT_CALL(mock_system_state_, device_policy()).Times(1);
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
}
TEST_F(ConnectionManagerTest, BlockUpdatesOver3GPerPolicyTest) {
policy::MockDevicePolicy block_3g_policy;
- EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ EXPECT_CALL(mock_system_state_, device_policy())
.Times(1)
.WillOnce(Return(&block_3g_policy));
@@ -262,7 +262,7 @@
TEST_F(ConnectionManagerTest, BlockUpdatesOver3GIfErrorInPolicyFetchTest) {
policy::MockDevicePolicy allow_3g_policy;
- EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ EXPECT_CALL(mock_system_state_, device_policy())
.Times(1)
.WillOnce(Return(&allow_3g_policy));
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 9750067..09b8999 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -166,7 +166,7 @@
public:
AnyHttpFetcherTest()
: mock_connection_manager_(&mock_system_state_) {
- mock_system_state_.SetConnectionManager(&mock_connection_manager_);
+ mock_system_state_.set_connection_manager(&mock_connection_manager_);
}
virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
@@ -398,7 +398,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
@@ -426,7 +426,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet))
@@ -462,7 +462,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWimax), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWimax))
@@ -541,7 +541,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetCellular), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetCellular))
@@ -617,7 +617,7 @@
delegate.fetcher_->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- delegate.fetcher_->GetSystemState()->GetConnectionManager());
+ delegate.fetcher_->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
@@ -675,7 +675,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
@@ -757,7 +757,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet))
@@ -849,7 +849,7 @@
fetcher->set_delegate(&delegate);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet))
@@ -967,7 +967,7 @@
delegate.fetcher_.reset(fetcher_in);
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher_in->GetSystemState()->GetConnectionManager());
+ fetcher_in->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
@@ -1163,7 +1163,7 @@
bool is_allowed = (i != 0);
scoped_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
- fetcher->GetSystemState()->GetConnectionManager());
+ fetcher->GetSystemState()->connection_manager());
EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
.WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 59f065e..7d82462 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -45,7 +45,7 @@
bool LibcurlHttpFetcher::IsUpdateAllowedOverCurrentConnection() const {
NetworkConnectionType type;
ConcreteDbusGlib dbus_iface;
- ConnectionManager* connection_manager = system_state_->GetConnectionManager();
+ ConnectionManager* connection_manager = system_state_->connection_manager();
TEST_AND_RETURN_FALSE(connection_manager->GetConnectionType(&dbus_iface,
&type));
bool is_allowed = connection_manager->IsUpdateAllowedOver(type);
diff --git a/main.cc b/main.cc
index 43ee5b4..833afb2 100644
--- a/main.cc
+++ b/main.cc
@@ -22,7 +22,6 @@
#include "update_engine/dbus_interface.h"
#include "update_engine/dbus_service.h"
#include "update_engine/gpio_handler.h"
-#include "update_engine/prefs.h"
#include "update_engine/subprocess.h"
#include "update_engine/terminator.h"
#include "update_engine/update_attempter.h"
@@ -168,19 +167,13 @@
// Create the single GMainLoop
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
- chromeos_update_engine::Prefs prefs;
- LOG_IF(ERROR, !prefs.Init(FilePath("/var/lib/update_engine/prefs")))
- << "Failed to initialize preferences.";
-
chromeos_update_engine::RealSystemState real_system_state;
-
- MetricsLibrary metrics_lib;
- metrics_lib.Init();
- real_system_state.set_metrics_lib(&metrics_lib);
+ LOG_IF(ERROR, !real_system_state.Initialize())
+ << "Failed to initialize system state.";
// Sets static members for the certificate checker.
- chromeos_update_engine::CertificateChecker::set_metrics_lib(&metrics_lib);
- chromeos_update_engine::CertificateChecker::set_prefs(&prefs);
+ chromeos_update_engine::CertificateChecker::set_system_state(
+ &real_system_state);
chromeos_update_engine::OpenSSLWrapper openssl_wrapper;
chromeos_update_engine::CertificateChecker::set_openssl_wrapper(
&openssl_wrapper);
@@ -196,10 +189,9 @@
// Create the update attempter:
chromeos_update_engine::ConcreteDbusGlib dbus;
- chromeos_update_engine::UpdateAttempter update_attempter(&prefs,
+ chromeos_update_engine::UpdateAttempter update_attempter(&real_system_state,
&dbus,
- &gpio_handler,
- &real_system_state);
+ &gpio_handler);
// Create the dbus service object:
dbus_g_object_type_install_info(UPDATE_ENGINE_TYPE_SERVICE,
diff --git a/mock_http_fetcher.h b/mock_http_fetcher.h
index d3cc75a..db528c7 100644
--- a/mock_http_fetcher.h
+++ b/mock_http_fetcher.h
@@ -41,7 +41,7 @@
fail_transfer_(false),
never_use_(false),
mock_connection_manager_(&mock_system_state_) {
- mock_system_state_.SetConnectionManager(&mock_connection_manager_);
+ mock_system_state_.set_connection_manager(&mock_connection_manager_);
data_.insert(data_.end(), data, data + size);
}
diff --git a/mock_payload_state.h b/mock_payload_state.h
new file mode 100644
index 0000000..bb60e98
--- /dev/null
+++ b/mock_payload_state.h
@@ -0,0 +1,20 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_PAYLOAD_STATE_H__
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_PAYLOAD_STATE_H__
+
+#include "gmock/gmock.h"
+#include "update_engine/payload_state.h"
+
+namespace chromeos_update_engine {
+
+class MockPayloadState: public PayloadState {
+ public:
+ MOCK_METHOD1(UpdateFailed, void(ActionExitCode error));
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_PAYLOAD_STATE_H__
diff --git a/mock_system_state.h b/mock_system_state.h
index a2e5cd2..2f82345 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -10,6 +10,8 @@
#include <metrics/metrics_library_mock.h>
#include <policy/mock_device_policy.h>
+#include "update_engine/mock_payload_state.h"
+#include "update_engine/prefs_mock.h"
#include "update_engine/system_state.h"
namespace chromeos_update_engine {
@@ -18,37 +20,61 @@
// OOBE is completed even when there's no such marker file, etc.
class MockSystemState : public SystemState {
public:
- MockSystemState() {
- // By default, provide a mock metrics library. If the caller wants,
- // they can override this by using set_metrics_lib() method.
- metrics_lib_ = &mock_metrics_lib_;
+ MockSystemState() : prefs_(&mock_prefs_) {
+ mock_payload_state_.Initialize(&mock_prefs_);
}
virtual ~MockSystemState() {}
MOCK_METHOD0(IsOOBEComplete, bool());
- MOCK_METHOD1(SetDevicePolicy, void(const policy::DevicePolicy*));
- MOCK_CONST_METHOD0(GetDevicePolicy, const policy::DevicePolicy*());
+ MOCK_METHOD1(set_device_policy, void(const policy::DevicePolicy*));
+ MOCK_CONST_METHOD0(device_policy, const policy::DevicePolicy*());
- void SetConnectionManager(ConnectionManager* connection_manager) {
- connection_manager_ = connection_manager;
- }
-
- virtual ConnectionManager* GetConnectionManager() {
+ virtual ConnectionManager* connection_manager() {
return connection_manager_;
}
- void set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
- metrics_lib_ = metrics_lib;
- }
virtual MetricsLibraryInterface* metrics_lib() {
- return metrics_lib_;
+ return &mock_metrics_lib_;
}
+ virtual PrefsInterface* prefs() {
+ return prefs_;
+ }
+
+ virtual PayloadState* payload_state() {
+ return &mock_payload_state_;
+ }
+
+ // MockSystemState-specific public method.
+ void set_connection_manager(ConnectionManager* connection_manager) {
+ connection_manager_ = connection_manager;
+ }
+
+ MetricsLibraryMock* mock_metrics_lib() {
+ return &mock_metrics_lib_;
+ }
+
+ void set_prefs(PrefsInterface* prefs) {
+ prefs_ = prefs;
+ }
+
+ testing::NiceMock<PrefsMock> *mock_prefs() {
+ return &mock_prefs_;
+ }
+
+ MockPayloadState* mock_payload_state() {
+ return &mock_payload_state_;
+ }
private:
- ConnectionManager* connection_manager_;
+ // These are Mock objects or objects we own.
MetricsLibraryMock mock_metrics_lib_;
- MetricsLibraryInterface* metrics_lib_;
+ testing::NiceMock<PrefsMock> mock_prefs_;
+ MockPayloadState mock_payload_state_;
+
+ // These are pointers to objects which caller can override.
+ PrefsInterface* prefs_;
+ ConnectionManager* connection_manager_;
};
} // namespeace chromeos_update_engine
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 3c86c6b..1a3517b 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -183,12 +183,12 @@
return string(reinterpret_cast<const char *>(str.get()));
}
-OmahaRequestAction::OmahaRequestAction(PrefsInterface* prefs,
+OmahaRequestAction::OmahaRequestAction(SystemState* system_state,
OmahaRequestParams* params,
OmahaEvent* event,
HttpFetcher* http_fetcher,
bool ping_only)
- : prefs_(prefs),
+ : system_state_(system_state),
params_(params),
event_(event),
http_fetcher_(http_fetcher),
@@ -202,7 +202,7 @@
int OmahaRequestAction::CalculatePingDays(const string& key) {
int days = kNeverPinged;
int64_t last_ping = 0;
- if (prefs_->GetInt64(key, &last_ping) && last_ping >= 0) {
+ if (system_state_->prefs()->GetInt64(key, &last_ping) && last_ping >= 0) {
days = (Time::Now() - Time::FromInternalValue(last_ping)).InDays();
if (days < 0) {
// If |days| is negative, then the system clock must have jumped
@@ -244,7 +244,7 @@
ping_only_,
ping_active_days_,
ping_roll_call_days_,
- prefs_));
+ system_state_->prefs()));
http_fetcher_->SetPostData(request_post.data(), request_post.size(),
kHttpContentTypeTextXml);
@@ -366,26 +366,28 @@
CHECK_GE(nodeset->nodeNr, 1);
xmlNode* update_check_node = nodeset->nodeTab[0];
- // TODO(jaysri): The PollInterval is not supported by Omaha server currently.
- // But still keeping this existing code in case we ever decide to slow down
- // the request rate from the server-side. Note that the PollInterval is not
- // persisted, so it has to be sent by the server on every response to
- // guarantee that the UpdateCheckScheduler uses this value (otherwise, if the
- // device got rebooted after the last server-indicated value, it'll revert to
- // the default value). Also kDefaultMaxUpdateChecks value for the scattering
- // logic is based on the assumption that we perform an update check every
- // hour so that the max value of 8 will roughly be equivalent to one work
- // day. If we decide to use PollInterval permanently, we should update
- // the max_update_checks_allowed to take PollInterval into account. Note:
- // The parsing for PollInterval happens even before parsing of the status
- // because we may want to specify the PollInterval even when there's no
- // update.
+ // chromium-os:37289: The PollInterval is not supported by Omaha server
+ // currently. But still keeping this existing code in case we ever decide to
+ // slow down the request rate from the server-side. Note that the
+ // PollInterval is not persisted, so it has to be sent by the server on every
+ // response to guarantee that the UpdateCheckScheduler uses this value
+ // (otherwise, if the device got rebooted after the last server-indicated
+ // value, it'll revert to the default value). Also kDefaultMaxUpdateChecks
+ // value for the scattering logic is based on the assumption that we perform
+ // an update check every hour so that the max value of 8 will roughly be
+ // equivalent to one work day. If we decide to use PollInterval permanently,
+ // we should update the max_update_checks_allowed to take PollInterval into
+ // account. Note: The parsing for PollInterval happens even before parsing
+ // of the status because we may want to specify the PollInterval even when
+ // there's no update.
base::StringToInt(XmlGetProperty(update_check_node, "PollInterval"),
&output_object->poll_interval);
if (!ParseStatus(update_check_node, output_object, completer))
return false;
+ // Note: ParseUrls MUST be called before ParsePackage as ParsePackage
+ // appends the package name to the URLs populated in this method.
if (!ParseUrls(doc, output_object, completer))
return false;
@@ -395,6 +397,10 @@
if (!ParseParams(doc, output_object, completer))
return false;
+ output_object->update_exists = true;
+ SetOutputObject(*output_object);
+ completer->set_code(kActionCodeSuccess);
+
return true;
}
@@ -443,19 +449,20 @@
CHECK(nodeset) << "XPath missing " << kUpdateUrlNodeXPath;
CHECK_GE(nodeset->nodeNr, 1);
- // TODO(jaysri): For now, use the last URL. In subsequent
- // check-ins, we'll add the support to honor multiples URLs.
- LOG(INFO) << "Processing last of " << nodeset->nodeNr << " url(s)";
- xmlNode* url_node = nodeset->nodeTab[nodeset->nodeNr - 1];
+ LOG(INFO) << "Found " << nodeset->nodeNr << " url(s)";
+ output_object->payload_urls.clear();
+ for (int i = 0; i < nodeset->nodeNr; i++) {
+ xmlNode* url_node = nodeset->nodeTab[i];
- const string codebase(XmlGetProperty(url_node, "codebase"));
- if (codebase.empty()) {
- LOG(ERROR) << "Omaha Response URL has empty codebase";
- completer->set_code(kActionCodeOmahaResponseInvalid);
- return false;
+ const string codebase(XmlGetProperty(url_node, "codebase"));
+ if (codebase.empty()) {
+ LOG(ERROR) << "Omaha Response URL has empty codebase";
+ completer->set_code(kActionCodeOmahaResponseInvalid);
+ return false;
+ }
+ output_object->payload_urls.push_back(codebase);
}
- output_object->codebase = codebase;
return true;
}
@@ -483,7 +490,7 @@
// Get package properties one by one.
- // Parse the payload name to be appended to the Url codebase.
+ // Parse the payload name to be appended to the base Url value.
const string package_name(XmlGetProperty(package_node, "name"));
LOG(INFO) << "Omaha Response package name = " << package_name;
if (package_name.empty()) {
@@ -491,7 +498,14 @@
completer->set_code(kActionCodeOmahaResponseInvalid);
return false;
}
- output_object->codebase += package_name;
+
+ // Append the package name to each URL in our list so that we don't
+ // propagate the urlBase vs packageName distinctions beyond this point.
+ // From now on, we only need to use payload_urls.
+ for (size_t i = 0; i < output_object->payload_urls.size(); i++) {
+ output_object->payload_urls[i] += package_name;
+ LOG(INFO) << "Url" << i << ": " << output_object->payload_urls[i];
+ }
// Parse the payload size.
off_t size = ParseInt(XmlGetProperty(package_node, "size"));
@@ -502,8 +516,7 @@
}
output_object->size = size;
- LOG(INFO) << "Download URL = " << output_object->codebase
- << ", Paylod size = " << output_object->size;
+ LOG(INFO) << "Payload size = " << output_object->size << " bytes";
return true;
}
@@ -554,7 +567,7 @@
return false;
}
- // Get the optional properties one by one.
+ // Get the optional properties one by one.
output_object->display_version =
XmlGetProperty(pie_action_node, "DisplayVersion");
output_object->more_info_url = XmlGetProperty(pie_action_node, "MoreInfo");
@@ -569,10 +582,6 @@
output_object->max_days_to_scatter =
ParseInt(XmlGetProperty(pie_action_node, "MaxDaysToScatter"));
- output_object->update_exists = true;
- SetOutputObject(*output_object);
- completer->set_code(kActionCodeSuccess);
-
return true;
}
@@ -582,8 +591,8 @@
void OmahaRequestAction::TransferComplete(HttpFetcher *fetcher,
bool successful) {
ScopedActionCompleter completer(processor_, this);
- LOG(INFO) << "Omaha request response: " << string(response_buffer_.begin(),
- response_buffer_.end());
+ string current_response(response_buffer_.begin(), response_buffer_.end());
+ LOG(INFO) << "Omaha request response: " << current_response;
// Events are best effort transactions -- assume they always succeed.
if (IsEvent()) {
@@ -626,7 +635,7 @@
ShouldPing(ping_roll_call_days_) ||
ping_active_days_ == kPingTimeJump ||
ping_roll_call_days_ == kPingTimeJump) {
- LOG_IF(ERROR, !UpdateLastPingDays(doc.get(), prefs_))
+ LOG_IF(ERROR, !UpdateLastPingDays(doc.get(), system_state_->prefs()))
<< "Failed to update the last ping day preferences!";
}
@@ -661,6 +670,12 @@
completer.set_code(kActionCodeOmahaUpdateDeferredPerPolicy);
return;
}
+
+ // Update the payload state with the current response. The payload state
+ // will automatically reset all stale state if this response is different
+ // from what's stored already.
+ PayloadState* payload_state = system_state_->payload_state();
+ payload_state->SetResponse(output_object);
}
bool OmahaRequestAction::ShouldDeferDownload(OmahaResponse* output_object) {
@@ -708,8 +723,9 @@
Time update_first_seen_at;
int64 update_first_seen_at_int;
- if (prefs_->Exists(kPrefsUpdateFirstSeenAt)) {
- if (prefs_->GetInt64(kPrefsUpdateFirstSeenAt, &update_first_seen_at_int)) {
+ if (system_state_->prefs()->Exists(kPrefsUpdateFirstSeenAt)) {
+ if (system_state_->prefs()->GetInt64(kPrefsUpdateFirstSeenAt,
+ &update_first_seen_at_int)) {
// Note: This timestamp could be that of ANY update we saw in the past
// (not necessarily this particular update we're considering to apply)
// but never got to apply because of some reason (e.g. stop AU policy,
@@ -732,7 +748,8 @@
} else {
update_first_seen_at = Time::Now();
update_first_seen_at_int = update_first_seen_at.ToInternalValue();
- if (prefs_->SetInt64(kPrefsUpdateFirstSeenAt, update_first_seen_at_int)) {
+ if (system_state_->prefs()->SetInt64(kPrefsUpdateFirstSeenAt,
+ update_first_seen_at_int)) {
LOG(INFO) << "Persisted the new value for UpdateFirstSeenAt: "
<< utils::ToString(update_first_seen_at);
}
@@ -807,8 +824,9 @@
bool OmahaRequestAction::IsUpdateCheckCountBasedWaitingSatisfied() {
int64 update_check_count_value;
- if (prefs_->Exists(kPrefsUpdateCheckCount)) {
- if (!prefs_->GetInt64(kPrefsUpdateCheckCount, &update_check_count_value)) {
+ if (system_state_->prefs()->Exists(kPrefsUpdateCheckCount)) {
+ if (!system_state_->prefs()->GetInt64(kPrefsUpdateCheckCount,
+ &update_check_count_value)) {
// We are unable to read the update check count from file for some reason.
// So let's proceed anyway so as to not stall the update.
LOG(ERROR) << "Unable to read update check count. "
@@ -826,7 +844,8 @@
<< update_check_count_value;
// Write out the initial value of update_check_count_value.
- if (!prefs_->SetInt64(kPrefsUpdateCheckCount, update_check_count_value)) {
+ if (!system_state_->prefs()->SetInt64(kPrefsUpdateCheckCount,
+ update_check_count_value)) {
// We weren't able to write the update check count file for some reason.
// So let's proceed anyway so as to not stall the update.
LOG(ERROR) << "Unable to write update check count. "
diff --git a/omaha_request_action.h b/omaha_request_action.h
index 8a5a5d5..bc9b4fe 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -10,6 +10,7 @@
#include <fcntl.h>
#include <string>
+#include <vector>
#include <base/memory/scoped_ptr.h>
#include <curl/curl.h>
@@ -46,7 +47,11 @@
// These are only valid if update_exists is true:
std::string display_version;
- std::string codebase;
+
+ // The ordered list of URLs in the Omaha response. Each item is a complete
+ // URL (i.e. in terms of Omaha XML, each value is a urlBase + packageName)
+ std::vector<std::string> payload_urls;
+
std::string more_info_url;
std::string hash;
std::string metadata_signature;
@@ -142,7 +147,7 @@
// OmahaRequestAction(..., new OmahaEvent(...), new WhateverHttpFetcher);
// or
// OmahaRequestAction(..., NULL, new WhateverHttpFetcher);
- OmahaRequestAction(PrefsInterface* prefs,
+ OmahaRequestAction(SystemState* system_state,
OmahaRequestParams* params,
OmahaEvent* event,
HttpFetcher* http_fetcher,
@@ -228,8 +233,8 @@
OmahaResponse* output_object,
ScopedActionCompleter* completer);
- // Access to the preferences store.
- PrefsInterface* prefs_;
+ // Global system context.
+ SystemState* system_state_;
// Contains state that is relevant in the processing of the Omaha request.
OmahaRequestParams* params_;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 90c01a4..ece38b8 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -17,7 +17,6 @@
#include "update_engine/omaha_request_action.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/prefs.h"
-#include "update_engine/prefs_mock.h"
#include "update_engine/test_utils.h"
#include "update_engine/utils.h"
@@ -209,8 +208,10 @@
if (fail_http_response_code >= 0) {
fetcher->FailTransfer(fail_http_response_code);
}
- NiceMock<PrefsMock> local_prefs;
- OmahaRequestAction action(prefs ? prefs : &local_prefs,
+ MockSystemState mock_system_state;
+ if (prefs)
+ mock_system_state.set_prefs(prefs);
+ OmahaRequestAction action(&mock_system_state,
¶ms,
NULL,
fetcher,
@@ -248,8 +249,8 @@
MockHttpFetcher* fetcher = new MockHttpFetcher(http_response.data(),
http_response.size(),
NULL);
- NiceMock<PrefsMock> prefs;
- OmahaRequestAction action(&prefs, ¶ms, event, fetcher, false);
+ MockSystemState mock_system_state;
+ OmahaRequestAction action(&mock_system_state, ¶ms, event, fetcher, false);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -300,7 +301,7 @@
EXPECT_TRUE(response.update_exists);
EXPECT_TRUE(response.update_exists);
EXPECT_EQ("1.2.3.4", response.display_version);
- EXPECT_EQ("http://code/base/file.signed", response.codebase);
+ EXPECT_EQ("http://code/base/file.signed", response.payload_urls[0]);
EXPECT_EQ("http://more/info", response.more_info_url);
EXPECT_EQ("HASH1234=", response.hash);
EXPECT_EQ(123, response.size);
@@ -615,9 +616,9 @@
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
OmahaRequestParams params = kDefaultTestParams;
- OmahaRequestAction action(&prefs, ¶ms, NULL,
+ OmahaRequestAction action(&mock_system_state, ¶ms, NULL,
new MockHttpFetcher(http_response.data(),
http_response.size(),
NULL),
@@ -747,7 +748,7 @@
NULL));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ("10.2.3.4", response.display_version);
- EXPECT_EQ("http://missing/field/test/f", response.codebase);
+ EXPECT_EQ("http://missing/field/test/f", response.payload_urls[0]);
EXPECT_EQ("", response.more_info_url);
EXPECT_EQ("lkq34j5345", response.hash);
EXPECT_EQ(587, response.size);
@@ -779,9 +780,9 @@
string http_response("doesn't matter");
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
OmahaRequestParams params = kDefaultTestParams;
- OmahaRequestAction action(&prefs, ¶ms, NULL,
+ OmahaRequestAction action(&mock_system_state, ¶ms, NULL,
new MockHttpFetcher(http_response.data(),
http_response.size(),
NULL),
@@ -864,7 +865,7 @@
NULL));
EXPECT_EQ(response.more_info_url, "testthe<url");
- EXPECT_EQ(response.codebase, "testthe&codebase/file.signed");
+ EXPECT_EQ(response.payload_urls[0], "testthe&codebase/file.signed");
EXPECT_EQ(response.deadline, "<20110101");
}
@@ -989,10 +990,10 @@
TEST(OmahaRequestActionTest, IsEventTest) {
string http_response("doesn't matter");
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
OmahaRequestParams params = kDefaultTestParams;
OmahaRequestAction update_check_action(
- &prefs,
+ &mock_system_state,
¶ms,
NULL,
new MockHttpFetcher(http_response.data(),
@@ -1003,7 +1004,7 @@
params = kDefaultTestParams;
OmahaRequestAction event_action(
- &prefs,
+ &mock_system_state,
¶ms,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
new MockHttpFetcher(http_response.data(),
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index a181542..ab083aa 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -20,8 +20,9 @@
const char OmahaResponseHandlerAction::kDeadlineFile[] =
"/tmp/update-check-response-deadline";
-OmahaResponseHandlerAction::OmahaResponseHandlerAction(PrefsInterface* prefs)
- : prefs_(prefs),
+OmahaResponseHandlerAction::OmahaResponseHandlerAction(
+ SystemState* system_state)
+ : system_state_(system_state),
got_no_update_response_(false),
key_path_(DeltaPerformer::kUpdatePayloadPublicKeyPath) {}
@@ -34,19 +35,28 @@
LOG(INFO) << "There are no updates. Aborting.";
return;
}
- install_plan_.download_url = response.codebase;
+
+ // All decisions as to which URL should be used have already been done. So,
+ // make the download URL as the payload URL at the current url index.
+ uint32_t url_index = system_state_->payload_state()->GetUrlIndex();
+ LOG(INFO) << "Using Url" << url_index << " as the download url this time";
+ CHECK(url_index < response.payload_urls.size());
+ install_plan_.download_url = response.payload_urls[url_index];
+
+ // Fill up the other properties based on the response.
install_plan_.payload_size = response.size;
install_plan_.payload_hash = response.hash;
install_plan_.metadata_size = response.metadata_size;
install_plan_.metadata_signature = response.metadata_signature;
install_plan_.hash_checks_mandatory = AreHashChecksMandatory(response);
install_plan_.is_resume =
- DeltaPerformer::CanResumeUpdate(prefs_, response.hash);
+ DeltaPerformer::CanResumeUpdate(system_state_->prefs(), response.hash);
if (!install_plan_.is_resume) {
- LOG_IF(WARNING, !DeltaPerformer::ResetUpdateProgress(prefs_, false))
+ LOG_IF(WARNING, !DeltaPerformer::ResetUpdateProgress(
+ system_state_->prefs(), false))
<< "Unable to reset the update progress.";
- LOG_IF(WARNING, !prefs_->SetString(kPrefsUpdateCheckResponseHash,
- response.hash))
+ LOG_IF(WARNING, !system_state_->prefs()->SetString(
+ kPrefsUpdateCheckResponseHash, response.hash))
<< "Unable to save the update check response hash.";
}
@@ -108,16 +118,22 @@
// checks for HTTPS until we have rolled out at least once and are confident
// nothing breaks. chromium-os:37082 tracks turning this on for HTTPS
// eventually.
- if (StartsWithASCII(response.codebase, "https://", false)) {
- LOG(INFO) << "Waiving payload hash checks since Omaha response "
- << "only has HTTPS URL(s)";
- return false;
+
+ // Even if there's a single non-HTTPS URL, make the hash checks as
+ // mandatory because we could be downloading the payload from any URL later
+ // on. It's really hard to do book-keeping based on each byte being
+ // downloaded to see whether we only used HTTPS throughout.
+ for (size_t i = 0; i < response.payload_urls.size(); i++) {
+ if (!StartsWithASCII(response.payload_urls[i], "https://", false)) {
+ LOG(INFO) << "Mandating payload hash checks since Omaha response "
+ << "contains non-HTTPS URL(s)";
+ return true;
+ }
}
- // No exceptions apply. So hash checks are mandatory, by default.
- LOG(INFO) << "Mandating payload hash checks since Omaha response "
- << "contains HTTP URL(s)";
- return true;
+ LOG(INFO) << "Waiving payload hash checks since Omaha response "
+ << "only has HTTPS URL(s)";
+ return false;
}
} // namespace chromeos_update_engine
diff --git a/omaha_response_handler_action.h b/omaha_response_handler_action.h
index 32c0f0f..27d4cec 100644
--- a/omaha_response_handler_action.h
+++ b/omaha_response_handler_action.h
@@ -12,6 +12,7 @@
#include "update_engine/action.h"
#include "update_engine/install_plan.h"
#include "update_engine/omaha_request_action.h"
+#include "update_engine/system_state.h"
// This class reads in an Omaha response and converts what it sees into
// an install plan which is passed out.
@@ -19,7 +20,6 @@
namespace chromeos_update_engine {
class OmahaResponseHandlerAction;
-class PrefsInterface;
template<>
class ActionTraits<OmahaResponseHandlerAction> {
@@ -32,7 +32,7 @@
public:
static const char kDeadlineFile[];
- OmahaResponseHandlerAction(PrefsInterface* prefs);
+ OmahaResponseHandlerAction(SystemState* system_state);
typedef ActionTraits<OmahaResponseHandlerAction>::InputObjectType
InputObjectType;
typedef ActionTraits<OmahaResponseHandlerAction>::OutputObjectType
@@ -70,8 +70,8 @@
// of the system and the contents of the Omaha response. False otherwise.
bool AreHashChecksMandatory(const OmahaResponse& response);
- // Update Engine preference store.
- PrefsInterface* prefs_;
+ // Global system context.
+ SystemState* system_state_;
// set to non-empty in unit tests
std::string boot_device_;
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index fd3f23e..c1c7e53 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -7,7 +7,7 @@
#include <gtest/gtest.h>
#include "update_engine/omaha_response_handler_action.h"
-#include "update_engine/prefs_mock.h"
+#include "update_engine/mock_system_state.h"
#include "update_engine/test_utils.h"
#include "update_engine/utils.h"
@@ -65,12 +65,13 @@
ObjectFeederAction<OmahaResponse> feeder_action;
feeder_action.set_obj(in);
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
if (in.update_exists) {
- EXPECT_CALL(prefs, SetString(kPrefsUpdateCheckResponseHash, in.hash))
+ EXPECT_CALL(*mock_system_state.mock_prefs(),
+ SetString(kPrefsUpdateCheckResponseHash, in.hash))
.WillOnce(Return(true));
}
- OmahaResponseHandlerAction response_handler_action(&prefs);
+ OmahaResponseHandlerAction response_handler_action(&mock_system_state);
response_handler_action.set_boot_device(boot_dev);
BondActions(&feeder_action, &response_handler_action);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -94,7 +95,7 @@
OmahaResponse in;
in.update_exists = true;
in.display_version = "a.b.c.d";
- in.codebase = "http://foo/the_update_a.b.c.d.tgz";
+ in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
in.more_info_url = "http://more/info";
in.hash = "HASH+";
in.size = 12;
@@ -103,7 +104,7 @@
in.deadline = "20101020";
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
- EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_EQ("/dev/sda5", install_plan.install_path);
string deadline;
@@ -121,7 +122,7 @@
OmahaResponse in;
in.update_exists = true;
in.display_version = "a.b.c.d";
- in.codebase = "http://foo/the_update_a.b.c.d.tgz";
+ in.payload_urls.push_back("http://foo/the_update_a.b.c.d.tgz");
in.more_info_url = "http://more/info";
in.hash = "HASHj+";
in.size = 12;
@@ -129,7 +130,7 @@
in.prompt = true;
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
- EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_EQ("/dev/sda3", install_plan.install_path);
string deadline;
@@ -141,7 +142,7 @@
OmahaResponse in;
in.update_exists = true;
in.display_version = "a.b.c.d";
- in.codebase = kLongName;
+ in.payload_urls.push_back(kLongName);
in.more_info_url = "http://more/info";
in.hash = "HASHj+";
in.size = 12;
@@ -150,7 +151,7 @@
in.deadline = "some-deadline";
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
- EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_EQ("/dev/sda5", install_plan.install_path);
string deadline;
@@ -175,13 +176,13 @@
OmahaResponse in;
in.update_exists = true;
in.display_version = "a.b.c.d";
- in.codebase = "http://test.should/need/hash.checks.signed";
+ in.payload_urls.push_back("http://test.should/need/hash.checks.signed");
in.more_info_url = "http://more/info";
in.hash = "HASHj+";
in.size = 12;
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
- EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_TRUE(install_plan.hash_checks_mandatory);
}
@@ -190,15 +191,32 @@
OmahaResponse in;
in.update_exists = true;
in.display_version = "a.b.c.d";
- in.codebase = "https://test.should.not/need/hash.checks.signed";
+ in.payload_urls.push_back("https://test.should.not/need/hash.checks.signed");
in.more_info_url = "http://more/info";
in.hash = "HASHj+";
in.size = 12;
InstallPlan install_plan;
EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
- EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_FALSE(install_plan.hash_checks_mandatory);
}
+TEST_F(OmahaResponseHandlerActionTest, HashChecksForBothHttpAndHttpsTest) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.display_version = "a.b.c.d";
+ in.payload_urls.push_back("http://test.should.still/need/hash.checks");
+ in.payload_urls.push_back("https://test.should.still/need/hash.checks");
+ in.more_info_url = "http://more/info";
+ in.hash = "HASHj+";
+ in.size = 12;
+ InstallPlan install_plan;
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
+ EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_TRUE(install_plan.hash_checks_mandatory);
+}
+
+
} // namespace chromeos_update_engine
diff --git a/payload_state.cc b/payload_state.cc
new file mode 100644
index 0000000..b153fa7
--- /dev/null
+++ b/payload_state.cc
@@ -0,0 +1,129 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/payload_state.h"
+
+#include <base/logging.h>
+#include <base/stringprintf.h>
+
+#include "update_engine/omaha_request_action.h"
+#include "update_engine/prefs.h"
+
+using std::string;
+
+namespace chromeos_update_engine {
+
+// Returns a string containing that subset of the fields from the OmahaResponse
+// which we're interested in persisting for the purpose of detecting whether
+// we should clear the rest of the payload state when we get a new
+// OmahaResponse.
+static string GetFilteredResponse(const OmahaResponse& response) {
+ string mini_response = StringPrintf("NumURLs = %d\n",
+ response.payload_urls.size());
+
+ for (size_t i = 0; i < response.payload_urls.size(); i++)
+ mini_response += StringPrintf("Url%d = %s\n",
+ i, response.payload_urls[i].c_str());
+
+ mini_response += StringPrintf("Payload Size = %llu\n"
+ "Payload Sha256 Hash = %s\n"
+ "Metadata Size = %llu\n"
+ "Metadata Signature = %s\n",
+ response.size,
+ response.hash.c_str(),
+ response.metadata_size,
+ response.metadata_signature.c_str());
+ return mini_response;
+}
+
+bool PayloadState::Initialize(PrefsInterface* prefs) {
+ CHECK(prefs);
+ prefs_ = prefs;
+ LoadResponse();
+ LoadUrlIndex();
+ LogPayloadState();
+ return true;
+}
+
+void PayloadState::LogPayloadState() {
+ LOG(INFO) << "Current Payload State:\n"
+ << "Current Response = \n" << response_
+ << "Current URL Index = " << url_index_;
+}
+
+void PayloadState::SetResponse(const OmahaResponse& omaha_response) {
+ CHECK(prefs_);
+ num_urls_ = omaha_response.payload_urls.size();
+ string new_response = GetFilteredResponse(omaha_response);
+ bool has_response_changed = (response_ != new_response);
+ response_ = new_response;
+ LOG(INFO) << "Stored Response = \n" << response_;
+ prefs_->SetString(kPrefsCurrentResponse, response_);
+
+ if (has_response_changed) {
+ LOG(INFO) << "Resetting all payload state as this is a new response";
+ SetUrlIndex(0);
+ }
+}
+
+void PayloadState::UpdateFailed(ActionExitCode error) {
+ LOG(INFO) << "Updating payload state for error code: " << error;
+
+ if (!num_urls_) {
+ // Since we don't persist num_urls_, it's possible that we get an error in
+ // our communication to Omaha before even OmahaRequestAction code had a
+ // chance to call SetResponse (which sets num_urls_). So we should not
+ // advance the url_index_ in such cases.
+ LOG(INFO) << "Ignoring failures until we get a valid Omaha response.";
+ return;
+ }
+
+ // chromium-os:37206: Classify the errors and advance the URL index at
+ // different rates for different errors in the next CL. Until then, advance
+ // URL index on every single error.
+ uint32_t next_url_index = GetUrlIndex() + 1;
+ if (next_url_index < num_urls_) {
+ LOG(INFO) << "Advancing the URL index for next attempt";
+ } else {
+ LOG(INFO) << "Resetting the current URL index (" << GetUrlIndex() << ") to "
+ << "0 as we only have " << num_urls_ << " URL(s)";
+ next_url_index = 0;
+
+ // TODO(jaysri): This is the place where we should increment the
+ // payload_attempt_number so that we can back-off appropriately.
+ }
+
+ SetUrlIndex(next_url_index);
+}
+
+string PayloadState::LoadResponse() {
+ CHECK(prefs_);
+ string stored_value;
+ if (prefs_->Exists(kPrefsCurrentResponse) &&
+ prefs_->GetString(kPrefsCurrentResponse, &stored_value)) {
+ response_ = stored_value;
+ }
+ return response_;
+}
+
+uint32_t PayloadState::LoadUrlIndex() {
+ CHECK(prefs_);
+ int64_t stored_value;
+ if (prefs_->Exists(kPrefsCurrentUrlIndex) &&
+ prefs_->GetInt64(kPrefsCurrentUrlIndex, &stored_value)) {
+ url_index_ = stored_value;
+ }
+ return url_index_;
+}
+
+void PayloadState::SetUrlIndex(uint32_t url_index) {
+ CHECK(prefs_);
+ // TODO(jaysri): When we implement failure count, make sure to reset
+ // the failure count when url index changes.
+ url_index_ = url_index;
+ LOG(INFO) << "Current URL Index = " << url_index_;
+ prefs_->SetInt64(kPrefsCurrentUrlIndex, url_index_);
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_state.h b/payload_state.h
new file mode 100644
index 0000000..6861e70
--- /dev/null
+++ b/payload_state.h
@@ -0,0 +1,99 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_PAYLOAD_STATE_H__
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_PAYLOAD_STATE_H__
+
+#include "base/file_path.h"
+
+#include "update_engine/action_processor.h"
+#include "update_engine/prefs_interface.h"
+
+namespace chromeos_update_engine {
+
+// Forward declaration here because we get a circular dependency if
+// we include omaha_request_action.h directly.
+struct OmahaResponse;
+
+// Encapsulates all the payload state required for download. This includes the
+// state necessary for handling multiple URLs in Omaha response, the back-off
+// state, etc. All state is persisted so that we use the most recently saved
+// value when resuming the update_engine process. All state is also cached in
+// memory so that we ensure we always make progress based on last known good
+// state even when there's any issue in reading/writing from the file system.
+class PayloadState {
+ public:
+
+ PayloadState() : prefs_(NULL), num_urls_(0), url_index_(0) {}
+
+ // Initializes a payload state object using |prefs| for storing the
+ // persisted state. It also performs the initial loading of all persisted
+ // state into memory and dumps the initial state for debugging purposes.
+ // Note: the other methods should be called only after calling Initialize
+ // on this object.
+ bool Initialize(PrefsInterface* prefs);
+
+ // Logs the current payload state.
+ void LogPayloadState();
+
+ // Sets the internal payload state based on the given Omaha response. This
+ // response could be the same or different from the one for which we've stored
+ // the internal state. If it's different, then this method resets all the
+ // internal state corresponding to the old response. Since the Omaha response
+ // has a lot of fields that are not related to payload state, it uses only
+ // a subset of the fields in the Omaha response to compare equality.
+ void SetResponse(const OmahaResponse& response);
+
+ // Updates the payload state when the current update attempt has failed.
+ void UpdateFailed(ActionExitCode error);
+
+ // Returns the internally stored subset of the response state as a string.
+ // This is logically a private method, but exposed here for unit tests.
+ std::string GetResponse() {
+ return response_;
+ }
+
+ // Returns the current URL index.
+ uint32_t GetUrlIndex() {
+ return url_index_;
+ }
+
+ private:
+ // Sets the stored response_ value from the currently persisted value for
+ // the response. Returns the same value.
+ std::string LoadResponse();
+
+ // Sets the url_index_ value from the currently persisted value for
+ // URL index. Returns the same value.
+ uint32_t LoadUrlIndex();
+
+ // Sets the current URL index.
+ void SetUrlIndex(uint32_t url_index);
+
+ // Interface object with which we read/write persisted state. This must
+ // be set by calling the Initialize method before calling any other method.
+ PrefsInterface* prefs_;
+
+ // Cached value of the latest subset of the Omaha response with which we're
+ // working off currently. This value is persisted so we load it off the next
+ // time when update_engine restarts. The rest of the state in this class will
+ // be cleared when we set a new response.
+ std::string response_;
+
+ // The number of urls in the current response. Not persisted.
+ uint32_t num_urls_;
+
+ // Cached value of the index of the current URL, to be used in case we are
+ // unable to read from the persisted store for any reason. This type is
+ // different from the one in the accessor methods because PrefsInterface
+ // supports only int64_t but we want to provide a stronger abstraction of
+ // uint32_t.
+ int64_t url_index_;
+
+ DISALLOW_COPY_AND_ASSIGN(PayloadState);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_PAYLOAD_STATE_H__
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
new file mode 100644
index 0000000..bfca903
--- /dev/null
+++ b/payload_state_unittest.cc
@@ -0,0 +1,176 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <glib.h>
+
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+#include "update_engine/omaha_request_action.h"
+#include "update_engine/payload_state.h"
+#include "update_engine/prefs_mock.h"
+#include "update_engine/test_utils.h"
+#include "update_engine/utils.h"
+
+using std::string;
+using testing::_;
+using testing::NiceMock;
+using testing::SetArgumentPointee;
+
+namespace chromeos_update_engine {
+
+class PayloadStateTest : public ::testing::Test { };
+
+TEST(PayloadStateTest, SetResponseWorksWithEmptyResponse) {
+ OmahaResponse response;
+ NiceMock<PrefsMock> prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
+ PayloadState payload_state;
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ payload_state.SetResponse(response);
+ string stored_response = payload_state.GetResponse();
+ string expected_response = "NumURLs = 0\n"
+ "Payload Size = 0\n"
+ "Payload Sha256 Hash = \n"
+ "Metadata Size = 0\n"
+ "Metadata Signature = \n";
+ EXPECT_EQ(expected_response, stored_response);
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+}
+
+TEST(PayloadStateTest, SetResponseWorksWithSingleUrl) {
+ OmahaResponse response;
+ response.payload_urls.push_back("http://single.url.test");
+ response.size = 123456789;
+ response.hash = "hash";
+ response.metadata_size = 58123;
+ response.metadata_signature = "msign";
+ NiceMock<PrefsMock> prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
+ PayloadState payload_state;
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ payload_state.SetResponse(response);
+ string stored_response = payload_state.GetResponse();
+ string expected_response = "NumURLs = 1\n"
+ "Url0 = http://single.url.test\n"
+ "Payload Size = 123456789\n"
+ "Payload Sha256 Hash = hash\n"
+ "Metadata Size = 58123\n"
+ "Metadata Signature = msign\n";
+ EXPECT_EQ(expected_response, stored_response);
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+}
+
+TEST(PayloadStateTest, SetResponseWorksWithMultipleUrls) {
+ OmahaResponse response;
+ response.payload_urls.push_back("http://multiple.url.test");
+ response.payload_urls.push_back("https://multiple.url.test");
+ response.size = 523456789;
+ response.hash = "rhash";
+ response.metadata_size = 558123;
+ response.metadata_signature = "metasign";
+ NiceMock<PrefsMock> prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
+ PayloadState payload_state;
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ payload_state.SetResponse(response);
+ string stored_response = payload_state.GetResponse();
+ string expected_response = "NumURLs = 2\n"
+ "Url0 = http://multiple.url.test\n"
+ "Url1 = https://multiple.url.test\n"
+ "Payload Size = 523456789\n"
+ "Payload Sha256 Hash = rhash\n"
+ "Metadata Size = 558123\n"
+ "Metadata Signature = metasign\n";
+ EXPECT_EQ(expected_response, stored_response);
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+}
+
+TEST(PayloadStateTest, CanAdvanceUrlIndexCorrectly) {
+ OmahaResponse response;
+ // For a variation, don't set the metadata signature, as it's optional.
+ response.payload_urls.push_back("http://set.url.index");
+ response.payload_urls.push_back("https://set.url.index");
+ response.size = 523456789;
+ response.hash = "rhash";
+
+ NiceMock<PrefsMock> prefs;
+ // Should be called 4 times, one for each UpdateFailed call plus one
+ // for SetResponse.
+ EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, _)).Times(4);
+ PayloadState payload_state;
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ payload_state.SetResponse(response);
+ string stored_response = payload_state.GetResponse();
+ string expected_response = "NumURLs = 2\n"
+ "Url0 = http://set.url.index\n"
+ "Url1 = https://set.url.index\n"
+ "Payload Size = 523456789\n"
+ "Payload Sha256 Hash = rhash\n"
+ "Metadata Size = 0\n"
+ "Metadata Signature = \n";
+ EXPECT_EQ(expected_response, stored_response);
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+
+ // Verify that on the first error, the URL index advances to 1.
+ ActionExitCode error = kActionCodeDownloadMetadataSignatureMismatch;
+ payload_state.UpdateFailed(error);
+ EXPECT_EQ(1, payload_state.GetUrlIndex());
+
+ // Verify that on the next error, the URL index wraps around to 0.
+ payload_state.UpdateFailed(error);
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+
+ // Verify that on the next error, it again advances to 1.
+ payload_state.UpdateFailed(error);
+ EXPECT_EQ(1, payload_state.GetUrlIndex());
+}
+
+TEST(PayloadStateTest, NewResponseResetsPayloadState) {
+ OmahaResponse response;
+ response.payload_urls.push_back("http://reset.url.index");
+ response.payload_urls.push_back("https://reset.url.index");
+ response.size = 523456789;
+ response.hash = "rhash";
+ response.metadata_size = 558123;
+ response.metadata_signature = "metasign";
+ NiceMock<PrefsMock> prefs;
+ EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(2);
+ EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 1)).Times(1);
+ PayloadState payload_state;
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ payload_state.SetResponse(response);
+ string stored_response = payload_state.GetResponse();
+ string expected_response = "NumURLs = 2\n"
+ "Url0 = http://reset.url.index\n"
+ "Url1 = https://reset.url.index\n"
+ "Payload Size = 523456789\n"
+ "Payload Sha256 Hash = rhash\n"
+ "Metadata Size = 558123\n"
+ "Metadata Signature = metasign\n";
+ EXPECT_EQ(expected_response, stored_response);
+
+ // Advance the URL index to 1 by faking an error.
+ ActionExitCode error = kActionCodeDownloadMetadataSignatureMismatch;
+ payload_state.UpdateFailed(error);
+ EXPECT_EQ(1, payload_state.GetUrlIndex());
+
+ // Now, slightly change the response and set it again.
+ response.hash = "newhash";
+ payload_state.SetResponse(response);
+ stored_response = payload_state.GetResponse();
+ expected_response = "NumURLs = 2\n"
+ "Url0 = http://reset.url.index\n"
+ "Url1 = https://reset.url.index\n"
+ "Payload Size = 523456789\n"
+ "Payload Sha256 Hash = newhash\n"
+ "Metadata Size = 558123\n"
+ "Metadata Signature = metasign\n";
+ // Verify the new response.
+ EXPECT_EQ(expected_response, stored_response);
+
+ // Make sure the url index was reset to 0 because of the new response.
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+}
+
+}
diff --git a/prefs.cc b/prefs.cc
index abce538..400056e 100644
--- a/prefs.cc
+++ b/prefs.cc
@@ -37,6 +37,9 @@
const char kPrefsWallClockWaitPeriod[] = "wall-clock-wait-period";
const char kPrefsUpdateFirstSeenAt[] = "update-first-seen-at";
+const char kPrefsCurrentResponse[] = "current-omaha-response";
+const char kPrefsCurrentUrlIndex[] = "current-url-index";
+
bool Prefs::Init(const FilePath& prefs_dir) {
prefs_dir_ = prefs_dir;
return true;
diff --git a/prefs_interface.h b/prefs_interface.h
index d0ff0d0..cc7c4d1 100644
--- a/prefs_interface.h
+++ b/prefs_interface.h
@@ -27,6 +27,8 @@
extern const char kPrefsUpdateCheckCount[];
extern const char kPrefsWallClockWaitPeriod[];
extern const char kPrefsUpdateFirstSeenAt[];
+extern const char kPrefsCurrentResponse[];
+extern const char kPrefsCurrentUrlIndex[];
// The prefs interface allows access to a persistent preferences
// store. The two reasons for providing this as an interface are
diff --git a/system_state.cc b/system_state.cc
index 0053fed..d13e18c 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -9,35 +9,54 @@
namespace chromeos_update_engine {
static const char kOOBECompletedMarker[] = "/home/chronos/.oobe_completed";
+static const char kPrefsDirectory[] = "/var/lib/update_engine/prefs";
RealSystemState::RealSystemState()
: device_policy_(NULL),
- connection_manager_(this),
- metrics_lib_(NULL) {}
+ connection_manager_(this) {}
+
+bool RealSystemState::Initialize() {
+ metrics_lib_.Init();
+
+ if (!prefs_.Init(FilePath(kPrefsDirectory))) {
+ LOG(ERROR) << "Failed to initialize preferences.";
+ return false;
+ }
+
+ if (!payload_state_.Initialize(&prefs_))
+ return false;
+
+ // All is well. Initialization successful.
+ return true;
+}
bool RealSystemState::IsOOBEComplete() {
return file_util::PathExists(FilePath(kOOBECompletedMarker));
}
-void RealSystemState::SetDevicePolicy(
+void RealSystemState::set_device_policy(
const policy::DevicePolicy* device_policy) {
device_policy_ = device_policy;
}
-const policy::DevicePolicy* RealSystemState::GetDevicePolicy() const {
+const policy::DevicePolicy* RealSystemState::device_policy() const {
return device_policy_;
}
-ConnectionManager* RealSystemState::GetConnectionManager() {
+ConnectionManager* RealSystemState::connection_manager() {
return &connection_manager_;
}
-void RealSystemState::set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
- metrics_lib_ = metrics_lib;
+MetricsLibraryInterface* RealSystemState::metrics_lib() {
+ return &metrics_lib_;
}
-MetricsLibraryInterface* RealSystemState::metrics_lib() {
- return metrics_lib_;
+PrefsInterface* RealSystemState::prefs() {
+ return &prefs_;
+}
+
+PayloadState* RealSystemState::payload_state() {
+ return &payload_state_;
}
} // namespace chromeos_update_engine
diff --git a/system_state.h b/system_state.h
index 7d7ac09..04ad382 100644
--- a/system_state.h
+++ b/system_state.h
@@ -10,11 +10,14 @@
#include <policy/libpolicy.h>
#include <update_engine/connection_manager.h>
+#include <update_engine/payload_state.h>
+#include <update_engine/prefs.h>
namespace chromeos_update_engine {
// An interface to global system context, including platform resources,
-// the current state of the system, high-level objects, system interfaces, etc.
+// the current state of the system, high-level objects whose lifetime is same
+// as main, system interfaces, etc.
// Carved out separately so it can be mocked for unit tests.
// Currently it has only one method, but we should start migrating other
// methods to use this as and when needed to unit test them.
@@ -29,15 +32,20 @@
virtual bool IsOOBEComplete() = 0;
// Sets or gets the latest device policy.
- virtual void SetDevicePolicy(const policy::DevicePolicy* device_policy) = 0;
- virtual const policy::DevicePolicy* GetDevicePolicy() const = 0;
+ virtual void set_device_policy(const policy::DevicePolicy* device_policy) = 0;
+ virtual const policy::DevicePolicy* device_policy() const = 0;
// Gets the connection manager object.
- virtual ConnectionManager* GetConnectionManager() = 0;
+ virtual ConnectionManager* connection_manager() = 0;
- // Sets or gets the Metrics Library interface for reporting UMA stats.
- virtual void set_metrics_lib(MetricsLibraryInterface* metrics_lib) = 0;
+ // Gets the Metrics Library interface for reporting UMA stats.
virtual MetricsLibraryInterface* metrics_lib() = 0;
+
+ // Gets the interface object for persisted store.
+ virtual PrefsInterface* prefs() = 0;
+
+ // Gets the URL State object.
+ virtual PayloadState* payload_state() = 0;
};
// A real implementation of the SystemStateInterface which is
@@ -50,14 +58,21 @@
virtual bool IsOOBEComplete();
- virtual void SetDevicePolicy(const policy::DevicePolicy* device_policy);
- virtual const policy::DevicePolicy* GetDevicePolicy() const;
+ virtual void set_device_policy(const policy::DevicePolicy* device_policy);
+ virtual const policy::DevicePolicy* device_policy() const;
- virtual ConnectionManager* GetConnectionManager();
+ virtual ConnectionManager* connection_manager();
- virtual void set_metrics_lib(MetricsLibraryInterface* metrics_lib);
virtual MetricsLibraryInterface* metrics_lib();
+ virtual PrefsInterface* prefs();
+
+ virtual PayloadState* payload_state();
+
+ // Initializs this concrete object. Other methods should be invoked only
+ // if the object has been initialized successfully.
+ bool Initialize();
+
private:
// The latest device policy object from the policy provider.
const policy::DevicePolicy* device_policy_;
@@ -67,7 +82,14 @@
ConnectionManager connection_manager_;
// The Metrics Library interface for reporting UMA stats.
- MetricsLibraryInterface* metrics_lib_;
+ MetricsLibrary metrics_lib_;
+
+ // Interface for persisted store.
+ Prefs prefs_;
+
+ // All state pertaining to payload state such as
+ // response, URL, back-off states.
+ PayloadState payload_state_;
};
} // namespace chromeos_update_engine
diff --git a/update_attempter.cc b/update_attempter.cc
index 3e0249b..d799c33 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -107,13 +107,12 @@
return code;
}
-UpdateAttempter::UpdateAttempter(PrefsInterface* prefs,
+UpdateAttempter::UpdateAttempter(SystemState* system_state,
DbusGlibInterface* dbus_iface,
- GpioHandler* gpio_handler,
- SystemState* system_state)
+ GpioHandler* gpio_handler)
: processor_(new ActionProcessor()),
+ system_state_(system_state),
dbus_service_(NULL),
- prefs_(prefs),
update_check_scheduler_(NULL),
fake_update_success_(false),
http_response_code_(0),
@@ -135,8 +134,8 @@
is_using_test_url_(false),
is_test_mode_(false),
is_test_update_attempted_(false),
- gpio_handler_(gpio_handler),
- system_state_(system_state) {
+ gpio_handler_(gpio_handler) {
+ prefs_ = system_state->prefs();
if (utils::FileExists(kUpdateCompletedMarker))
status_ = UPDATE_STATUS_UPDATED_NEED_REBOOT;
}
@@ -151,6 +150,7 @@
bool interactive,
bool is_test_mode,
bool is_user_initiated) {
+ LOG(INFO) << "Update called";
chrome_proxy_resolver_.Init();
fake_update_success_ = false;
if (status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
@@ -184,6 +184,7 @@
// before any update processing starts.
start_action_processor_ = true;
UpdateBootFlags();
+ LOG(INFO) << "Update finished";
}
bool UpdateAttempter::CalculateUpdateParams(const string& app_version,
@@ -215,7 +216,7 @@
device_policy.GetTargetVersionPrefix(
&omaha_request_params_.target_version_prefix);
- system_state_->SetDevicePolicy(&device_policy);
+ system_state_->set_device_policy(&device_policy);
set<string> allowed_types;
string allowed_types_str;
@@ -229,7 +230,7 @@
<< (allowed_types_str.empty() ? "all" : allowed_types_str);
} else {
LOG(INFO) << "No device policies present.";
- system_state_->SetDevicePolicy(NULL);
+ system_state_->set_device_policy(NULL);
}
CalculateScatteringParams(is_user_initiated);
@@ -287,7 +288,7 @@
// Take a copy of the old scatter value before we update it, as
// we need to update the waiting period if this value changes.
TimeDelta old_scatter_factor = scatter_factor_;
- const policy::DevicePolicy* device_policy = system_state_->GetDevicePolicy();
+ const policy::DevicePolicy* device_policy = system_state_->device_policy();
if (device_policy) {
int64 new_scatter_factor_in_secs = 0;
device_policy->GetScatterFactorInSeconds(&new_scatter_factor_in_secs);
@@ -414,19 +415,19 @@
update_check_fetcher->set_no_network_max_retries(interactive ? 1 : 3);
update_check_fetcher->set_check_certificate(CertificateChecker::kUpdate);
shared_ptr<OmahaRequestAction> update_check_action(
- new OmahaRequestAction(prefs_,
+ new OmahaRequestAction(system_state_,
&omaha_request_params_,
NULL,
update_check_fetcher, // passes ownership
false));
shared_ptr<OmahaResponseHandlerAction> response_handler_action(
- new OmahaResponseHandlerAction(prefs_));
+ new OmahaResponseHandlerAction(system_state_));
shared_ptr<FilesystemCopierAction> filesystem_copier_action(
new FilesystemCopierAction(false, false));
shared_ptr<FilesystemCopierAction> kernel_filesystem_copier_action(
new FilesystemCopierAction(true, false));
shared_ptr<OmahaRequestAction> download_started_action(
- new OmahaRequestAction(prefs_,
+ new OmahaRequestAction(system_state_,
&omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadStarted),
@@ -443,7 +444,7 @@
new MultiRangeHttpFetcher(
download_fetcher))); // passes ownership
shared_ptr<OmahaRequestAction> download_finished_action(
- new OmahaRequestAction(prefs_,
+ new OmahaRequestAction(system_state_,
&omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadFinished),
@@ -458,7 +459,7 @@
shared_ptr<PostinstallRunnerAction> postinstall_runner_action(
new PostinstallRunnerAction);
shared_ptr<OmahaRequestAction> update_complete_action(
- new OmahaRequestAction(prefs_,
+ new OmahaRequestAction(system_state_,
&omaha_request_params_,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
new LibcurlHttpFetcher(GetProxyResolver(),
@@ -878,11 +879,14 @@
if (error_event_.get() == NULL)
return false;
- LOG(INFO) << "Update failed -- reporting the error event.";
+ LOG(ERROR) << "Update failed.";
+ system_state_->payload_state()->UpdateFailed(error_event_->error_code);
+
+ LOG(INFO) << "Reporting the error event";
utils::SendErrorCodeToUma(system_state_->metrics_lib(),
error_event_->error_code);
shared_ptr<OmahaRequestAction> error_event_action(
- new OmahaRequestAction(prefs_,
+ new OmahaRequestAction(system_state_,
&omaha_request_params_,
error_event_.release(), // Pass ownership.
new LibcurlHttpFetcher(GetProxyResolver(),
@@ -998,7 +1002,7 @@
void UpdateAttempter::PingOmaha() {
if (!processor_->IsRunning()) {
shared_ptr<OmahaRequestAction> ping_action(
- new OmahaRequestAction(prefs_,
+ new OmahaRequestAction(system_state_,
&omaha_request_params_,
NULL,
new LibcurlHttpFetcher(GetProxyResolver(),
diff --git a/update_attempter.h b/update_attempter.h
index dd47f4d..df32536 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -60,10 +60,9 @@
public:
static const int kMaxDeltaUpdateFailures;
- UpdateAttempter(PrefsInterface* prefs,
+ UpdateAttempter(SystemState* system_state,
DbusGlibInterface* dbus_iface,
- GpioHandler* gpio_handler,
- SystemState* system_state);
+ GpioHandler* gpio_handler);
virtual ~UpdateAttempter();
// Checks for update and, if a newer version is available, attempts to update
@@ -275,6 +274,10 @@
std::vector<std::tr1::shared_ptr<AbstractAction> > actions_;
scoped_ptr<ActionProcessor> processor_;
+ // External state of the system outside the update_engine process
+ // carved out separately to mock out easily in unit tests.
+ SystemState* system_state_;
+
// If non-null, this UpdateAttempter will send status updates over this
// dbus service.
UpdateEngineService* dbus_service_;
@@ -285,7 +288,9 @@
// Pointer to the DownloadAction in the actions_ vector.
std::tr1::shared_ptr<DownloadAction> download_action_;
- // Pointer to the preferences store interface.
+ // Pointer to the preferences store interface. This is just a cached
+ // copy of system_state->prefs() because it's used in many methods and
+ // is convenient this way.
PrefsInterface* prefs_;
// The current UpdateCheckScheduler to notify of state transitions.
@@ -360,10 +365,6 @@
// The current scatter factor as found in the policy setting.
base::TimeDelta scatter_factor_;
- // External state of the system outside the update_engine process
- // carved out separately to mock out easily in unit tests.
- SystemState* system_state_;
-
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index d23bdb6..6432cd8 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -8,14 +8,16 @@
#include <gmock/gmock.h>
#include "update_engine/mock_dbus_interface.h"
+#include "update_engine/mock_system_state.h"
#include "update_engine/update_attempter.h"
namespace chromeos_update_engine {
class UpdateAttempterMock : public UpdateAttempter {
public:
- explicit UpdateAttempterMock(MockDbusGlib* dbus)
- : UpdateAttempter(NULL, dbus, NULL, NULL) {}
+ explicit UpdateAttempterMock(MockSystemState* mock_system_state,
+ MockDbusGlib* dbus)
+ : UpdateAttempter(mock_system_state, dbus, NULL) {}
MOCK_METHOD6(Update, void(const std::string& app_version,
const std::string& omaha_url,
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 937a70f..23e60fe 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -12,6 +12,7 @@
#include "update_engine/filesystem_copier_action.h"
#include "update_engine/mock_dbus_interface.h"
#include "update_engine/mock_http_fetcher.h"
+#include "update_engine/mock_payload_state.h"
#include "update_engine/mock_system_state.h"
#include "update_engine/postinstall_runner_action.h"
#include "update_engine/prefs.h"
@@ -37,22 +38,22 @@
// methods.
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
- explicit UpdateAttempterUnderTest(MockDbusGlib* dbus)
- : UpdateAttempter(NULL, dbus, NULL, NULL) {}
+ explicit UpdateAttempterUnderTest(MockSystemState* mock_system_state,
+ MockDbusGlib* dbus)
+ : UpdateAttempter(mock_system_state, dbus, NULL) {}
};
class UpdateAttempterTest : public ::testing::Test {
protected:
UpdateAttempterTest()
- : attempter_(&dbus_),
+ : attempter_(&mock_system_state_, &dbus_),
mock_connection_manager(&mock_system_state_),
loop_(NULL) {
- mock_system_state_.SetConnectionManager(&mock_connection_manager);
+ mock_system_state_.set_connection_manager(&mock_connection_manager);
}
virtual void SetUp() {
EXPECT_EQ(NULL, attempter_.dbus_service_);
- EXPECT_EQ(NULL, attempter_.prefs_);
- EXPECT_EQ(NULL, attempter_.system_state_);
+ EXPECT_TRUE(attempter_.system_state_ != NULL);
EXPECT_EQ(NULL, attempter_.update_check_scheduler_);
EXPECT_EQ(0, attempter_.http_response_code_);
EXPECT_EQ(utils::kCpuSharesNormal, attempter_.shares_);
@@ -65,8 +66,7 @@
EXPECT_EQ(0, attempter_.new_payload_size_);
processor_ = new ActionProcessorMock();
attempter_.processor_.reset(processor_); // Transfers ownership.
- attempter_.prefs_ = &prefs_;
- attempter_.system_state_ = &mock_system_state_;
+ prefs_ = mock_system_state_.mock_prefs();
}
void QuitMainLoop();
@@ -102,11 +102,11 @@
static gboolean StaticNoScatteringDoneDuringManualUpdateTestStart(
gpointer data);
+ MockSystemState mock_system_state_;
MockDbusGlib dbus_;
UpdateAttempterUnderTest attempter_;
ActionProcessorMock* processor_;
- NiceMock<PrefsMock> prefs_;
- MockSystemState mock_system_state_;
+ NiceMock<PrefsMock>* prefs_; // shortcut to mock_system_state_->mock_prefs()
MockConnectionManager mock_connection_manager;
GMainLoop* loop_;
};
@@ -114,8 +114,8 @@
TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) {
scoped_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, NULL));
fetcher->FailTransfer(503); // Sets the HTTP response code.
- DownloadAction action(&prefs_, NULL, fetcher.release());
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
+ DownloadAction action(prefs_, NULL, fetcher.release());
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(NULL, &action, kActionCodeSuccess);
EXPECT_EQ(503, attempter_.http_response_code());
EXPECT_EQ(UPDATE_STATUS_FINALIZING, attempter_.status());
@@ -126,7 +126,7 @@
ActionMock action;
EXPECT_CALL(action, Type()).WillRepeatedly(Return("ActionMock"));
attempter_.status_ = UPDATE_STATUS_DOWNLOADING;
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(Return(false));
attempter_.ActionCompleted(NULL, &action, kActionCodeError);
ASSERT_TRUE(attempter_.error_event_.get() != NULL);
@@ -136,7 +136,8 @@
scoped_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, NULL));
fetcher->FailTransfer(500); // Sets the HTTP response code.
OmahaRequestParams params;
- OmahaRequestAction action(&prefs_, ¶ms, NULL, fetcher.release(), false);
+ OmahaRequestAction action(&mock_system_state_, ¶ms, NULL,
+ fetcher.release(), false);
ObjectCollectorAction<OmahaResponse> collector_action;
BondActions(&action, &collector_action);
OmahaResponse response;
@@ -144,7 +145,7 @@
action.SetOutputObject(response);
UpdateCheckScheduler scheduler(&attempter_, NULL, &mock_system_state_);
attempter_.set_update_check_scheduler(&scheduler);
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(NULL, &action, kActionCodeSuccess);
EXPECT_EQ(500, attempter_.http_response_code());
EXPECT_EQ(UPDATE_STATUS_IDLE, attempter_.status());
@@ -156,8 +157,7 @@
extern const char* kUpdateCompletedMarker;
const FilePath kMarker(kUpdateCompletedMarker);
EXPECT_EQ(0, file_util::WriteFile(kMarker, "", 0));
- MockDbusGlib dbus;
- UpdateAttempterUnderTest attempter(&dbus);
+ UpdateAttempterUnderTest attempter(&mock_system_state_, &dbus_);
EXPECT_EQ(UPDATE_STATUS_UPDATED_NEED_REBOOT, attempter.status());
EXPECT_TRUE(file_util::Delete(kMarker, false));
}
@@ -169,10 +169,12 @@
GetErrorCodeForAction(NULL, kActionCodeSuccess));
OmahaRequestParams params;
- OmahaRequestAction omaha_request_action(NULL, ¶ms, NULL, NULL, false);
+ MockSystemState mock_system_state;
+ OmahaRequestAction omaha_request_action(&mock_system_state, ¶ms, NULL,
+ NULL, false);
EXPECT_EQ(kActionCodeOmahaRequestError,
GetErrorCodeForAction(&omaha_request_action, kActionCodeError));
- OmahaResponseHandlerAction omaha_response_handler_action(&prefs_);
+ OmahaResponseHandlerAction omaha_response_handler_action(&mock_system_state_);
EXPECT_EQ(kActionCodeOmahaResponseHandlerError,
GetErrorCodeForAction(&omaha_response_handler_action,
kActionCodeError));
@@ -191,40 +193,40 @@
TEST_F(UpdateAttempterTest, DisableDeltaUpdateIfNeededTest) {
attempter_.omaha_request_params_.delta_okay = true;
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(Return(false));
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_TRUE(attempter_.omaha_request_params_.delta_okay);
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(UpdateAttempter::kMaxDeltaUpdateFailures - 1),
Return(true)));
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_TRUE(attempter_.omaha_request_params_.delta_okay);
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(UpdateAttempter::kMaxDeltaUpdateFailures),
Return(true)));
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_FALSE(attempter_.omaha_request_params_.delta_okay);
- EXPECT_CALL(prefs_, GetInt64(_, _)).Times(0);
+ EXPECT_CALL(*prefs_, GetInt64(_, _)).Times(0);
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_FALSE(attempter_.omaha_request_params_.delta_okay);
}
TEST_F(UpdateAttempterTest, MarkDeltaUpdateFailureTest) {
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(Return(false))
.WillOnce(DoAll(SetArgumentPointee<1>(-1), Return(true)))
.WillOnce(DoAll(SetArgumentPointee<1>(1), Return(true)))
.WillOnce(DoAll(
SetArgumentPointee<1>(UpdateAttempter::kMaxDeltaUpdateFailures),
Return(true)));
- EXPECT_CALL(prefs_, SetInt64(Ne(kPrefsDeltaUpdateFailures), _))
+ 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,
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 1)).Times(2);
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 2)).Times(1);
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures,
UpdateAttempter::kMaxDeltaUpdateFailures + 1))
.Times(1);
for (int i = 0; i < 4; i ++)
@@ -234,7 +236,14 @@
TEST_F(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest) {
EXPECT_CALL(*processor_, EnqueueAction(_)).Times(0);
EXPECT_CALL(*processor_, StartProcessing()).Times(0);
+ EXPECT_CALL(*mock_system_state_.mock_payload_state(), UpdateFailed(_))
+ .Times(0);
+ OmahaResponse response;
+ response.payload_urls.push_back("http://url");
+ response.payload_urls.push_back("https://url");
+ mock_system_state_.mock_payload_state()->SetResponse(response);
attempter_.ScheduleErrorEventAction();
+ EXPECT_EQ(0, mock_system_state_.mock_payload_state()->GetUrlIndex());
}
TEST_F(UpdateAttempterTest, ScheduleErrorEventActionTest) {
@@ -243,10 +252,16 @@
OmahaRequestAction::StaticType())))
.Times(1);
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ OmahaResponse response;
+ response.payload_urls.push_back("http://url");
+ response.payload_urls.push_back("https://url");
+ mock_system_state_.mock_payload_state()->SetResponse(response);
+ ActionExitCode err = kActionCodeError;
attempter_.error_event_.reset(new OmahaEvent(OmahaEvent::kTypeUpdateComplete,
OmahaEvent::kResultError,
- kActionCodeError));
+ err));
attempter_.ScheduleErrorEventAction();
+ EXPECT_EQ(1, mock_system_state_.mock_payload_state()->GetUrlIndex());
EXPECT_EQ(UPDATE_STATUS_REPORTING_ERROR_EVENT, attempter_.status());
}
@@ -427,7 +442,7 @@
TEST_F(UpdateAttempterTest, CreatePendingErrorEventResumedTest) {
OmahaResponseHandlerAction *response_action =
- new OmahaResponseHandlerAction(&prefs_);
+ new OmahaResponseHandlerAction(&mock_system_state_);
response_action->install_plan_.is_resume = true;
attempter_.response_handler_action_.reset(response_action);
ActionMock action;
@@ -545,7 +560,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).WillRepeatedly(
+ EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
Return(device_policy));
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
@@ -592,7 +607,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).WillRepeatedly(
+ EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
Return(device_policy));
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
@@ -659,7 +674,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).WillRepeatedly(
+ EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
Return(device_policy));
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index 2e7e62d..bc1a560 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -30,19 +30,23 @@
// GLib and utils in tests. There're explicit unit test for the wrapper methods.
class UpdateCheckSchedulerUnderTest : public UpdateCheckScheduler {
public:
- UpdateCheckSchedulerUnderTest(UpdateAttempter* update_attempter)
- : UpdateCheckScheduler(update_attempter, NULL, &mock_system_state_) {}
+ UpdateCheckSchedulerUnderTest(UpdateAttempter* update_attempter,
+ MockSystemState* mock_system_state)
+ : UpdateCheckScheduler(update_attempter, NULL, mock_system_state),
+ mock_system_state_(mock_system_state) {}
MOCK_METHOD2(GTimeoutAddSeconds, guint(guint seconds, GSourceFunc function));
MOCK_METHOD0(IsBootDeviceRemovable, bool());
MOCK_METHOD0(IsOfficialBuild, bool());
- MockSystemState mock_system_state_;
+ MockSystemState* mock_system_state_;
};
class UpdateCheckSchedulerTest : public ::testing::Test {
public:
- UpdateCheckSchedulerTest() : scheduler_(&attempter_), attempter_(&dbus_) {}
+ UpdateCheckSchedulerTest()
+ : attempter_(&mock_system_state_, &dbus_),
+ scheduler_(&attempter_, &mock_system_state_) {}
protected:
virtual void SetUp() {
@@ -66,9 +70,10 @@
return test_->source_callback_.Call(data);
}
- UpdateCheckSchedulerUnderTest scheduler_;
+ MockSystemState mock_system_state_;
MockDbusGlib dbus_;
UpdateAttempterMock attempter_;
+ UpdateCheckSchedulerUnderTest scheduler_;
MockFunction<gboolean(gpointer data)> source_callback_;
GMainLoop* loop_;
static UpdateCheckSchedulerTest* test_;
@@ -273,7 +278,8 @@
TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBECompleteTest) {
scheduler_.scheduled_ = true;
- EXPECT_CALL(scheduler_.mock_system_state_,
+ EXPECT_TRUE(scheduler_.mock_system_state_ != NULL);
+ EXPECT_CALL(*scheduler_.mock_system_state_,
IsOOBEComplete()).Times(1).WillOnce(Return(true));
EXPECT_CALL(attempter_, Update("", "", false, false, false, false))
.Times(1)
@@ -285,7 +291,7 @@
TEST_F(UpdateCheckSchedulerTest, StaticCheckOOBENotCompleteTest) {
scheduler_.scheduled_ = true;
- EXPECT_CALL(scheduler_.mock_system_state_,
+ EXPECT_CALL(*scheduler_.mock_system_state_,
IsOOBEComplete()).Times(1).WillOnce(Return(false));
EXPECT_CALL(attempter_, Update("", "", _, _, _,_)).Times(0);
int interval_min, interval_max;