Reland 2: Promises: Add AbstractPromise

This is the internal promise representation. It implements the machinery
needed to marshal and dispatch promises as they become ready for
execution.

Care has been taken to try and minimize the size of the AbstractPromise
class and to keep down the number of heap allocations. The promise result
and the executor are both stored in the base::unique_any since they are
never needed at the same time. The size of AbstractPromise on x64 is 96
bytes in builds without DCHECKS.

In builds with DCHECKS it's larger because there's additional storage
used to diagnose and prevent various usage hazards:

* Unhandled rejection
* Double move of promise results to callbacks
* Mixed move and non-move semantics of promise results to callbacks

The base::any_internal type has been adjusted so it can store the
SmallUniqueObject<> inline, which is used to store the promise executor.
The largest anticipated promise executor base::All is the size of
3x sizeof(void*) to hold a vtable and a std::vector.

Design: https://docs.google.com/document/d/1l12PAJgEtlrqTXKiw6mk2cR2jP7FAfCCDr-DGIdiC9w/edit

TBR=fdoray@chromium.org, etiennep@chromium.org

Bug: 906125
Change-Id: I62cecf23ecc8ea3ef7417d6ad40c0911fac5339c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1607643Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658982}

base/BUILD.gn

@@ -751,8 +751,13 @@ jumbo_component("base") {
     "task/lazy_task_runner.h",
     "task/post_task.cc",
     "task/post_task.h",
+    "task/promise/abstract_promise.cc",
+    "task/promise/abstract_promise.h",
     "task/promise/dependent_list.cc",
     "task/promise/dependent_list.h",
+    "task/promise/no_op_promise_executor.cc",
+    "task/promise/no_op_promise_executor.h",
+    "task/promise/small_unique_object.h",
     "task/scoped_set_task_priority_for_current_thread.cc",
     "task/scoped_set_task_priority_for_current_thread.h",
     "task/sequence_manager/associated_thread_id.cc",
@@ -2626,6 +2631,7 @@ test("base_unittests") {
     "task/common/task_annotator_unittest.cc",
     "task/lazy_task_runner_unittest.cc",
     "task/post_task_unittest.cc",
+    "task/promise/abstract_promise_unittest.cc",
     "task/promise/dependent_list_unittest.cc",
     "task/scoped_set_task_priority_for_current_thread_unittest.cc",
     "task/sequence_manager/atomic_flag_set_unittest.cc",

base/containers/any_internal.h

@@ -85,8 +85,8 @@ class BASE_EXPORT AnyInternal {
   };
 
   struct alignas(sizeof(void*)) InlineAlloc {
-    // Holds a T if small.
-    char bytes[sizeof(void*)];
+    // Holds a T if small. Tweaked to hold a promise executor inline.
+    char bytes[sizeof(void*) * 3];
 
     template <typename T>
     T& value_as() {

base/containers/any_internal_unittest.cc

@@ -13,6 +13,8 @@ namespace {
 struct OutOfLineStruct {
   void* one;
   void* two;
+  void* three;
+  void* four;
 };
 }  // namespace
 
@@ -26,7 +28,7 @@ TEST(AnyInternalTest, InlineOrOutlineStorage) {
       "std::unique_ptr<int> should be stored inline");
   static_assert(
       !AnyInternal::InlineStorageHelper<OutOfLineStruct>::kUseInlineStorage,
-      "A struct with two pointers should be stored out of line");
+      "A struct with four pointers should be stored out of line");
 }
 
 }  // namespace internal

base/task/promise/dependent_list.cc

@@ -4,6 +4,8 @@
 
 #include "base/task/promise/dependent_list.h"
 
+#include "base/task/promise/abstract_promise.h"
+
 namespace base {
 namespace internal {
 
@@ -17,6 +19,13 @@ DependentList::~DependentList() = default;
 
 DependentList::Node::Node() = default;
 
+DependentList::Node::Node(Node&& other) {
+  dependent = std::move(other.dependent);
+  DCHECK_EQ(other.next, nullptr);
+}
+
+DependentList::Node::~Node() = default;
+
 DependentList::InsertResult DependentList::Insert(Node* node) {
   // This method uses std::memory_order_acquire semantics on read (the failure
   // case of compare_exchange_weak() below is a read) to ensure setting
@@ -83,6 +92,12 @@ DependentList::Node* DependentList::ConsumeOnceForCancel() {
   return reinterpret_cast<Node*>(prev_head);
 }
 
+bool DependentList::IsSettled() const {
+  uintptr_t value = head_.load(std::memory_order_acquire);
+  return value == kResolvedSentinel || value == kRejectedSentinel ||
+         value == kCanceledSentinel;
+}
+
 bool DependentList::IsResolved() const {
   return head_.load(std::memory_order_acquire) == kResolvedSentinel;
 }
@@ -91,7 +106,7 @@ bool DependentList::IsRejected() const {
   return head_.load(std::memory_order_acquire) == kRejectedSentinel;
 }
 
-bool DependentList::IsCancelled() const {
+bool DependentList::IsCanceled() const {
   return head_.load(std::memory_order_acquire) == kCanceledSentinel;
 }
 

base/task/promise/dependent_list.h

@@ -10,6 +10,7 @@
 #include "base/base_export.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 
 namespace base {
 namespace internal {
@@ -41,9 +42,10 @@ class BASE_EXPORT DependentList {
 
   struct BASE_EXPORT Node {
     Node();
+    explicit Node(Node&& other) noexcept;
+    ~Node();
 
-    // TODO(alexclarke): Make this a scoped_refptr.
-    AbstractPromise* dependent;
+    scoped_refptr<AbstractPromise> dependent;
     std::atomic<Node*> next{nullptr};
   };
 
@@ -61,9 +63,10 @@ class BASE_EXPORT DependentList {
   // A ConsumeXXX function may only be called once.
   Node* ConsumeOnceForCancel();
 
+  bool IsSettled() const;
   bool IsResolved() const;
   bool IsRejected() const;
-  bool IsCancelled() const;
+  bool IsCanceled() const;
 
  private:
   std::atomic<uintptr_t> head_;

base/task/promise/dependent_list_unittest.cc

@@ -13,8 +13,9 @@ TEST(DependentList, ConstructUnresolved) {
   DependentList::Node node;
   EXPECT_EQ(DependentList::InsertResult::SUCCESS, list.Insert(&node));
   EXPECT_FALSE(list.IsRejected());
-  EXPECT_FALSE(list.IsCancelled());
+  EXPECT_FALSE(list.IsCanceled());
   EXPECT_FALSE(list.IsResolved());
+  EXPECT_FALSE(list.IsSettled());
 }
 
 TEST(DependentList, ConstructResolved) {
@@ -24,7 +25,8 @@ TEST(DependentList, ConstructResolved) {
             list.Insert(&node));
   EXPECT_TRUE(list.IsResolved());
   EXPECT_FALSE(list.IsRejected());
-  EXPECT_FALSE(list.IsCancelled());
+  EXPECT_FALSE(list.IsCanceled());
+  EXPECT_TRUE(list.IsSettled());
 }
 
 TEST(DependentList, ConstructRejected) {
@@ -33,8 +35,9 @@ TEST(DependentList, ConstructRejected) {
   EXPECT_EQ(DependentList::InsertResult::FAIL_PROMISE_REJECTED,
             list.Insert(&node));
   EXPECT_TRUE(list.IsRejected());
-  EXPECT_FALSE(list.IsCancelled());
+  EXPECT_FALSE(list.IsCanceled());
   EXPECT_FALSE(list.IsResolved());
+  EXPECT_TRUE(list.IsSettled());
 }
 
 TEST(DependentList, ConsumeOnceForResolve) {
@@ -47,10 +50,12 @@ TEST(DependentList, ConsumeOnceForResolve) {
   EXPECT_EQ(DependentList::InsertResult::SUCCESS, list.Insert(&node3));
 
   EXPECT_FALSE(list.IsResolved());
+  EXPECT_FALSE(list.IsSettled());
   DependentList::Node* result = list.ConsumeOnceForResolve();
   EXPECT_TRUE(list.IsResolved());
   EXPECT_FALSE(list.IsRejected());
-  EXPECT_FALSE(list.IsCancelled());
+  EXPECT_FALSE(list.IsCanceled());
+  EXPECT_TRUE(list.IsSettled());
 
   EXPECT_EQ(&node3, result);
   EXPECT_EQ(&node2, result->next.load());
@@ -73,10 +78,12 @@ TEST(DependentList, ConsumeOnceForReject) {
   EXPECT_EQ(DependentList::InsertResult::SUCCESS, list.Insert(&node3));
 
   EXPECT_FALSE(list.IsRejected());
+  EXPECT_FALSE(list.IsSettled());
   DependentList::Node* result = list.ConsumeOnceForReject();
   EXPECT_TRUE(list.IsRejected());
   EXPECT_FALSE(list.IsResolved());
-  EXPECT_FALSE(list.IsCancelled());
+  EXPECT_FALSE(list.IsCanceled());
+  EXPECT_TRUE(list.IsSettled());
 
   EXPECT_EQ(&node3, result);
   EXPECT_EQ(&node2, result->next.load());
@@ -98,11 +105,13 @@ TEST(DependentList, ConsumeOnceForCancel) {
   EXPECT_EQ(DependentList::InsertResult::SUCCESS, list.Insert(&node2));
   EXPECT_EQ(DependentList::InsertResult::SUCCESS, list.Insert(&node3));
 
-  EXPECT_FALSE(list.IsCancelled());
+  EXPECT_FALSE(list.IsCanceled());
+  EXPECT_FALSE(list.IsSettled());
   DependentList::Node* result = list.ConsumeOnceForCancel();
-  EXPECT_TRUE(list.IsCancelled());
+  EXPECT_TRUE(list.IsCanceled());
   EXPECT_FALSE(list.IsResolved());
   EXPECT_FALSE(list.IsRejected());
+  EXPECT_TRUE(list.IsSettled());
 
   EXPECT_EQ(&node3, result);
   EXPECT_EQ(&node2, result->next.load());

base/task/promise/no_op_promise_executor.cc

+// Copyright 2019 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 "base/task/promise/no_op_promise_executor.h"
+#include "base/task_runner.h"
+
+namespace base {
+namespace internal {
+
+NoOpPromiseExecutor::NoOpPromiseExecutor(bool can_resolve, bool can_reject)
+#if DCHECK_IS_ON()
+    : can_resolve_(can_resolve),
+      can_reject_(can_reject)
+#endif
+{
+}
+
+NoOpPromiseExecutor::~NoOpPromiseExecutor() {}
+
+AbstractPromise::Executor::PrerequisitePolicy
+NoOpPromiseExecutor::GetPrerequisitePolicy() {
+  return PrerequisitePolicy::kNever;
+}
+
+bool NoOpPromiseExecutor::IsCancelled() const {
+  return false;
+}
+
+#if DCHECK_IS_ON()
+AbstractPromise::Executor::ArgumentPassingType
+NoOpPromiseExecutor::ResolveArgumentPassingType() const {
+  return ArgumentPassingType::kNoCallback;
+}
+
+AbstractPromise::Executor::ArgumentPassingType
+NoOpPromiseExecutor::RejectArgumentPassingType() const {
+  return ArgumentPassingType::kNoCallback;
+}
+
+bool NoOpPromiseExecutor::CanResolve() const {
+  return can_resolve_;
+}
+
+bool NoOpPromiseExecutor::CanReject() const {
+  return can_reject_;
+}
+#endif
+
+void NoOpPromiseExecutor::Execute(AbstractPromise* promise) {}
+
+// static
+scoped_refptr<internal::AbstractPromise> NoOpPromiseExecutor::Create(
+    Location from_here,
+    bool can_resolve,
+    bool can_reject,
+    RejectPolicy reject_policy) {
+  return MakeRefCounted<internal::AbstractPromise>(
+      nullptr, from_here, nullptr, reject_policy,
+      internal::AbstractPromise::ConstructWith<
+          internal::DependentList::ConstructUnresolved,
+          internal::NoOpPromiseExecutor>(),
+      can_resolve, can_reject);
+}
+
+}  // namespace internal
+}  // namespace base

base/task/promise/no_op_promise_executor.h

+// Copyright 2019 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 BASE_TASK_PROMISE_NO_OP_PROMISE_EXECUTOR_H_
+#define BASE_TASK_PROMISE_NO_OP_PROMISE_EXECUTOR_H_
+
+#include "base/macros.h"
+#include "base/task/promise/abstract_promise.h"
+
+namespace base {
+namespace internal {
+
+// An Executor that doesn't do anything.
+class BASE_EXPORT NoOpPromiseExecutor : public AbstractPromise::Executor {
+ public:
+  NoOpPromiseExecutor(bool can_resolve, bool can_reject);
+
+  ~NoOpPromiseExecutor() override;
+
+  static scoped_refptr<internal::AbstractPromise> Create(
+      Location from_here,
+      bool can_resolve,
+      bool can_reject,
+      RejectPolicy reject_policy);
+
+  PrerequisitePolicy GetPrerequisitePolicy() override;
+  bool IsCancelled() const override;
+
+#if DCHECK_IS_ON()
+  ArgumentPassingType ResolveArgumentPassingType() const override;
+  ArgumentPassingType RejectArgumentPassingType() const override;
+  bool CanResolve() const override;
+  bool CanReject() const override;
+#endif
+  void Execute(AbstractPromise* promise) override;
+
+ private:
+#if DCHECK_IS_ON()
+  bool can_resolve_;
+  bool can_reject_;
+#endif
+};
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_PROMISE_NO_OP_PROMISE_EXECUTOR_H_

base/task/promise/small_unique_object.h

+// Copyright 2019 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 BASE_TASK_PROMISE_SMALL_UNIQUE_OBJECT_H_
+#define BASE_TASK_PROMISE_SMALL_UNIQUE_OBJECT_H_
+
+#include <cstring>
+
+#include "base/macros.h"
+#include "base/template_util.h"
+
+namespace base {
+namespace internal {
+
+// A container intended for inline storage of a derived virtual object up to a
+// maximum size. This is useful to avoid unnecessary heap allocations.
+template <typename T, size_t MaxSize = sizeof(int*) * 3>
+class BASE_EXPORT SmallUniqueObject {
+ public:
+  SmallUniqueObject() { memset(&union_, 0, sizeof(union_)); }
+
+  template <typename Derived, typename... Args>
+  SmallUniqueObject(in_place_type_t<Derived>, Args&&... args) noexcept {
+    static_assert(std::is_base_of<T, Derived>::value,
+                  "T is not a base of Derived");
+    static_assert(sizeof(Derived) <= MaxSize,
+                  "Derived is too big to be held by SmallUniqueObject");
+    static_assert(sizeof(T) <= MaxSize,
+                  "T is too big to be held by SmallUniqueObject");
+    new (union_.storage) Derived(std::forward<Args>(args)...);
+  }
+
+  ~SmallUniqueObject() { reset(); }
+
+  explicit operator bool() const { return !Empty(); }
+
+  bool Empty() const {
+    for (size_t i = 0; i < kMaxInts; i++) {
+      if (union_.flag[i])
+        return false;
+    }
+    return true;
+  }
+
+  void reset() {
+    if (!Empty()) {
+      get()->~T();
+      memset(&union_, 0, sizeof(union_));
+    }
+  }
+
+  T* get() noexcept {
+    if (Empty())
+      return nullptr;
+    return reinterpret_cast<T*>(union_.storage);
+  }
+
+  const T* get() const noexcept {
+    if (Empty())
+      return nullptr;
+    return reinterpret_cast<const T*>(union_.storage);
+  }
+
+  const T& operator*() const {
+    DCHECK(!Empty());
+    return *get();
+  }
+
+  T& operator*() {
+    DCHECK(!Empty());
+    return *get();
+  }
+
+  const T* operator->() const noexcept {
+    DCHECK(!Empty());
+    return get();
+  }
+
+  T* operator->() noexcept {
+    DCHECK(!Empty());
+    return get();
+  }
+
+ private:
+  static constexpr size_t kMaxInts = MaxSize / sizeof(int);
+  union {
+    // The vtable will be stored somewhere within |storage|, the actual location
+    // is compiler specific but where ever it is, it can't be zero.
+    int flag[kMaxInts];
+    char storage[MaxSize];
+  } union_;
+};
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_PROMISE_SMALL_UNIQUE_OBJECT_H_

base/task_runner.cc

@@ -6,8 +6,10 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/task/promise/abstract_promise.h"
 #include "base/threading/post_task_and_reply_impl.h"
 
 namespace base {
@@ -51,6 +53,14 @@ bool TaskRunner::PostTaskAndReply(const Location& from_here,
       from_here, std::move(task), std::move(reply));
 }
 
+bool TaskRunner::PostPromiseInternal(
+    const scoped_refptr<internal::AbstractPromise>& promise,
+    base::TimeDelta delay) {
+  return PostDelayedTask(
+      promise->from_here(),
+      BindOnce(&internal::AbstractPromise::Execute, std::move(promise)), delay);
+}
+
 TaskRunner::TaskRunner() = default;
 
 TaskRunner::~TaskRunner() = default;

base/task_runner.h

@@ -15,6 +15,10 @@
 
 namespace base {
 
+namespace internal {
+class AbstractPromise;
+}  // namespace internal
+
 struct TaskRunnerTraits;
 
 // A TaskRunner is an object that runs posted tasks (in the form of
@@ -133,6 +137,12 @@ class BASE_EXPORT TaskRunner
                         OnceClosure task,
                         OnceClosure reply);
 
+  // TODO(alexclarke): This should become pure virtual and replace
+  // PostDelayedTask. NB passing by reference to reduce binary size.
+  bool PostPromiseInternal(
+      const scoped_refptr<internal::AbstractPromise>& promise,
+      base::TimeDelta delay);
+
  protected:
   friend struct TaskRunnerTraits;
 

base/test/BUILD.gn

@@ -51,6 +51,8 @@ static_library("test_support") {
     "bind_test_util.h",
     "copy_only_int.cc",
     "copy_only_int.h",
+    "do_nothing_promise.cc",
+    "do_nothing_promise.h",
     "fuzzed_data_provider.h",
     "gtest_util.cc",
     "gtest_util.h",

base/test/do_nothing_promise.cc

+// Copyright 2019 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 "base/test/do_nothing_promise.h"
+
+namespace base {
+
+DoNothingPromiseBuilder::operator scoped_refptr<internal::AbstractPromise>()
+    const {
+  return internal::NoOpPromiseExecutor::Create(from_here, can_resolve,
+                                               can_reject, reject_policy);
+}
+
+}  // namespace base

base/test/do_nothing_promise.h

+// Copyright 2019 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 BASE_TEST_DO_NOTHING_PROMISE_H_
+#define BASE_TEST_DO_NOTHING_PROMISE_H_
+
+#include "base/task/promise/no_op_promise_executor.h"
+
+namespace base {
+
+// Creates a promise whose executor doesn't do anything.
+struct DoNothingPromiseBuilder {
+  explicit DoNothingPromiseBuilder(Location from_here) : from_here(from_here) {}
+
+  const Location from_here;
+  bool can_resolve = false;
+  bool can_reject = false;
+  RejectPolicy reject_policy = RejectPolicy::kMustCatchRejection;
+
+  DoNothingPromiseBuilder& SetCanResolve(bool can_resolve) {
+    this->can_resolve = can_resolve;
+    return *this;
+  }
+
+  DoNothingPromiseBuilder& SetCanReject(bool can_reject) {
+    this->can_reject = can_reject;
+    return *this;
+  }
+
+  DoNothingPromiseBuilder& SetRejectPolicy(RejectPolicy reject_policy) {
+    this->reject_policy = reject_policy;
+    return *this;
+  }
+
+  operator scoped_refptr<internal::AbstractPromise>() const;
+};
+
+}  // namespace base
+
+#endif  // BASE_TEST_DO_NOTHING_PROMISE_H_