commit 978c1081373dba279c2c9d5b308e093c3e13707a
author Colin Howes <chowes@google.com> Mon Dec 03 11:46:12 2018 -0800
committer chrome-bot <chrome-bot@chromium.org> Tue Dec 04 14:00:05 2018 -0800
tree f5c046107a867f22ce1fb142cc3b5ed6e91d0326
parent f9e5656462ee760b83bb89dbdf533565c8543921

update_attempter: Set is_install_ to false when doing a rollback.

We weren't setting is_install_ when performing a rollback, meaning it
was potentially left uninitialized or had a stale value from a previous
install/update attempt. Explicitly set is_install_ in Rollback() and in
the UpdateAttempter constructor.

BUG=chromium:905704
TEST=Add a unittest.

Change-Id: I19b646d689cab869a2aca0cd82def252c370184b
Reviewed-on: https://chromium-review.googlesource.com/1359109
Commit-Ready: Colin Howes <chowes@google.com>
Tested-by: Colin Howes <chowes@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

update_attempter.cc

diff --git a/update_attempter.cc b/update_attempter.cc
index f47a63e..50c72f0 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -130,7 +130,8 @@
                                  CertificateChecker* cert_checker)
     : processor_(new ActionProcessor()),
       system_state_(system_state),
-      cert_checker_(cert_checker) {}
+      cert_checker_(cert_checker),
+      is_install_(false) {}
 
 UpdateAttempter::~UpdateAttempter() {
   // CertificateChecker might not be initialized in unittests.
@@ -703,6 +704,7 @@
 }
 
 bool UpdateAttempter::Rollback(bool powerwash) {
+  is_install_ = false;
   if (!CanRollback()) {
     return false;
   }

update_attempter.h

diff --git a/update_attempter.h b/update_attempter.h
index 07a66b9..7371507 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -275,6 +275,7 @@
   FRIEND_TEST(UpdateAttempterTest, PingOmahaTest);
   FRIEND_TEST(UpdateAttempterTest, ReportDailyMetrics);
   FRIEND_TEST(UpdateAttempterTest, RollbackNotAllowed);
+  FRIEND_TEST(UpdateAttempterTest, RollbackAfterInstall);
   FRIEND_TEST(UpdateAttempterTest, RollbackAllowed);
   FRIEND_TEST(UpdateAttempterTest, RollbackAllowedSetAndReset);
   FRIEND_TEST(UpdateAttempterTest, RollbackMetricsNotRollbackFailure);
@@ -286,6 +287,7 @@
   FRIEND_TEST(UpdateAttempterTest, SetRollbackHappenedNotRollback);
   FRIEND_TEST(UpdateAttempterTest, SetRollbackHappenedRollback);
   FRIEND_TEST(UpdateAttempterTest, TargetVersionPrefixSetAndReset);
+  FRIEND_TEST(UpdateAttempterTest, UpdateAfterInstall);
   FRIEND_TEST(UpdateAttempterTest, UpdateAttemptFlagsCachedAtUpdateStart);
   FRIEND_TEST(UpdateAttempterTest, UpdateDeferredByPolicyTest);
   FRIEND_TEST(UpdateAttempterTest, UpdateIsNotRunningWhenUpdateAvailable);

update_attempter_unittest.cc

diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index a395e1b..f2905bc 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -1216,6 +1216,18 @@
   EXPECT_EQ("", attempter_.forced_omaha_url());
 }
 
+TEST_F(UpdateAttempterTest, RollbackAfterInstall) {
+  attempter_.is_install_ = true;
+  attempter_.Rollback(false);
+  EXPECT_FALSE(attempter_.is_install_);
+}
+
+TEST_F(UpdateAttempterTest, UpdateAfterInstall) {
+  attempter_.is_install_ = true;
+  attempter_.CheckForUpdate("", "", UpdateAttemptFlags::kNone);
+  EXPECT_FALSE(attempter_.is_install_);
+}
+
 TEST_F(UpdateAttempterTest, TargetVersionPrefixSetAndReset) {
   attempter_.CalculateUpdateParams("", "", "", "1234", false, false, false);
   EXPECT_EQ("1234",