update_engine: Revert caching the update payload manifest
This CL basically revert this CL:
https://android-review.googlesource.com/c/platform/system/update_engine/+/1360759
The purpose of that CL was to cache the payload manifest in the
pref (wrong place) to read it next time the update is being
resumed. However, there seems to be some problems with it. For example
it is disregarding the p2p updates and is messing it up. The code seems
to be wrong (it weirdly creates a DeltaPerformer again). And it doesn't
have any unittests so we can't accept it in CrOS. This won't revert the
changes in the DownloadAction that Android uses, so it doesn't have any
effect on aosp.
BUG=b:171829801
TEST=cros_workon_make --board reef --test update_engine
Change-Id: Id1ed36a336970ffd119385dbe476f6c64103b407
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2776258
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Vyshu Khota <vyshu@google.com>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/cros/download_action_chromeos.cc b/cros/download_action_chromeos.cc
index 814c208..ea84f2c 100644
--- a/cros/download_action_chromeos.cc
+++ b/cros/download_action_chromeos.cc
@@ -205,76 +205,19 @@
StartDownloading();
}
-bool DownloadActionChromeos::LoadCachedManifest(int64_t manifest_size) {
- std::string cached_manifest_bytes;
- if (!prefs_->GetString(kPrefsManifestBytes, &cached_manifest_bytes) ||
- cached_manifest_bytes.size() <= 0) {
- LOG(INFO) << "Cached Manifest data not found";
- return false;
- }
- if (static_cast<int64_t>(cached_manifest_bytes.size()) != manifest_size) {
- LOG(WARNING) << "Cached metadata has unexpected size: "
- << cached_manifest_bytes.size() << " vs. " << manifest_size;
- return false;
- }
-
- ErrorCode error;
- const bool success =
- delta_performer_->Write(
- cached_manifest_bytes.data(), cached_manifest_bytes.size(), &error) &&
- delta_performer_->IsManifestValid();
- if (success) {
- LOG(INFO) << "Successfully parsed cached manifest";
- } else {
- // If parsing of cached data failed, fall back to fetch them using HTTP
- LOG(WARNING) << "Cached manifest data fails to load, error code:"
- << static_cast<int>(error) << "," << error;
- }
- return success;
-}
-
void DownloadActionChromeos::StartDownloading() {
download_active_ = true;
http_fetcher_->ClearRanges();
- if (writer_ && writer_ != delta_performer_.get()) {
- LOG(INFO) << "Using writer for test.";
- } else {
- delta_performer_.reset(new DeltaPerformer(prefs_,
- boot_control_,
- hardware_,
- delegate_,
- &install_plan_,
- payload_,
- interactive_));
- writer_ = delta_performer_.get();
- }
-
if (install_plan_.is_resume &&
payload_ == &install_plan_.payloads[resume_payload_index_]) {
- // Resuming an update so parse the cached manifest first
+ // Resuming an update so fetch the update manifest metadata first.
int64_t manifest_metadata_size = 0;
int64_t manifest_signature_size = 0;
prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
-
- // TODO(zhangkelvin) Add unittest for success and fallback route
- if (!LoadCachedManifest(manifest_metadata_size + manifest_signature_size)) {
- if (delta_performer_) {
- // Create a new DeltaPerformer to reset all its state
- delta_performer_ = std::make_unique<DeltaPerformer>(prefs_,
- boot_control_,
- hardware_,
- delegate_,
- &install_plan_,
- payload_,
- interactive_);
- writer_ = delta_performer_.get();
- }
- http_fetcher_->AddRange(base_offset_,
- manifest_metadata_size + manifest_signature_size);
- }
-
+ http_fetcher_->AddRange(base_offset_,
+ manifest_metadata_size + manifest_signature_size);
// If there're remaining unprocessed data blobs, fetch them. Be careful not
// to request data beyond the end of the payload to avoid 416 HTTP response
// error codes.
@@ -298,6 +241,19 @@
}
}
+ if (writer_ && writer_ != delta_performer_.get()) {
+ LOG(INFO) << "Using writer for test.";
+ } else {
+ delta_performer_.reset(new DeltaPerformer(prefs_,
+ boot_control_,
+ hardware_,
+ delegate_,
+ &install_plan_,
+ payload_,
+ interactive_));
+ writer_ = delta_performer_.get();
+ }
+
if (SystemState::Get() != nullptr) {
const PayloadStateInterface* payload_state =
SystemState::Get()->payload_state();
diff --git a/cros/download_action_chromeos.h b/cros/download_action_chromeos.h
index 9e8760c..7e27131 100644
--- a/cros/download_action_chromeos.h
+++ b/cros/download_action_chromeos.h
@@ -113,10 +113,6 @@
// called or if CloseP2PSharingFd() has been called.
void WriteToP2PFile(const void* data, size_t length, off_t file_offset);
- // Attempt to load cached manifest data from prefs
- // return true on success, false otherwise.
- bool LoadCachedManifest(int64_t manifest_size);
-
// Start downloading the current payload using delta_performer.
void StartDownloading();
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 015d00c..3f35835 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -577,10 +577,11 @@
if ((*error = ValidateManifest()) != ErrorCode::kSuccess)
return false;
manifest_valid_ = true;
+#ifndef __CHROMEOS__
if (!install_plan_->is_resume) {
prefs_->SetString(kPrefsManifestBytes, {buffer_.begin(), buffer_.end()});
}
-
+#endif // __CHROMEOS__
// Clear the download buffer.
DiscardBuffer(false, metadata_size_);
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index 91fbe60..f5be64b 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -28,7 +28,6 @@
#include <base/stl_util.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
-#include <gmock/gmock-matchers.h>
#include <google/protobuf/repeated_field.h>
#include <gtest/gtest.h>
#include <openssl/pem.h>