Commit aa9940a9 authored by Peter Van der Beken's avatar Peter Van der Beken
Browse files

Bug 1788969 - Align async iterable code more closely with the spec. r=edgar

Implement the common steps for the next method from
https://webidl.spec.whatwg.org/#es-asynchronous-iterator-prototype-object in
a base class, that all async iterable iterator objects inherit from. Natives
that implement an async iterable only need to implement the "getting the
next iteration result" part in their GetNextPromise method. This means they
don't have to create the object according to "CreateIterResultObject"
themselves, but can just create promise and often resolve it with a native
value directly. We've switched to a special JS::Value to signal "end of
iteration", but that's hidden inside the
iterator_utils::ResolvePromiseForFinished helper.
The WebIDL parser now uses the right return type for the generated "next"
method, which means that any exceptions in the binding code itself will
actually be correctly converted to a rejected promise instead of being
rethrown.
This also uses a class for the generated iterable iterator that's not
exposed outside the binding code. No other code should create and/or
wrap these anyway.

Differential Revision: https://phabricator.services.mozilla.com/D156323
parent 7650d454
Loading
Loading
Loading
Loading
+10 −8
Original line number Diff line number Diff line
@@ -35,7 +35,6 @@ from Configuration import (
    getAllTypes,
    Descriptor,
    MemberIsLegacyUnforgeable,
    iteratorNativeType,
)
AUTOGENERATED_WARNING_COMMENT = (
@@ -5305,7 +5304,7 @@ class CastableObjectUnwrapper:
                }
                """,
                exceptionCode=exceptionCode,
                **self.substitution
                **self.substitution,
            )
        else:
            self.substitution["codeOnFailure"] = codeOnFailure
@@ -5325,7 +5324,7 @@ class CastableObjectUnwrapper:
              }
            }
            """,
            **substitution
            **substitution,
        )
@@ -12812,7 +12811,7 @@ class CGUnionStruct(CGThing):
                    mType = e${name};
                    return mValue.m${name}.SetValue(${ctorArgs});
                    """,
                    **vars
                    **vars,
                )
                # bodyInHeader must be false for return values because they own
@@ -12879,7 +12878,7 @@ class CGUnionStruct(CGThing):
                mValue.m${name}.Destroy();
                mType = eUninitialized;
                """,
                **vars
                **vars,
            )
            methods.append(
                ClassMethod(
@@ -12897,7 +12896,7 @@ class CGUnionStruct(CGThing):
                MOZ_RELEASE_ASSERT(Is${name}(), "Wrong type!");
                return mValue.m${name}.Value();
                """,
                **vars
                **vars,
            )
            # The non-const version of GetAs* returns our internal type
            getterReturnType = "%s&" % vars["structType"]
@@ -13244,7 +13243,7 @@ class CGUnionConversionStruct(CGThing):
                    mUnion.mType = mUnion.e${name};
                    return mUnion.mValue.m${name}.SetValue(${ctorArgs});
                    """,
                    **vars
                    **vars,
                )
                methods.append(
                    ClassMethod(
@@ -22229,14 +22228,17 @@ class CGIterableMethodGenerator(CGGeneric):
            return
        if descriptor.interface.isIterable():
            assert descriptor.interface.maplikeOrSetlikeOrIterable.isPairIterator()
            assert len(args) == 0
            binding = descriptor.interface.identifier.name + "Iterator_Binding"
            iterClass = f"mozilla::dom::binding_detail::WrappableIterableIterator<{descriptor.nativeType}>"
            init = ""
        else:
            assert descriptor.interface.isAsyncIterable()
            binding = descriptor.interface.identifier.name + "AsyncIterator_Binding"
            iterClass = f"mozilla::dom::binding_detail::WrappableAsyncIterableIterator<{descriptor.nativeType}>"
            init = fill(
                """
                {
@@ -22260,7 +22262,7 @@ class CGIterableMethodGenerator(CGGeneric):
                                                   &${binding}::Wrap));
                $*{init}
                """,
                iterClass=iteratorNativeType(descriptor),
                iterClass=iterClass,
                itrMethod=methodName.title(),
                binding=binding,
                init=init,
+6 −1
Original line number Diff line number Diff line
@@ -757,6 +757,8 @@ class Descriptor(DescriptorProvider):
        if name in self.implicitJSContext:
            attrs.append("implicitJSContext")
        if member.isMethod():
            if self.interface.isAsyncIteratorInterface() and name == "next":
                attrs.append("implicitJSContext")
            # JSObject-returning [NewObject] methods must be fallible,
            # since they have to (fallibly) allocate the new JSObject.
            if member.getExtendedAttribute("NewObject"):
@@ -1050,7 +1052,10 @@ def iteratorNativeType(descriptor):
    assert iterableDecl.isPairIterator() or descriptor.interface.isAsyncIterable()
    if descriptor.interface.isIterable():
        return "mozilla::dom::IterableIterator<%s>" % descriptor.nativeType
    return "mozilla::dom::AsyncIterableIterator<%s>" % descriptor.nativeType
    return (
        "mozilla::dom::binding_detail::AsyncIterableIteratorNoReturn<%s>"
        % descriptor.nativeType
    )


def findInnermostType(t):
+153 −34
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "mozilla/dom/IterableIterator.h"
#include "mozilla/dom/Promise-inl.h"

namespace mozilla::dom {

@@ -31,7 +32,8 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IterableIteratorBase)
NS_INTERFACE_MAP_END

namespace iterator_utils {
void DictReturn(JSContext* aCx, JS::MutableHandle<JSObject*> aResult,

void DictReturn(JSContext* aCx, JS::MutableHandle<JS::Value> aResult,
                bool aDone, JS::Handle<JS::Value> aValue, ErrorResult& aRv) {
  RootedDictionary<IterableKeyOrValueResult> dict(aCx);
  dict.mDone = aDone;
@@ -41,6 +43,16 @@ void DictReturn(JSContext* aCx, JS::MutableHandle<JSObject*> aResult,
    aRv.Throw(NS_ERROR_FAILURE);
    return;
  }
  aResult.set(dictValue);
}

void DictReturn(JSContext* aCx, JS::MutableHandle<JSObject*> aResult,
                bool aDone, JS::Handle<JS::Value> aValue, ErrorResult& aRv) {
  JS::Rooted<JS::Value> dictValue(aCx);
  DictReturn(aCx, &dictValue, aDone, aValue, aRv);
  if (aRv.Failed()) {
    return;
  }
  aResult.set(&dictValue.toObject());
}

@@ -67,46 +79,153 @@ void KeyAndValueReturn(JSContext* aCx, JS::Handle<JS::Value> aKey,
  aResult.set(&dictValue.toObject());
}

void ResolvePromiseForFinished(JSContext* aCx, Promise* aPromise) {
  MOZ_ASSERT(aPromise);
}  // namespace iterator_utils

namespace binding_detail {

  ErrorResult error;
  JS::Rooted<JSObject*> dict(aCx);
  DictReturn(aCx, &dict, true, JS::UndefinedHandleValue, error);
  if (error.Failed()) {
    aPromise->MaybeReject(std::move(error));
    return;
static already_AddRefed<Promise> PromiseOrErr(
    Result<RefPtr<Promise>, nsresult>&& aResult, ErrorResult& aError) {
  if (aResult.isErr()) {
    aError.Throw(aResult.unwrapErr());
    return nullptr;
  }
  aPromise->MaybeResolve(dict);

  return aResult.unwrap().forget();
}

void ResolvePromiseWithKeyOrValue(JSContext* aCx, Promise* aPromise,
                                  JS::Handle<JS::Value> aKeyOrValue) {
  MOZ_ASSERT(aPromise);
already_AddRefed<Promise> AsyncIterableNextImpl::NextSteps(
    JSContext* aCx, AsyncIterableIteratorBase* aObject,
    nsIGlobalObject* aGlobalObject, ErrorResult& aRv) {
  // 2. If object’s is finished is true, then:
  if (aObject->mIsFinished) {
    // 1. Let result be CreateIterResultObject(undefined, true).
    JS::Rooted<JS::Value> dict(aCx);
    iterator_utils::DictReturn(aCx, &dict, true, JS::UndefinedHandleValue, aRv);
    if (aRv.Failed()) {
      return Promise::CreateRejectedWithErrorResult(aGlobalObject, aRv);
    }

  ErrorResult error;
  JS::Rooted<JSObject*> dict(aCx);
  DictReturn(aCx, &dict, false, aKeyOrValue, error);
  if (error.Failed()) {
    aPromise->MaybeReject(std::move(error));
    return;
    // 2. Perform ! Call(nextPromiseCapability.[[Resolve]], undefined,
    //    «result»).
    // 3. Return nextPromiseCapability.[[Promise]].
    return Promise::Resolve(aGlobalObject, aCx, dict, aRv);
  }

  // 4. Let nextPromise be the result of getting the next iteration result with
  //    object’s target and object.
  RefPtr<Promise> nextPromise = GetNextPromise(aRv);

  // 5. Let fulfillSteps be the following steps, given next:
  auto fulfillSteps = [](JSContext* aCx, JS::Handle<JS::Value> aNext,
                         ErrorResult& aRv,
                         const RefPtr<AsyncIterableIteratorBase>& aObject,
                         const nsCOMPtr<nsIGlobalObject>& aGlobalObject)
      -> already_AddRefed<Promise> {
    // 1. Set object’s ongoing promise to null.
    aObject->mOngoingPromise = nullptr;

    // 2. If next is end of iteration, then:
    JS::Rooted<JS::Value> dict(aCx);
    if (aNext.isMagic(binding_details::END_OF_ITERATION)) {
      // 1. Set object’s is finished to true.
      aObject->mIsFinished = true;
      // 2. Return CreateIterResultObject(undefined, true).
      iterator_utils::DictReturn(aCx, &dict, true, JS::UndefinedHandleValue,
                                 aRv);
      if (aRv.Failed()) {
        return nullptr;
      }
    } else {
      // 3. Otherwise, if interface has a pair asynchronously iterable
      // declaration:
      //   1. Assert: next is a value pair.
      //   2. Return the iterator result for next and kind.
      // 4. Otherwise:
      //   1. Assert: interface has a value asynchronously iterable declaration.
      //   2. Assert: next is a value of the type that appears in the
      //   declaration.
      //   3. Let value be next, converted to an ECMAScript value.
      //   4. Return CreateIterResultObject(value, false).
      iterator_utils::DictReturn(aCx, &dict, false, aNext, aRv);
      if (aRv.Failed()) {
        return nullptr;
      }
    }
  aPromise->MaybeResolve(dict);
    // Note that ThenCatchWithCycleCollectedArgs expects a Promise, so
    // we use Promise::Resolve here. The specs do convert this to a
    // promise too at another point, but the end result should be the
    // same.
    return Promise::Resolve(aGlobalObject, aCx, dict, aRv);
  };
  // 7. Let rejectSteps be the following steps, given reason:
  auto rejectSteps = [](JSContext* aCx, JS::Handle<JS::Value> aReason,
                        ErrorResult& aRv,
                        const RefPtr<AsyncIterableIteratorBase>& aObject,
                        const nsCOMPtr<nsIGlobalObject>& aGlobalObject) {
    // 1. Set object’s ongoing promise to null.
    aObject->mOngoingPromise = nullptr;
    // 2. Set object’s is finished to true.
    aObject->mIsFinished = true;
    // 3. Throw reason.
    return Promise::Reject(aGlobalObject, aCx, aReason, aRv);
  };
  // 9. Perform PerformPromiseThen(nextPromise, onFulfilled, onRejected,
  //    nextPromiseCapability).
  Result<RefPtr<Promise>, nsresult> result =
      nextPromise->ThenCatchWithCycleCollectedArgs(
          std::move(fulfillSteps), std::move(rejectSteps), RefPtr{aObject},
          nsCOMPtr{aGlobalObject});

  // 10. Return nextPromiseCapability.[[Promise]].
  return PromiseOrErr(std::move(result), aRv);
}

void ResolvePromiseWithKeyAndValue(JSContext* aCx, Promise* aPromise,
                                   JS::Handle<JS::Value> aKey,
                                   JS::Handle<JS::Value> aValue) {
  MOZ_ASSERT(aPromise);
already_AddRefed<Promise> AsyncIterableNextImpl::Next(
    JSContext* aCx, AsyncIterableIteratorBase* aObject,
    nsISupports* aGlobalObject, ErrorResult& aRv) {
  nsCOMPtr<nsIGlobalObject> globalObject = do_QueryInterface(aGlobalObject);

  // 3.7.10.2. Asynchronous iterator prototype object
  // …
  // 10. If ongoingPromise is not null, then:
  if (aObject->mOngoingPromise) {
    // 1. Let afterOngoingPromiseCapability be
    //    ! NewPromiseCapability(%Promise%).
    // 2. Let onSettled be CreateBuiltinFunction(nextSteps, « »).

    // aObject is the same object as 'this', so it's fine to capture 'this'
    // without taking a strong reference, because we already take a strong
    // reference to it through aObject.
    auto onSettled = [this](JSContext* aCx, JS::Handle<JS::Value> aValue,
                            ErrorResult& aRv,
                            const RefPtr<AsyncIterableIteratorBase>& aObject,
                            const nsCOMPtr<nsIGlobalObject>& aGlobalObject) {
      return NextSteps(aCx, aObject, aGlobalObject, aRv);
    };

    // 3. Perform PerformPromiseThen(ongoingPromise, onSettled, onSettled,
    //    afterOngoingPromiseCapability).
    Result<RefPtr<Promise>, nsresult> afterOngoingPromise =
        aObject->mOngoingPromise->ThenCatchWithCycleCollectedArgs(
            onSettled, onSettled, RefPtr{aObject}, std::move(globalObject));
    if (afterOngoingPromise.isErr()) {
      aRv.Throw(afterOngoingPromise.unwrapErr());
      return nullptr;
    }

  ErrorResult error;
  JS::Rooted<JSObject*> dict(aCx);
  KeyAndValueReturn(aCx, aKey, aValue, &dict, error);
  if (error.Failed()) {
    aPromise->MaybeReject(std::move(error));
    return;
    // 4. Set object’s ongoing promise to
    //    afterOngoingPromiseCapability.[[Promise]].
    aObject->mOngoingPromise = afterOngoingPromise.unwrap().forget();
  } else {
    // 11. Otherwise:
    //   1. Set object’s ongoing promise to the result of running nextSteps.
    aObject->mOngoingPromise = NextSteps(aCx, aObject, globalObject, aRv);
  }
  aPromise->MaybeResolve(dict);

  // 12. Return object’s ongoing promise.
  return do_AddRef(aObject->mOngoingPromise);
}
}  // namespace iterator_utils

}  // namespace binding_detail

}  // namespace mozilla::dom
+121 −40
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@
#include "js/TypeDecls.h"
#include "js/Value.h"
#include "nsISupports.h"
#include "mozilla/AlreadyAddRefed.h"
#include "mozilla/dom/IterableIteratorBinding.h"
#include "mozilla/dom/Promise.h"
#include "mozilla/dom/RootedDictionary.h"
@@ -39,7 +40,19 @@

namespace mozilla::dom {

namespace binding_details {

// JS::MagicValue(END_OF_ITERATION) is the value we use for
// https://webidl.spec.whatwg.org/#end-of-iteration. It shouldn't be returned to
// JS, because AsyncIterableIteratorBase::NextSteps will detect it and will
// return the result of CreateIterResultObject(undefined, true) instead
// (discarding the magic value).
static const JSWhyMagic END_OF_ITERATION = JS_GENERIC_MAGIC;

}  // namespace binding_details

namespace iterator_utils {

void DictReturn(JSContext* aCx, JS::MutableHandle<JSObject*> aResult,
                bool aDone, JS::Handle<JS::Value> aValue, ErrorResult& aRv);

@@ -47,14 +60,16 @@ void KeyAndValueReturn(JSContext* aCx, JS::Handle<JS::Value> aKey,
                       JS::Handle<JS::Value> aValue,
                       JS::MutableHandle<JSObject*> aResult, ErrorResult& aRv);

void ResolvePromiseForFinished(JSContext* aCx, Promise* aPromise);
inline void ResolvePromiseForFinished(Promise* aPromise) {
  aPromise->MaybeResolve(JS::MagicValue(binding_details::END_OF_ITERATION));
}

void ResolvePromiseWithKeyOrValue(JSContext* aCx, Promise* aPromise,
                                  JS::Handle<JS::Value> aKeyOrValue);
template <typename Key, typename Value>
void ResolvePromiseWithKeyAndValue(Promise* aPromise, const Key& aKey,
                                   const Value& aValue) {
  aPromise->MaybeResolve(MakeTuple(aKey, aValue));
}

void ResolvePromiseWithKeyAndValue(JSContext* aCx, Promise* aPromise,
                                   JS::Handle<JS::Value> aKey,
                                   JS::Handle<JS::Value> aValue);
}  // namespace iterator_utils

class IterableIteratorBase : public nsISupports {
@@ -209,44 +224,46 @@ class IterableIterator final : public IterableIteratorBase {
  uint32_t mIndex;
};

namespace binding_detail {

class AsyncIterableNextImpl;

}  // namespace binding_detail

class AsyncIterableIteratorBase : public IterableIteratorBase {
 public:
  IteratorType GetIteratorType() { return mIteratorType; }

 protected:
  explicit AsyncIterableIteratorBase(IteratorType aIteratorType)
      : mIteratorType(aIteratorType) {}

 private:
  friend class binding_detail::AsyncIterableNextImpl;

  // 3.7.10.1. Default asynchronous iterator objects
  // Target is in AsyncIterableIterator
  // Kind
  IteratorType mIteratorType;
  // Ongoing promise
  RefPtr<Promise> mOngoingPromise;
  // Is finished
  bool mIsFinished = false;
};

template <typename T>
class AsyncIterableIterator final : public IterableIteratorBase,
class AsyncIterableIterator : public AsyncIterableIteratorBase,
                              public SupportsWeakPtr {
 public:
  typedef bool (*WrapFunc)(JSContext* aCx, AsyncIterableIterator<T>* aObject,
                           JS::Handle<JSObject*> aGivenProto,
                           JS::MutableHandle<JSObject*> aReflector);

  explicit AsyncIterableIterator(T* aIterableObj, IteratorType aIteratorType,
                                 WrapFunc aWrapFunc)
      : mIterableObj(aIterableObj),
        mIteratorType(aIteratorType),
        mWrapFunc(aWrapFunc) {
  AsyncIterableIterator(T* aIterableObj, IteratorType aIteratorType)
      : AsyncIterableIteratorBase(aIteratorType), mIterableObj(aIterableObj) {
    MOZ_ASSERT(mIterableObj);
    MOZ_ASSERT(mWrapFunc);
  }

  void SetData(void* aData) { mData = aData; }

  void* GetData() { return mData; }

  IteratorType GetIteratorType() { return mIteratorType; }

  void Next(JSContext* aCx, JS::MutableHandle<JSObject*> aResult,
            ErrorResult& aRv) {
    RefPtr<Promise> promise = mIterableObj->GetNextPromise(aCx, this, aRv);
    if (!promise) {
      aRv.Throw(NS_ERROR_FAILURE);
      return;
    }
    aResult.set(promise->PromiseObj());
  }

  bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,
                  JS::MutableHandle<JSObject*> aObj) {
    return (*mWrapFunc)(aCx, this, aGivenProto, aObj);
  }

 protected:
  virtual ~AsyncIterableIterator() {
    // As long as iterable object does not hold strong ref to its iterators,
@@ -272,16 +289,80 @@ class AsyncIterableIterator final : public IterableIteratorBase,
    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIterableObj);
  }

  // Binding Implementation object that we're iterating over.
  // 3.7.10.1. Default asynchronous iterator objects
  // Target
  RefPtr<T> mIterableObj;
  // Tells whether this is a key, value, or entries iterator.
  IteratorType mIteratorType;
  // Function pointer to binding-type-specific Wrap() call for this iterator.
  WrapFunc mWrapFunc;
  // Kind
  // Ongoing promise
  // Is finished
  // See AsyncIterableIteratorBase

  // Opaque data of the backing object.
  void* mData{nullptr};
};

namespace binding_detail {

template <typename T>
using WrappableIterableIterator = IterableIterator<T>;

class AsyncIterableNextImpl {
 protected:
  already_AddRefed<Promise> Next(JSContext* aCx,
                                 AsyncIterableIteratorBase* aObject,
                                 nsISupports* aGlobalObject, ErrorResult& aRv);
  virtual already_AddRefed<Promise> GetNextPromise(ErrorResult& aRv) = 0;

 private:
  already_AddRefed<Promise> NextSteps(JSContext* aCx,
                                      AsyncIterableIteratorBase* aObject,
                                      nsIGlobalObject* aGlobalObject,
                                      ErrorResult& aRv);
};

template <typename T>
class AsyncIterableIteratorNoReturn : public AsyncIterableIterator<T>,
                                      public AsyncIterableNextImpl {
 public:
  using WrapFunc = bool (*)(JSContext* aCx,
                            AsyncIterableIteratorNoReturn<T>* aObject,
                            JS::Handle<JSObject*> aGivenProto,
                            JS::MutableHandle<JSObject*> aReflector);
  using AsyncIterableIteratorBase::IteratorType;

  AsyncIterableIteratorNoReturn(T* aIterableObj, IteratorType aIteratorType,
                                WrapFunc aWrapFunc)
      : AsyncIterableIterator<T>(aIterableObj, aIteratorType),
        mWrapFunc(aWrapFunc) {
    MOZ_ASSERT(mWrapFunc);
  }

  bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,
                  JS::MutableHandle<JSObject*> aObj) {
    return (*mWrapFunc)(aCx, this, aGivenProto, aObj);
  }

  already_AddRefed<Promise> Next(JSContext* aCx, ErrorResult& aRv) {
    return AsyncIterableNextImpl::Next(
        aCx, this, this->mIterableObj->GetParentObject(), aRv);
  }

 protected:
  already_AddRefed<Promise> GetNextPromise(ErrorResult& aRv) override {
    return this->mIterableObj->GetNextPromise(
        static_cast<AsyncIterableIterator<T>*>(this), aRv);
  }

 private:
  // Function pointer to binding-type-specific Wrap() call for this iterator.
  WrapFunc mWrapFunc;
};

template <typename T>
using WrappableAsyncIterableIterator = AsyncIterableIteratorNoReturn<T>;

}  // namespace binding_detail

}  // namespace mozilla::dom

#endif  // mozilla_dom_IterableIterator_h
+28 −0
Original line number Diff line number Diff line
@@ -406,6 +406,34 @@ template <typename T>
  return true;
}

// Accept tuple of other things we accept. The result will be a JS array object.
template <typename... Elements>
[[nodiscard]] bool ToJSValue(JSContext* aCx,
                             const Tuple<Elements...>& aArguments,
                             JS::MutableHandle<JS::Value> aValue) {
  // Make sure we're called in a compartment
  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));

  JS::RootedVector<JS::Value> v(aCx);
  if (!v.resize(sizeof...(Elements))) {
    return false;
  }
  bool ok = true;
  size_t i = 0;
  ForEach(aArguments, [aCx, &ok, &v, &i](auto& aElem) {
    ok = ok && ToJSValue(aCx, aElem, v[i++]);
  });
  if (!ok) {
    return false;
  }
  JSObject* arrayObj = JS::NewArrayObject(aCx, v);
  if (!arrayObj) {
    return false;
  }
  aValue.setObject(*arrayObj);
  return true;
}

// Accept records of other things we accept. N.B. This assumes that
// keys are either UTF-8 or UTF-16-ish. See Bug 1706058.
template <typename K, typename V>
Loading