Always powerwash on channel change if arbitrary channel allowed.

Merged is_powerwash_allowed() and to_more_stable_channel() into
ShouldPowerwash().

Bug: 73082835
Test: update_engine_unittests
Change-Id: I6b7af0d1dac28d5fa9cddf4391bbd9cdf2acb57b
diff --git a/mock_omaha_request_params.h b/mock_omaha_request_params.h
index 5d5d47b..6d8d3d8 100644
--- a/mock_omaha_request_params.h
+++ b/mock_omaha_request_params.h
@@ -32,9 +32,6 @@
     // Delegate all calls to the parent instance by default. This helps the
     // migration from tests using the real RequestParams when they should have
     // use a fake or mock.
-    ON_CALL(*this, to_more_stable_channel())
-        .WillByDefault(testing::Invoke(
-            this, &MockOmahaRequestParams::fake_to_more_stable_channel));
     ON_CALL(*this, GetAppId())
         .WillByDefault(testing::Invoke(
             this, &MockOmahaRequestParams::FakeGetAppId));
@@ -44,27 +41,22 @@
     ON_CALL(*this, UpdateDownloadChannel())
         .WillByDefault(testing::Invoke(
             this, &MockOmahaRequestParams::FakeUpdateDownloadChannel));
-    ON_CALL(*this, is_powerwash_allowed())
+    ON_CALL(*this, ShouldPowerwash())
         .WillByDefault(testing::Invoke(
-            this, &MockOmahaRequestParams::fake_is_powerwash_allowed));
+            this, &MockOmahaRequestParams::FakeShouldPowerwash));
   }
 
-  MOCK_CONST_METHOD0(to_more_stable_channel, bool(void));
   MOCK_CONST_METHOD0(GetAppId, std::string(void));
   MOCK_METHOD3(SetTargetChannel, bool(const std::string& channel,
                                       bool is_powerwash_allowed,
                                       std::string* error));
   MOCK_METHOD0(UpdateDownloadChannel, void(void));
-  MOCK_CONST_METHOD0(is_powerwash_allowed, bool(void));
   MOCK_CONST_METHOD0(IsUpdateUrlOfficial, bool(void));
+  MOCK_CONST_METHOD0(ShouldPowerwash, bool(void));
 
  private:
   // Wrappers to call the parent class and behave like the real object by
   // default. See "Delegating Calls to a Parent Class" in gmock's documentation.
-  bool fake_to_more_stable_channel() const {
-    return OmahaRequestParams::to_more_stable_channel();
-  }
-
   std::string FakeGetAppId() const {
     return OmahaRequestParams::GetAppId();
   }
@@ -81,8 +73,8 @@
     return OmahaRequestParams::UpdateDownloadChannel();
   }
 
-  bool fake_is_powerwash_allowed() const {
-    return OmahaRequestParams::is_powerwash_allowed();
+  bool FakeShouldPowerwash() const {
+    return OmahaRequestParams::ShouldPowerwash();
   }
 };
 
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index a8b8a94..d58612c 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -239,9 +239,7 @@
   // If we are downgrading to a more stable channel and we are allowed to do
   // powerwash, then pass 0.0.0.0 as the version. This is needed to get the
   // highest-versioned payload on the destination channel.
-  bool is_potential_downgrade =
-      params->to_more_stable_channel() && params->is_powerwash_allowed();
-  if (is_potential_downgrade) {
+  if (params->ShouldPowerwash()) {
     LOG(INFO) << "Passing OS version as 0.0.0.0 as we are set to powerwash "
               << "on downgrading to the version in the more stable channel";
     app_versions = "version=\"0.0.0.0\" from_version=\"" +
@@ -291,7 +289,7 @@
   }
 
   string product_components_args;
-  if (!is_potential_downgrade && !app_data.product_components.empty()) {
+  if (!params->ShouldPowerwash() && !app_data.product_components.empty()) {
     brillo::KeyValueStore store;
     if (store.LoadFromString(app_data.product_components)) {
       for (const string& key : store.GetKeys()) {
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 87dc404..2f466dd 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -2188,8 +2188,7 @@
   params.set_current_channel("canary-channel");
   EXPECT_TRUE(params.SetTargetChannel("stable-channel", true, nullptr));
   params.UpdateDownloadChannel();
-  EXPECT_TRUE(params.to_more_stable_channel());
-  EXPECT_TRUE(params.is_powerwash_allowed());
+  EXPECT_TRUE(params.ShouldPowerwash());
   ASSERT_FALSE(TestUpdateCheck(&params,
                                "invalid xml>",
                                -1,
@@ -2223,8 +2222,7 @@
   params.set_current_channel("stable-channel");
   EXPECT_TRUE(params.SetTargetChannel("canary-channel", false, nullptr));
   params.UpdateDownloadChannel();
-  EXPECT_FALSE(params.to_more_stable_channel());
-  EXPECT_FALSE(params.is_powerwash_allowed());
+  EXPECT_FALSE(params.ShouldPowerwash());
   ASSERT_FALSE(TestUpdateCheck(&params,
                                "invalid xml>",
                                -1,
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index 8301d7b..9e78a93 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -222,18 +222,24 @@
   return -1;
 }
 
-bool OmahaRequestParams::to_more_stable_channel() const {
+bool OmahaRequestParams::ToMoreStableChannel() const {
   int current_channel_index = GetChannelIndex(image_props_.current_channel);
   int download_channel_index = GetChannelIndex(download_channel_);
 
-  // If any of the two channels are arbitrary channels, stability is unknown, so
-  // always powerwash if allowed.
-  if (current_channel_index < 0 || download_channel_index < 0)
-    return true;
-
   return download_channel_index > current_channel_index;
 }
 
+bool OmahaRequestParams::ShouldPowerwash() const {
+  if (!mutable_image_props_.is_powerwash_allowed)
+    return false;
+  // If arbitrary channels are allowed, always powerwash on channel change.
+  if (image_props_.allow_arbitrary_channels)
+    return image_props_.current_channel != download_channel_;
+  // Otherwise only powerwash if we are moving from less stable (higher version)
+  // to more stable channel (lower version).
+  return ToMoreStableChannel();
+}
+
 string OmahaRequestParams::GetAppId() const {
   return download_channel_ == "canary-channel" ? image_props_.canary_product_id
                                                : image_props_.product_id;
diff --git a/omaha_request_params.h b/omaha_request_params.h
index a9215ae..60619f9 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -198,10 +198,6 @@
     return max_update_checks_allowed_;
   }
 
-  // True if we're trying to update to a more stable channel.
-  // i.e. index(target_channel) > index(current_channel).
-  virtual bool to_more_stable_channel() const;
-
   // Returns the app id corresponding to the current value of the
   // download channel.
   virtual std::string GetAppId() const;
@@ -238,9 +234,8 @@
   // or Init is called again.
   virtual void UpdateDownloadChannel();
 
-  virtual bool is_powerwash_allowed() const {
-    return mutable_image_props_.is_powerwash_allowed;
-  }
+  // Returns whether we should powerwash for this update.
+  virtual bool ShouldPowerwash() const;
 
   // Check if the provided update URL is official, meaning either the default
   // autoupdate server or the autoupdate autotest server.
@@ -259,7 +254,10 @@
   FRIEND_TEST(OmahaRequestParamsTest, ChannelIndexTest);
   FRIEND_TEST(OmahaRequestParamsTest, CollectECFWVersionsTest);
   FRIEND_TEST(OmahaRequestParamsTest, IsValidChannelTest);
+  FRIEND_TEST(OmahaRequestParamsTest, SetIsPowerwashAllowedTest);
   FRIEND_TEST(OmahaRequestParamsTest, SetTargetChannelInvalidTest);
+  FRIEND_TEST(OmahaRequestParamsTest, SetTargetChannelTest);
+  FRIEND_TEST(OmahaRequestParamsTest, ShouldPowerwashTest);
   FRIEND_TEST(OmahaRequestParamsTest, ToMoreStableChannelFlagTest);
 
   // Returns true if |channel| is a valid channel, otherwise write error to
@@ -273,6 +271,10 @@
   // Returns the index of the given channel.
   int GetChannelIndex(const std::string& channel) const;
 
+  // True if we're trying to update to a more stable channel.
+  // i.e. index(target_channel) > index(current_channel).
+  bool ToMoreStableChannel() const;
+
   // Returns True if we should store the fw/ec versions based on our hwid_.
   // Compares hwid to a set of whitelisted prefixes.
   bool CollectECFWVersions() const;
diff --git a/omaha_request_params_unittest.cc b/omaha_request_params_unittest.cc
index 2de45de..ce77f31 100644
--- a/omaha_request_params_unittest.cc
+++ b/omaha_request_params_unittest.cc
@@ -117,12 +117,12 @@
     params.set_root(tempdir_.GetPath().value());
     EXPECT_TRUE(params.Init("", "", false));
     EXPECT_TRUE(params.SetTargetChannel("canary-channel", false, nullptr));
-    EXPECT_FALSE(params.is_powerwash_allowed());
+    EXPECT_FALSE(params.mutable_image_props_.is_powerwash_allowed);
   }
   params_.set_root(tempdir_.GetPath().value());
   EXPECT_TRUE(params_.Init("", "", false));
   EXPECT_EQ("canary-channel", params_.target_channel());
-  EXPECT_FALSE(params_.is_powerwash_allowed());
+  EXPECT_FALSE(params_.mutable_image_props_.is_powerwash_allowed);
 }
 
 TEST_F(OmahaRequestParamsTest, SetIsPowerwashAllowedTest) {
@@ -131,12 +131,12 @@
     params.set_root(tempdir_.GetPath().value());
     EXPECT_TRUE(params.Init("", "", false));
     EXPECT_TRUE(params.SetTargetChannel("canary-channel", true, nullptr));
-    EXPECT_TRUE(params.is_powerwash_allowed());
+    EXPECT_TRUE(params.mutable_image_props_.is_powerwash_allowed);
   }
   params_.set_root(tempdir_.GetPath().value());
   EXPECT_TRUE(params_.Init("", "", false));
   EXPECT_EQ("canary-channel", params_.target_channel());
-  EXPECT_TRUE(params_.is_powerwash_allowed());
+  EXPECT_TRUE(params_.mutable_image_props_.is_powerwash_allowed);
 }
 
 TEST_F(OmahaRequestParamsTest, SetTargetChannelInvalidTest) {
@@ -151,12 +151,12 @@
         params.SetTargetChannel("dogfood-channel", true, &error_message));
     // The error message should include a message about the valid channels.
     EXPECT_NE(string::npos, error_message.find("stable-channel"));
-    EXPECT_FALSE(params.is_powerwash_allowed());
+    EXPECT_FALSE(params.mutable_image_props_.is_powerwash_allowed);
   }
   params_.set_root(tempdir_.GetPath().value());
   EXPECT_TRUE(params_.Init("", "", false));
   EXPECT_EQ("stable-channel", params_.target_channel());
-  EXPECT_FALSE(params_.is_powerwash_allowed());
+  EXPECT_FALSE(params_.mutable_image_props_.is_powerwash_allowed);
 }
 
 TEST_F(OmahaRequestParamsTest, IsValidChannelTest) {
@@ -232,13 +232,25 @@
 TEST_F(OmahaRequestParamsTest, ToMoreStableChannelFlagTest) {
   params_.image_props_.current_channel = "canary-channel";
   params_.download_channel_ = "stable-channel";
-  EXPECT_TRUE(params_.to_more_stable_channel());
+  EXPECT_TRUE(params_.ToMoreStableChannel());
   params_.image_props_.current_channel = "stable-channel";
-  EXPECT_FALSE(params_.to_more_stable_channel());
+  EXPECT_FALSE(params_.ToMoreStableChannel());
   params_.download_channel_ = "beta-channel";
-  EXPECT_FALSE(params_.to_more_stable_channel());
-  params_.download_channel_ = "some-channel";
-  EXPECT_TRUE(params_.to_more_stable_channel());
+  EXPECT_FALSE(params_.ToMoreStableChannel());
+}
+
+TEST_F(OmahaRequestParamsTest, ShouldPowerwashTest) {
+  params_.mutable_image_props_.is_powerwash_allowed = false;
+  EXPECT_FALSE(params_.ShouldPowerwash());
+  params_.mutable_image_props_.is_powerwash_allowed = true;
+  params_.image_props_.allow_arbitrary_channels = true;
+  params_.image_props_.current_channel = "foo-channel";
+  params_.download_channel_ = "bar-channel";
+  EXPECT_TRUE(params_.ShouldPowerwash());
+  params_.image_props_.allow_arbitrary_channels = false;
+  params_.image_props_.current_channel = "canary-channel";
+  params_.download_channel_ = "stable-channel";
+  EXPECT_TRUE(params_.ShouldPowerwash());
 }
 
 TEST_F(OmahaRequestParamsTest, CollectECFWVersionsTest) {
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 9c5fb4a..2d6105a 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -139,7 +139,7 @@
   system_state_->prefs()->SetString(current_channel_key,
                                     params->download_channel());
 
-  if (params->to_more_stable_channel() && params->is_powerwash_allowed())
+  if (params->ShouldPowerwash())
     install_plan_.powerwash_required = true;
 
   TEST_AND_RETURN(HasOutputPipe());
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 4e9c03f..9e2cdd1 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -393,8 +393,7 @@
 #endif  // __ANDROID__
   EXPECT_TRUE(params.SetTargetChannel("stable-channel", true, nullptr));
   params.UpdateDownloadChannel();
-  EXPECT_TRUE(params.to_more_stable_channel());
-  EXPECT_TRUE(params.is_powerwash_allowed());
+  EXPECT_TRUE(params.ShouldPowerwash());
 
   fake_system_state_.set_request_params(&params);
   InstallPlan install_plan;
@@ -426,8 +425,7 @@
 #endif  // __ANDROID__
   EXPECT_TRUE(params.SetTargetChannel("canary-channel", false, nullptr));
   params.UpdateDownloadChannel();
-  EXPECT_FALSE(params.to_more_stable_channel());
-  EXPECT_FALSE(params.is_powerwash_allowed());
+  EXPECT_FALSE(params.ShouldPowerwash());
 
   fake_system_state_.set_request_params(&params);
   InstallPlan install_plan;