Commit bf248bc3 authored by Nicholas Hollingum's avatar Nicholas Hollingum Committed by Chromium LUCI CQ

borealis: Replace BorealisContextManager::Resut with Expected

This is a cleaner/more general implementation and lets us remove the
one-off version the context manager uses.

Bug: b/175360169
Change-Id: I3356edc8b98d539476c686d624179cdfc7eaa906
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2603232
Commit-Queue: Nic Hollingum <hollingum@google.com>
Reviewed-by: default avatarDaniel Ng <danielng@google.com>
Cr-Commit-Position: refs/heads/master@{#839699}
parent d5feee8b
...@@ -880,7 +880,6 @@ source_set("chromeos") { ...@@ -880,7 +880,6 @@ source_set("chromeos") {
"borealis/borealis_app_launcher.h", "borealis/borealis_app_launcher.h",
"borealis/borealis_context.cc", "borealis/borealis_context.cc",
"borealis/borealis_context.h", "borealis/borealis_context.h",
"borealis/borealis_context_manager.cc",
"borealis/borealis_context_manager.h", "borealis/borealis_context_manager.h",
"borealis/borealis_context_manager_impl.cc", "borealis/borealis_context_manager_impl.cc",
"borealis/borealis_context_manager_impl.h", "borealis/borealis_context_manager_impl.h",
......
...@@ -89,14 +89,15 @@ void BorealisAppLauncher::Launch(std::string app_id, ...@@ -89,14 +89,15 @@ void BorealisAppLauncher::Launch(std::string app_id,
base::BindOnce( base::BindOnce(
[](std::string app_id, [](std::string app_id,
BorealisAppLauncher::OnLaunchedCallback callback, BorealisAppLauncher::OnLaunchedCallback callback,
BorealisContextManager::Result result) { BorealisContextManager::ContextOrFailure result) {
if (!result.Ok()) { if (!result) {
LOG(ERROR) << "Failed to launch " << app_id << ": " LOG(ERROR) << "Failed to launch " << app_id << "(code "
<< result.FailureReason(); << result.Error().error()
<< "): " << result.Error().description();
std::move(callback).Run(LaunchResult::kError); std::move(callback).Run(LaunchResult::kError);
return; return;
} }
BorealisAppLauncher::Launch(result.Success(), std::move(app_id), BorealisAppLauncher::Launch(*result.Value(), std::move(app_id),
std::move(callback)); std::move(callback));
}, },
std::move(app_id), std::move(callback))); std::move(app_id), std::move(callback)));
......
// 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_context_manager.h"
namespace borealis {
BorealisContextManager::Result::Result(const BorealisContext* ctx)
: result_(BorealisStartupResult::kSuccess), failure_reason_(), ctx_(ctx) {
DCHECK(ctx);
}
BorealisContextManager::Result::Result(BorealisStartupResult result,
std::string failure_reason)
: result_(result),
failure_reason_(std::move(failure_reason)),
ctx_(nullptr) {
DCHECK(result != BorealisStartupResult::kSuccess);
}
BorealisContextManager::Result::~Result() = default;
bool BorealisContextManager::Result::Ok() const {
return result_ == BorealisStartupResult::kSuccess;
}
BorealisStartupResult BorealisContextManager::Result::Failure() const {
DCHECK(!Ok());
return result_;
}
const std::string& BorealisContextManager::Result::FailureReason() const {
DCHECK(!Ok());
return failure_reason_;
}
const BorealisContext& BorealisContextManager::Result::Success() const {
DCHECK(Ok());
return *ctx_;
}
} // namespace borealis
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include "base/callback.h" #include "base/callback.h"
#include "chrome/browser/chromeos/borealis/borealis_metrics.h" #include "chrome/browser/chromeos/borealis/borealis_metrics.h"
#include "chrome/browser/chromeos/borealis/infra/described.h"
#include "chrome/browser/chromeos/borealis/infra/expected.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
namespace borealis { namespace borealis {
...@@ -19,38 +21,12 @@ class BorealisContextManager : public KeyedService { ...@@ -19,38 +21,12 @@ class BorealisContextManager : public KeyedService {
public: public:
// 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 { using ContextOrFailure =
public: Expected<BorealisContext*, Described<BorealisStartupResult>>;
// Used to indicate that the result was a success.
explicit Result(const BorealisContext* ctx);
// Used to indicate the the result was a failure.
Result(BorealisStartupResult result, std::string failure_reason);
~Result();
// Returns true if the result was successful.
bool Ok() const;
// In the event of a failed launch, returns the result code for that error.
BorealisStartupResult Failure() const;
// In the event of a failed launch, returns the message provided.
const std::string& FailureReason() const;
// In the event of a successful launch, returns a handle to the context
// created for that launch.
const BorealisContext& Success() const;
private:
BorealisStartupResult result_;
std::string failure_reason_;
const BorealisContext* ctx_;
};
// Convenience definition for the callback provided by clients wanting to // Convenience definition for the callback provided by clients wanting to
// launch borealis. // launch borealis.
using ResultCallback = base::OnceCallback<void(Result)>; using ResultCallback = base::OnceCallback<void(ContextOrFailure)>;
BorealisContextManager() = default; BorealisContextManager() = default;
BorealisContextManager(const BorealisContextManager&) = delete; BorealisContextManager(const BorealisContextManager&) = delete;
......
...@@ -87,7 +87,8 @@ BorealisContextManagerImpl::~BorealisContextManagerImpl() = default; ...@@ -87,7 +87,8 @@ BorealisContextManagerImpl::~BorealisContextManagerImpl() = default;
void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) { void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) {
if (context_) { if (context_) {
std::move(callback).Run(BorealisContextManager::Result(context_.get())); std::move(callback).Run(
BorealisContextManager::ContextOrFailure(context_.get()));
return; return;
} }
AddCallback(std::move(callback)); AddCallback(std::move(callback));
...@@ -156,20 +157,22 @@ void BorealisContextManagerImpl::Complete(Startup::Result completion_result) { ...@@ -156,20 +157,22 @@ void BorealisContextManagerImpl::Complete(Startup::Result completion_result) {
DCHECK(in_progress_startup_); DCHECK(in_progress_startup_);
in_progress_startup_.reset(); in_progress_startup_.reset();
BorealisContextManager::Result completion_result_for_clients = BorealisContextManager::ContextOrFailure completion_result_for_clients =
completion_result.Handle( completion_result.Handle(
base::BindOnce( base::BindOnce(
[](std::unique_ptr<BorealisContext>* out_context, [](std::unique_ptr<BorealisContext>* out_context,
std::unique_ptr<BorealisContext>& success) { std::unique_ptr<BorealisContext>& success) {
std::swap(*out_context, success); std::swap(*out_context, success);
return BorealisContextManager::Result(out_context->get()); return BorealisContextManager::ContextOrFailure(
out_context->get());
}, },
&context_), &context_),
base::BindOnce([](Described<BorealisStartupResult>& failure) { base::BindOnce([](Described<BorealisStartupResult>& failure) {
LOG(ERROR) << "Startup failed: failure=" << failure.error() LOG(ERROR) << "Startup failed: failure=" << failure.error()
<< " message=" << failure.description(); << " message=" << failure.description();
return BorealisContextManager::Result(failure.error(), return BorealisContextManager::ContextOrFailure::Unexpected(
failure.description()); Described<BorealisStartupResult>{failure.error(),
failure.description()});
})); }));
while (!callback_queue_.empty()) { while (!callback_queue_.empty()) {
......
...@@ -79,7 +79,7 @@ class BorealisContextManagerImpl : public BorealisContextManager { ...@@ -79,7 +79,7 @@ class BorealisContextManagerImpl : public BorealisContextManager {
// 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::ContextOrFailure GetResult(
const Startup::Result& completion_result); const Startup::Result& completion_result);
Profile* const profile_; Profile* const profile_;
......
...@@ -26,13 +26,14 @@ namespace borealis { ...@@ -26,13 +26,14 @@ namespace borealis {
namespace { namespace {
MATCHER(IsSuccessResult, "") { MATCHER(IsSuccessResult, "") {
return arg.Ok() && arg.Success().vm_name() == "test_vm_name"; return arg && arg.Value()->vm_name() == "test_vm_name";
} }
MATCHER(IsFailureResult, "") { MATCHER(IsFailureResult, "") {
return !arg.Ok() && return !arg &&
arg.Failure() == borealis::BorealisStartupResult::kStartVmFailed && arg.Error().error() ==
arg.FailureReason() == "Something went wrong!"; borealis::BorealisStartupResult::kStartVmFailed &&
arg.Error().description() == "Something went wrong!";
} }
class MockTask : public BorealisTask { class MockTask : public BorealisTask {
...@@ -83,7 +84,7 @@ class ResultCallbackHandler { ...@@ -83,7 +84,7 @@ class ResultCallbackHandler {
return base::BindOnce(&ResultCallbackHandler::Callback, return base::BindOnce(&ResultCallbackHandler::Callback,
base::Unretained(this)); base::Unretained(this));
} }
MOCK_METHOD(void, Callback, (BorealisContextManager::Result), ()); MOCK_METHOD(void, Callback, (BorealisContextManager::ContextOrFailure), ());
}; };
class BorealisContextManagerTest : public testing::Test { class BorealisContextManagerTest : public testing::Test {
...@@ -131,11 +132,12 @@ TEST_F(BorealisContextManagerTest, NoTasksImpliesSuccess) { ...@@ -131,11 +132,12 @@ TEST_F(BorealisContextManagerTest, NoTasksImpliesSuccess) {
BorealisContextManagerImplForTesting context_manager( BorealisContextManagerImplForTesting context_manager(
profile_.get(), /*tasks=*/0, /*success=*/true); profile_.get(), /*tasks=*/0, /*success=*/true);
EXPECT_CALL(callback_expectation, Callback(testing::_)) EXPECT_CALL(callback_expectation, Callback(testing::_))
.WillOnce(testing::Invoke([](BorealisContextManager::Result result) { .WillOnce(
EXPECT_TRUE(result.Ok()); testing::Invoke([](BorealisContextManager::ContextOrFailure result) {
// Even with no tasks, the context will give the VM a name. EXPECT_TRUE(result);
EXPECT_EQ(result.Success().vm_name(), "borealis"); // Even with no tasks, the context will give the VM a name.
})); EXPECT_EQ(result.Value()->vm_name(), "borealis");
}));
context_manager.StartBorealis(callback_expectation.GetCallback()); context_manager.StartBorealis(callback_expectation.GetCallback());
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
} }
...@@ -252,10 +254,12 @@ class NeverCompletingContextManager : public BorealisContextManagerImpl { ...@@ -252,10 +254,12 @@ class NeverCompletingContextManager : public BorealisContextManagerImpl {
TEST_F(BorealisContextManagerTest, ShutDownCancelsRequestsAndTerminatesVm) { TEST_F(BorealisContextManagerTest, ShutDownCancelsRequestsAndTerminatesVm) {
testing::StrictMock<ResultCallbackHandler> callback_expectation; testing::StrictMock<ResultCallbackHandler> callback_expectation;
EXPECT_CALL(callback_expectation, Callback(testing::_)) EXPECT_CALL(callback_expectation, Callback(testing::_))
.WillOnce(testing::Invoke([](BorealisContextManager::Result result) { .WillOnce(
EXPECT_FALSE(result.Ok()); testing::Invoke([](BorealisContextManager::ContextOrFailure result) {
EXPECT_EQ(result.Failure(), BorealisStartupResult::kCancelled); EXPECT_FALSE(result);
})); EXPECT_EQ(result.Error().error(),
BorealisStartupResult::kCancelled);
}));
NeverCompletingContextManager context_manager(profile_.get()); NeverCompletingContextManager context_manager(profile_.get());
context_manager.StartBorealis(callback_expectation.GetCallback()); context_manager.StartBorealis(callback_expectation.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