Fixed channel switch when user decides to switch to another channel in

he middle of current channel transition.

For example:
1. User is currently on the beta channel and decides to switch to the dev channel.
2. In the middle of the switch the user decides to switch to the stable channel
   instead of currently downloading dev.

But update engine moves to UPDATE_STATUS_REPORTING_ERROR_EVENT because
target channel isn't equal to downloading channel, so request to start
downloading of stable channel fails. Almost immediately update engine moves
to UPDATE_STATUS_IDLE, but user needs to reload chrome://help page to initiate
update check. Current changelist handles the issue and implements automatic
update checks when update engine is idle.

BUG=311162
TEST=unit_tests:VersionUpdaterCrosTest.*

Review URL: https://codereview.chromium.org/578293002

Cr-Commit-Position: refs/heads/master@{#296693}

chrome/browser/ui/webui/help/version_updater_chromeos.cc

@@ -113,6 +113,7 @@ VersionUpdater* VersionUpdater::Create() {
 
 void VersionUpdaterCros::GetUpdateStatus(const StatusCallback& callback) {
   callback_ = callback;
+
   if (!EnsureCanUpdate(callback))
     return;
 
@@ -136,12 +137,18 @@ void VersionUpdaterCros::CheckForUpdate(const StatusCallback& callback) {
   if (!update_engine_client->HasObserver(this))
     update_engine_client->AddObserver(this);
 
+  if (update_engine_client->GetLastStatus().status !=
+      UpdateEngineClient::UPDATE_STATUS_IDLE) {
+    check_for_update_when_idle_ = true;
+    return;
+  }
+  check_for_update_when_idle_ = false;
+
   // Make sure that libcros is loaded and OOBE is complete.
   if (!WizardController::default_controller() ||
       chromeos::StartupUtils::IsDeviceRegistered()) {
-    update_engine_client->RequestUpdateCheck(
-        base::Bind(&VersionUpdaterCros::OnUpdateCheck,
-                   weak_ptr_factory_.GetWeakPtr()));
+    update_engine_client->RequestUpdateCheck(base::Bind(
+        &VersionUpdaterCros::OnUpdateCheck, weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
@@ -170,6 +177,7 @@ void VersionUpdaterCros::GetChannel(bool get_current_channel,
 
 VersionUpdaterCros::VersionUpdaterCros()
     : last_operation_(UpdateEngineClient::UPDATE_STATUS_IDLE),
+      check_for_update_when_idle_(false),
       weak_ptr_factory_(this) {
 }
 
@@ -229,6 +237,11 @@ void VersionUpdaterCros::UpdateStatusChanged(
 
   callback_.Run(my_status, progress, message);
   last_operation_ = status.status;
+
+  if (check_for_update_when_idle_ &&
+      status.status == UpdateEngineClient::UPDATE_STATUS_IDLE) {
+    CheckForUpdate(callback_);
+  }
 }
 
 void VersionUpdaterCros::OnUpdateCheck(

chrome/browser/ui/webui/help/version_updater_chromeos.h

@@ -45,6 +45,9 @@ class VersionUpdaterCros : public VersionUpdater,
   // Last state received via UpdateStatusChanged().
   chromeos::UpdateEngineClient::UpdateStatusOperation last_operation_;
 
+  // True if an update check should be scheduled when the update engine is idle.
+  bool check_for_update_when_idle_;
+
   base::WeakPtrFactory<VersionUpdaterCros> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(VersionUpdaterCros);

chrome/browser/ui/webui/help/version_updater_chromeos_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/webui/help/version_updater_chromeos.h"
+
+#include "base/bind.h"
+#include "base/compiler_specific.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "chrome/browser/chromeos/login/users/mock_user_manager.h"
+#include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/browser/chromeos/settings/device_settings_service.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_update_engine_client.h"
+#include "chromeos/dbus/shill_service_client.h"
+#include "chromeos/network/network_handler.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+using ::testing::AtLeast;
+using ::testing::Return;
+
+namespace chromeos {
+
+namespace {
+
+void CheckNotification(VersionUpdater::Status /* status */,
+                       int /* progress */,
+                       const base::string16& /* message */) {
+}
+
+}  // namespace
+
+class VersionUpdaterCrosTest : public ::testing::Test {
+ protected:
+  VersionUpdaterCrosTest()
+      : version_updater_(VersionUpdater::Create()),
+        fake_update_engine_client_(NULL),
+        mock_user_manager_(new MockUserManager()),
+        user_manager_enabler_(mock_user_manager_) {}
+
+  virtual ~VersionUpdaterCrosTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    fake_update_engine_client_ = new FakeUpdateEngineClient();
+    scoped_ptr<DBusThreadManagerSetter> dbus_setter =
+        DBusThreadManager::GetSetterForTesting();
+    dbus_setter->SetUpdateEngineClient(
+        scoped_ptr<UpdateEngineClient>(fake_update_engine_client_).Pass());
+
+    EXPECT_CALL(*mock_user_manager_, IsCurrentUserOwner())
+        .WillRepeatedly(Return(false));
+    EXPECT_CALL(*mock_user_manager_, Shutdown()).Times(AtLeast(0));
+
+    DeviceSettingsService::Initialize();
+    CrosSettings::Initialize();
+
+    NetworkHandler::Initialize();
+    ShillServiceClient::TestInterface* service_test =
+        DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
+    service_test->AddService("/service/eth",
+                             "eth" /* guid */,
+                             "eth",
+                             shill::kTypeEthernet, shill::kStateOnline,
+                             true /* visible */);
+    loop_.RunUntilIdle();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    NetworkHandler::Shutdown();
+
+    CrosSettings::Shutdown();
+    DeviceSettingsService::Shutdown();
+  }
+
+  scoped_ptr<VersionUpdater> version_updater_;
+  FakeUpdateEngineClient* fake_update_engine_client_;  // Not owned.
+
+  MockUserManager* mock_user_manager_;  // Not owned.
+  ScopedUserManagerEnabler user_manager_enabler_;
+
+  base::MessageLoop loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(VersionUpdaterCrosTest);
+};
+
+// The test checks following behaviour:
+// 1. The device is currently on the dev channel and an user decides to switch
+// to the beta channel.
+// 2. In the middle of channel switch the user decides to switch to the stable
+// channel.
+// 3. Update engine reports an error because downloading channel (beta) is not
+// equal
+// to the target channel (stable).
+// 4. When update engine becomes idle downloading of the stable channel is
+// initiated.
+TEST_F(VersionUpdaterCrosTest, TwoOverlappingSetChannelRequests) {
+  version_updater_->SetChannel("beta-channel", true);
+
+  {
+    UpdateEngineClient::Status status;
+    status.status = UpdateEngineClient::UPDATE_STATUS_IDLE;
+    fake_update_engine_client_->set_default_status(status);
+    fake_update_engine_client_->NotifyObserversThatStatusChanged(status);
+  }
+
+  EXPECT_EQ(0, fake_update_engine_client_->request_update_check_call_count());
+
+  // IDLE -> DOWNLOADING transition after update check.
+  version_updater_->CheckForUpdate(base::Bind(&CheckNotification));
+  EXPECT_EQ(1, fake_update_engine_client_->request_update_check_call_count());
+
+  {
+    UpdateEngineClient::Status status;
+    status.status = UpdateEngineClient::UPDATE_STATUS_DOWNLOADING;
+    status.download_progress = 0.1;
+    fake_update_engine_client_->set_default_status(status);
+    fake_update_engine_client_->NotifyObserversThatStatusChanged(status);
+  }
+
+  version_updater_->SetChannel("stable-channel", true);
+
+  // DOWNLOADING -> REPORTING_ERROR_EVENT transition since target channel is not
+  // equal to downloading channel now.
+  {
+    UpdateEngineClient::Status status;
+    status.status = UpdateEngineClient::UPDATE_STATUS_REPORTING_ERROR_EVENT;
+    fake_update_engine_client_->set_default_status(status);
+    fake_update_engine_client_->NotifyObserversThatStatusChanged(status);
+  }
+
+  version_updater_->CheckForUpdate(base::Bind(&CheckNotification));
+  EXPECT_EQ(1, fake_update_engine_client_->request_update_check_call_count());
+
+  // REPORTING_ERROR_EVENT -> IDLE transition, update check should be
+  // automatically scheduled.
+  {
+    UpdateEngineClient::Status status;
+    status.status = UpdateEngineClient::UPDATE_STATUS_IDLE;
+    fake_update_engine_client_->set_default_status(status);
+    fake_update_engine_client_->NotifyObserversThatStatusChanged(status);
+  }
+
+  EXPECT_EQ(2, fake_update_engine_client_->request_update_check_call_count());
+}
+
+}  // namespace chromeos

chrome/chrome_tests_unit.gypi

@@ -1220,6 +1220,7 @@
       'browser/ui/webui/chromeos/login/l10n_util_unittest.cc',
       'browser/ui/webui/chromeos/login/signin_userlist_unittest.cc',
       'browser/ui/webui/fileicon_source_unittest.cc',
+      'browser/ui/webui/help/version_updater_chromeos_unittest.cc',
       'browser/ui/webui/history_ui_unittest.cc',
       'browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc',
       'browser/ui/webui/ntp/suggestions_combiner_unittest.cc',

chromeos/dbus/fake_update_engine_client.cc

@@ -10,6 +10,7 @@ FakeUpdateEngineClient::FakeUpdateEngineClient()
     : update_check_result_(UpdateEngineClient::UPDATE_RESULT_SUCCESS),
       can_rollback_stub_result_(false),
       reboot_after_update_call_count_(0),
+      request_update_check_call_count_(0),
       rollback_call_count_(0),
       can_rollback_call_count_(0) {
 }
@@ -29,11 +30,12 @@ void FakeUpdateEngineClient::RemoveObserver(Observer* observer) {
 }
 
 bool FakeUpdateEngineClient::HasObserver(Observer* observer) {
-  return false;
+  return observers_.HasObserver(observer);
 }
 
 void FakeUpdateEngineClient::RequestUpdateCheck(
     const UpdateCheckCallback& callback) {
+  request_update_check_call_count_++;
   callback.Run(update_check_result_);
 }
 

chromeos/dbus/fake_update_engine_client.h

@@ -65,6 +65,11 @@ class FakeUpdateEngineClient : public UpdateEngineClient {
       return reboot_after_update_call_count_;
   }
 
+  // Returns how many times RequestUpdateCheck() is called.
+  int request_update_check_call_count() const {
+    return request_update_check_call_count_;
+  }
+
   // Returns how many times Rollback() is called.
   int rollback_call_count() const { return rollback_call_count_; }
 
@@ -78,6 +83,7 @@ class FakeUpdateEngineClient : public UpdateEngineClient {
   UpdateEngineClient::UpdateCheckResult update_check_result_;
   bool can_rollback_stub_result_;
   int reboot_after_update_call_count_;
+  int request_update_check_call_count_;
   int rollback_call_count_;
   int can_rollback_call_count_;
 };