borealis: Allow Expected.Handle to return a value

Figured this would be a useful API extension while refactoring.

Bug: b/175360169
Change-Id: If6490aefacdb5110d3fe5dc423d311e6f7618c12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2584434
Commit-Queue: Nic Hollingum <hollingum@google.com>
Reviewed-by: Daniel Ng <danielng@google.com>
Cr-Commit-Position: refs/heads/master@{#837750}

chrome/browser/chromeos/borealis/borealis_context_manager_impl.cc

@@ -156,16 +156,22 @@ void BorealisContextManagerImpl::Complete(Startup::Result completion_result) {
   DCHECK(in_progress_startup_);
   in_progress_startup_.reset();
 
-  if (completion_result) {
-    context_ = std::move(completion_result.Value());
-  } else {
-    Described<BorealisStartupResult>& failure = completion_result.Error();
-    LOG(ERROR) << "Startup failed: failure=" << failure.error()
-               << " message=" << failure.description();
-  }
-
   BorealisContextManager::Result completion_result_for_clients =
-      GetResult(completion_result);
+      completion_result.Handle(
+          base::BindOnce(
+              [](std::unique_ptr<BorealisContext>* out_context,
+                 std::unique_ptr<BorealisContext>& success) {
+                std::swap(*out_context, success);
+                return BorealisContextManager::Result(out_context->get());
+              },
+              &context_),
+          base::BindOnce([](Described<BorealisStartupResult>& failure) {
+            LOG(ERROR) << "Startup failed: failure=" << failure.error()
+                       << " message=" << failure.description();
+            return BorealisContextManager::Result(failure.error(),
+                                                  failure.description());
+          }));
+
   while (!callback_queue_.empty()) {
     ResultCallback callback = std::move(callback_queue_.front());
     callback_queue_.pop();
@@ -173,13 +179,4 @@ void BorealisContextManagerImpl::Complete(Startup::Result completion_result) {
   }
 }
 
-BorealisContextManager::Result BorealisContextManagerImpl::GetResult(
-    const Startup::Result& completion_result) {
-  if (completion_result) {
-    return BorealisContextManager::Result(context_.get());
-  }
-  const Described<BorealisStartupResult>& failure = completion_result.Error();
-  return BorealisContextManager::Result(failure.error(), failure.description());
-}
-
 }  // namespace borealis

chrome/browser/chromeos/borealis/infra/expected.h

@@ -26,10 +26,6 @@ class Expected {
   using value_t = T;
   using error_t = E;
 
-  // Convenience callbacks for handling the various states.
-  using ValueCallback = base::OnceCallback<void(T&)>;
-  using ErrorCallback = base::OnceCallback<void(E&)>;
-
   // TODO(b/172501195): This implementation is only partial, either complete it
   // or replace it with the standard. Until then |T| and |E| should probably be
   // movable and copy-able respectively.
@@ -73,11 +69,14 @@ class Expected {
 
   // Invoke exactly one of the |on_value| or |on_error| callbacks, depending on
   // the state of |this|. Works a bit like absl::visit() but more chrome-ey.
-  void Handle(ValueCallback on_value, ErrorCallback on_error) {
+  // Returns whatever type those callbacks return (which must be the same).
+  template <typename R>
+  R Handle(base::OnceCallback<R(T&)> on_value,
+           base::OnceCallback<R(E&)> on_error) {
     if (*this) {
-      std::move(on_value).Run(absl::get<T>(storage_));
+      return std::move(on_value).Run(absl::get<T>(storage_));
     } else {
-      std::move(on_error).Run(absl::get<E>(storage_));
+      return std::move(on_error).Run(absl::get<E>(storage_));
     }
   }
 

chrome/browser/chromeos/borealis/infra/expected_unittest.cc

@@ -64,18 +64,27 @@ TEST(ExpectedTest, HandleCallsCorrectCallback) {
   CallbackFactory<A> a_callback;
   CallbackFactory<B> b_callback;
 
-  Expected<A, B> a{A()};
   EXPECT_CALL(a_callback, Call).Times(1);
-  a.Handle(
+  Expected<A, B>{A()}.Handle(
       base::BindOnce(&CallbackFactory<A>::Call, base::Unretained(&a_callback)),
       base::BindOnce(&CallbackFactory<B>::Call, base::Unretained(&b_callback)));
 
-  Expected<A, B> b = Unexpected<A>(B{});
   EXPECT_CALL(b_callback, Call).Times(1);
-  b.Handle(
+  Unexpected<A>(B{}).Handle(
       base::BindOnce(&CallbackFactory<A>::Call, base::Unretained(&a_callback)),
       base::BindOnce(&CallbackFactory<B>::Call, base::Unretained(&b_callback)));
 }
 
+TEST(ExpectedTest, HandleCanReturn) {
+  using Exp = Expected<A, B>;
+  EXPECT_EQ("expected",
+            Exp{A()}.Handle(base::BindOnce([](A&) { return "expected"; }),
+                            base::BindOnce([](B&) { return "unexpected"; })));
+
+  EXPECT_EQ("unexpected", Unexpected<A>(B{}).Handle(
+                              base::BindOnce([](A&) { return "expected"; }),
+                              base::BindOnce([](B&) { return "unexpected"; })));
+}
+
 }  // namespace
 }  // namespace borealis