commit 9cf526274905ff47ff26eab088c803916e7112d3
author xunchang <xunchang@google.com> Fri Jan 25 11:04:58 2019 -0800
committer Tianjie Xu <xunchang@google.com> Wed Feb 20 23:29:25 2019 +0000
tree ae1f467608255d4baabee8a67f38ad2be4bc468c
parent d58a12117777fd32b5d953ab40f24491e2d5cb83

Metrics: PayloadAttemptNumber should persist across attempts

We should only reset the PayloadAttemptNumber if the update succeeds, or
we switch to a different payload. The old code has already checked for
the payload id; and we should only delete this pref upon successful
updates.

Bug: 122841006
Test: unit tests pass
Change-Id: I7743b721d2899ba59375f8465468cc0e2a9b3568

update_attempter_android.cc

diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index c738e4e..1cc8505 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -596,6 +596,9 @@
   CollectAndReportUpdateMetricsOnUpdateFinished(error_code);
   ClearMetricsPrefs();
   if (error_code == ErrorCode::kSuccess) {
+    // We should only reset the PayloadAttemptNumber if the update succeeds, or
+    // we switch to a different payload.
+    prefs_->Delete(kPrefsPayloadAttemptNumber);
     metrics_utils::SetSystemUpdatedMarker(clock_.get(), prefs_);
     // Clear the total bytes downloaded if and only if the update succeeds.
     prefs_->SetInt64(kPrefsTotalBytesDownloaded, 0);
@@ -826,7 +829,6 @@
   CHECK(prefs_);
   prefs_->Delete(kPrefsCurrentBytesDownloaded);
   prefs_->Delete(kPrefsNumReboots);
-  prefs_->Delete(kPrefsPayloadAttemptNumber);
   prefs_->Delete(kPrefsSystemUpdatedMarker);
   prefs_->Delete(kPrefsUpdateTimestampStart);
   prefs_->Delete(kPrefsUpdateBootTimestampStart);

update_attempter_android.h

diff --git a/update_attempter_android.h b/update_attempter_android.h
index e4b40de..c4710ad 100644
--- a/update_attempter_android.h
+++ b/update_attempter_android.h
@@ -151,9 +151,9 @@
   void UpdatePrefsOnUpdateStart(bool is_resume);
 
   // Prefs to delete:
-  //   |kPrefsNumReboots|, |kPrefsPayloadAttemptNumber|,
+  //   |kPrefsNumReboots|, |kPrefsCurrentBytesDownloaded|
   //   |kPrefsSystemUpdatedMarker|, |kPrefsUpdateTimestampStart|,
-  //   |kPrefsUpdateBootTimestampStart|, |kPrefsCurrentBytesDownloaded|
+  //   |kPrefsUpdateBootTimestampStart|
   void ClearMetricsPrefs();
 
   DaemonStateInterface* daemon_state_;

update_attempter_android_unittest.cc

diff --git a/update_attempter_android_unittest.cc b/update_attempter_android_unittest.cc
index 2593d44..3be0b7e 100644
--- a/update_attempter_android_unittest.cc
+++ b/update_attempter_android_unittest.cc
@@ -111,9 +111,10 @@
   update_attempter_android_.Init();
   // Check that we reset the metric prefs.
   EXPECT_FALSE(prefs_.Exists(kPrefsNumReboots));
-  EXPECT_FALSE(prefs_.Exists(kPrefsPayloadAttemptNumber));
   EXPECT_FALSE(prefs_.Exists(kPrefsUpdateTimestampStart));
   EXPECT_FALSE(prefs_.Exists(kPrefsSystemUpdatedMarker));
+  // PayloadAttemptNumber should persist across reboots.
+  EXPECT_TRUE(prefs_.Exists(kPrefsPayloadAttemptNumber));
 }
 
 TEST_F(UpdateAttempterAndroidTest, ReportMetricsOnUpdateTerminated) {