AU: Optimize checkpointing a bit and decide on new update vs. resume.

BUG=7390
TEST=unit tests, gmerged on device

Change-Id: Ibed6082fe697e6b28b03fb1cc39d700826bf2bfe

Review URL: http://codereview.chromium.org/3541016
diff --git a/delta_performer.cc b/delta_performer.cc
index 6143ec5..92d4733 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -222,7 +222,7 @@
     // that if the operation gets interrupted, we don't try to resume the
     // update.
     if (!IsIdempotentOperation(op)) {
-      ResetUpdateProgress();
+      ResetUpdateProgress(prefs_);
     }
     if (op.type() == DeltaArchiveManifest_InstallOperation_Type_REPLACE ||
         op.type() == DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
@@ -512,19 +512,57 @@
   buffer_.erase(buffer_.begin(), buffer_.begin() + count);
 }
 
-bool DeltaPerformer::ResetUpdateProgress() {
-  TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextOperation,
-                                         kUpdateStateOperationInvalid));
+bool DeltaPerformer::CanResumeUpdate(PrefsInterface* prefs,
+                                     string update_check_response_hash) {
+  int64_t next_operation = kUpdateStateOperationInvalid;
+  TEST_AND_RETURN_FALSE(prefs->GetInt64(kPrefsUpdateStateNextOperation,
+                                        &next_operation) &&
+                        next_operation != kUpdateStateOperationInvalid &&
+                        next_operation > 0);
+
+  string interrupted_hash;
+  TEST_AND_RETURN_FALSE(prefs->GetString(kPrefsUpdateCheckResponseHash,
+                                         &interrupted_hash) &&
+                        !interrupted_hash.empty() &&
+                        interrupted_hash == update_check_response_hash);
+
+  // Sanity check the rest.
+  int64_t next_data_offset = -1;
+  TEST_AND_RETURN_FALSE(prefs->GetInt64(kPrefsUpdateStateNextDataOffset,
+                                        &next_data_offset) &&
+                        next_data_offset >= 0);
+
+  string signed_sha256_context;
+  TEST_AND_RETURN_FALSE(
+      prefs->GetString(kPrefsUpdateStateSignedSHA256Context,
+                       &signed_sha256_context) &&
+      !signed_sha256_context.empty());
+
+  int64_t manifest_metadata_size = 0;
+  TEST_AND_RETURN_FALSE(prefs->GetInt64(kPrefsManifestMetadataSize,
+                                        &manifest_metadata_size) &&
+                        manifest_metadata_size > 0);
+
+  return true;
+}
+
+bool DeltaPerformer::ResetUpdateProgress(PrefsInterface* prefs) {
+  TEST_AND_RETURN_FALSE(prefs->SetInt64(kPrefsUpdateStateNextOperation,
+                                        kUpdateStateOperationInvalid));
   return true;
 }
 
 bool DeltaPerformer::CheckpointUpdateProgress() {
   // First reset the progress in case we die in the middle of the state update.
-  ResetUpdateProgress();
-  TEST_AND_RETURN_FALSE(prefs_->SetString(kPrefsUpdateStateSignedSHA256Context,
-                                          hash_calculator_.GetContext()));
-  TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextDataOffset,
-                                         buffer_offset_));
+  ResetUpdateProgress(prefs_);
+  if (last_updated_buffer_offset_ != buffer_offset_) {
+    TEST_AND_RETURN_FALSE(
+        prefs_->SetString(kPrefsUpdateStateSignedSHA256Context,
+                          hash_calculator_.GetContext()));
+    TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextDataOffset,
+                                           buffer_offset_));
+    last_updated_buffer_offset_ = buffer_offset_;
+  }
   TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextOperation,
                                          next_operation_num_));
   return true;
diff --git a/delta_performer.h b/delta_performer.h
index 2f9fd8a..87a27dd 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -31,6 +31,7 @@
         manifest_valid_(false),
         next_operation_num_(0),
         buffer_offset_(0),
+        last_updated_buffer_offset_(kuint64max),
         block_size_(0) {}
 
   // Opens the kernel. Should be called before or after Open(), but before
@@ -72,6 +73,15 @@
       uint64_t full_length,
       std::string* positions_string);
 
+  // Returns true if a previous update attempt can be continued based on the
+  // persistent preferences and the new update check response hash.
+  static bool CanResumeUpdate(PrefsInterface* prefs,
+                              std::string update_check_response_hash);
+
+  // Resets the persistent update progress state to indicate that an update
+  // can't be resumed. Returns true on success, false otherwise.
+  static bool ResetUpdateProgress(PrefsInterface* prefs);
+
  private:
   // Returns true if enough of the delta file has been passed via Write()
   // to be able to perform a given install operation.
@@ -102,7 +112,8 @@
   // updates the hash calculator with these bytes before discarding them.
   void DiscardBufferHeadBytes(size_t count, bool do_hash);
 
-  bool ResetUpdateProgress();
+  // Checkpoints the update progress into persistent storage to allow this
+  // update attempt to be resumed after reboot.
   bool CheckpointUpdateProgress();
 
   // Update Engine preference store.
@@ -131,6 +142,9 @@
   // Offset of buffer_ in the binary blobs section of the update.
   uint64_t buffer_offset_;
 
+  // Last |buffer_offset_| value updated as part of the progress update.
+  uint64_t last_updated_buffer_offset_;
+
   // The block size (parsed from the manifest).
   uint32_t block_size_;
 
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 3e8aabd..7571fb4 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -102,6 +102,7 @@
       OmahaHashCalculator::OmahaHashOfData(data);
   uint64_t size = data.size() + (size_test ? 1 : 0);
   InstallPlan install_plan(true,
+                           false,
                            "",
                            size,
                            hash,
@@ -227,7 +228,7 @@
 
     // takes ownership of passed in HttpFetcher
     ObjectFeederAction<InstallPlan> feeder_action;
-    InstallPlan install_plan(true, "", 0, "", temp_file.GetPath(), "");
+    InstallPlan install_plan(true, false, "", 0, "", temp_file.GetPath(), "");
     feeder_action.set_obj(install_plan);
     PrefsMock prefs;
     DownloadAction download_action(&prefs,
@@ -326,6 +327,7 @@
 
   // takes ownership of passed in HttpFetcher
   InstallPlan install_plan(true,
+                           false,
                            "",
                            1,
                            OmahaHashCalculator::OmahaHashOfString("x"),
@@ -364,7 +366,7 @@
   DirectFileWriter writer;
 
   // takes ownership of passed in HttpFetcher
-  InstallPlan install_plan(true, "", 0, "", path, "");
+  InstallPlan install_plan(true, false, "", 0, "", path, "");
   ObjectFeederAction<InstallPlan> feeder_action;
   feeder_action.set_obj(install_plan);
   PrefsMock prefs;
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index 03aa86a..f82097e 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -231,7 +231,7 @@
 
   ObjectFeederAction<InstallPlan> feeder_action;
   const char* kUrl = "http://some/url";
-  InstallPlan install_plan(true, kUrl, 0, "", "", "");
+  InstallPlan install_plan(true, false, kUrl, 0, "", "", "");
   feeder_action.set_obj(install_plan);
   FilesystemCopierAction copier_action(false);
   ObjectCollectorAction<InstallPlan> collector_action;
@@ -256,7 +256,13 @@
   processor.set_delegate(&delegate);
 
   ObjectFeederAction<InstallPlan> feeder_action;
-  InstallPlan install_plan(false, "", 0, "", "/no/such/file", "/no/such/file");
+  InstallPlan install_plan(false,
+                           false,
+                           "",
+                           0,
+                           "",
+                           "/no/such/file",
+                           "/no/such/file");
   feeder_action.set_obj(install_plan);
   FilesystemCopierAction copier_action(false);
   ObjectCollectorAction<InstallPlan> collector_action;
diff --git a/install_plan.h b/install_plan.h
index 19fee8a..8dc42aa 100644
--- a/install_plan.h
+++ b/install_plan.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -15,20 +15,23 @@
 
 struct InstallPlan {
   InstallPlan(bool is_full,
+              bool is_resume,
               const std::string& url,
               uint64_t size,
               const std::string& hash,
               const std::string& install_path,
               const std::string& kernel_install_path)
       : is_full_update(is_full),
+        is_resume(is_resume),
         download_url(url),
         size(size),
         download_hash(hash),
         install_path(install_path),
         kernel_install_path(kernel_install_path) {}
-  InstallPlan() : is_full_update(false) {}
+  InstallPlan() : is_full_update(false), is_resume(false), size(0) {}
 
   bool is_full_update;
+  bool is_resume;
   std::string download_url;  // url to download from
   uint64_t size;  // size of the download url's data
   std::string download_hash;  // hash of the data at the url
@@ -37,11 +40,12 @@
 
   bool operator==(const InstallPlan& that) const {
     return (is_full_update == that.is_full_update) &&
-           (download_url == that.download_url) &&
-           (size == that.size) &&
-           (download_hash == that.download_hash) &&
-           (install_path == that.install_path) &&
-           (kernel_install_path == that.kernel_install_path);
+        (is_resume == that.is_resume) &&
+        (download_url == that.download_url) &&
+        (size == that.size) &&
+        (download_hash == that.download_hash) &&
+        (install_path == that.install_path) &&
+        (kernel_install_path == that.kernel_install_path);
   }
   bool operator!=(const InstallPlan& that) const {
     return !((*this) == that);
@@ -49,6 +53,7 @@
   void Dump() const {
     LOG(INFO) << "InstallPlan: "
               << (is_full_update ? "full_update" : "delta_update")
+              << (is_resume ? ", resume" : ", new_update")
               << ", url: " << download_url
               << ", size: " << size
               << ", hash: " << download_hash
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index cdcf735..d3142a7 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -8,6 +8,7 @@
 
 #include <base/logging.h>
 
+#include "update_engine/delta_performer.h"
 #include "update_engine/prefs_interface.h"
 #include "update_engine/utils.h"
 
@@ -27,12 +28,17 @@
   install_plan_.download_url = response.codebase;
   install_plan_.size = response.size;
   install_plan_.download_hash = response.hash;
-  // TODO(petkov): Decide here if this is going to be a regular update or
-  // resume-after-boot. This should also set the number of ops performed so far
-  // to invalid if no need to resume.
-  LOG_IF(WARNING, !prefs_->SetString(kPrefsUpdateCheckResponseHash,
-                                     response.hash))
-      << "Unable to save the update check response hash.";
+
+  install_plan_.is_resume =
+      DeltaPerformer::CanResumeUpdate(prefs_, response.hash);
+  if (!install_plan_.is_resume) {
+    LOG_IF(WARNING, !DeltaPerformer::ResetUpdateProgress(prefs_))
+        << "Unable to reset the update progress.";
+    LOG_IF(WARNING, !prefs_->SetString(kPrefsUpdateCheckResponseHash,
+                                       response.hash))
+        << "Unable to save the update check response hash.";
+  }
+
   TEST_AND_RETURN(GetInstallDev(
       (!boot_device_.empty() ? boot_device_ : utils::BootDevice()),
       &install_plan_.install_path));