Track bytes received across multiple update files am: 9321f50102
am: daed65ff80
Change-Id: Ib8b390b9648d55b2f23dc83048944f3dfede2a93
diff --git a/mock_file_writer.h b/mock_file_writer.h
index 72d6a86..26cd45d 100644
--- a/mock_file_writer.h
+++ b/mock_file_writer.h
@@ -24,7 +24,8 @@
class MockFileWriter : public FileWriter {
public:
- MOCK_METHOD2(Write, ssize_t(const void* bytes, size_t count));
+ MOCK_METHOD2(Write, bool(const void* bytes, size_t count));
+ MOCK_METHOD3(Write, bool(const void* bytes, size_t count, ErrorCode* error));
MOCK_METHOD0(Close, int());
};
diff --git a/mock_service_observer.h b/mock_service_observer.h
new file mode 100644
index 0000000..032f55a
--- /dev/null
+++ b/mock_service_observer.h
@@ -0,0 +1,38 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef UPDATE_ENGINE_MOCK_SERVICE_OBSERVER_H_
+#define UPDATE_ENGINE_MOCK_SERVICE_OBSERVER_H_
+
+#include <gmock/gmock.h>
+#include "update_engine/service_observer_interface.h"
+
+namespace chromeos_update_engine {
+
+class MockServiceObserver : public ServiceObserverInterface {
+ public:
+ MOCK_METHOD5(SendStatusUpdate,
+ void(int64_t last_checked_time,
+ double progress,
+ update_engine::UpdateStatus status,
+ const std::string& new_version,
+ int64_t new_size));
+ MOCK_METHOD1(SendPayloadApplicationComplete, void(ErrorCode error_code));
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_MOCK_SERVICE_OBSERVER_H_
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index c3a5016..cd25962 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -173,6 +173,7 @@
install_plan_.Dump();
bytes_received_ = 0;
+ bytes_received_previous_payloads_ = 0;
bytes_total_ = 0;
for (const auto& payload : install_plan_.payloads)
bytes_total_ += payload.size;
@@ -317,8 +318,10 @@
}
bytes_received_ += length;
+ uint64_t bytes_downloaded_total =
+ bytes_received_previous_payloads_ + bytes_received_;
if (delegate_ && download_active_) {
- delegate_->BytesReceived(length, bytes_received_, bytes_total_);
+ delegate_->BytesReceived(length, bytes_downloaded_total, bytes_total_);
}
if (writer_ && !writer_->Write(bytes, length, &code_)) {
if (code_ != ErrorCode::kSuccess) {
@@ -349,13 +352,16 @@
void DownloadAction::TransferComplete(HttpFetcher* fetcher, bool successful) {
if (writer_) {
LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer.";
- writer_ = nullptr;
+ if (delta_performer_.get() == writer_) {
+ // no delta_performer_ in tests, so leave the test writer in place
+ writer_ = nullptr;
+ }
}
download_active_ = false;
ErrorCode code =
successful ? ErrorCode::kSuccess : ErrorCode::kDownloadTransferError;
- if (code == ErrorCode::kSuccess && delta_performer_.get()) {
- if (!payload_->already_applied)
+ if (code == ErrorCode::kSuccess) {
+ if (delta_performer_ && !payload_->already_applied)
code = delta_performer_->VerifyPayload(payload_->hash, payload_->size);
if (code != ErrorCode::kSuccess) {
LOG(ERROR) << "Download of " << install_plan_.download_url
@@ -365,10 +371,12 @@
CloseP2PSharingFd(true);
} else if (payload_ < &install_plan_.payloads.back() &&
system_state_->payload_state()->NextPayload()) {
+ LOG(INFO) << "Incrementing to next payload";
// No need to reset if this payload was already applied.
- if (!payload_->already_applied)
+ if (delta_performer_ && !payload_->already_applied)
DeltaPerformer::ResetUpdateProgress(prefs_, false);
// Start downloading next payload.
+ bytes_received_previous_payloads_ += payload_->size;
payload_++;
install_plan_.download_url =
system_state_->payload_state()->GetCurrentUrl();
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index d0e6000..786db20 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -166,7 +166,8 @@
// For reporting status to outsiders
DownloadActionDelegate* delegate_;
- uint64_t bytes_received_{0};
+ uint64_t bytes_received_{0}; // per file/range
+ uint64_t bytes_received_previous_payloads_{0};
uint64_t bytes_total_{0};
bool download_active_{false};
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index 7d3ac6c..f42b1d8 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -41,6 +41,7 @@
#include "update_engine/common/utils.h"
#include "update_engine/fake_p2p_manager_configuration.h"
#include "update_engine/fake_system_state.h"
+#include "update_engine/mock_file_writer.h"
#include "update_engine/payload_consumer/mock_download_action.h"
#include "update_engine/update_manager/fake_update_manager.h"
@@ -56,6 +57,7 @@
using testing::AtLeast;
using testing::InSequence;
using testing::Return;
+using testing::SetArgPointee;
using testing::_;
class DownloadActionTest : public ::testing::Test { };
@@ -242,6 +244,92 @@
false); // use_download_delegate
}
+TEST(DownloadActionTest, MultiPayloadProgressTest) {
+ std::vector<brillo::Blob> payload_datas;
+ // the first payload must be the largest, as it's the actual payload used by
+ // the MockHttpFetcher for all downloaded data.
+ payload_datas.emplace_back(4 * kMockHttpFetcherChunkSize + 256);
+ payload_datas.emplace_back(2 * kMockHttpFetcherChunkSize);
+ brillo::FakeMessageLoop loop(nullptr);
+ loop.SetAsCurrent();
+ FakeSystemState fake_system_state;
+ EXPECT_CALL(*fake_system_state.mock_payload_state(), NextPayload())
+ .WillOnce(Return(true));
+
+ MockFileWriter mock_file_writer;
+ EXPECT_CALL(mock_file_writer, Close()).WillRepeatedly(Return(0));
+ EXPECT_CALL(mock_file_writer, Write(_, _, _))
+ .WillRepeatedly(
+ DoAll(SetArgPointee<2>(ErrorCode::kSuccess), Return(true)));
+
+ InstallPlan install_plan;
+ uint64_t total_expected_download_size{0};
+ for (const auto& data : payload_datas) {
+ uint64_t size = data.size();
+ install_plan.payloads.push_back(
+ {.size = size, .type = InstallPayloadType::kFull});
+ total_expected_download_size += size;
+ }
+ ObjectFeederAction<InstallPlan> feeder_action;
+ feeder_action.set_obj(install_plan);
+ MockPrefs prefs;
+ MockHttpFetcher* http_fetcher = new MockHttpFetcher(
+ payload_datas[0].data(), payload_datas[0].size(), nullptr);
+ // takes ownership of passed in HttpFetcher
+ DownloadAction download_action(&prefs,
+ fake_system_state.boot_control(),
+ fake_system_state.hardware(),
+ &fake_system_state,
+ http_fetcher);
+ download_action.SetTestFileWriter(&mock_file_writer);
+ BondActions(&feeder_action, &download_action);
+ MockDownloadActionDelegate download_delegate;
+ {
+ InSequence s;
+ download_action.set_delegate(&download_delegate);
+ // these are hand-computed based on the payloads specified above
+ EXPECT_CALL(download_delegate,
+ BytesReceived(kMockHttpFetcherChunkSize,
+ kMockHttpFetcherChunkSize,
+ total_expected_download_size));
+ EXPECT_CALL(download_delegate,
+ BytesReceived(kMockHttpFetcherChunkSize,
+ kMockHttpFetcherChunkSize * 2,
+ total_expected_download_size));
+ EXPECT_CALL(download_delegate,
+ BytesReceived(kMockHttpFetcherChunkSize,
+ kMockHttpFetcherChunkSize * 3,
+ total_expected_download_size));
+ EXPECT_CALL(download_delegate,
+ BytesReceived(kMockHttpFetcherChunkSize,
+ kMockHttpFetcherChunkSize * 4,
+ total_expected_download_size));
+ EXPECT_CALL(download_delegate,
+ BytesReceived(256,
+ kMockHttpFetcherChunkSize * 4 + 256,
+ total_expected_download_size));
+ EXPECT_CALL(download_delegate,
+ BytesReceived(kMockHttpFetcherChunkSize,
+ kMockHttpFetcherChunkSize * 5 + 256,
+ total_expected_download_size));
+ EXPECT_CALL(download_delegate,
+ BytesReceived(kMockHttpFetcherChunkSize,
+ total_expected_download_size,
+ total_expected_download_size));
+ }
+ ActionProcessor processor;
+ processor.EnqueueAction(&feeder_action);
+ processor.EnqueueAction(&download_action);
+
+ loop.PostTask(
+ FROM_HERE,
+ base::Bind(
+ [](ActionProcessor* processor) { processor->StartProcessing(); },
+ base::Unretained(&processor)));
+ loop.Run();
+ EXPECT_FALSE(loop.PendingTasks());
+}
+
namespace {
class TerminateEarlyTestProcessorDelegate : public ActionProcessorDelegate {
public:
diff --git a/update_attempter.h b/update_attempter.h
index b4e2f60..9dd844d 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -252,17 +252,20 @@
FRIEND_TEST(UpdateAttempterTest, ActionCompletedDownloadTest);
FRIEND_TEST(UpdateAttempterTest, ActionCompletedErrorTest);
FRIEND_TEST(UpdateAttempterTest, ActionCompletedOmahaRequestTest);
+ FRIEND_TEST(UpdateAttempterTest, BootTimeInUpdateMarkerFile);
+ FRIEND_TEST(UpdateAttempterTest, BroadcastCompleteDownloadTest);
+ FRIEND_TEST(UpdateAttempterTest, ChangeToDownloadingOnReceivedBytesTest);
FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventTest);
FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventResumedTest);
FRIEND_TEST(UpdateAttempterTest, DisableDeltaUpdateIfNeededTest);
+ FRIEND_TEST(UpdateAttempterTest, DownloadProgressAccumulationTest);
FRIEND_TEST(UpdateAttempterTest, MarkDeltaUpdateFailureTest);
FRIEND_TEST(UpdateAttempterTest, PingOmahaTest);
+ FRIEND_TEST(UpdateAttempterTest, ReportDailyMetrics);
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest);
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionTest);
- FRIEND_TEST(UpdateAttempterTest, UpdateTest);
- FRIEND_TEST(UpdateAttempterTest, ReportDailyMetrics);
- FRIEND_TEST(UpdateAttempterTest, BootTimeInUpdateMarkerFile);
FRIEND_TEST(UpdateAttempterTest, TargetVersionPrefixSetAndReset);
+ FRIEND_TEST(UpdateAttempterTest, UpdateTest);
// CertificateChecker::Observer method.
// Report metrics about the certificate being checked.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 4ebc85c..911f664 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -48,6 +48,7 @@
#include "update_engine/fake_system_state.h"
#include "update_engine/mock_p2p_manager.h"
#include "update_engine/mock_payload_state.h"
+#include "update_engine/mock_service_observer.h"
#include "update_engine/payload_consumer/filesystem_verifier_action.h"
#include "update_engine/payload_consumer/install_plan.h"
#include "update_engine/payload_consumer/payload_constants.h"
@@ -219,6 +220,7 @@
EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(nullptr, &action, ErrorCode::kSuccess);
EXPECT_EQ(UpdateStatus::FINALIZING, attempter_.status());
+ EXPECT_EQ(0.0, attempter_.download_progress_);
ASSERT_EQ(nullptr, attempter_.error_event_.get());
}
@@ -232,6 +234,79 @@
ASSERT_NE(nullptr, attempter_.error_event_.get());
}
+TEST_F(UpdateAttempterTest, DownloadProgressAccumulationTest) {
+ // Simple test case, where all the values match (nothing was skipped)
+ uint64_t bytes_progressed_1 = 1024 * 1024; // 1MB
+ uint64_t bytes_progressed_2 = 1024 * 1024; // 1MB
+ uint64_t bytes_received_1 = bytes_progressed_1;
+ uint64_t bytes_received_2 = bytes_received_1 + bytes_progressed_2;
+ uint64_t bytes_total = 20 * 1024 * 1024; // 20MB
+
+ double progress_1 =
+ static_cast<double>(bytes_received_1) / static_cast<double>(bytes_total);
+ double progress_2 =
+ static_cast<double>(bytes_received_2) / static_cast<double>(bytes_total);
+
+ EXPECT_EQ(0.0, attempter_.download_progress_);
+ // This is set via inspecting the InstallPlan payloads when the
+ // OmahaResponseAction is completed
+ attempter_.new_payload_size_ = bytes_total;
+ NiceMock<MockServiceObserver> observer;
+ EXPECT_CALL(observer,
+ SendStatusUpdate(
+ _, progress_1, UpdateStatus::DOWNLOADING, _, bytes_total));
+ EXPECT_CALL(observer,
+ SendStatusUpdate(
+ _, progress_2, UpdateStatus::DOWNLOADING, _, bytes_total));
+ attempter_.AddObserver(&observer);
+ attempter_.BytesReceived(bytes_progressed_1, bytes_received_1, bytes_total);
+ EXPECT_EQ(progress_1, attempter_.download_progress_);
+ // This iteration validates that a later set of updates to the variables are
+ // properly handled (so that |getStatus()| will return the same progress info
+ // as the callback is receiving.
+ attempter_.BytesReceived(bytes_progressed_2, bytes_received_2, bytes_total);
+ EXPECT_EQ(progress_2, attempter_.download_progress_);
+}
+
+TEST_F(UpdateAttempterTest, ChangeToDownloadingOnReceivedBytesTest) {
+ // The transition into UpdateStatus::DOWNLOADING happens when the
+ // first bytes are received.
+ uint64_t bytes_progressed = 1024 * 1024; // 1MB
+ uint64_t bytes_received = 2 * 1024 * 1024; // 2MB
+ uint64_t bytes_total = 20 * 1024 * 1024; // 300MB
+ attempter_.status_ = UpdateStatus::CHECKING_FOR_UPDATE;
+ // This is set via inspecting the InstallPlan payloads when the
+ // OmahaResponseAction is completed
+ attempter_.new_payload_size_ = bytes_total;
+ EXPECT_EQ(0.0, attempter_.download_progress_);
+ NiceMock<MockServiceObserver> observer;
+ EXPECT_CALL(
+ observer,
+ SendStatusUpdate(_, _, UpdateStatus::DOWNLOADING, _, bytes_total));
+ attempter_.AddObserver(&observer);
+ attempter_.BytesReceived(bytes_progressed, bytes_received, bytes_total);
+ EXPECT_EQ(UpdateStatus::DOWNLOADING, attempter_.status_);
+}
+
+TEST_F(UpdateAttempterTest, BroadcastCompleteDownloadTest) {
+ // There is a special case to ensure that at 100% downloaded,
+ // download_progress_ is updated and that value broadcast. This test confirms
+ // that.
+ uint64_t bytes_progressed = 0; // ignored
+ uint64_t bytes_received = 5 * 1024 * 1024; // ignored
+ uint64_t bytes_total = 5 * 1024 * 1024; // 300MB
+ attempter_.status_ = UpdateStatus::DOWNLOADING;
+ attempter_.new_payload_size_ = bytes_total;
+ EXPECT_EQ(0.0, attempter_.download_progress_);
+ NiceMock<MockServiceObserver> observer;
+ EXPECT_CALL(
+ observer,
+ SendStatusUpdate(_, 1.0, UpdateStatus::DOWNLOADING, _, bytes_total));
+ attempter_.AddObserver(&observer);
+ attempter_.BytesReceived(bytes_progressed, bytes_received, bytes_total);
+ EXPECT_EQ(1.0, attempter_.download_progress_);
+}
+
TEST_F(UpdateAttempterTest, ActionCompletedOmahaRequestTest) {
unique_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, nullptr));
fetcher->FailTransfer(500); // Sets the HTTP response code.