Load the and store rollback version into the correct variable.

Somehow missed this and added unittest to help never have this issue again.
In this CL I also got rid of the powerwash_prefs_ logic in GetPersistedValue
because it was just useless noise in my prev CL that isn't used anymore.

BUG=chromium:285381
TEST=unittests that actually catch error.

Change-Id: I4ed680c5b5c2e37549d58aa0a7c0d35a62623fe7
Reviewed-on: https://chromium-review.googlesource.com/168073
Tested-by: Chris Sosa <sosa@chromium.org>
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: Chris Sosa <sosa@chromium.org>
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 8e77afa..3cf78d3 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -50,9 +50,12 @@
   // 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;
+  if(!rollback_version.empty()) {
+    LOG(INFO) << "Detected previous rollback from version " << rollback_version;
+    if(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,
diff --git a/payload_state.cc b/payload_state.cc
index 1463bc0..90dc3d3 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -625,18 +625,13 @@
   }
 }
 
-int64_t PayloadState::GetPersistedValue(const string& key,
-                                        bool across_powerwash) {
-  PrefsInterface* prefs = prefs_;
-  if (across_powerwash)
-    prefs = powerwash_safe_prefs_;
-
+int64_t PayloadState::GetPersistedValue(const string& key) {
   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) {
@@ -690,13 +685,12 @@
 }
 
 void PayloadState::LoadPayloadAttemptNumber() {
-  SetPayloadAttemptNumber(GetPersistedValue(kPrefsPayloadAttemptNumber,
-                                            false));
+  SetPayloadAttemptNumber(GetPersistedValue(kPrefsPayloadAttemptNumber));
 }
 
 void PayloadState::LoadFullPayloadAttemptNumber() {
-  SetFullPayloadAttemptNumber(GetPersistedValue(kPrefsFullPayloadAttemptNumber,
-                                            false));
+  SetFullPayloadAttemptNumber(GetPersistedValue(
+      kPrefsFullPayloadAttemptNumber));
 }
 
 void PayloadState::SetPayloadAttemptNumber(int payload_attempt_number) {
@@ -716,7 +710,7 @@
 }
 
 void PayloadState::LoadUrlIndex() {
-  SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex, false));
+  SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex));
 }
 
 void PayloadState::SetUrlIndex(uint32_t url_index) {
@@ -731,7 +725,7 @@
 }
 
 void PayloadState::LoadUrlSwitchCount() {
-  SetUrlSwitchCount(GetPersistedValue(kPrefsUrlSwitchCount, false));
+  SetUrlSwitchCount(GetPersistedValue(kPrefsUrlSwitchCount));
 }
 
 void PayloadState::SetUrlSwitchCount(uint32_t url_switch_count) {
@@ -742,8 +736,7 @@
 }
 
 void PayloadState::LoadUrlFailureCount() {
-  SetUrlFailureCount(GetPersistedValue(kPrefsCurrentUrlFailureCount,
-                                       false));
+  SetUrlFailureCount(GetPersistedValue(kPrefsCurrentUrlFailureCount));
 }
 
 void PayloadState::SetUrlFailureCount(uint32_t url_failure_count) {
@@ -877,11 +870,16 @@
 }
 
 void PayloadState::LoadNumReboots() {
-  SetNumReboots(GetPersistedValue(kPrefsNumReboots, false));
+  SetNumReboots(GetPersistedValue(kPrefsNumReboots));
 }
 
 void PayloadState::LoadRollbackVersion() {
-  SetNumReboots(GetPersistedValue(kPrefsRollbackVersion, true));
+  CHECK(powerwash_safe_prefs_);
+  string rollback_version;
+  if (powerwash_safe_prefs_->GetString(kPrefsRollbackVersion,
+                                       &rollback_version)) {
+    SetRollbackVersion(rollback_version);
+  }
 }
 
 void PayloadState::SetRollbackVersion(const string& rollback_version) {
@@ -990,7 +988,7 @@
 
 void PayloadState::LoadCurrentBytesDownloaded(DownloadSource source) {
   string key = GetPrefsKey(kPrefsCurrentBytesDownloaded, source);
-  SetCurrentBytesDownloaded(source, GetPersistedValue(key, false), true);
+  SetCurrentBytesDownloaded(source, GetPersistedValue(key), true);
 }
 
 void PayloadState::SetCurrentBytesDownloaded(
@@ -1014,7 +1012,7 @@
 
 void PayloadState::LoadTotalBytesDownloaded(DownloadSource source) {
   string key = GetPrefsKey(kPrefsTotalBytesDownloaded, source);
-  SetTotalBytesDownloaded(source, GetPersistedValue(key, false), true);
+  SetTotalBytesDownloaded(source, GetPersistedValue(key), true);
 }
 
 void PayloadState::SetTotalBytesDownloaded(
@@ -1038,7 +1036,7 @@
 }
 
 void PayloadState::LoadNumResponsesSeen() {
-  SetNumResponsesSeen(GetPersistedValue(kPrefsNumResponsesSeen, false));
+  SetNumResponsesSeen(GetPersistedValue(kPrefsNumResponsesSeen));
 }
 
 void PayloadState::SetNumResponsesSeen(int num_responses_seen) {
@@ -1241,7 +1239,7 @@
 }
 
 void PayloadState::LoadP2PNumAttempts() {
-  SetP2PNumAttempts(GetPersistedValue(kPrefsP2PNumAttempts, false));
+  SetP2PNumAttempts(GetPersistedValue(kPrefsP2PNumAttempts));
 }
 
 Time PayloadState::GetP2PFirstAttemptTimestamp() {
@@ -1258,8 +1256,7 @@
 }
 
 void PayloadState::LoadP2PFirstAttemptTimestamp() {
-  int64_t stored_value = GetPersistedValue(kPrefsP2PFirstAttemptTimestamp,
-                                           false);
+  int64_t stored_value = GetPersistedValue(kPrefsP2PFirstAttemptTimestamp);
   Time stored_time = Time::FromInternalValue(stored_value);
   SetP2PFirstAttemptTimestamp(stored_time);
 }
diff --git a/payload_state.h b/payload_state.h
index f358bda..ad0f339 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -114,6 +114,7 @@
   FRIEND_TEST(PayloadStateTest, RebootAfterUpdateFailedMetric);
   FRIEND_TEST(PayloadStateTest, RebootAfterUpdateSucceed);
   FRIEND_TEST(PayloadStateTest, RebootAfterCanceledUpdate);
+  FRIEND_TEST(PayloadStateTest, RollbackVersion);
   FRIEND_TEST(PayloadStateTest, UpdateSuccessWithWipedPrefs);
 
   // Increments the payload attempt number used for metrics.
@@ -181,10 +182,9 @@
   // 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. If |across_powerwash| is True,
-  // get the value that will persist across a powerwash.
-  int64_t GetPersistedValue(const std::string& key, bool across_powerwash);
+  // Returns the persisted value from prefs_ 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
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 613940e..d8032fe 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -988,6 +988,17 @@
   payload_state.Rollback();
 
   EXPECT_EQ(rollback_version, payload_state.GetRollbackVersion());
+
+  // Change it up a little and verify we load it correctly.
+  rollback_version = "2345.0.1";
+  // Let's verify we can reload it correctly.
+  EXPECT_CALL(*mock_powerwash_safe_prefs, GetString(
+      kPrefsRollbackVersion, _)).WillOnce(DoAll(
+          SetArgumentPointee<1>(rollback_version), Return(true)));
+  EXPECT_CALL(*mock_powerwash_safe_prefs, SetString(kPrefsRollbackVersion,
+                                                    rollback_version));
+  payload_state.LoadRollbackVersion();
+  EXPECT_EQ(rollback_version, payload_state.GetRollbackVersion());
 }
 
 TEST(PayloadStateTest, DurationsAreCorrect) {