Blacklist versions as part of Rollback along with unittests.

This CL adds version blacklisting as part of AU Rollback. A few additional
things:

1) Since this pref must persist across rollback I have introduced a
powerwash_safe_prefs as part of system_state that will persist across
powerwashes.
2) Fixed bug where we needed to read the device policy (which is read during an
update_check before Rollback would work).
3) Some refactoring to move pref constants to constants.
4) Passing keepimg into our powerwash command so we don't wipe the old
partitions.

BUG=chromium:252589 chromium:254217
TEST=Unittests + test on device + using rollback with and without powerwash
checking preserve state.

Change-Id: I991fad944594944425fd9941e10b30a919f2b83b
Reviewed-on: https://gerrit.chromium.org/gerrit/59518
Reviewed-by: Chris Sosa <sosa@chromium.org>
Tested-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: Chris Sosa <sosa@chromium.org>
diff --git a/constants.cc b/constants.cc
index e4aa199..7f14ddb 100644
--- a/constants.cc
+++ b/constants.cc
@@ -7,13 +7,19 @@
 namespace chromeos_update_engine {
 
 const char kPowerwashMarkerFile[] =
-  "/mnt/stateful_partition/factory_install_reset";
+    "/mnt/stateful_partition/factory_install_reset";
+
+const char kPowerwashCommand[] = "safe fast keepimg\n";
+
+const char kPowerwashSafePrefsDir[] =
+    "/mnt/stateful_partition/unencrypted/preserve/update_engine/prefs";
+
+const char kPrefsDirectory[] = "/var/lib/update_engine/prefs";
+
+const char kStatefulPartition[] = "/mnt/stateful_partition";
 
 const char kSystemRebootedMarkerFile[] = "/tmp/update_engine_update_recorded";
 
-const char kPowerwashCommand[] = "safe fast\n";
-
-const char kStatefulPartition[] = "/mnt/stateful_partition";
 
 // Constants defining keys for the persisted state of update engine.
 const char kPrefsBackoffExpiryTime[] = "backoff-expiry-time";
@@ -34,6 +40,7 @@
 const char kPrefsPayloadAttemptNumber[] = "payload-attempt-number";
 const char kPrefsPreviousVersion[] = "previous-version";
 const char kPrefsResumedUpdateFailures[] = "resumed-update-failures";
+const char kPrefsRollbackVersion[] = "rollback-version";
 const char kPrefsSystemUpdatedMarker[] = "system-updated-marker";
 const char kPrefsTotalBytesDownloaded[] = "total-bytes-downloaded";
 const char kPrefsUpdateCheckCount[] = "update-check-count";
diff --git a/constants.h b/constants.h
index f2dfc5a..dc39d5a 100644
--- a/constants.h
+++ b/constants.h
@@ -17,6 +17,12 @@
 // The contents of the powerwash marker file.
 extern const char kPowerwashCommand[];
 
+// Directory for AU prefs that are preserved across powerwash.
+extern const char kPowerwashSafePrefsDir[];
+
+// The location where we store the AU preferences (state etc).
+extern const char kPrefsDirectory[];
+
 // Path to the stateful partition on the root filesystem.
 extern const char kStatefulPartition[];
 
@@ -37,6 +43,7 @@
 extern const char kPrefsPayloadAttemptNumber[];
 extern const char kPrefsPreviousVersion[];
 extern const char kPrefsResumedUpdateFailures[];
+extern const char kPrefsRollbackVersion[];
 extern const char kPrefsSystemUpdatedMarker[];
 extern const char kPrefsTotalBytesDownloaded[];
 extern const char kPrefsUpdateCheckCount[];
diff --git a/dbus_service.cc b/dbus_service.cc
index 0a284fe..e75b2b2 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -105,8 +105,7 @@
                                                 bool powerwash,
                                                 GError **error) {
   LOG(INFO) << "Attempting rollback to non-active partitions.";
-  self->system_state_->update_attempter()->Rollback(powerwash);
-  return TRUE;
+  return self->system_state_->update_attempter()->Rollback(powerwash);
 }
 
 gboolean update_engine_service_reset_status(UpdateEngineService* self,
diff --git a/mock_payload_state.h b/mock_payload_state.h
index 3dcf03c..8959ca1 100644
--- a/mock_payload_state.h
+++ b/mock_payload_state.h
@@ -27,6 +27,7 @@
   MOCK_METHOD1(UpdateFailed, void(ErrorCode error));
   MOCK_METHOD0(ShouldBackoffDownload, bool());
   MOCK_METHOD0(UpdateEngineStarted, void());
+  MOCK_METHOD0(Rollback, void());
 
   // Getters.
   MOCK_METHOD0(GetResponseSignature, std::string());
@@ -41,6 +42,7 @@
   MOCK_METHOD1(GetCurrentBytesDownloaded, uint64_t(DownloadSource source));
   MOCK_METHOD1(GetTotalBytesDownloaded, uint64_t(DownloadSource source));
   MOCK_METHOD0(GetNumReboots, uint32_t());
+  MOCK_METHOD0(GetRollbackVersion, std::string());
 };
 
 }  // namespace chromeos_update_engine
diff --git a/mock_system_state.cc b/mock_system_state.cc
index 152347a..d76de05 100644
--- a/mock_system_state.cc
+++ b/mock_system_state.cc
@@ -11,7 +11,8 @@
 // OOBE is completed even when there's no such marker file, etc.
 MockSystemState::MockSystemState()
   : default_request_params_(this),
-    prefs_(&mock_prefs_) {
+    prefs_(&mock_prefs_),
+    powerwash_safe_prefs_(&mock_powerwash_safe_prefs_) {
   clock_ = &default_clock_;
   request_params_ = &default_request_params_;
   mock_payload_state_.Initialize(this);
diff --git a/mock_system_state.h b/mock_system_state.h
index e03546e..8b04f7e 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -52,6 +52,10 @@
     return prefs_;
   }
 
+  inline virtual PrefsInterface* powerwash_safe_prefs() {
+    return powerwash_safe_prefs_;
+  }
+
   inline virtual PayloadStateInterface* payload_state() {
     return &mock_payload_state_;
   }
@@ -83,10 +87,18 @@
     prefs_ = prefs;
   }
 
+  inline void set_powerwash_safe_prefs(PrefsInterface* prefs) {
+    powerwash_safe_prefs_ = prefs;
+  }
+
   inline testing::NiceMock<PrefsMock> *mock_prefs() {
     return &mock_prefs_;
   }
 
+  inline testing::NiceMock<PrefsMock> *mock_powerwash_safe_prefs() {
+    return &mock_powerwash_safe_prefs_;
+  }
+
   inline MockPayloadState* mock_payload_state() {
     return &mock_payload_state_;
   }
@@ -99,6 +111,7 @@
   // These are Mock objects or objects we own.
   testing::NiceMock<MetricsLibraryMock> mock_metrics_lib_;
   testing::NiceMock<PrefsMock> mock_prefs_;
+  testing::NiceMock<PrefsMock> mock_powerwash_safe_prefs_;
   testing::NiceMock<MockPayloadState> mock_payload_state_;
   testing::NiceMock<MockGpioHandler>* mock_gpio_handler_;
   testing::NiceMock<UpdateAttempterMock>* mock_update_attempter_;
@@ -111,6 +124,7 @@
   // These are pointers to objects which caller can override.
   ClockInterface* clock_;
   PrefsInterface* prefs_;
+  PrefsInterface* powerwash_safe_prefs_;
   ConnectionManager* connection_manager_;
   OmahaRequestParams* request_params_;
 };
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 6a6f87a..2c45c7d 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -636,7 +636,7 @@
   // Set the version.
   output_object->version = XmlGetProperty(manifest_node, kTagVersion);
   if (output_object->version.empty()) {
-    LOG(ERROR) << "Omaha Response has manifest version";
+    LOG(ERROR) << "Omaha Response does not have version in manifest!";
     completer->set_code(kErrorCodeOmahaResponseInvalid);
     return false;
   }
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 358988c..21832c5 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -38,6 +38,14 @@
     return;
   }
 
+  // Note: policy decision to not update to a version we rolled back from.
+  string rollback_version =
+      system_state_->payload_state()->GetRollbackVersion();
+  if (!rollback_version.empty() && rollback_version == response.version) {
+    LOG(INFO) << "Received version that we rolled back from. Aborting.";
+    return;
+  }
+
   // All decisions as to which URL should be used have already been done. So,
   // make the current URL as the download URL.
   string current_url = system_state_->payload_state()->GetCurrentUrl();
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 3516cd6..f3b4b99 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -59,6 +59,7 @@
     "very_long_name_and_no_slashes-very_long_name_and_no_slashes"
     "very_long_name_and_no_slashes-very_long_name_and_no_slashes"
     "-the_update_a.b.c.d_DELTA_.tgz";
+const string kBadVersion = "don't update me";
 }  // namespace {}
 
 bool OmahaResponseHandlerActionTest::DoTestCommon(
@@ -72,7 +73,7 @@
 
   ObjectFeederAction<OmahaResponse> feeder_action;
   feeder_action.set_obj(in);
-  if (in.update_exists) {
+  if (in.update_exists and in.version != kBadVersion) {
     EXPECT_CALL(*(mock_system_state->mock_prefs()),
                 SetString(kPrefsUpdateCheckResponseHash, in.hash))
         .WillOnce(Return(true));
@@ -81,6 +82,8 @@
   string current_url = in.payload_urls.size() ? in.payload_urls[0] : "";
   EXPECT_CALL(*(mock_system_state->mock_payload_state()), GetCurrentUrl())
       .WillRepeatedly(Return(current_url));
+  EXPECT_CALL(*(mock_system_state->mock_payload_state()), GetRollbackVersion())
+        .WillRepeatedly(Return(kBadVersion));
 
   OmahaResponseHandlerAction response_handler_action(mock_system_state);
   response_handler_action.set_boot_device(boot_dev);
@@ -187,6 +190,31 @@
   EXPECT_EQ("", install_plan.install_path);
 }
 
+TEST_F(OmahaResponseHandlerActionTest, RollbackVersionTest) {
+  string version_ok = "124.0.0";
+
+  InstallPlan install_plan;
+  OmahaResponse in;
+  in.update_exists = true;
+  in.version = kBadVersion;
+  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;
+  in.prompt = true;
+
+  // Version is blacklisted for first call so no update.
+  EXPECT_FALSE(DoTest(in, "/dev/sda5", &install_plan));
+  EXPECT_EQ("", install_plan.download_url);
+  EXPECT_EQ("", install_plan.payload_hash);
+  EXPECT_EQ("", install_plan.install_path);
+
+  // Version isn't blacklisted.
+  in.version = version_ok;
+  EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+  EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
+}
+
 TEST_F(OmahaResponseHandlerActionTest, HashChecksForHttpTest) {
   OmahaResponse in;
   in.update_exists = true;
diff --git a/payload_state.cc b/payload_state.cc
index 1823ce7..20e2cac 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -44,6 +44,7 @@
 bool PayloadState::Initialize(SystemState* system_state) {
   system_state_ = system_state;
   prefs_ = system_state_->prefs();
+  powerwash_safe_prefs_ = system_state_->powerwash_safe_prefs();
   LoadResponseSignature();
   LoadPayloadAttemptNumber();
   LoadUrlIndex();
@@ -61,6 +62,7 @@
   }
   LoadNumReboots();
   LoadNumResponsesSeen();
+  LoadRollbackVersion();
   return true;
 }
 
@@ -310,6 +312,10 @@
   return true;
 }
 
+void PayloadState::Rollback() {
+  SetRollbackVersion(system_state_->request_params()->app_version());
+}
+
 void PayloadState::IncrementPayloadAttemptNumber() {
   if (response_.is_delta_payload) {
     LOG(INFO) << "Not incrementing payload attempt number for delta payloads";
@@ -544,6 +550,13 @@
   SetUpdateTimestampEnd(Time()); // Set to null time
   SetUpdateDurationUptime(TimeDelta::FromSeconds(0));
   ResetDownloadSourcesOnNewUpdate();
+  ResetRollbackVersion();
+}
+
+void PayloadState::ResetRollbackVersion() {
+  CHECK(powerwash_safe_prefs_);
+  rollback_version_ = "";
+  powerwash_safe_prefs_->Delete(kPrefsRollbackVersion);
 }
 
 void PayloadState::ResetDownloadSourcesOnNewUpdate() {
@@ -556,13 +569,18 @@
   }
 }
 
-int64_t PayloadState::GetPersistedValue(const string& key) {
+int64_t PayloadState::GetPersistedValue(const string& key,
+                                        bool across_powerwash) {
+  PrefsInterface* prefs = prefs_;
+  if (across_powerwash)
+    prefs = powerwash_safe_prefs_;
+
   CHECK(prefs_);
-  if (!prefs_->Exists(key))
+  if (!prefs->Exists(key))
     return 0;
 
   int64_t stored_value;
-  if (!prefs_->GetInt64(key, &stored_value))
+  if (!prefs->GetInt64(key, &stored_value))
     return 0;
 
   if (stored_value < 0) {
@@ -616,7 +634,8 @@
 }
 
 void PayloadState::LoadPayloadAttemptNumber() {
-  SetPayloadAttemptNumber(GetPersistedValue(kPrefsPayloadAttemptNumber));
+  SetPayloadAttemptNumber(GetPersistedValue(kPrefsPayloadAttemptNumber,
+                                            false));
 }
 
 void PayloadState::SetPayloadAttemptNumber(uint32_t payload_attempt_number) {
@@ -627,7 +646,7 @@
 }
 
 void PayloadState::LoadUrlIndex() {
-  SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex));
+  SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex, false));
 }
 
 void PayloadState::SetUrlIndex(uint32_t url_index) {
@@ -642,7 +661,7 @@
 }
 
 void PayloadState::LoadUrlSwitchCount() {
-  SetUrlSwitchCount(GetPersistedValue(kPrefsUrlSwitchCount));
+  SetUrlSwitchCount(GetPersistedValue(kPrefsUrlSwitchCount, false));
 }
 
 void PayloadState::SetUrlSwitchCount(uint32_t url_switch_count) {
@@ -653,7 +672,8 @@
 }
 
 void PayloadState::LoadUrlFailureCount() {
-  SetUrlFailureCount(GetPersistedValue(kPrefsCurrentUrlFailureCount));
+  SetUrlFailureCount(GetPersistedValue(kPrefsCurrentUrlFailureCount,
+                                       false));
 }
 
 void PayloadState::SetUrlFailureCount(uint32_t url_failure_count) {
@@ -787,7 +807,18 @@
 }
 
 void PayloadState::LoadNumReboots() {
-  SetNumReboots(GetPersistedValue(kPrefsNumReboots));
+  SetNumReboots(GetPersistedValue(kPrefsNumReboots, false));
+}
+
+void PayloadState::LoadRollbackVersion() {
+  SetNumReboots(GetPersistedValue(kPrefsRollbackVersion, true));
+}
+
+void PayloadState::SetRollbackVersion(const string& rollback_version) {
+  CHECK(powerwash_safe_prefs_);
+  LOG(INFO) << "Blacklisting version "<< rollback_version;
+  rollback_version_ = rollback_version;
+  powerwash_safe_prefs_->SetString(kPrefsRollbackVersion, rollback_version);
 }
 
 void PayloadState::SetUpdateDurationUptimeExtended(const TimeDelta& value,
@@ -852,7 +883,7 @@
 
 void PayloadState::LoadCurrentBytesDownloaded(DownloadSource source) {
   string key = GetPrefsKey(kPrefsCurrentBytesDownloaded, source);
-  SetCurrentBytesDownloaded(source, GetPersistedValue(key), true);
+  SetCurrentBytesDownloaded(source, GetPersistedValue(key, false), true);
 }
 
 void PayloadState::SetCurrentBytesDownloaded(
@@ -876,7 +907,7 @@
 
 void PayloadState::LoadTotalBytesDownloaded(DownloadSource source) {
   string key = GetPrefsKey(kPrefsTotalBytesDownloaded, source);
-  SetTotalBytesDownloaded(source, GetPersistedValue(key), true);
+  SetTotalBytesDownloaded(source, GetPersistedValue(key, false), true);
 }
 
 void PayloadState::SetTotalBytesDownloaded(
@@ -900,7 +931,7 @@
 }
 
 void PayloadState::LoadNumResponsesSeen() {
-  SetNumResponsesSeen(GetPersistedValue(kPrefsNumResponsesSeen));
+  SetNumResponsesSeen(GetPersistedValue(kPrefsNumResponsesSeen, false));
 }
 
 void PayloadState::SetNumResponsesSeen(int num_responses_seen) {
diff --git a/payload_state.h b/payload_state.h
index 7c0d0d3..3c88622 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -40,6 +40,7 @@
   virtual void UpdateSucceeded();
   virtual void UpdateFailed(ErrorCode error);
   virtual bool ShouldBackoffDownload();
+  virtual void Rollback();
 
   virtual inline std::string GetResponseSignature() {
     return response_signature_;
@@ -87,6 +88,10 @@
 
   virtual void UpdateEngineStarted();
 
+  virtual inline std::string GetRollbackVersion() {
+    return rollback_version_;
+  }
+
  private:
   // Increments the payload attempt number which governs the backoff behavior
   // at the time of the next update check.
@@ -138,8 +143,9 @@
   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);
+  // the value returned is non-negative. If |across_powerwash| is True,
+  // get the value that will persist across a powerwash.
+  int64_t GetPersistedValue(const std::string& key, bool across_powerwash);
 
   // Calculates the response "signature", which is basically a string composed
   // of the subset of the fields in the current response that affect the
@@ -249,6 +255,16 @@
                                uint64_t total_bytes_downloaded,
                                bool log);
 
+  // Loads the blacklisted version from our prefs file.
+  void LoadRollbackVersion();
+
+  // Blacklists this version from getting AU'd to until we receive a new update
+  // response.
+  void SetRollbackVersion(const std::string& rollback_version);
+
+  // Clears any blacklisted version.
+  void ResetRollbackVersion();
+
   inline uint32_t GetUrlIndex() {
     return url_index_;
   }
@@ -297,6 +313,11 @@
   // be set by calling the Initialize method before calling any other method.
   PrefsInterface* prefs_;
 
+  // Interface object with which we read/write persisted state. This must
+  // be set by calling the Initialize method before calling any other method.
+  // This object persists across powerwashes.
+  PrefsInterface* powerwash_safe_prefs_;
+
   // This is the current response object from Omaha.
   OmahaResponse response_;
 
@@ -390,6 +411,12 @@
   // allowed as per device policy.
   std::vector<std::string> candidate_urls_;
 
+  // This stores a blacklisted version set as part of rollback. When we rollback
+  // we store the version of the os from which we are rolling back from in order
+  // to guarantee that we do not re-update to it on the next au attempt after
+  // reboot.
+  std::string rollback_version_;
+
   DISALLOW_COPY_AND_ASSIGN(PayloadState);
 };
 
diff --git a/payload_state_interface.h b/payload_state_interface.h
index a7129c9..54c7c9f 100644
--- a/payload_state_interface.h
+++ b/payload_state_interface.h
@@ -58,6 +58,9 @@
   // depending on the type of the error that happened.
   virtual void UpdateFailed(ErrorCode error) = 0;
 
+  // This method should be called every time we initiate a Rollback.
+  virtual void Rollback() = 0;
+
   // Returns true if we should backoff the current download attempt.
   // False otherwise.
   virtual bool ShouldBackoffDownload() = 0;
@@ -112,6 +115,10 @@
 
   // Called at update_engine startup to do various house-keeping.
   virtual void UpdateEngineStarted() = 0;
+
+  // Returns the version from before a rollback if our last update was a
+  // rollback.
+  virtual std::string GetRollbackVersion() = 0;
  };
 
 }  // namespace chromeos_update_engine
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index c935178..dcdcd11 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -786,6 +786,30 @@
   EXPECT_EQ(0, payload_state.GetNumReboots());
 }
 
+TEST(PayloadStateTest, RollbackVersion) {
+  MockSystemState mock_system_state;
+  PayloadState payload_state;
+
+  NiceMock<PrefsMock>* mock_powerwash_safe_prefs =
+      mock_system_state.mock_powerwash_safe_prefs();
+  EXPECT_TRUE(payload_state.Initialize(&mock_system_state));
+
+  // Verify pre-conditions are good.
+  EXPECT_TRUE(payload_state.GetRollbackVersion().empty());
+
+  // Mock out the os version and make sure it's blacklisted correctly.
+  string rollback_version = "2345.0.0";
+  OmahaRequestParams params(&mock_system_state);
+  params.Init(rollback_version, "", false);
+  mock_system_state.set_request_params(&params);
+
+  EXPECT_CALL(*mock_powerwash_safe_prefs, SetString(kPrefsRollbackVersion,
+                                                    rollback_version));
+  payload_state.Rollback();
+
+  EXPECT_EQ(rollback_version, payload_state.GetRollbackVersion());
+}
+
 TEST(PayloadStateTest, DurationsAreCorrect) {
   OmahaResponse response;
   PayloadState payload_state;
diff --git a/real_system_state.h b/real_system_state.h
index 83f3ac1..9ced628 100644
--- a/real_system_state.h
+++ b/real_system_state.h
@@ -52,6 +52,10 @@
     return &prefs_;
   }
 
+  virtual inline PrefsInterface* powerwash_safe_prefs() {
+      return &powerwash_safe_prefs_;
+    }
+
   virtual inline PayloadStateInterface* payload_state() {
     return &payload_state_;
   }
@@ -95,6 +99,9 @@
   // Interface for persisted store.
   Prefs prefs_;
 
+  // Interface for persisted store that persists across powerwashes.
+  Prefs powerwash_safe_prefs_;
+
   // All state pertaining to payload state such as
   // response, URL, backoff states.
   PayloadState payload_state_;
diff --git a/system_state.cc b/system_state.cc
index 2f7d0dd..d9d0818 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -10,7 +10,6 @@
 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),
@@ -26,6 +25,11 @@
     return false;
   }
 
+  if (!powerwash_safe_prefs_.Init(FilePath(kPowerwashSafePrefsDir))) {
+    LOG(ERROR) << "Failed to initialize powerwash preferences.";
+    return false;
+  }
+
   if (!utils::FileExists(kSystemRebootedMarkerFile)) {
     if (!utils::WriteFile(kSystemRebootedMarkerFile, "", 0)) {
       LOG(ERROR) << "Could not create reboot marker file";
diff --git a/system_state.h b/system_state.h
index 4f3c98f..56d38a6 100644
--- a/system_state.h
+++ b/system_state.h
@@ -60,6 +60,12 @@
   // Gets the interface object for persisted store.
   virtual PrefsInterface* prefs() = 0;
 
+  // Gets the interface object for the persisted store that persists across
+  // powerwashes. Please note that this should be used very seldomly and must
+  // be forwards and backwards compatible as powerwash is used to go back and
+  // forth in system versions.
+  virtual PrefsInterface* powerwash_safe_prefs() = 0;
+
   // Gets the interface for the payload state object.
   virtual PayloadStateInterface* payload_state() = 0;
 
diff --git a/update_attempter.cc b/update_attempter.cc
index 799cd20..ca6643b 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -544,14 +544,18 @@
   }
 }
 
-void UpdateAttempter::Rollback(bool powerwash) {
+bool UpdateAttempter::Rollback(bool powerwash) {
   CHECK(!processor_->IsRunning());
   processor_->set_delegate(this);
 
+  // TODO(sosa): crbug.com/252539 -- refactor channel into system_state and
+  // check for != stable-channel here.
+  RefreshDevicePolicy();
+
   LOG(INFO) << "Setting rollback options.";
   InstallPlan install_plan;
-  TEST_AND_RETURN(utils::GetInstallDev(utils::BootDevice(),
-                                       &install_plan.install_path));
+  TEST_AND_RETURN_FALSE(utils::GetInstallDev(utils::BootDevice(),
+                                             &install_plan.install_path));
   install_plan.kernel_install_path = utils::BootKernelDevice(
       install_plan.install_path);
   install_plan.powerwash_required = powerwash;
@@ -563,6 +567,12 @@
       new InstallPlanAction(install_plan));
   actions_.push_back(shared_ptr<AbstractAction>(install_plan_action));
 
+  // Initialize the default request params.
+  if (!omaha_request_params_->Init("", "", true)) {
+    LOG(ERROR) << "Unable to initialize Omaha request params.";
+    return false;
+  }
+
   BuildPostInstallActions(install_plan_action.get());
 
   // Enqueue the actions
@@ -570,6 +580,10 @@
        it != actions_.end(); ++it) {
     processor_->EnqueueAction(it->get());
   }
+
+  // Update the payload state for Rollback.
+  system_state_->payload_state()->Rollback();
+
   SetStatusAndNotify(UPDATE_STATUS_ATTEMPTING_ROLLBACK,
                      kUpdateNoticeUnspecified);
 
@@ -578,6 +592,7 @@
   // actions we just posted.
   start_action_processor_ = true;
   UpdateBootFlags();
+  return true;
 }
 
 void UpdateAttempter::CheckForUpdate(const string& app_version,
diff --git a/update_attempter.h b/update_attempter.h
index a9dadf0..a98fcbf 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -147,8 +147,8 @@
   // This is the internal entry point for going through a rollback. This will
   // attempt to run the postinstall on the non-active partition and set it as
   // the partition to boot from. If |powerwash| is True, perform a powerwash
-  // as part of rollback.
-  void Rollback(bool powerwash);
+  // as part of rollback. Returns True on success.
+  bool Rollback(bool powerwash);
 
   // Initiates a reboot if the current state is
   // UPDATED_NEED_REBOOT. Returns true on sucess, false otherwise.