Commit 35ee38bc authored by danielng's avatar danielng Committed by Commit Bot

borealis: Installer changes for tests

Some minor changes for cleaning up some of the installer code and
making it more testable. Unit tests being introduced here CL:2392163
and browser tests here CL:2392285.

Tests: Tested manually on my test device and ran the new tests.
Bug: b:161650651

Change-Id: Ia241ec790096b33b56b54fcb579b08eef3a9ace8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391891
Commit-Queue: Daniel Ng <danielng@google.com>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarNic Hollingum <hollingum@google.com>
Cr-Commit-Position: refs/heads/master@{#804857}
parent e21b2cf3
...@@ -5621,7 +5621,7 @@ ...@@ -5621,7 +5621,7 @@
Your device is low on storage. To increase free space, delete files from device. Your device is low on storage. To increase free space, delete files from device.
</message> </message>
<message name="IDS_BOREALIS_GENERIC_ERROR_MESSAGE" desc="" translateable="false"> <message name="IDS_BOREALIS_GENERIC_ERROR_MESSAGE" desc="" translateable="false">
Couldn't install <ph name="APP_NAME">$1<ex>BOREALIS</ex></ph>. Please try again later. Couldn't install <ph name="APP_NAME">$1<ex>BOREALIS</ex></ph>. Please try again later. Error code: <ph name="ERROR_CODE">$2<ex>7</ex></ph>.
</message> </message>
<!-- <!--
================================================================================== ==================================================================================
......
...@@ -3114,8 +3114,6 @@ source_set("unit_tests") { ...@@ -3114,8 +3114,6 @@ source_set("unit_tests") {
"authpolicy/authpolicy_helper.unittest.cc", "authpolicy/authpolicy_helper.unittest.cc",
"base/file_flusher_unittest.cc", "base/file_flusher_unittest.cc",
"bluetooth/debug_logs_manager_unittest.cc", "bluetooth/debug_logs_manager_unittest.cc",
"borealis/borealis_installer_mock.cc",
"borealis/borealis_installer_mock.h",
"cert_provisioning/cert_provisioning_invalidator_unittest.cc", "cert_provisioning/cert_provisioning_invalidator_unittest.cc",
"cert_provisioning/cert_provisioning_platform_keys_helpers_unittest.cc", "cert_provisioning/cert_provisioning_platform_keys_helpers_unittest.cc",
"cert_provisioning/cert_provisioning_scheduler_unittest.cc", "cert_provisioning/cert_provisioning_scheduler_unittest.cc",
......
...@@ -20,14 +20,4 @@ std::string BorealisInstaller::GetInstallingStateName(InstallingState state) { ...@@ -20,14 +20,4 @@ std::string BorealisInstaller::GetInstallingStateName(InstallingState state) {
} }
} }
void BorealisInstaller::AddObserver(Observer* observer) {
DCHECK(!observer);
observers_.AddObserver(observer);
}
void BorealisInstaller::RemoveObserver(Observer* observer) {
DCHECK(!observer);
observers_.RemoveObserver(observer);
}
} // namespace borealis } // namespace borealis
...@@ -55,8 +55,8 @@ class BorealisInstaller : public KeyedService { ...@@ -55,8 +55,8 @@ class BorealisInstaller : public KeyedService {
// Cancels the installation process. // Cancels the installation process.
virtual void Cancel() = 0; virtual void Cancel() = 0;
void AddObserver(Observer* observer); virtual void AddObserver(Observer* observer) = 0;
void RemoveObserver(Observer* observer); virtual void RemoveObserver(Observer* observer) = 0;
protected: protected:
~BorealisInstaller() override; ~BorealisInstaller() override;
......
...@@ -43,12 +43,22 @@ void BorealisInstallerImpl::Cancel() { ...@@ -43,12 +43,22 @@ void BorealisInstallerImpl::Cancel() {
} }
} }
void BorealisInstallerImpl::AddObserver(Observer* observer) {
DCHECK(observer);
observers_.AddObserver(observer);
}
void BorealisInstallerImpl::RemoveObserver(Observer* observer) {
DCHECK(observer);
observers_.RemoveObserver(observer);
}
void BorealisInstallerImpl::StartDlcInstallation() { void BorealisInstallerImpl::StartDlcInstallation() {
state_ = State::kInstalling; state_ = State::kInstalling;
UpdateInstallingState(InstallingState::kInstallingDlc); UpdateInstallingState(InstallingState::kInstallingDlc);
chromeos::DlcserviceClient::Get()->Install( chromeos::DlcserviceClient::Get()->Install(
"borealis-dlc", kBorealisDlcName,
base::BindOnce(&BorealisInstallerImpl::OnDlcInstallationCompleted, base::BindOnce(&BorealisInstallerImpl::OnDlcInstallationCompleted,
weak_ptr_factory_.GetWeakPtr()), weak_ptr_factory_.GetWeakPtr()),
base::BindRepeating( base::BindRepeating(
...@@ -57,8 +67,13 @@ void BorealisInstallerImpl::StartDlcInstallation() { ...@@ -57,8 +67,13 @@ void BorealisInstallerImpl::StartDlcInstallation() {
} }
void BorealisInstallerImpl::InstallationEnded(InstallationResult result) { void BorealisInstallerImpl::InstallationEnded(InstallationResult result) {
state_ = State::kIdle; // If another installation is in progress, we don't want to reset any states
installing_state_ = InstallingState::kInactive; // and interfere with the process. When that process completes, it will reset
// these states.
if (result != InstallationResult::kOperationInProgress) {
state_ = State::kIdle;
installing_state_ = InstallingState::kInactive;
}
for (auto& observer : observers_) { for (auto& observer : observers_) {
observer.OnInstallationEnded(result); observer.OnInstallationEnded(result);
} }
...@@ -119,10 +134,6 @@ void BorealisInstallerImpl::OnDlcInstallationCompleted( ...@@ -119,10 +134,6 @@ void BorealisInstallerImpl::OnDlcInstallationCompleted(
const chromeos::DlcserviceClient::InstallResult& install_result) { const chromeos::DlcserviceClient::InstallResult& install_result) {
DCHECK_EQ(installing_state_, InstallingState::kInstallingDlc); DCHECK_EQ(installing_state_, InstallingState::kInstallingDlc);
if (state_ == State::kCancelling) { if (state_ == State::kCancelling) {
// Since DLC installation is currently the only step of installation,
// Borealis has actually been installed by this stage. This is calling
// CancelFinished() instead of InstallFinished(), to help provide clarity
// and also make it easier to add new installation steps in the future.
InstallationEnded(InstallationResult::kCancelled); InstallationEnded(InstallationResult::kCancelled);
return; return;
} }
...@@ -149,7 +160,7 @@ void BorealisInstallerImpl::OnDlcInstallationCompleted( ...@@ -149,7 +160,7 @@ void BorealisInstallerImpl::OnDlcInstallationCompleted(
} else if (install_result.error == dlcservice::kErrorNeedReboot) { } else if (install_result.error == dlcservice::kErrorNeedReboot) {
LOG(ERROR) LOG(ERROR)
<< "Device has pending update and needs a reboot to use Borealis DLC."; << "Device has pending update and needs a reboot to use Borealis DLC.";
result = InstallationResult::kDlcBusy; result = InstallationResult::kDlcNeedReboot;
} else if (install_result.error == dlcservice::kErrorAllocation) { } else if (install_result.error == dlcservice::kErrorAllocation) {
LOG(ERROR) << "Device needs to free space to use Borealis DLC."; LOG(ERROR) << "Device needs to free space to use Borealis DLC.";
result = InstallationResult::kDlcNeedSpace; result = InstallationResult::kDlcNeedSpace;
......
...@@ -29,6 +29,9 @@ class BorealisInstallerImpl : public BorealisInstaller { ...@@ -29,6 +29,9 @@ class BorealisInstallerImpl : public BorealisInstaller {
// Cancels the installation process. // Cancels the installation process.
void Cancel() override; void Cancel() override;
void AddObserver(Observer* observer) override;
void RemoveObserver(Observer* observer) override;
private: private:
enum class State { enum class State {
kIdle, kIdle,
......
// Copyright 2020 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/chromeos/borealis/borealis_installer_mock.h"
namespace borealis {
namespace test {
BorealisInstallerMock::BorealisInstallerMock() = default;
BorealisInstallerMock::~BorealisInstallerMock() = default;
} // namespace test
} // namespace borealis
// Copyright 2020 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.
#ifndef CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_INSTALLER_MOCK_H_
#define CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_INSTALLER_MOCK_H_
#include "chrome/browser/chromeos/borealis/borealis_installer.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace borealis {
namespace test {
class BorealisInstallerMock : public BorealisInstaller {
public:
BorealisInstallerMock();
~BorealisInstallerMock();
BorealisInstallerMock(const BorealisInstallerMock&) = delete;
BorealisInstallerMock& operator=(const BorealisInstallerMock&) = delete;
MOCK_METHOD(bool, IsProcessing, (), ());
MOCK_METHOD(void, Start, (), ());
MOCK_METHOD(void, Cancel, (), ());
};
} // namespace test
} // namespace borealis
#endif // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_INSTALLER_MOCK_H_
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
namespace borealis { namespace borealis {
const char kBorealisAppId[] = "dkecggknbdokeipkgnhifhiokailichf"; const char kBorealisAppId[] = "dkecggknbdokeipkgnhifhiokailichf";
const char kBorealisDlcName[] = "borealis-dlc";
bool IsBorealisAllowed() { bool IsBorealisAllowed() {
// Check that the Borealis feature is enabled. // Check that the Borealis feature is enabled.
......
...@@ -12,6 +12,8 @@ namespace borealis { ...@@ -12,6 +12,8 @@ namespace borealis {
// This is used by the Borealis app and the Borealis installer. // This is used by the Borealis app and the Borealis installer.
// Generated by crx_file::id_util::GenerateId("org.chromium.borealis"); // Generated by crx_file::id_util::GenerateId("org.chromium.borealis");
extern const char kBorealisAppId[]; extern const char kBorealisAppId[];
// This is used to install the Borealis DLC component.
extern const char kBorealisDlcName[];
// Checks if Borealis is allowed to run in the current environment. // Checks if Borealis is allowed to run in the current environment.
bool IsBorealisAllowed(); bool IsBorealisAllowed();
......
...@@ -257,6 +257,7 @@ base::string16 BorealisInstallerView::GetPrimaryMessage() const { ...@@ -257,6 +257,7 @@ base::string16 BorealisInstallerView::GetPrimaryMessage() const {
DCHECK(result_); DCHECK(result_);
switch (*result_) { switch (*result_) {
case borealis::BorealisInstaller::InstallationResult::kNotAllowed: case borealis::BorealisInstaller::InstallationResult::kNotAllowed:
case borealis::BorealisInstaller::InstallationResult::kDlcUnsupported:
return l10n_util::GetStringFUTF16( return l10n_util::GetStringFUTF16(
IDS_BOREALIS_INSTALLER_NOT_ALLOWED_TITLE, app_name_); IDS_BOREALIS_INSTALLER_NOT_ALLOWED_TITLE, app_name_);
default: default:
...@@ -303,15 +304,22 @@ base::string16 BorealisInstallerView::GetSecondaryMessage() const { ...@@ -303,15 +304,22 @@ base::string16 BorealisInstallerView::GetSecondaryMessage() const {
return l10n_util::GetStringFUTF16( return l10n_util::GetStringFUTF16(
IDS_BOREALIS_DLC_NEED_REBOOT_FAILED_MESSAGE, app_name_); IDS_BOREALIS_DLC_NEED_REBOOT_FAILED_MESSAGE, app_name_);
case ResultEnum::kDlcNeedSpace: case ResultEnum::kDlcNeedSpace:
return l10n_util::GetStringFUTF16( return l10n_util::GetStringUTF16(
IDS_BOREALIS_INSUFFICIENT_DISK_SPACE_MESSAGE, app_name_); IDS_BOREALIS_INSUFFICIENT_DISK_SPACE_MESSAGE);
case ResultEnum::kDlcUnknown: case ResultEnum::kDlcUnknown:
return l10n_util::GetStringFUTF16(IDS_BOREALIS_GENERIC_ERROR_MESSAGE, return l10n_util::GetStringFUTF16(
app_name_); IDS_BOREALIS_GENERIC_ERROR_MESSAGE, app_name_,
base::NumberToString16(
static_cast<std::underlying_type_t<ResultEnum>>(*result_)));
} }
} }
} }
void BorealisInstallerView::SetInstallingStateForTesting(
InstallingState new_state) {
installing_state_ = new_state;
}
int BorealisInstallerView::GetCurrentDialogButtons() const { int BorealisInstallerView::GetCurrentDialogButtons() const {
switch (state_) { switch (state_) {
case State::kInstalling: case State::kInstalling:
......
...@@ -24,6 +24,8 @@ class Profile; ...@@ -24,6 +24,8 @@ class Profile;
class BorealisInstallerView : public views::DialogDelegateView, class BorealisInstallerView : public views::DialogDelegateView,
public borealis::BorealisInstaller::Observer { public borealis::BorealisInstaller::Observer {
public: public:
using InstallingState = borealis::BorealisInstaller::InstallingState;
explicit BorealisInstallerView(Profile* profile); explicit BorealisInstallerView(Profile* profile);
// Disallow copy and assign. // Disallow copy and assign.
...@@ -51,6 +53,8 @@ class BorealisInstallerView : public views::DialogDelegateView, ...@@ -51,6 +53,8 @@ class BorealisInstallerView : public views::DialogDelegateView,
base::string16 GetPrimaryMessage() const; base::string16 GetPrimaryMessage() const;
base::string16 GetSecondaryMessage() const; base::string16 GetSecondaryMessage() const;
void SetInstallingStateForTesting(InstallingState new_state);
private: private:
class TitleLabel; class TitleLabel;
enum class State { enum class State {
...@@ -60,8 +64,6 @@ class BorealisInstallerView : public views::DialogDelegateView, ...@@ -60,8 +64,6 @@ class BorealisInstallerView : public views::DialogDelegateView,
kError, // Something unexpected happened. kError, // Something unexpected happened.
}; };
using InstallingState = borealis::BorealisInstaller::InstallingState;
~BorealisInstallerView() override; ~BorealisInstallerView() override;
// Returns the dialog buttons that should be displayed, based on the current // Returns the dialog buttons that should be displayed, based on the current
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment