Fix memory leak with unsettled ManualPromiseResolver

Added a destructor to ManualPromiseResolver which cancels the
promise if unsettled.

Bug: 906125
Change-Id: I4fe26178c53322e8818e98c015121831bdf1e6b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626855
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Auto-Submit: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663097}

base/task/promise/abstract_promise.cc

@@ -234,8 +234,10 @@ AbstractPromise::GetPrerequisitePolicy() {
 }
 
 void AbstractPromise::Execute() {
-  if (IsCanceled())
+  if (IsCanceled()) {
+    OnCanceled();
     return;
+  }
 
 #if DCHECK_IS_ON()
   // Clear |must_catch_ancestor_that_could_reject_| if we can catch it.

base/task/promise/promise.h

@@ -423,6 +423,13 @@ class ManualPromiseResolver {
       RejectPolicy reject_policy = RejectPolicy::kMustCatchRejection)
       : promise_(SequencedTaskRunnerHandle::Get(), from_here, reject_policy) {}
 
+  ~ManualPromiseResolver() {
+    // If the promise wasn't resolved or rejected, then cancel it to make sure
+    // we don't leak memory.
+    if (!promise_.abstract_promise_->IsSettled())
+      promise_.abstract_promise_->OnCanceled();
+  }
+
   template <typename... Args>
   void Resolve(Args&&... arg) noexcept {
     DCHECK(!promise_.abstract_promise_->IsResolved());

base/task/promise/promise_unittest.cc

@@ -57,6 +57,18 @@ class MockObject {
 
 struct DummyError {};
 
+struct Cancelable {
+  Cancelable() : weak_ptr_factory(this) {}
+
+  void LogTask(std::vector<std::string>* log, std::string value) {
+    log->push_back(value);
+  }
+
+  void NopTask() {}
+
+  WeakPtrFactory<Cancelable> weak_ptr_factory;
+};
+
 }  // namespace
 
 class PromiseTest : public testing::Test {
@@ -324,6 +336,45 @@ TEST_F(PromiseTest, ThenAndCatchOnCurrentReturnTypes) {
           BindOnce([]() -> PromiseResult<C, D> { return C{}; }));
 }
 
+TEST_F(PromiseTest, UnsettledManualPromiseResolverCancelsChain) {
+  bool delete_flag = false;
+  Promise<void> p2;
+
+  {
+    ManualPromiseResolver<int> p1(FROM_HERE);
+    p2 = p1.promise().ThenOnCurrent(
+        FROM_HERE, BindOnce([](scoped_refptr<ObjectToDelete> v) {},
+                            MakeRefCounted<ObjectToDelete>(&delete_flag)));
+  }
+
+  EXPECT_TRUE(delete_flag);
+  EXPECT_TRUE(p2.IsCancelledForTesting());
+}
+
+TEST_F(PromiseTest, CancellationSpottedByExecute) {
+  bool delete_flag = false;
+  Promise<void> p3;
+
+  {
+    Cancelable cancelable;
+    ManualPromiseResolver<void> p1(FROM_HERE);
+    Promise<void> p2 = p1.promise().ThenOnCurrent(
+        FROM_HERE, BindOnce(&Cancelable::NopTask,
+                            cancelable.weak_ptr_factory.GetWeakPtr()));
+
+    p1.Resolve();
+    cancelable.weak_ptr_factory.InvalidateWeakPtrs();
+
+    p3 = p2.ThenOnCurrent(
+        FROM_HERE, BindOnce([](scoped_refptr<ObjectToDelete> v) {},
+                            MakeRefCounted<ObjectToDelete>(&delete_flag)));
+  }
+
+  RunLoop().RunUntilIdle();
+  EXPECT_TRUE(delete_flag);
+  EXPECT_TRUE(p3.IsCancelledForTesting());
+}
+
 TEST_F(PromiseTest, RejectAndReReject) {
   ManualPromiseResolver<int, std::string> p(FROM_HERE);
   RunLoop run_loop;
@@ -1202,20 +1253,6 @@ TEST_F(PromiseTest, RejectFinallySkipsThens) {
   run_loop.Run();
 }
 
-namespace {
-struct Cancelable {
-  Cancelable() : weak_ptr_factory(this) {}
-
-  void LogTask(std::vector<std::string>* log, std::string value) {
-    log->push_back(value);
-  }
-
-  void NopTask() {}
-
-  WeakPtrFactory<Cancelable> weak_ptr_factory;
-};
-}  // namespace
-
 TEST_F(PromiseTest, CancelViaWeakPtr) {
   std::vector<std::string> log;
   ManualPromiseResolver<void, std::string> mpr(FROM_HERE,