Add metrics to report the number of bytes downloaded per protocol.
This CL adds these two basic metrics:
1. The number of megabytes downloaded using each protocol (or source
in general, as we want to consider HTTP downloads from server and possibly
a peer in future as two different sources) for each successful update. The
totals across all protocols will equal the payload size exactly.
2. The total number of megabytes downloaded since the last update attempt.
If there are no errors, this should be same as above. Otherwise, this will
be larger and will help us to compute the update efficiency.
BUG=chromium:225953
TEST=Unit tests pass, new Unit tests added.
TEST=chrome://histograms shows metrics correctly.
Change-Id: Ic02e218f46568427df99a8a9df2011860aee84f3
Reviewed-on: https://gerrit.chromium.org/gerrit/48069
Reviewed-by: Chris Sosa <[email protected]>
Commit-Queue: Jay Srinivasan <[email protected]>
Tested-by: Jay Srinivasan <[email protected]>
diff --git a/constants.cc b/constants.cc
index de7bb91..3064c40 100644
--- a/constants.cc
+++ b/constants.cc
@@ -11,13 +11,13 @@
const char kPowerwashCommand[] = "safe fast\n";
-
// Constants defining keys for the persisted state of update engine.
const char kPrefsBackoffExpiryTime[] = "backoff-expiry-time";
const char kPrefsCertificateReportToSendDownload[] =
"certificate-report-to-send-download";
const char kPrefsCertificateReportToSendUpdate[] =
"certificate-report-to-send-update";
+const char kPrefsCurrentBytesDownloaded[] = "current-bytes-downloaded";
const char kPrefsCurrentResponseSignature[] = "current-response-signature";
const char kPrefsCurrentUrlFailureCount[] = "current-url-failure-count";
const char kPrefsCurrentUrlIndex[] = "current-url-index";
@@ -28,6 +28,7 @@
const char kPrefsPayloadAttemptNumber[] = "payload-attempt-number";
const char kPrefsPreviousVersion[] = "previous-version";
const char kPrefsResumedUpdateFailures[] = "resumed-update-failures";
+const char kPrefsTotalBytesDownloaded[] = "total-bytes-downloaded";
const char kPrefsUpdateCheckCount[] = "update-check-count";
const char kPrefsUpdateCheckResponseHash[] = "update-check-response-hash";
const char kPrefsUpdateFirstSeenAt[] = "update-first-seen-at";
diff --git a/constants.h b/constants.h
index 4f335c3..b2c1de2 100644
--- a/constants.h
+++ b/constants.h
@@ -18,6 +18,7 @@
extern const char kPrefsBackoffExpiryTime[];
extern const char kPrefsCertificateReportToSendDownload[];
extern const char kPrefsCertificateReportToSendUpdate[];
+extern const char kPrefsCurrentBytesDownloaded[];
extern const char kPrefsCurrentResponseSignature[];
extern const char kPrefsCurrentUrlFailureCount[];
extern const char kPrefsCurrentUrlIndex[];
@@ -28,6 +29,7 @@
extern const char kPrefsPayloadAttemptNumber[];
extern const char kPrefsPreviousVersion[];
extern const char kPrefsResumedUpdateFailures[];
+extern const char kPrefsTotalBytesDownloaded[];
extern const char kPrefsUpdateCheckCount[];
extern const char kPrefsUpdateCheckResponseHash[];
extern const char kPrefsUpdateFirstSeenAt[];
@@ -41,6 +43,23 @@
extern const char kPrefsUpdateTimestampStart[];
extern const char kPrefsUpdateDurationUptime[];
+// A download source is any combination of protocol and server (that's of
+// interest to us when looking at UMA metrics) using which we may download
+// the payload.
+typedef enum {
+ kDownloadSourceHttpsServer, // UMA Binary representation: 0001
+ kDownloadSourceHttpServer, // UMA Binary representation: 0010
+
+ // Note: Add new sources only above this line.
+ kNumDownloadSources
+} DownloadSource;
+
+// The default number of UMA buckets for metrics.
+const int kNumDefaultUmaBuckets = 50;
+
+// General constants
+const int kNumBytesInOneMiB = 1024 * 1024;
+
} // namespace chromeos_update_engine
#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_CONSTANTS_H
diff --git a/mock_payload_state.h b/mock_payload_state.h
index a2c80f4..c32ff78 100644
--- a/mock_payload_state.h
+++ b/mock_payload_state.h
@@ -13,7 +13,7 @@
class MockPayloadState: public PayloadStateInterface {
public:
- bool Initialize(PrefsInterface* prefs) {
+ bool Initialize(SystemState* system_state) {
return true;
}
@@ -21,6 +21,7 @@
MOCK_METHOD1(SetResponse, void(const OmahaResponse& response));
MOCK_METHOD0(DownloadComplete, void());
MOCK_METHOD1(DownloadProgress, void(size_t count));
+ MOCK_METHOD0(UpdateRestarted, void());
MOCK_METHOD0(UpdateSucceeded, void());
MOCK_METHOD1(UpdateFailed, void(ActionExitCode error));
MOCK_METHOD0(ShouldBackoffDownload, bool());
@@ -33,6 +34,8 @@
MOCK_METHOD0(GetBackoffExpiryTime, base::Time());
MOCK_METHOD0(GetUpdateDuration, base::TimeDelta());
MOCK_METHOD0(GetUpdateDurationUptime, base::TimeDelta());
+ MOCK_METHOD1(GetCurrentBytesDownloaded, uint64_t(DownloadSource source));
+ MOCK_METHOD1(GetTotalBytesDownloaded, uint64_t(DownloadSource source));
};
} // namespace chromeos_update_engine
diff --git a/mock_system_state.cc b/mock_system_state.cc
index d25f3c9..b7ed27b 100644
--- a/mock_system_state.cc
+++ b/mock_system_state.cc
@@ -13,7 +13,7 @@
: default_request_params_(this),
prefs_(&mock_prefs_) {
request_params_ = &default_request_params_;
- mock_payload_state_.Initialize(&mock_prefs_);
+ mock_payload_state_.Initialize(this);
mock_gpio_handler_ = new testing::NiceMock<MockGpioHandler>();
mock_update_attempter_ = new testing::NiceMock<UpdateAttempterMock>(
this, &dbus_);
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index a9f77a8..af47289 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -54,6 +54,7 @@
install_plan_.is_resume =
DeltaPerformer::CanResumeUpdate(system_state_->prefs(), response.hash);
if (!install_plan_.is_resume) {
+ system_state_->payload_state()->UpdateRestarted();
LOG_IF(WARNING, !DeltaPerformer::ResetUpdateProgress(
system_state_->prefs(), false))
<< "Unable to reset the update progress.";
diff --git a/payload_state.cc b/payload_state.cc
index 1699f64..bdc9bb4 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -7,9 +7,12 @@
#include <algorithm>
#include <base/logging.h>
+#include "base/string_util.h"
#include <base/stringprintf.h>
#include "update_engine/constants.h"
+#include "update_engine/prefs.h"
+#include "update_engine/system_state.h"
#include "update_engine/utils.h"
using base::Time;
@@ -27,9 +30,18 @@
// We want to randomize retry attempts after the backoff by +/- 6 hours.
static const uint32_t kMaxBackoffFuzzMinutes = 12 * 60;
-bool PayloadState::Initialize(PrefsInterface* prefs) {
- CHECK(prefs);
- prefs_ = prefs;
+PayloadState::PayloadState()
+ : prefs_(NULL),
+ payload_attempt_number_(0),
+ url_index_(0),
+ url_failure_count_(0) {
+ for (int i = 0; i <= kNumDownloadSources; i++)
+ total_bytes_downloaded_[i] = current_bytes_downloaded_[i] = 0;
+}
+
+bool PayloadState::Initialize(SystemState* system_state) {
+ system_state_ = system_state;
+ prefs_ = system_state_->prefs();
LoadResponseSignature();
LoadPayloadAttemptNumber();
LoadUrlIndex();
@@ -39,6 +51,11 @@
// The LoadUpdateDurationUptime() method relies on LoadUpdateTimestampStart()
// being called before it. Don't reorder.
LoadUpdateDurationUptime();
+ for (int i = 0; i < kNumDownloadSources; i++) {
+ DownloadSource source = static_cast<DownloadSource>(i);
+ LoadCurrentBytesDownloaded(source);
+ LoadTotalBytesDownloaded(source);
+ }
return true;
}
@@ -70,6 +87,10 @@
ResetPersistedState();
return;
}
+
+ // Update the current download source which depends on the latest value of
+ // the response.
+ UpdateCurrentDownloadSource();
}
void PayloadState::DownloadComplete() {
@@ -82,6 +103,7 @@
return;
CalculateUpdateDurationUptime();
+ UpdateBytesDownloaded(count);
// We've received non-zero bytes from a recent download operation. Since our
// URL failure count is meant to penalize a URL only for consecutive
@@ -100,9 +122,16 @@
SetUrlFailureCount(0);
}
+void PayloadState::UpdateRestarted() {
+ LOG(INFO) << "Starting a new update";
+ ResetDownloadSourcesOnNewUpdate();
+}
+
void PayloadState::UpdateSucceeded() {
+ // Send the relevant metrics that are tracked in this class to UMA.
CalculateUpdateDurationUptime();
SetUpdateTimestampEnd(Time::Now());
+ ReportBytesDownloadedMetrics();
}
void PayloadState::UpdateFailed(ActionExitCode error) {
@@ -328,6 +357,64 @@
SetBackoffExpiryTime(Time::Now() + next_backoff_interval);
}
+void PayloadState::UpdateCurrentDownloadSource() {
+ current_download_source_ = kNumDownloadSources;
+
+ if (GetUrlIndex() < response_.payload_urls.size()) {
+ string current_url = response_.payload_urls[GetUrlIndex()];
+ if (StartsWithASCII(current_url, "https://", false))
+ current_download_source_ = kDownloadSourceHttpsServer;
+ else if (StartsWithASCII(current_url, "http://", false))
+ current_download_source_ = kDownloadSourceHttpServer;
+ }
+
+ LOG(INFO) << "Current download source: "
+ << utils::ToString(current_download_source_);
+}
+
+void PayloadState::UpdateBytesDownloaded(size_t count) {
+ SetCurrentBytesDownloaded(
+ current_download_source_,
+ GetCurrentBytesDownloaded(current_download_source_) + count,
+ false);
+ SetTotalBytesDownloaded(
+ current_download_source_,
+ GetTotalBytesDownloaded(current_download_source_) + count,
+ false);
+}
+
+void PayloadState::ReportBytesDownloadedMetrics() {
+ // Report metrics collected from all known download sources to UMA.
+ // The reported data is in Megabytes in order to represent a larger
+ // sample range.
+ for (int i = 0; i < kNumDownloadSources; i++) {
+ DownloadSource source = static_cast<DownloadSource>(i);
+ const int kMaxMiBs = 10240; // Anything above 10GB goes in the last bucket.
+
+ string metric = "Installer.SuccessfulMBsDownloadedFrom" +
+ utils::ToString(source);
+ uint64_t mbs = GetCurrentBytesDownloaded(source) / kNumBytesInOneMiB;
+ LOG(INFO) << "Uploading " << mbs << " (MBs) for metric " << metric;
+ system_state_->metrics_lib()->SendToUMA(metric,
+ mbs,
+ 0, // min
+ kMaxMiBs,
+ kNumDefaultUmaBuckets);
+ SetCurrentBytesDownloaded(source, 0, true);
+
+ metric = "Installer.TotalMBsDownloadedFrom" + utils::ToString(source);
+ mbs = GetTotalBytesDownloaded(source) / kNumBytesInOneMiB;
+ LOG(INFO) << "Uploading " << mbs << " (MBs) for metric " << metric;
+ system_state_->metrics_lib()->SendToUMA(metric,
+ mbs,
+ 0, // min
+ kMaxMiBs,
+ kNumDefaultUmaBuckets);
+
+ SetTotalBytesDownloaded(source, 0, true);
+ }
+}
+
void PayloadState::ResetPersistedState() {
SetPayloadAttemptNumber(0);
SetUrlIndex(0);
@@ -336,6 +423,35 @@
SetUpdateTimestampStart(Time::Now());
SetUpdateTimestampEnd(Time()); // Set to null time
SetUpdateDurationUptime(TimeDelta::FromSeconds(0));
+ ResetDownloadSourcesOnNewUpdate();
+}
+
+void PayloadState::ResetDownloadSourcesOnNewUpdate() {
+ for (int i = 0; i < kNumDownloadSources; i++) {
+ DownloadSource source = static_cast<DownloadSource>(i);
+ SetCurrentBytesDownloaded(source, 0, true);
+ // Note: Not resetting the TotalBytesDownloaded as we want that metric
+ // to count the bytes downloaded across various update attempts until
+ // we have successfully applied the update.
+ }
+}
+
+int64_t PayloadState::GetPersistedValue(const string& key) {
+ CHECK(prefs_);
+ if (!prefs_->Exists(key))
+ return 0;
+
+ int64_t stored_value;
+ if (!prefs_->GetInt64(key, &stored_value))
+ return 0;
+
+ if (stored_value < 0) {
+ LOG(ERROR) << key << ": Invalid value (" << stored_value
+ << ") in persisted state. Defaulting to 0";
+ return 0;
+ }
+
+ return stored_value;
}
string PayloadState::CalculateResponseSignature() {
@@ -372,7 +488,7 @@
}
}
-void PayloadState::SetResponseSignature(string response_signature) {
+void PayloadState::SetResponseSignature(const string& response_signature) {
CHECK(prefs_);
response_signature_ = response_signature;
LOG(INFO) << "Current Response Signature = \n" << response_signature_;
@@ -380,17 +496,7 @@
}
void PayloadState::LoadPayloadAttemptNumber() {
- CHECK(prefs_);
- int64_t stored_value;
- if (prefs_->Exists(kPrefsPayloadAttemptNumber) &&
- prefs_->GetInt64(kPrefsPayloadAttemptNumber, &stored_value)) {
- if (stored_value < 0) {
- LOG(ERROR) << "Invalid payload attempt number (" << stored_value
- << ") in persisted state. Defaulting to 0";
- stored_value = 0;
- }
- SetPayloadAttemptNumber(stored_value);
- }
+ SetPayloadAttemptNumber(GetPersistedValue(kPrefsPayloadAttemptNumber));
}
void PayloadState::SetPayloadAttemptNumber(uint32_t payload_attempt_number) {
@@ -401,19 +507,7 @@
}
void PayloadState::LoadUrlIndex() {
- CHECK(prefs_);
- int64_t stored_value;
- if (prefs_->Exists(kPrefsCurrentUrlIndex) &&
- prefs_->GetInt64(kPrefsCurrentUrlIndex, &stored_value)) {
- // We only check for basic sanity value here. Detailed check will be
- // done in SetResponse once the first response comes in.
- if (stored_value < 0) {
- LOG(ERROR) << "Invalid URL Index (" << stored_value
- << ") in persisted state. Defaulting to 0";
- stored_value = 0;
- }
- SetUrlIndex(stored_value);
- }
+ SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex));
}
void PayloadState::SetUrlIndex(uint32_t url_index) {
@@ -421,20 +515,14 @@
url_index_ = url_index;
LOG(INFO) << "Current URL Index = " << url_index_;
prefs_->SetInt64(kPrefsCurrentUrlIndex, url_index_);
+
+ // Also update the download source, which is purely dependent on the
+ // current URL index alone.
+ UpdateCurrentDownloadSource();
}
void PayloadState::LoadUrlFailureCount() {
- CHECK(prefs_);
- int64_t stored_value;
- if (prefs_->Exists(kPrefsCurrentUrlFailureCount) &&
- prefs_->GetInt64(kPrefsCurrentUrlFailureCount, &stored_value)) {
- if (stored_value < 0) {
- LOG(ERROR) << "Invalid URL Failure count (" << stored_value
- << ") in persisted state. Defaulting to 0";
- stored_value = 0;
- }
- SetUrlFailureCount(stored_value);
- }
+ SetUrlFailureCount(GetPersistedValue(kPrefsCurrentUrlFailureCount));
}
void PayloadState::SetUrlFailureCount(uint32_t url_failure_count) {
@@ -593,4 +681,57 @@
SetUpdateDurationUptimeExtended(new_uptime, now, false);
}
+string PayloadState::GetPrefsKey(const string& prefix, DownloadSource source) {
+ return prefix + "-from-" + utils::ToString(source);
+}
+
+void PayloadState::LoadCurrentBytesDownloaded(DownloadSource source) {
+ string key = GetPrefsKey(kPrefsCurrentBytesDownloaded, source);
+ SetCurrentBytesDownloaded(source, GetPersistedValue(key), true);
+}
+
+void PayloadState::SetCurrentBytesDownloaded(
+ DownloadSource source,
+ uint64_t current_bytes_downloaded,
+ bool log) {
+ CHECK(prefs_);
+
+ if (source >= kNumDownloadSources)
+ return;
+
+ // Update the in-memory value.
+ current_bytes_downloaded_[source] = current_bytes_downloaded;
+
+ string prefs_key = GetPrefsKey(kPrefsCurrentBytesDownloaded, source);
+ prefs_->SetInt64(prefs_key, current_bytes_downloaded);
+ LOG_IF(INFO, log) << "Current bytes downloaded for "
+ << utils::ToString(source) << " = "
+ << GetCurrentBytesDownloaded(source);
+}
+
+void PayloadState::LoadTotalBytesDownloaded(DownloadSource source) {
+ string key = GetPrefsKey(kPrefsTotalBytesDownloaded, source);
+ SetTotalBytesDownloaded(source, GetPersistedValue(key), true);
+}
+
+void PayloadState::SetTotalBytesDownloaded(
+ DownloadSource source,
+ uint64_t total_bytes_downloaded,
+ bool log) {
+ CHECK(prefs_);
+
+ if (source >= kNumDownloadSources)
+ return;
+
+ // Update the in-memory value.
+ total_bytes_downloaded_[source] = total_bytes_downloaded;
+
+ // Persist.
+ string prefs_key = GetPrefsKey(kPrefsTotalBytesDownloaded, source);
+ prefs_->SetInt64(prefs_key, total_bytes_downloaded);
+ LOG_IF(INFO, log) << "Total bytes downloaded for "
+ << utils::ToString(source) << " = "
+ << GetTotalBytesDownloaded(source);
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_state.h b/payload_state.h
index 0557d0a..b34422f 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -12,6 +12,8 @@
namespace chromeos_update_engine {
+class SystemState;
+
// Encapsulates all the payload state required for download. This includes the
// state necessary for handling multiple URLs in Omaha response, the backoff
// state, etc. All state is persisted so that we use the most recently saved
@@ -20,26 +22,20 @@
// state even when there's any issue in reading/writing from the file system.
class PayloadState : public PayloadStateInterface {
public:
- PayloadState()
- : prefs_(NULL),
- payload_attempt_number_(0),
- url_index_(0),
- url_failure_count_(0) {}
-
+ PayloadState();
virtual ~PayloadState() {}
- // 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);
-
+ // Initializes a payload state object using the given global system state.
+ // It 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(SystemState* system_state);
// Implementation of PayloadStateInterface methods.
virtual void SetResponse(const OmahaResponse& response);
virtual void DownloadComplete();
virtual void DownloadProgress(size_t count);
+ virtual void UpdateRestarted();
virtual void UpdateSucceeded();
virtual void UpdateFailed(ActionExitCode error);
virtual bool ShouldBackoffDownload();
@@ -68,6 +64,14 @@
virtual base::TimeDelta GetUpdateDurationUptime();
+ virtual inline uint64_t GetCurrentBytesDownloaded(DownloadSource source) {
+ return source < kNumDownloadSources ? current_bytes_downloaded_[source] : 0;
+ }
+
+ virtual inline uint64_t GetTotalBytesDownloaded(DownloadSource source) {
+ return source < kNumDownloadSources ? total_bytes_downloaded_[source] : 0;
+ }
+
private:
// Increments the payload attempt number which governs the backoff behavior
// at the time of the next update check.
@@ -88,10 +92,30 @@
// payload attempt number.
void UpdateBackoffExpiryTime();
+ // Updates the value of current download source based on the current URL
+ // index. If the download source is not one of the known sources, it's set
+ // to kNumDownloadSources.
+ void UpdateCurrentDownloadSource();
+
+ // Updates the various metrics corresponding with the given number of bytes
+ // that were downloaded recently.
+ void UpdateBytesDownloaded(size_t count);
+
+ // Reports the various metrics related to the number of bytes downloaded.
+ void ReportBytesDownloadedMetrics();
+
// Resets all the persisted state values which are maintained relative to the
// current response signature. The response signature itself is not reset.
void ResetPersistedState();
+ // Resets the appropriate state related to download sources that need to be
+ // reset on a new update.
+ void ResetDownloadSourcesOnNewUpdate();
+
+ // Returns the persisted value for the given key. It also validates that
+ // the value returned is non-negative.
+ int64_t GetPersistedValue(const std::string& key);
+
// Calculates the response "signature", which is basically a string composed
// of the subset of the fields in the current response that affect the
// behavior of the PayloadState.
@@ -103,7 +127,7 @@
// Sets the response signature to the given value. Also persists the value
// being set so that we resume from the save value in case of a process
// restart.
- void SetResponseSignature(std::string response_signature);
+ void SetResponseSignature(const std::string& response_signature);
// Initializes the payload attempt number from the persisted state.
void LoadPayloadAttemptNumber();
@@ -167,6 +191,36 @@
// sets |update_duration_uptime_timestamp_| to current monotonic time.
void CalculateUpdateDurationUptime();
+ // Returns the full key for a download source given the prefix.
+ std::string GetPrefsKey(const std::string& prefix, DownloadSource source);
+
+ // Loads the number of bytes that have been currently downloaded through the
+ // previous attempts from the persisted state for the given source. It's
+ // reset to 0 everytime we begin a full update and is continued from previous
+ // attempt if we're resuming the update.
+ void LoadCurrentBytesDownloaded(DownloadSource source);
+
+ // Sets the number of bytes that have been currently downloaded for the
+ // given source. This value is also persisted.
+ void SetCurrentBytesDownloaded(DownloadSource source,
+ uint64_t current_bytes_downloaded,
+ bool log);
+
+ // Loads the total number of bytes that have been downloaded (since the last
+ // successful update) from the persisted state for the given source. It's
+ // reset to 0 everytime we successfully apply an update and counts the bytes
+ // downloaded for both successful and failed attempts since then.
+ void LoadTotalBytesDownloaded(DownloadSource source);
+
+ // Sets the total number of bytes that have been downloaded so far for the
+ // given source. This value is also persisted.
+ void SetTotalBytesDownloaded(DownloadSource source,
+ uint64_t total_bytes_downloaded,
+ bool log);
+
+ // The global state of the system.
+ SystemState* system_state_;
+
// 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_;
@@ -199,6 +253,12 @@
// persisted so we resume from the same value in case of a process restart.
int64_t url_failure_count_;
+ // The current download source based on the current URL. This value is
+ // not persisted as it can be recomputed everytime we update the URL.
+ // We're storing this so as not to recompute this on every few bytes of
+ // data we read from the socket.
+ DownloadSource current_download_source_;
+
// The timestamp until which we've to wait before attempting to download the
// payload again, so as to backoff repeated downloads.
base::Time backoff_expiry_time_;
@@ -219,6 +279,23 @@
// The monotonic time when |update_duration_uptime_| was last set
base::Time update_duration_uptime_timestamp_;
+ // The number of bytes that have been downloaded for each source for each new
+ // update attempt. If we resume an update, we'll continue from the previous
+ // value, but if we get a new response or if the previous attempt failed,
+ // we'll reset this to 0 to start afresh. Each update to this value is
+ // persisted so we resume from the same value in case of a process restart.
+ // The extra index in the array is to no-op accidental access in case the
+ // return value from GetCurrentDownloadSource is used without validation.
+ uint64_t current_bytes_downloaded_[kNumDownloadSources + 1];
+
+ // The number of bytes that have been downloaded for each source since the
+ // the last successful update. This is used to compute the overhead we incur.
+ // Each update to this value is persisted so we resume from the same value in
+ // case of a process restart.
+ // The extra index in the array is to no-op accidental access in case the
+ // return value from GetCurrentDownloadSource is used without validation.
+ uint64_t total_bytes_downloaded_[kNumDownloadSources + 1];
+
// Returns the number of URLs in the current response.
// Note: This value will be 0 if this method is called before we receive
// the first valid Omaha response in this process.
diff --git a/payload_state_interface.h b/payload_state_interface.h
index d681154..133124d 100644
--- a/payload_state_interface.h
+++ b/payload_state_interface.h
@@ -7,6 +7,7 @@
#include <string>
+#include "update_engine/constants.h"
#include "update_engine/action_processor.h"
#include "update_engine/omaha_response.h"
@@ -39,7 +40,14 @@
// able to make forward progress with the current URL.
virtual void DownloadProgress(size_t count) = 0;
- // This method should be called whenever an update attempt succeeds.
+ // This method should be called every time we begin a new update. This method
+ // should not be called when we resume an update from the previously
+ // downloaded point. This is used to reset the metrics for each new update.
+ virtual void UpdateRestarted() = 0;
+
+ // This method should be called once after an update attempt succeeds. This
+ // is when the relevant UMA metrics that are tracked on a per-update-basis
+ // are uploaded to the UMA server.
virtual void UpdateSucceeded() = 0;
// This method should be called whenever an update attempt fails with the
@@ -76,6 +84,17 @@
// the device is powered off or sleeping. If the update has not
// completed, returns the time spent so far.
virtual base::TimeDelta GetUpdateDurationUptime() = 0;
+
+ // Returns the number of bytes that have been downloaded for each source for
+ // each new update attempt. If we resume an update, we'll continue from the
+ // previous value, but if we get a new response or if the previous attempt
+ // failed, we'll reset this to 0 to start afresh.
+ virtual uint64_t GetCurrentBytesDownloaded(DownloadSource source) = 0;
+
+ // Returns the total number of bytes that have been downloaded for each
+ // source since the the last successful update. This is used to compute the
+ // overhead we incur.
+ virtual uint64_t GetTotalBytesDownloaded(DownloadSource source) = 0;
};
} // namespace chromeos_update_engine
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 62de02b..ffd1b27 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -9,6 +9,7 @@
#include "gtest/gtest.h"
#include "update_engine/constants.h"
+#include "update_engine/mock_system_state.h"
#include "update_engine/omaha_request_action.h"
#include "update_engine/payload_state.h"
#include "update_engine/prefs_mock.h"
@@ -26,6 +27,15 @@
namespace chromeos_update_engine {
+const char* kCurrentBytesDownloadedFromHttps =
+ "current-bytes-downloaded-from-HttpsServer";
+const char* kTotalBytesDownloadedFromHttps =
+ "total-bytes-downloaded-from-HttpsServer";
+const char* kCurrentBytesDownloadedFromHttp =
+ "current-bytes-downloaded-from-HttpServer";
+const char* kTotalBytesDownloadedFromHttp =
+ "total-bytes-downloaded-from-HttpServer";
+
static void SetupPayloadStateWith2Urls(string hash,
PayloadState* payload_state,
OmahaResponse* response) {
@@ -72,15 +82,25 @@
TEST(PayloadStateTest, SetResponseWorksWithEmptyResponse) {
OmahaResponse response;
- NiceMock<PrefsMock> prefs;
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateTimestampStart, _)).Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateDurationUptime, _)).Times(AtLeast(1));
+ MockSystemState mock_system_state;
+ NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, 0)).Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsUpdateTimestampStart, _))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsUpdateDurationUptime, _))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttps, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttp, 0))
+ .Times(AtLeast(1));
PayloadState payload_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
payload_state.SetResponse(response);
string stored_response_sign = payload_state.GetResponseSignature();
string expected_response_sign = "NumURLs = 0\n"
@@ -103,15 +123,27 @@
response.hash = "hash";
response.metadata_size = 58123;
response.metadata_signature = "msign";
- NiceMock<PrefsMock> prefs;
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateTimestampStart, _)).Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateDurationUptime, _)).Times(AtLeast(1));
+ MockSystemState mock_system_state;
+ NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsUpdateTimestampStart, _))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsUpdateDurationUptime, _))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttps, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttp, 0))
+ .Times(AtLeast(1));
PayloadState payload_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
payload_state.SetResponse(response);
string stored_response_sign = payload_state.GetResponseSignature();
string expected_response_sign = "NumURLs = 1\n"
@@ -136,15 +168,23 @@
response.hash = "rhash";
response.metadata_size = 558123;
response.metadata_signature = "metasign";
- NiceMock<PrefsMock> prefs;
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateTimestampStart, _)).Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateDurationUptime, _)).Times(AtLeast(1));
+ MockSystemState mock_system_state;
+ NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttps, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttp, 0))
+ .Times(AtLeast(1));
PayloadState payload_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
payload_state.SetResponse(response);
string stored_response_sign = payload_state.GetResponseSignature();
string expected_response_sign = "NumURLs = 2\n"
@@ -164,27 +204,28 @@
TEST(PayloadStateTest, CanAdvanceUrlIndexCorrectly) {
OmahaResponse response;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
+ NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
PayloadState payload_state;
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
// Payload attempt should start with 0 and then advance to 1.
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0)).Times(1);
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 1)).Times(1);
- EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(2);
-
- // Durations will be set
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateTimestampStart, _)).Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateDurationUptime, _)).Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 1))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(AtLeast(2));
// Url index should go from 0 to 1 twice.
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(2);
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 1)).Times(2);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 1)).Times(AtLeast(1));
// Failure count should be called each times url index is set, so that's
// 4 times for this test.
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0)).Times(4);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0))
+ .Times(AtLeast(4));
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
// This does a SetResponse which causes all the states to be set to 0 for
// the first time.
@@ -207,10 +248,10 @@
TEST(PayloadStateTest, NewResponseResetsPayloadState) {
OmahaResponse response;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
PayloadState payload_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
// Set the first response.
SetupPayloadStateWith2Urls("Hash5823", &payload_state, &response);
@@ -226,30 +267,58 @@
// Make sure the url index was reset to 0 because of the new response.
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
+ EXPECT_EQ(0,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(0,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(0, payload_state.GetCurrentBytesDownloaded(
+ kDownloadSourceHttpsServer));
+ EXPECT_EQ(0,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
}
TEST(PayloadStateTest, AllCountersGetUpdatedProperlyOnErrorCodesAndEvents) {
OmahaResponse response;
PayloadState payload_state;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
+ int progress_bytes = 100;
+ NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0)).Times(2);
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 1)).Times(1);
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 2)).Times(1);
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
+ .Times(AtLeast(2));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 1))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 2))
+ .Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(4);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(AtLeast(4));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(4);
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 1)).Times(2);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(AtLeast(4));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 1)).Times(AtLeast(2));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0)).Times(7);
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 1)).Times(2);
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 2)).Times(1);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0))
+ .Times(AtLeast(7));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 1))
+ .Times(AtLeast(2));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 2))
+ .Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateTimestampStart, _)).Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateDurationUptime, _)).Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsUpdateTimestampStart, _))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsUpdateDurationUptime, _))
+ .Times(AtLeast(1));
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttps, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttp, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kCurrentBytesDownloadedFromHttp, progress_bytes))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kTotalBytesDownloadedFromHttp, progress_bytes))
+ .Times(AtLeast(1));
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash5873", &payload_state, &response);
@@ -306,7 +375,7 @@
// And that failure count should be reset when we download some bytes
// afterwards.
- payload_state.DownloadProgress(100);
+ payload_state.DownloadProgress(progress_bytes);
EXPECT_EQ(2, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
@@ -325,20 +394,24 @@
TEST(PayloadStateTest, PayloadAttemptNumberIncreasesOnSuccessfulDownload) {
OmahaResponse response;
PayloadState payload_state;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
+ NiceMock<PrefsMock>* prefs = mock_system_state.mock_prefs();
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0)).Times(1);
- EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 1)).Times(1);
+ EXPECT_CALL(*prefs, SetInt64(_,_)).Times(AtLeast(0));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsPayloadAttemptNumber, 1))
+ .Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(2);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsBackoffExpiryTime, _))
+ .Times(AtLeast(2));
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(1);
- EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0)).Times(1);
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlIndex, 0))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0))
+ .Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateTimestampStart, _)).Times(AtLeast(1));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateDurationUptime, _)).Times(AtLeast(1));
-
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash8593", &payload_state, &response);
@@ -353,9 +426,9 @@
TEST(PayloadStateTest, SetResponseResetsInvalidUrlIndex) {
OmahaResponse response;
PayloadState payload_state;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash4427", &payload_state, &response);
// Generate enough events to advance URL index, failure count and
@@ -370,21 +443,22 @@
// Now, simulate a corrupted url index on persisted store which gets
// loaded when update_engine restarts. Using a different prefs object
// so as to not bother accounting for the uninteresting calls above.
- NiceMock<PrefsMock> prefs2;
- EXPECT_CALL(prefs2, Exists(_)).WillRepeatedly(Return(true));
- EXPECT_CALL(prefs2, GetInt64(kPrefsPayloadAttemptNumber, _));
- EXPECT_CALL(prefs2, GetInt64(kPrefsBackoffExpiryTime, _));
- EXPECT_CALL(prefs2, GetInt64(kPrefsCurrentUrlIndex, _))
- .WillOnce(DoAll(SetArgumentPointee<1>(2), Return(true)));
- EXPECT_CALL(prefs2, GetInt64(kPrefsCurrentUrlFailureCount, _));
- EXPECT_CALL(prefs2, GetInt64(kPrefsUpdateTimestampStart, _));
- EXPECT_CALL(prefs2, GetInt64(kPrefsUpdateDurationUptime, _));
+ MockSystemState mock_system_state2;
+ NiceMock<PrefsMock>* prefs2 = mock_system_state2.mock_prefs();
+ EXPECT_CALL(*prefs2, Exists(_)).WillRepeatedly(Return(true));
+ EXPECT_CALL(*prefs2, GetInt64(_,_)).Times(AtLeast(1));
+ EXPECT_CALL(*prefs2, GetInt64(kPrefsPayloadAttemptNumber, _))
+ .Times(AtLeast(1));
+ EXPECT_CALL(*prefs2, GetInt64(kPrefsCurrentUrlIndex, _))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(2), Return(true)));
+ EXPECT_CALL(*prefs2, GetInt64(kPrefsCurrentUrlFailureCount, _))
+ .Times(AtLeast(1));
// Note: This will be a different payload object, but the response should
// have the same hash as before so as to not trivially reset because the
// response was different. We want to specifically test that even if the
// response is same, we should reset the state if we find it corrupted.
- EXPECT_TRUE(payload_state.Initialize(&prefs2));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state2));
SetupPayloadStateWith2Urls("Hash4427", &payload_state, &response);
// Make sure all counters get reset to 0 because of the corrupted URL index
@@ -398,9 +472,9 @@
OmahaResponse response;
response.is_delta_payload = true;
PayloadState payload_state;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash6437", &payload_state, &response);
// Simulate a successful download and see that we're ready to download
@@ -440,9 +514,9 @@
OmahaResponse response;
response.is_delta_payload = false;
PayloadState payload_state;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash8939", &payload_state, &response);
CheckPayloadBackoffState(&payload_state, 1, TimeDelta::FromDays(1));
@@ -461,9 +535,9 @@
OmahaResponse response;
response.disable_payload_backoff = true;
PayloadState payload_state;
- NiceMock<PrefsMock> prefs;
+ MockSystemState mock_system_state;
- EXPECT_TRUE(payload_state.Initialize(&prefs));
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
SetupPayloadStateWith2Urls("Hash8939", &payload_state, &response);
// Simulate a successful download and see that we are ready to download
@@ -481,4 +555,122 @@
EXPECT_FALSE(payload_state.ShouldBackoffDownload());
}
+TEST(PayloadStateTest, BytesDownloadedMetricsGetAddedToCorrectSources) {
+ OmahaResponse response;
+ response.disable_payload_backoff = true;
+ PayloadState payload_state;
+ MockSystemState mock_system_state;
+ int https_total = 0;
+ int http_total = 0;
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+ SetupPayloadStateWith2Urls("Hash3286", &payload_state, &response);
+
+ // Simulate a successful download and see that we are ready to download
+ // again without any backoff.
+ int first_chunk = 5000000;
+ http_total += first_chunk;
+ payload_state.DownloadProgress(first_chunk);
+ // Test that first all progress is made on HTTP and none on HTTPs.
+ EXPECT_EQ(first_chunk,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(http_total,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(0, payload_state.GetCurrentBytesDownloaded(
+ kDownloadSourceHttpsServer));
+ EXPECT_EQ(https_total,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
+
+ // Simulate an error that'll cause the url index to point to https.
+ ActionExitCode error = kActionCodeDownloadMetadataSignatureMismatch;
+ payload_state.UpdateFailed(error);
+
+ // Test that no new progress is made on HTTP and new progress is on HTTPs.
+ int second_chunk = 23456789;
+ https_total += second_chunk;
+ payload_state.DownloadProgress(second_chunk);
+ EXPECT_EQ(first_chunk,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(http_total,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(second_chunk, payload_state.GetCurrentBytesDownloaded(
+ kDownloadSourceHttpsServer));
+ EXPECT_EQ(https_total,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
+
+ // Simulate error to go back to http.
+ payload_state.UpdateFailed(error);
+ int third_chunk = 32345678;
+ int http_chunk = first_chunk + third_chunk;
+ http_total += third_chunk;
+ int https_chunk = second_chunk;
+ payload_state.DownloadProgress(third_chunk);
+
+ // Test that third chunk is again back on HTTP. HTTPS remains on second chunk.
+ EXPECT_EQ(http_chunk,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(http_chunk,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(second_chunk, payload_state.GetCurrentBytesDownloaded(
+ kDownloadSourceHttpsServer));
+ EXPECT_EQ(https_total,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
+
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.SuccessfulMBsDownloadedFromHttpServer",
+ http_chunk / kNumBytesInOneMiB, _, _, _));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.TotalMBsDownloadedFromHttpServer",
+ http_total / kNumBytesInOneMiB, _, _, _));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.SuccessfulMBsDownloadedFromHttpsServer",
+ https_chunk / kNumBytesInOneMiB, _, _, _));
+ EXPECT_CALL(*mock_system_state.mock_metrics_lib(), SendToUMA(
+ "Installer.TotalMBsDownloadedFromHttpsServer",
+ https_total / kNumBytesInOneMiB, _, _, _));
+
+ payload_state.UpdateSucceeded();
+
+ // Make sure the metrics are reset after a successful update.
+ EXPECT_EQ(0,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(0,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(0, payload_state.GetCurrentBytesDownloaded(
+ kDownloadSourceHttpsServer));
+ EXPECT_EQ(0,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
+}
+
+TEST(PayloadStateTest, RestartingUpdateResetsMetrics) {
+ OmahaResponse response;
+ MockSystemState mock_system_state;
+ PayloadState payload_state;
+
+ EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+
+ // Set the first response.
+ SetupPayloadStateWith2Urls("Hash5823", &payload_state, &response);
+
+ int num_bytes = 10000;
+ payload_state.DownloadProgress(num_bytes);
+ EXPECT_EQ(num_bytes,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(num_bytes,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(0, payload_state.GetCurrentBytesDownloaded(
+ kDownloadSourceHttpsServer));
+ EXPECT_EQ(0,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpsServer));
+
+ payload_state.UpdateRestarted();
+ // Make sure the current bytes downloaded is reset, but not the total bytes.
+ EXPECT_EQ(0,
+ payload_state.GetCurrentBytesDownloaded(kDownloadSourceHttpServer));
+ EXPECT_EQ(num_bytes,
+ payload_state.GetTotalBytesDownloaded(kDownloadSourceHttpServer));
+}
+
+
+
}
diff --git a/system_state.cc b/system_state.cc
index 0d553df..432b99f 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -24,7 +24,7 @@
return false;
}
- if (!payload_state_.Initialize(&prefs_))
+ if (!payload_state_.Initialize(this))
return false;
// Initialize the GPIO handler as instructed.
diff --git a/update_attempter.cc b/update_attempter.cc
index 9e5e49b..1360865 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -639,10 +639,10 @@
prefs_->Delete(kPrefsUpdateFirstSeenAt);
prefs_->Delete(kPrefsUpdateTimestampStart);
prefs_->Delete(kPrefsUpdateDurationUptime);
- LOG(INFO) << "Update successfully applied, waiting to reboot.";
SetStatusAndNotify(UPDATE_STATUS_UPDATED_NEED_REBOOT,
kUpdateNoticeUnspecified);
+ LOG(INFO) << "Update successfully applied, waiting to reboot.";
// Also report the success code so that the percentiles can be
// interpreted properly for the remaining error codes in UMA.
diff --git a/utils.cc b/utils.cc
index c6bc6b8..6e14f22 100644
--- a/utils.cc
+++ b/utils.cc
@@ -718,6 +718,18 @@
return (b ? "true" : "false");
}
+std::string ToString(DownloadSource source) {
+ switch (source) {
+ case kDownloadSourceHttpsServer: return "HttpsServer";
+ case kDownloadSourceHttpServer: return "HttpServer";
+ case kNumDownloadSources: return "Unknown";
+ // Don't add a default case to let the compiler warn about newly added
+ // download sources which should be added here.
+ }
+
+ return "Unknown";
+}
+
ActionExitCode GetBaseErrorCode(ActionExitCode code) {
// Ignore the higher order bits in the code by applying the mask as
// we want the enumerations to be in the small contiguous range
diff --git a/utils.h b/utils.h
index 46b06e1..53e58fe 100644
--- a/utils.h
+++ b/utils.h
@@ -19,6 +19,7 @@
#include "metrics/metrics_library.h"
#include "update_engine/action.h"
+#include "update_engine/constants.h"
#include "update_engine/action_processor.h"
namespace chromeos_update_engine {
@@ -158,6 +159,9 @@
// Returns true or false depending on the value of b.
std::string ToString(bool b);
+// Returns a string representation of the given enum.
+std::string ToString(DownloadSource source);
+
enum BootLoader {
BootLoader_SYSLINUX = 0,
BootLoader_CHROME_FIRMWARE = 1
