Commit 6d80bc1f authored by danielng's avatar danielng Committed by Commit Bot

borealis: emitting startup metrics

Using the metrics added in earlier to track information about the
start-up flow for borealis.

Cq-Depend: 2509072
Bug: b/171339461
Change-Id: If0e0f3c212ab1ff4cfddf43b10104f453a99e0e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517347
Commit-Queue: Daniel Ng <danielng@google.com>
Reviewed-by: default avatarNic Hollingum <hollingum@google.com>
Cr-Commit-Position: refs/heads/master@{#825740}
parent 9187fd7a
...@@ -7,27 +7,27 @@ ...@@ -7,27 +7,27 @@
namespace borealis { namespace borealis {
BorealisContextManager::Result::Result(const BorealisContext* ctx) BorealisContextManager::Result::Result(const BorealisContext* ctx)
: status_(Status::kSuccess), failure_reason_(), ctx_(ctx) { : result_(BorealisStartupResult::kSuccess), failure_reason_(), ctx_(ctx) {
DCHECK(ctx); DCHECK(ctx);
} }
BorealisContextManager::Result::Result(Status status, BorealisContextManager::Result::Result(BorealisStartupResult result,
std::string failure_reason) std::string failure_reason)
: status_(status), : result_(result),
failure_reason_(std::move(failure_reason)), failure_reason_(std::move(failure_reason)),
ctx_(nullptr) { ctx_(nullptr) {
DCHECK(status != Status::kSuccess); DCHECK(result != BorealisStartupResult::kSuccess);
} }
BorealisContextManager::Result::~Result() = default; BorealisContextManager::Result::~Result() = default;
bool BorealisContextManager::Result::Ok() const { bool BorealisContextManager::Result::Ok() const {
return status_ == Status::kSuccess; return result_ == BorealisStartupResult::kSuccess;
} }
BorealisContextManager::Status BorealisContextManager::Result::Failure() const { BorealisStartupResult BorealisContextManager::Result::Failure() const {
DCHECK(!Ok()); DCHECK(!Ok());
return status_; return result_;
} }
const std::string& BorealisContextManager::Result::FailureReason() const { const std::string& BorealisContextManager::Result::FailureReason() const {
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <string> #include <string>
#include "base/callback.h" #include "base/callback.h"
#include "chrome/browser/chromeos/borealis/borealis_metrics.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
namespace borealis { namespace borealis {
...@@ -16,33 +17,23 @@ class BorealisContext; ...@@ -16,33 +17,23 @@ class BorealisContext;
class BorealisContextManager : public KeyedService { class BorealisContextManager : public KeyedService {
public: public:
// A list of possible outcomes for an attempt to startup borealis.
enum class Status {
kSuccess,
kCancelled,
kMountFailed,
kDiskImageFailed,
kStartVmFailed,
kAwaitBorealisStartupFailed,
};
// An attempt to launch borealis. If the launch succeeds, holds a reference to // An attempt to launch borealis. If the launch succeeds, holds a reference to
// the context created for that launch, otherwise holds an error. // the context created for that launch, otherwise holds an error.
class Result { class Result {
public: public:
// Used to indicate the the result was a success. // Used to indicate that the result was a success.
explicit Result(const BorealisContext* ctx); explicit Result(const BorealisContext* ctx);
// Used to indicate the the result was a failure. // Used to indicate the the result was a failure.
Result(Status status, std::string failure_reason); Result(BorealisStartupResult result, std::string failure_reason);
~Result(); ~Result();
// Returns true if the result was successful. // Returns true if the result was successful.
bool Ok() const; bool Ok() const;
// In the event of a failed launch, returns the status code for that error. // In the event of a failed launch, returns the result code for that error.
Status Failure() const; BorealisStartupResult Failure() const;
// In the event of a failed launch, returns the message provided. // In the event of a failed launch, returns the message provided.
const std::string& FailureReason() const; const std::string& FailureReason() const;
...@@ -52,7 +43,7 @@ class BorealisContextManager : public KeyedService { ...@@ -52,7 +43,7 @@ class BorealisContextManager : public KeyedService {
const BorealisContext& Success() const; const BorealisContext& Success() const;
private: private:
Status status_; BorealisStartupResult result_;
std::string failure_reason_; std::string failure_reason_;
const BorealisContext* ctx_; const BorealisContext* ctx_;
}; };
...@@ -71,7 +62,7 @@ class BorealisContextManager : public KeyedService { ...@@ -71,7 +62,7 @@ class BorealisContextManager : public KeyedService {
// Stop the current running state, re-initializing the context manager // Stop the current running state, re-initializing the context manager
// to the state it was in prior to being started. All pending callbacks are // to the state it was in prior to being started. All pending callbacks are
// invoked with kCancelled status. // invoked with kCancelled result.
virtual void ShutDownBorealis() = 0; virtual void ShutDownBorealis() = 0;
}; };
......
...@@ -20,24 +20,6 @@ namespace { ...@@ -20,24 +20,6 @@ namespace {
// need to determine the name instead. // need to determine the name instead.
constexpr char kBorealisVmName[] = "borealis"; constexpr char kBorealisVmName[] = "borealis";
std::ostream& operator<<(std::ostream& stream,
borealis::BorealisContextManager::Status status) {
switch (status) {
case borealis::BorealisContextManager::Status::kSuccess:
return stream << "Success";
case borealis::BorealisContextManager::Status::kCancelled:
return stream << "Cancelled";
case borealis::BorealisContextManager::Status::kMountFailed:
return stream << "Mount Failed";
case borealis::BorealisContextManager::Status::kDiskImageFailed:
return stream << "Disk Image Failed";
case borealis::BorealisContextManager::Status::kStartVmFailed:
return stream << "Start VM Failed";
case borealis::BorealisContextManager::Status::kAwaitBorealisStartupFailed:
return stream << "Await Borealis Startup Failed";
}
}
} // namespace } // namespace
namespace borealis { namespace borealis {
...@@ -57,6 +39,8 @@ void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) { ...@@ -57,6 +39,8 @@ void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) {
context_ = base::WrapUnique(new BorealisContext(profile_)); context_ = base::WrapUnique(new BorealisContext(profile_));
context_->set_vm_name(kBorealisVmName); context_->set_vm_name(kBorealisVmName);
task_queue_ = GetTasks(); task_queue_ = GetTasks();
startup_start_tick_ = base::TimeTicks::Now();
RecordBorealisStartupNumAttemptsHistogram();
NextTask(); NextTask();
} }
} }
...@@ -82,7 +66,7 @@ void BorealisContextManagerImpl::ShutDownBorealis() { ...@@ -82,7 +66,7 @@ void BorealisContextManagerImpl::ShutDownBorealis() {
<< response.value().failure_reason(); << response.value().failure_reason();
} }
})); }));
Complete(BorealisContextManager::Status::kCancelled, "shut down"); Complete(BorealisStartupResult::kCancelled, "shut down");
} }
base::queue<std::unique_ptr<BorealisTask>> base::queue<std::unique_ptr<BorealisTask>>
...@@ -102,7 +86,9 @@ void BorealisContextManagerImpl::AddCallback(ResultCallback callback) { ...@@ -102,7 +86,9 @@ void BorealisContextManagerImpl::AddCallback(ResultCallback callback) {
void BorealisContextManagerImpl::NextTask() { void BorealisContextManagerImpl::NextTask() {
if (task_queue_.empty()) { if (task_queue_.empty()) {
Complete(Status::kSuccess, ""); RecordBorealisStartupOverallTimeHistogram(base::TimeTicks::Now() -
startup_start_tick_);
Complete(BorealisStartupResult::kSuccess, "");
return; return;
} }
task_queue_.front()->Run( task_queue_.front()->Run(
...@@ -110,21 +96,22 @@ void BorealisContextManagerImpl::NextTask() { ...@@ -110,21 +96,22 @@ void BorealisContextManagerImpl::NextTask() {
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
} }
void BorealisContextManagerImpl::TaskCallback(Status status, void BorealisContextManagerImpl::TaskCallback(BorealisStartupResult result,
std::string error) { std::string error) {
task_queue_.pop(); task_queue_.pop();
if (status == Status::kSuccess) { if (result == BorealisStartupResult::kSuccess) {
NextTask(); NextTask();
return; return;
} }
LOG(ERROR) << "Startup failed: failure=" << status << " message=" << error; LOG(ERROR) << "Startup failed: failure=" << result << " message=" << error;
Complete(status, std::move(error)); Complete(result, std::move(error));
} }
void BorealisContextManagerImpl::Complete(BorealisContextManager::Status status, void BorealisContextManagerImpl::Complete(BorealisStartupResult result,
std::string error_or_empty) { std::string error_or_empty) {
startup_status_ = status; startup_result_ = result;
startup_error_ = error_or_empty; startup_error_ = error_or_empty;
RecordBorealisStartupResultHistogram(result);
while (!callback_queue_.empty()) { while (!callback_queue_.empty()) {
ResultCallback callback = std::move(callback_queue_.front()); ResultCallback callback = std::move(callback_queue_.front());
...@@ -132,7 +119,7 @@ void BorealisContextManagerImpl::Complete(BorealisContextManager::Status status, ...@@ -132,7 +119,7 @@ void BorealisContextManagerImpl::Complete(BorealisContextManager::Status status,
std::move(callback).Run(GetResult()); std::move(callback).Run(GetResult());
} }
if (startup_status_ == BorealisContextManager::Status::kSuccess) if (startup_result_ == BorealisStartupResult::kSuccess)
return; return;
task_queue_ = {}; task_queue_ = {};
...@@ -141,10 +128,10 @@ void BorealisContextManagerImpl::Complete(BorealisContextManager::Status status, ...@@ -141,10 +128,10 @@ void BorealisContextManagerImpl::Complete(BorealisContextManager::Status status,
} }
BorealisContextManager::Result BorealisContextManagerImpl::GetResult() { BorealisContextManager::Result BorealisContextManagerImpl::GetResult() {
if (startup_status_ == Status::kSuccess) { if (startup_result_ == BorealisStartupResult::kSuccess) {
return BorealisContextManager::Result(context_.get()); return BorealisContextManager::Result(context_.get());
} }
return BorealisContextManager::Result(startup_status_, startup_error_); return BorealisContextManager::Result(startup_result_, startup_error_);
} }
} // namespace borealis } // namespace borealis
...@@ -37,20 +37,20 @@ class BorealisContextManagerImpl : public BorealisContextManager { ...@@ -37,20 +37,20 @@ class BorealisContextManagerImpl : public BorealisContextManager {
private: private:
void AddCallback(ResultCallback callback); void AddCallback(ResultCallback callback);
void NextTask(); void NextTask();
void TaskCallback(Status status, std::string error); void TaskCallback(BorealisStartupResult result, std::string error);
// Completes the startup with the given |status| and error messgae, invoking // Completes the startup with the given |result| and error messgae, invoking
// all callbacks with the result. For any status except kSuccess the state of // all callbacks with the result. For any result except kSuccess the state of
// the system will be as though StartBorealis() had not been called. // the system will be as though StartBorealis() had not been called.
void Complete(BorealisContextManager::Status status, void Complete(BorealisStartupResult result, std::string error_or_empty);
std::string error_or_empty);
// Returns the result of the startup (i.e. the context if it succeeds, or an // Returns the result of the startup (i.e. the context if it succeeds, or an
// error if it doesn't). // error if it doesn't).
BorealisContextManager::Result GetResult(); BorealisContextManager::Result GetResult();
Profile* profile_ = nullptr; Profile* profile_ = nullptr;
BorealisContextManager::Status startup_status_ = Status::kSuccess; BorealisStartupResult startup_result_ = BorealisStartupResult::kSuccess;
base::TimeTicks startup_start_tick_;
std::string startup_error_; std::string startup_error_;
std::unique_ptr<BorealisContext> context_; std::unique_ptr<BorealisContext> context_;
base::queue<ResultCallback> callback_queue_; base::queue<ResultCallback> callback_queue_;
......
...@@ -10,8 +10,10 @@ ...@@ -10,8 +10,10 @@
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/containers/queue.h" #include "base/containers/queue.h"
#include "base/test/bind.h" #include "base/test/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h"
#include "chrome/browser/chromeos/borealis/borealis_metrics.h"
#include "chrome/browser/chromeos/borealis/borealis_task.h" #include "chrome/browser/chromeos/borealis/borealis_task.h"
#include "chrome/browser/chromeos/login/users/mock_user_manager.h" #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -30,7 +32,7 @@ MATCHER(IsSuccessResult, "") { ...@@ -30,7 +32,7 @@ MATCHER(IsSuccessResult, "") {
MATCHER(IsFailureResult, "") { MATCHER(IsFailureResult, "") {
return !arg.Ok() && return !arg.Ok() &&
arg.Failure() == BorealisContextManager::Status::kStartVmFailed && arg.Failure() == borealis::BorealisStartupResult::kStartVmFailed &&
arg.FailureReason() == "Something went wrong!"; arg.FailureReason() == "Something went wrong!";
} }
...@@ -40,10 +42,10 @@ class MockTask : public BorealisTask { ...@@ -40,10 +42,10 @@ class MockTask : public BorealisTask {
void RunInternal(BorealisContext* context) override { void RunInternal(BorealisContext* context) override {
if (success_) { if (success_) {
context->set_vm_name("test_vm_name"); context->set_vm_name("test_vm_name");
Complete(BorealisContextManager::Status::kSuccess, ""); Complete(borealis::BorealisStartupResult::kSuccess, "");
} else { } else {
// Just use a random error. // Just use a random error.
Complete(BorealisContextManager::Status::kStartVmFailed, Complete(borealis::BorealisStartupResult::kStartVmFailed,
"Something went wrong!"); "Something went wrong!");
} }
} }
...@@ -97,16 +99,18 @@ class BorealisContextManagerTest : public testing::Test { ...@@ -97,16 +99,18 @@ class BorealisContextManagerTest : public testing::Test {
void SetUp() override { void SetUp() override {
CreateProfile(); CreateProfile();
chromeos::DBusThreadManager::Initialize(); chromeos::DBusThreadManager::Initialize();
histogram_tester_ = std::make_unique<base::HistogramTester>();
} }
void TearDown() override { void TearDown() override {
chromeos::DBusThreadManager::Shutdown(); chromeos::DBusThreadManager::Shutdown();
profile_.reset(); profile_.reset();
histogram_tester_.reset();
} }
content::BrowserTaskEnvironment task_environment_; content::BrowserTaskEnvironment task_environment_;
std::unique_ptr<TestingProfile> profile_; std::unique_ptr<TestingProfile> profile_;
std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_; std::unique_ptr<base::HistogramTester> histogram_tester_;
private: private:
void CreateProfile() { void CreateProfile() {
...@@ -203,6 +207,31 @@ TEST_F(BorealisContextManagerTest, ...@@ -203,6 +207,31 @@ TEST_F(BorealisContextManagerTest,
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
} }
TEST_F(BorealisContextManagerTest, StartupSucceedsMetricsRecorded) {
BorealisContextManagerImplForTesting context_manager(
profile_.get(), /*tasks=*/1, /*success=*/true);
context_manager.StartBorealis(base::DoNothing());
task_environment_.RunUntilIdle();
histogram_tester_->ExpectTotalCount(kBorealisStartupNumAttemptsHistogram, 1);
histogram_tester_->ExpectUniqueSample(kBorealisStartupResultHistogram,
BorealisStartupResult::kSuccess, 1);
histogram_tester_->ExpectTotalCount(kBorealisStartupOverallTimeHistogram, 1);
}
TEST_F(BorealisContextManagerTest, StartupFailsMetricsRecorded) {
BorealisContextManagerImplForTesting context_manager(
profile_.get(), /*tasks=*/1, /*success=*/false);
context_manager.StartBorealis(base::DoNothing());
task_environment_.RunUntilIdle();
histogram_tester_->ExpectTotalCount(kBorealisStartupNumAttemptsHistogram, 1);
histogram_tester_->ExpectUniqueSample(kBorealisStartupResultHistogram,
BorealisStartupResult::kStartVmFailed,
1);
histogram_tester_->ExpectTotalCount(kBorealisStartupOverallTimeHistogram, 0);
}
class NeverCompletingContextManager : public BorealisContextManagerImpl { class NeverCompletingContextManager : public BorealisContextManagerImpl {
public: public:
explicit NeverCompletingContextManager(Profile* profile) explicit NeverCompletingContextManager(Profile* profile)
...@@ -226,7 +255,7 @@ TEST_F(BorealisContextManagerTest, ShutDownCancelsRequestsAndTerminatesVm) { ...@@ -226,7 +255,7 @@ TEST_F(BorealisContextManagerTest, ShutDownCancelsRequestsAndTerminatesVm) {
EXPECT_CALL(callback_expectation, Callback(testing::_)) EXPECT_CALL(callback_expectation, Callback(testing::_))
.WillOnce(testing::Invoke([](BorealisContextManager::Result result) { .WillOnce(testing::Invoke([](BorealisContextManager::Result result) {
EXPECT_FALSE(result.Ok()); EXPECT_FALSE(result.Ok());
EXPECT_EQ(result.Failure(), BorealisContextManager::Status::kCancelled); EXPECT_EQ(result.Failure(), BorealisStartupResult::kCancelled);
})); }));
NeverCompletingContextManager context_manager(profile_.get()); NeverCompletingContextManager context_manager(profile_.get());
...@@ -256,7 +285,7 @@ class TaskThatDoesSomethingAfterCompletion : public BorealisTask { ...@@ -256,7 +285,7 @@ class TaskThatDoesSomethingAfterCompletion : public BorealisTask {
: something_(std::move(something)) {} : something_(std::move(something)) {}
void RunInternal(BorealisContext* context) override { void RunInternal(BorealisContext* context) override {
Complete(BorealisContextManager::Status::kSuccess, ""); Complete(BorealisStartupResult::kSuccess, "");
std::move(something_).Run(); std::move(something_).Run();
} }
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "chrome/browser/chromeos/borealis/borealis_metrics.h" #include "chrome/browser/chromeos/borealis/borealis_metrics.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
namespace borealis { namespace borealis {
...@@ -47,3 +48,21 @@ void RecordBorealisStartupOverallTimeHistogram(base::TimeDelta startup_time) { ...@@ -47,3 +48,21 @@ void RecordBorealisStartupOverallTimeHistogram(base::TimeDelta startup_time) {
} }
} // namespace borealis } // namespace borealis
std::ostream& operator<<(std::ostream& stream,
borealis::BorealisStartupResult result) {
switch (result) {
case borealis::BorealisStartupResult::kSuccess:
return stream << "Success";
case borealis::BorealisStartupResult::kCancelled:
return stream << "Cancelled";
case borealis::BorealisStartupResult::kMountFailed:
return stream << "Mount Failed";
case borealis::BorealisStartupResult::kDiskImageFailed:
return stream << "Disk Image Failed";
case borealis::BorealisStartupResult::kStartVmFailed:
return stream << "Start VM Failed";
case borealis::BorealisStartupResult::kAwaitBorealisStartupFailed:
return stream << "Await Borealis Startup Failed";
}
}
...@@ -53,4 +53,7 @@ void RecordBorealisStartupOverallTimeHistogram(base::TimeDelta startup_time); ...@@ -53,4 +53,7 @@ void RecordBorealisStartupOverallTimeHistogram(base::TimeDelta startup_time);
} // namespace borealis } // namespace borealis
std::ostream& operator<<(std::ostream& stream,
borealis::BorealisStartupResult result);
#endif // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_METRICS_H_ #endif // CHROME_BROWSER_CHROMEOS_BOREALIS_BOREALIS_METRICS_H_
...@@ -24,13 +24,12 @@ BorealisTask::BorealisTask() = default; ...@@ -24,13 +24,12 @@ BorealisTask::BorealisTask() = default;
BorealisTask::~BorealisTask() = default; BorealisTask::~BorealisTask() = default;
void BorealisTask::Run(BorealisContext* context, void BorealisTask::Run(BorealisContext* context,
CompletionStatusCallback callback) { CompletionResultCallback callback) {
callback_ = std::move(callback); callback_ = std::move(callback);
RunInternal(context); RunInternal(context);
} }
void BorealisTask::Complete(BorealisContextManager::Status status, void BorealisTask::Complete(BorealisStartupResult status, std::string message) {
std::string message) {
// Task completion is self-mutually-exclusive, because tasks are deleted once // Task completion is self-mutually-exclusive, because tasks are deleted once
// complete. // complete.
base::SequencedTaskRunnerHandle::Get()->PostTask( base::SequencedTaskRunnerHandle::Get()->PostTask(
...@@ -55,11 +54,11 @@ void MountDlc::OnMountDlc( ...@@ -55,11 +54,11 @@ void MountDlc::OnMountDlc(
BorealisContext* context, BorealisContext* context,
const chromeos::DlcserviceClient::InstallResult& install_result) { const chromeos::DlcserviceClient::InstallResult& install_result) {
if (install_result.error != dlcservice::kErrorNone) { if (install_result.error != dlcservice::kErrorNone) {
Complete(BorealisContextManager::Status::kMountFailed, Complete(BorealisStartupResult::kMountFailed,
"Mounting the DLC for Borealis failed: " + install_result.error); "Mounting the DLC for Borealis failed: " + install_result.error);
} else { } else {
context->set_root_path(install_result.root_path); context->set_root_path(install_result.root_path);
Complete(BorealisContextManager::Status::kSuccess, ""); Complete(BorealisStartupResult::kSuccess, "");
} }
} }
...@@ -85,7 +84,7 @@ void CreateDiskImage::OnCreateDiskImage( ...@@ -85,7 +84,7 @@ void CreateDiskImage::OnCreateDiskImage(
base::Optional<vm_tools::concierge::CreateDiskImageResponse> response) { base::Optional<vm_tools::concierge::CreateDiskImageResponse> response) {
if (!response) { if (!response) {
context->set_disk_path(base::FilePath()); context->set_disk_path(base::FilePath());
Complete(BorealisContextManager::Status::kDiskImageFailed, Complete(BorealisStartupResult::kDiskImageFailed,
"Failed to create disk image for Borealis: Empty response."); "Failed to create disk image for Borealis: Empty response.");
return; return;
} }
...@@ -93,13 +92,13 @@ void CreateDiskImage::OnCreateDiskImage( ...@@ -93,13 +92,13 @@ void CreateDiskImage::OnCreateDiskImage(
if (response->status() != vm_tools::concierge::DISK_STATUS_EXISTS && if (response->status() != vm_tools::concierge::DISK_STATUS_EXISTS &&
response->status() != vm_tools::concierge::DISK_STATUS_CREATED) { response->status() != vm_tools::concierge::DISK_STATUS_CREATED) {
context->set_disk_path(base::FilePath()); context->set_disk_path(base::FilePath());
Complete(BorealisContextManager::Status::kDiskImageFailed, Complete(BorealisStartupResult::kDiskImageFailed,
"Failed to create disk image for Borealis: " + "Failed to create disk image for Borealis: " +
response->failure_reason()); response->failure_reason());
return; return;
} }
context->set_disk_path(base::FilePath(response->disk_path())); context->set_disk_path(base::FilePath(response->disk_path()));
Complete(BorealisContextManager::Status::kSuccess, ""); Complete(BorealisStartupResult::kSuccess, "");
} }
StartBorealisVm::StartBorealisVm() = default; StartBorealisVm::StartBorealisVm() = default;
...@@ -140,18 +139,18 @@ void StartBorealisVm::OnStartBorealisVm( ...@@ -140,18 +139,18 @@ void StartBorealisVm::OnStartBorealisVm(
BorealisContext* context, BorealisContext* context,
base::Optional<vm_tools::concierge::StartVmResponse> response) { base::Optional<vm_tools::concierge::StartVmResponse> response) {
if (!response) { if (!response) {
Complete(BorealisContextManager::Status::kStartVmFailed, Complete(BorealisStartupResult::kStartVmFailed,
"Failed to start Borealis VM: Empty response."); "Failed to start Borealis VM: Empty response.");
return; return;
} }
if (response->status() == vm_tools::concierge::VM_STATUS_RUNNING || if (response->status() == vm_tools::concierge::VM_STATUS_RUNNING ||
response->status() == vm_tools::concierge::VM_STATUS_STARTING) { response->status() == vm_tools::concierge::VM_STATUS_STARTING) {
Complete(BorealisContextManager::Status::kSuccess, ""); Complete(BorealisStartupResult::kSuccess, "");
return; return;
} }
Complete(BorealisContextManager::Status::kStartVmFailed, Complete(BorealisStartupResult::kStartVmFailed,
"Failed to start Borealis VM: " + response->failure_reason() + "Failed to start Borealis VM: " + response->failure_reason() +
" (code " + base::NumberToString(response->status()) + ")"); " (code " + base::NumberToString(response->status()) + ")");
} }
...@@ -175,11 +174,11 @@ void AwaitBorealisStartup::OnAwaitBorealisStartup( ...@@ -175,11 +174,11 @@ void AwaitBorealisStartup::OnAwaitBorealisStartup(
BorealisContext* context, BorealisContext* context,
base::Optional<std::string> container) { base::Optional<std::string> container) {
if (!container) { if (!container) {
Complete(BorealisContextManager::Status::kAwaitBorealisStartupFailed, Complete(BorealisStartupResult::kAwaitBorealisStartupFailed,
"Awaiting for Borealis launch failed: timed out"); "Awaiting for Borealis launch failed: timed out");
return; return;
} }
context->set_container_name(container.value()); context->set_container_name(container.value());
Complete(BorealisContextManager::Status::kSuccess, ""); Complete(BorealisStartupResult::kSuccess, "");
} }
} // namespace borealis } // namespace borealis
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
#include "chrome/browser/chromeos/borealis/borealis_launch_watcher.h" #include "chrome/browser/chromeos/borealis/borealis_launch_watcher.h"
#include "chrome/browser/chromeos/borealis/borealis_metrics.h"
#include "chromeos/dbus/concierge_client.h" #include "chromeos/dbus/concierge_client.h"
#include "chromeos/dbus/dlcservice/dlcservice_client.h" #include "chromeos/dbus/dlcservice/dlcservice_client.h"
...@@ -19,26 +20,25 @@ class BorealisContext; ...@@ -19,26 +20,25 @@ class BorealisContext;
// Borealis Context Manager. // Borealis Context Manager.
class BorealisTask { class BorealisTask {
public: public:
// Callback to be run when the task completes. The |status| should reflect // Callback to be run when the task completes. The |result| should reflect
// if the task succeeded with kSuccess and an empty string. If it fails, a // if the task succeeded with kSuccess and an empty string. If it fails, a
// different status should be used, and an error string provided. // different result should be used, and an error string provided.
using CompletionStatusCallback = using CompletionResultCallback =
base::OnceCallback<void(BorealisContextManager::Status, std::string)>; base::OnceCallback<void(BorealisStartupResult, std::string)>;
BorealisTask(); BorealisTask();
BorealisTask(const BorealisTask&) = delete; BorealisTask(const BorealisTask&) = delete;
BorealisTask& operator=(const BorealisTask&) = delete; BorealisTask& operator=(const BorealisTask&) = delete;
virtual ~BorealisTask(); virtual ~BorealisTask();
void Run(BorealisContext* context, CompletionStatusCallback callback); void Run(BorealisContext* context, CompletionResultCallback callback);
protected: protected:
virtual void RunInternal(BorealisContext* context) = 0; virtual void RunInternal(BorealisContext* context) = 0;
void Complete(BorealisContextManager::Status status, std::string message); void Complete(BorealisStartupResult result, std::string message);
private: private:
CompletionStatusCallback callback_; CompletionResultCallback callback_;
}; };
// Mounts the Borealis DLC. // Mounts the Borealis DLC.
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "chrome/browser/chromeos/borealis/borealis_context.h" #include "chrome/browser/chromeos/borealis/borealis_context.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
#include "chrome/browser/chromeos/borealis/borealis_metrics.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
...@@ -26,15 +27,12 @@ namespace { ...@@ -26,15 +27,12 @@ namespace {
class CallbackForTesting { class CallbackForTesting {
public: public:
BorealisTask::CompletionStatusCallback GetCallback() { BorealisTask::CompletionResultCallback GetCallback() {
return base::BindOnce(&CallbackForTesting::Callback, return base::BindOnce(&CallbackForTesting::Callback,
base::Unretained(this)); base::Unretained(this));
} }
MOCK_METHOD(void, MOCK_METHOD(void, Callback, (BorealisStartupResult, std::string), ());
Callback,
(BorealisContextManager::Status, std::string),
());
}; };
class BorealisTasksTest : public testing::Test { class BorealisTasksTest : public testing::Test {
...@@ -91,7 +89,7 @@ TEST_F(BorealisTasksTest, MountDlcSucceedsAndCallbackRanWithResults) { ...@@ -91,7 +89,7 @@ TEST_F(BorealisTasksTest, MountDlcSucceedsAndCallbackRanWithResults) {
EXPECT_EQ(context_->root_path(), ""); EXPECT_EQ(context_->root_path(), "");
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
MountDlc task; MountDlc task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -109,7 +107,7 @@ TEST_F(BorealisTasksTest, CreateDiskSucceedsAndCallbackRanWithResults) { ...@@ -109,7 +107,7 @@ TEST_F(BorealisTasksTest, CreateDiskSucceedsAndCallbackRanWithResults) {
EXPECT_EQ(context_->disk_path(), base::FilePath()); EXPECT_EQ(context_->disk_path(), base::FilePath());
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
CreateDiskImage task; CreateDiskImage task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -129,7 +127,7 @@ TEST_F(BorealisTasksTest, ...@@ -129,7 +127,7 @@ TEST_F(BorealisTasksTest,
EXPECT_EQ(context_->disk_path(), base::FilePath()); EXPECT_EQ(context_->disk_path(), base::FilePath());
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
CreateDiskImage task; CreateDiskImage task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -145,7 +143,7 @@ TEST_F(BorealisTasksTest, StartBorealisVmSucceedsAndCallbackRanWithResults) { ...@@ -145,7 +143,7 @@ TEST_F(BorealisTasksTest, StartBorealisVmSucceedsAndCallbackRanWithResults) {
fake_concierge_client_->set_start_vm_response(std::move(response)); fake_concierge_client_->set_start_vm_response(std::move(response));
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
StartBorealisVm task; StartBorealisVm task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -161,7 +159,7 @@ TEST_F(BorealisTasksTest, ...@@ -161,7 +159,7 @@ TEST_F(BorealisTasksTest,
fake_concierge_client_->set_start_vm_response(std::move(response)); fake_concierge_client_->set_start_vm_response(std::move(response));
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
StartBorealisVm task; StartBorealisVm task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -179,7 +177,7 @@ TEST_F(BorealisTasksTest, ...@@ -179,7 +177,7 @@ TEST_F(BorealisTasksTest,
signal.set_container_name("penguin"); signal.set_container_name("penguin");
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
AwaitBorealisStartup task(context_->profile(), context_->vm_name()); AwaitBorealisStartup task(context_->profile(), context_->vm_name());
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -197,7 +195,7 @@ TEST_F(BorealisTasksTest, ...@@ -197,7 +195,7 @@ TEST_F(BorealisTasksTest,
signal.set_container_name("penguin"); signal.set_container_name("penguin");
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kSuccess, _)); EXPECT_CALL(callback, Callback(BorealisStartupResult::kSuccess, _));
AwaitBorealisStartup task(context_->profile(), context_->vm_name()); AwaitBorealisStartup task(context_->profile(), context_->vm_name());
fake_cicerone_client_->NotifyContainerStarted(std::move(signal)); fake_cicerone_client_->NotifyContainerStarted(std::move(signal));
...@@ -210,8 +208,7 @@ TEST_F(BorealisTasksTest, ...@@ -210,8 +208,7 @@ TEST_F(BorealisTasksTest,
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL( EXPECT_CALL(
callback, callback,
Callback(BorealisContextManager::Status::kAwaitBorealisStartupFailed, Callback(BorealisStartupResult::kAwaitBorealisStartupFailed, StrNe("")));
StrNe("")));
AwaitBorealisStartup task(context_->profile(), context_->vm_name()); AwaitBorealisStartup task(context_->profile(), context_->vm_name());
task.GetWatcherForTesting().SetTimeoutForTesting( task.GetWatcherForTesting().SetTimeoutForTesting(
...@@ -226,8 +223,8 @@ class BorealisTasksTestDlc : public BorealisTasksTest, ...@@ -226,8 +223,8 @@ class BorealisTasksTestDlc : public BorealisTasksTest,
TEST_P(BorealisTasksTestDlc, MountDlcFailsAndCallbackRanWithResults) { TEST_P(BorealisTasksTestDlc, MountDlcFailsAndCallbackRanWithResults) {
fake_dlcservice_client_->set_install_error(GetParam()); fake_dlcservice_client_->set_install_error(GetParam());
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kMountFailed, EXPECT_CALL(callback,
StrNe(""))); Callback(BorealisStartupResult::kMountFailed, StrNe("")));
MountDlc task; MountDlc task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -255,9 +252,8 @@ TEST_P(BorealisTasksTestDiskImage, CreateDiskFailsAndCallbackRanWithResults) { ...@@ -255,9 +252,8 @@ TEST_P(BorealisTasksTestDiskImage, CreateDiskFailsAndCallbackRanWithResults) {
EXPECT_EQ(context_->disk_path(), base::FilePath()); EXPECT_EQ(context_->disk_path(), base::FilePath());
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL( EXPECT_CALL(callback,
callback, Callback(BorealisStartupResult::kDiskImageFailed, StrNe("")));
Callback(BorealisContextManager::Status::kDiskImageFailed, StrNe("")));
CreateDiskImage task; CreateDiskImage task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
...@@ -288,8 +284,8 @@ TEST_P(BorealisTasksTestsStartBorealisVm, ...@@ -288,8 +284,8 @@ TEST_P(BorealisTasksTestsStartBorealisVm,
fake_concierge_client_->set_start_vm_response(std::move(response)); fake_concierge_client_->set_start_vm_response(std::move(response));
testing::StrictMock<CallbackForTesting> callback; testing::StrictMock<CallbackForTesting> callback;
EXPECT_CALL(callback, Callback(BorealisContextManager::Status::kStartVmFailed, EXPECT_CALL(callback,
StrNe(""))); Callback(BorealisStartupResult::kStartVmFailed, StrNe("")));
StartBorealisVm task; StartBorealisVm task;
task.Run(context_.get(), callback.GetCallback()); task.Run(context_.get(), callback.GetCallback());
......
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