Use BuildState::UpdateType instead of installed_version

BuildState::installed_version() returns the platform version instead of
the browser version number. So, using BuildState::UpdateType to make
sure that the current update is not a rollback.

Bug: 1048607
Change-Id: I726548db6df944f5176b766dacd65fba33a5f1af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2237889Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Saurabh Nijhara <snijhara@google.com>
Cr-Commit-Position: refs/heads/master@{#778999}

chrome/browser/chromeos/policy/minimum_version_policy_handler.cc

@@ -63,6 +63,10 @@ std::string GetEnterpriseDomainName() {
       ->GetEnterpriseDisplayDomain();
 }
 
+BuildState* GetBuildState() {
+  return g_browser_process->GetBuildState();
+}
+
 }  // namespace
 
 const char MinimumVersionPolicyHandler::kChromeVersion[] = "chrome_version";
@@ -125,7 +129,7 @@ MinimumVersionPolicyHandler::MinimumVersionPolicyHandler(
 }
 
 MinimumVersionPolicyHandler::~MinimumVersionPolicyHandler() {
-  g_browser_process->GetBuildState()->RemoveObserver(this);
+  GetBuildState()->RemoveObserver(this);
   StopObservingNetwork();
 }
 
@@ -216,9 +220,11 @@ void MinimumVersionPolicyHandler::OnPolicyChanged() {
       requirements_met_ = false;
       FetchEolInfo();
     }
-  } else if (state_) {
+  } else {
     // Update is not required as the requirements of all of the configs in the
-    // policy are satisfied by the current chrome version.
+    // policy are satisfied by the current chrome version. We could also reach
+    // here at the time of login if the device was rebooted to apply the
+    // downloaded update, in which case it is needed to reset the local state.
     HandleUpdateNotRequired();
   }
 }
@@ -239,7 +245,7 @@ void MinimumVersionPolicyHandler::Reset() {
   eol_reached_ = false;
   update_required_deadline_timer_.Stop();
   notification_timer_.Stop();
-  g_browser_process->GetBuildState()->RemoveObserver(this);
+  GetBuildState()->RemoveObserver(this);
   state_.reset();
   HideNotification();
   notification_handler_.reset();
@@ -247,6 +253,14 @@ void MinimumVersionPolicyHandler::Reset() {
   StopObservingNetwork();
 }
 
+void MinimumVersionPolicyHandler::ResetOnUpdateCompleted() {
+  update_required_deadline_timer_.Stop();
+  notification_timer_.Stop();
+  GetBuildState()->RemoveObserver(this);
+  HideNotification();
+  notification_handler_.reset();
+}
+
 void MinimumVersionPolicyHandler::FetchEolInfo() {
   // Return if update required state is null meaning all requirements are
   // satisfied.
@@ -329,6 +343,14 @@ void MinimumVersionPolicyHandler::HandleUpdateRequired(
   if (deadline > previous_deadline)
     UpdateLocalState(warning_time);
 
+  // The device has already downloaded the update in-session and waiting for
+  // reboot to apply it.
+  if (GetBuildState()->update_type() == BuildState::UpdateType::kNormalUpdate) {
+    // TODO(https://crbug.com/1048607): May be adjust relaunch notification
+    // timer as per new deadline.
+    return;
+  }
+
   StartDeadlineTimer(deadline);
   if (!eol_reached_)
     StartObservingUpdate();
@@ -363,7 +385,7 @@ void MinimumVersionPolicyHandler::StartDeadlineTimer(base::Time deadline) {
 }
 
 void MinimumVersionPolicyHandler::StartObservingUpdate() {
-  auto* build_state = g_browser_process->GetBuildState();
+  auto* build_state = GetBuildState();
   if (!build_state->HasObserver(this))
     build_state->AddObserver(this);
 }
@@ -432,10 +454,10 @@ void MinimumVersionPolicyHandler::ShowAndScheduleNotification(
 }
 
 void MinimumVersionPolicyHandler::OnUpdate(const BuildState* build_state) {
-  // Reset the state if new version is greater or equal the required version.
-  // Hide update required screen if it is shown.
-  if (build_state->installed_version()->CompareTo(state_->version()) >= 0)
-    HandleUpdateNotRequired();
+  // If the device has been successfully updated, the relaunch notifications
+  // will reboot it for applying the updates.
+  if (build_state->update_type() == BuildState::UpdateType::kNormalUpdate)
+    ResetOnUpdateCompleted();
 }
 
 void MinimumVersionPolicyHandler::HideNotification() const {
@@ -453,6 +475,8 @@ void MinimumVersionPolicyHandler::DefaultNetworkChanged(
 }
 
 void MinimumVersionPolicyHandler::StopObservingNetwork() {
+  if (!chromeos::NetworkHandler::IsInitialized())
+    return;
   chromeos::NetworkStateHandler* network_state_handler =
       chromeos::NetworkHandler::Get()->network_state_handler();
   network_state_handler->RemoveObserver(this, FROM_HERE);

chrome/browser/chromeos/policy/minimum_version_policy_handler.h

@@ -160,6 +160,10 @@ class MinimumVersionPolicyHandler
   bool IsPolicyApplicable();
   void Reset();
 
+  // Handles post update completed actions like reset timers, hide update
+  // required notification and stop observing build state.
+  void ResetOnUpdateCompleted();
+
   // Handles the state when update is required as per the policy. If on the
   // login screen, update required screen is shown, else the user is logged out
   // if the device is not updated within the given |warning_time|. The

chrome/browser/chromeos/policy/minimum_version_policy_handler_browsertest.cc

@@ -57,10 +57,13 @@ const char kNewVersion[] = "99999.4.2";
 const int kNoWarning = 0;
 const int kShortWarningInDays = 2;
 const int kLongWarningInDays = 10;
+const int kVeryLongWarningInDays = 100;
 constexpr base::TimeDelta kShortWarning =
     base::TimeDelta::FromDays(kShortWarningInDays);
 constexpr base::TimeDelta kLongWarning =
     base::TimeDelta::FromDays(kLongWarningInDays);
+constexpr base::TimeDelta kVeryLongWarning =
+    base::TimeDelta::FromDays(kVeryLongWarningInDays);
 const char kPublicSessionId[] = "demo@example.com";
 const char kUpdateRequiredNotificationId[] = "policy.update_required";
 const char kWifiServicePath[] = "/service/wifi2";
@@ -263,7 +266,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, CriticalUpdateInSession) {
 }
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NonCriticalUpdateGoodNetwork) {
-  // Login the user into the session and mark as managed.
+  // Login the user into the session.
   Login();
 
   // Check deadline timer is not running and local state is not set.
@@ -323,17 +326,81 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NonCriticalUpdateGoodNetwork) {
   EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
 
   // Simulate update installed from update_engine_client and check that timer
-  // and the local state is reset.
+  // is reset but local state is not.
   update_engine::StatusResult status;
   status.set_current_operation(update_engine::Operation::UPDATED_NEED_REBOOT);
   status.set_new_version("99999.9");
   fake_update_engine_client_->set_default_status(status);
   fake_update_engine_client_->NotifyObserversThatStatusChanged(status);
+  EXPECT_FALSE(
+      GetMinimumVersionPolicyHandler()->IsDeadlineTimerRunningForTesting());
+  EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
+  EXPECT_FALSE(prefs->GetTime(prefs::kUpdateRequiredTimerStartTime).is_null());
+
+  // New policy after update is downloaded does not restart the timer but just
+  // updates the local state with longer warning period.
+  base::Value requirement_very_long_warning(base::Value::Type::LIST);
+  requirement_very_long_warning.Append(
+      CreateRequirement(kNewVersion, kVeryLongWarningInDays, kNoWarning));
+  SetDevicePolicyAndWaitForSettingChange(requirement_very_long_warning);
+  EXPECT_EQ(prefs->GetTime(prefs::kUpdateRequiredTimerStartTime),
+            timer_start_time);
+  EXPECT_EQ(prefs->GetTimeDelta(prefs::kUpdateRequiredWarningPeriod),
+            kVeryLongWarning);
+  EXPECT_FALSE(
+      GetMinimumVersionPolicyHandler()->IsDeadlineTimerRunningForTesting());
+  EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
+}
+
+IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, DeviceUpdateStatusChange) {
+  // Login the user into the session.
+  Login();
+
+  // Create and set policy value with warning time.
+  base::Value requirement_short_warning(base::Value::Type::LIST);
+  requirement_short_warning.Append(
+      CreateRequirement(kNewVersion, kShortWarningInDays, kShortWarningInDays));
+  SetDevicePolicyAndWaitForSettingChange(requirement_short_warning);
+
+  // Policy handler starts the deadline timer.
+  EXPECT_TRUE(
+      GetMinimumVersionPolicyHandler()->IsDeadlineTimerRunningForTesting());
+  EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
+
+  // Simulate channel switch rollback from update_engine_client and check that
+  // timer is not reset.
+  update_engine::StatusResult rollback_status;
+  rollback_status.set_current_operation(
+      update_engine::Operation::UPDATED_NEED_REBOOT);
+  rollback_status.set_will_powerwash_after_reboot(true);
+  fake_update_engine_client_->set_default_status(rollback_status);
+  fake_update_engine_client_->NotifyObserversThatStatusChanged(rollback_status);
+
+  EXPECT_TRUE(
+      GetMinimumVersionPolicyHandler()->IsDeadlineTimerRunningForTesting());
+  EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
+
+  // Simulate enterprise rollback from update_engine_client and check that timer
+  // is not reset.
+  rollback_status.set_is_enterprise_rollback(true);
+  fake_update_engine_client_->set_default_status(rollback_status);
+  fake_update_engine_client_->NotifyObserversThatStatusChanged(rollback_status);
+
+  EXPECT_TRUE(
+      GetMinimumVersionPolicyHandler()->IsDeadlineTimerRunningForTesting());
+  EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
+
+  // Simulate update installed from update_engine_client and check that timer is
+  // reset.
+  update_engine::StatusResult update_status;
+  update_status.set_current_operation(
+      update_engine::Operation::UPDATED_NEED_REBOOT);
+  fake_update_engine_client_->set_default_status(update_status);
+  fake_update_engine_client_->NotifyObserversThatStatusChanged(update_status);
 
   EXPECT_FALSE(
       GetMinimumVersionPolicyHandler()->IsDeadlineTimerRunningForTesting());
-  EXPECT_FALSE(GetMinimumVersionPolicyHandler()->GetState());
-  EXPECT_TRUE(prefs->GetTime(prefs::kUpdateRequiredTimerStartTime).is_null());
+  EXPECT_TRUE(GetMinimumVersionPolicyHandler()->GetState());
 }
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest,