Bug 1729458 - Part 1: Simplify DomPromiseListener, r=smaug,rpl

This makes a couple of changes, including removing the call to
AppendNativeHandler from the constructor, and using the nullable nature of
std::function instead of wrapping them in Maybe.

The main advantage of this change, however, is that it no longer acquires a
reference to `this` during the constructor, which could be unsafe.

Differential Revision: https://phabricator.services.mozilla.com/D124827

dom/promise/PromiseNativeHandler.cpp

@@ -13,31 +13,23 @@ namespace dom {
 
 NS_IMPL_ISUPPORTS0(DomPromiseListener)
 
-DomPromiseListener::DomPromiseListener(dom::Promise* aDOMPromise) {
-  aDOMPromise->AppendNativeHandler(this);
-}
-
-DomPromiseListener::DomPromiseListener(dom::Promise* aDOMPromise,
-                                       CallbackTypeResolved&& aResolve,
+DomPromiseListener::DomPromiseListener(CallbackTypeResolved&& aResolve,
                                        CallbackTypeRejected&& aReject)
-    : mResolve(Some(std::move(aResolve))), mReject(Some(std::move(aReject))) {
-  aDOMPromise->AppendNativeHandler(this);
-}
+    : mResolve(std::move(aResolve)), mReject(std::move(aReject)) {}
 
 DomPromiseListener::~DomPromiseListener() {
   if (mReject) {
-    (*mReject)(NS_BINDING_ABORTED);
+    mReject(NS_BINDING_ABORTED);
   }
 }
 
 void DomPromiseListener::ResolvedCallback(JSContext* aCx,
                                           JS::Handle<JS::Value> aValue) {
   if (mResolve) {
-    (*mResolve)(aCx, aValue);
+    mResolve(aCx, aValue);
   }
   // Let's clear the lambdas in case we have a cycle to ourselves.
-  mResolve.reset();
-  mReject.reset();
+  Clear();
 }
 
 void DomPromiseListener::RejectedCallback(JSContext* aCx,
@@ -49,17 +41,15 @@ void DomPromiseListener::RejectedCallback(JSContext* aCx,
     } else {
       errorCode = nsresult(aValue.toInt32());
     }
-    (*mReject)(errorCode);
+    mReject(errorCode);
   }
   // Let's clear the lambdas in case we have a cycle to ourselves.
-  mResolve.reset();
-  mReject.reset();
+  Clear();
 }
 
-void DomPromiseListener::SetResolvers(CallbackTypeResolved&& aResolve,
-                                      CallbackTypeRejected&& aReject) {
-  mResolve = Some(std::move(aResolve));
-  mReject = Some(std::move(aReject));
+void DomPromiseListener::Clear() {
+  mResolve = nullptr;
+  mReject = nullptr;
 }
 
 }  // namespace dom

dom/promise/PromiseNativeHandler.h

@@ -16,8 +16,6 @@
 namespace mozilla {
 namespace dom {
 
-class Promise;
-
 /*
  * PromiseNativeHandler allows C++ to react to a Promise being
  * rejected/resolved. A PromiseNativeHandler can be appended to a Promise using
@@ -47,18 +45,18 @@ class DomPromiseListener final : public PromiseNativeHandler {
       std::function<void(JSContext*, JS::Handle<JS::Value>)>;
   using CallbackTypeRejected = std::function<void(nsresult)>;
 
-  explicit DomPromiseListener(Promise* aDOMPromise);
-  DomPromiseListener(Promise* aDOMPromise, CallbackTypeResolved&& aResolve,
+  DomPromiseListener(CallbackTypeResolved&& aResolve,
                      CallbackTypeRejected&& aReject);
-  void SetResolvers(CallbackTypeResolved&& aResolve,
-                    CallbackTypeRejected&& aReject);
+
+  void Clear();
+
   void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override;
   void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override;
 
  private:
   ~DomPromiseListener();
-  Maybe<CallbackTypeResolved> mResolve;
-  Maybe<CallbackTypeRejected> mReject;
+  CallbackTypeResolved mResolve;
+  CallbackTypeRejected mReject;
 };
 
 }  // namespace dom

toolkit/components/extensions/webidl-api/ExtensionEventListener.cpp

@@ -59,11 +59,11 @@ class SendResponseCallback final : public nsISupports {
     // Create a promise monitor that invalidates the sendResponse
     // callback if the promise has been already resolved or rejected.
     mPromiseListener = new dom::DomPromiseListener(
-        mPromise,
         [self = RefPtr{this}](JSContext* aCx, JS::Handle<JS::Value> aValue) {
           self->Cleanup();
         },
         [self = RefPtr{this}](nsresult aError) { self->Cleanup(); });
+    mPromise->AppendNativeHandler(mPromiseListener);
   }
 
   void Cleanup(bool aIsDestroying = false) {
@@ -73,11 +73,9 @@ class SendResponseCallback final : public nsISupports {
     }
 
     NS_WARNING("SendResponseCallback::Cleanup");
-    // Override the promise listener's resolvers to release the
+    // Clear the promise listener's resolvers to release the
     // RefPtr captured by the ones initially set.
-    mPromiseListener->SetResolvers(
-        [](JSContext* aCx, JS::Handle<JS::Value> aValue) {},
-        [](nsresult aError) {});
+    mPromiseListener->Clear();
     mPromiseListener = nullptr;
 
     if (mPromise) {

xpcom/threads/MozPromiseInlines.h

@@ -4,14 +4,15 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#if !defined(MozPromiseInlines_h_)
-#  define MozPromiseInlines_h_
+#ifndef MozPromiseInlines_h_
+#define MozPromiseInlines_h_
 
-#  include <type_traits>
+#include <type_traits>
 
-#  include "mozilla/MozPromise.h"
-#  include "mozilla/dom/PrimitiveConversions.h"
-#  include "mozilla/dom/PromiseNativeHandler.h"
+#include "mozilla/MozPromise.h"
+#include "mozilla/dom/PrimitiveConversions.h"
+#include "mozilla/dom/Promise.h"
+#include "mozilla/dom/PromiseNativeHandler.h"
 
 namespace mozilla {
 
@@ -26,7 +27,6 @@ MozPromise<ResolveValueT, RejectValueT, IsExclusive>::FromDomPromise(
                 "Reject type must be nsresult");
   RefPtr<Private> p = new Private(__func__);
   RefPtr<dom::DomPromiseListener> listener = new dom::DomPromiseListener(
-      aDOMPromise,
       [p](JSContext* aCx, JS::Handle<JS::Value> aValue) {
         ResolveValueT value;
         bool ok = dom::ValueToPrimitive<ResolveValueT,
@@ -39,6 +39,7 @@ MozPromise<ResolveValueT, RejectValueT, IsExclusive>::FromDomPromise(
         p->Resolve(value, __func__);
       },
       [p](nsresult aError) { p->Reject(aError, __func__); });
+  aDOMPromise->AppendNativeHandler(listener);
   return p;
 }