1040263 - Make promises always have a wrapper

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Then we can use it in their "report that we have no catch handlers" code.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Eagerly create and preserve Promise reflectors so we always have them available during unlink

I assumed we wanted to actually throw on failure to wrap, as opposed to
MOZ_CRASHing, but doing the latter might also be a viable option.

Bobby, could you take a look at the jsapi shenanigans in Create?

Kyle, I think you're most familiar the widest range of the code this is
touching, but let me know if you want me to break up the reviews for this
somehow instead of just dumping it all on you.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=506ad50d9217

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8458170 [details] [diff] [review]
Eagerly create and preserve Promise reflectors so we always have them available during unlink

Review of attachment 8458170 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on Promise::Create. Thanks for doing this.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8458170 [details] [diff] [review]
Eagerly create and preserve Promise reflectors so we always have them available during unlink

Review of attachment 8458170 [details] [diff] [review]:
-----------------------------------------------------------------

Stealing khuey's review per IRC discussion.

Would it make sense to modify the codegen to force Promise returning methods to be annotated with [Throws]?

::: dom/base/SubtleCrypto.cpp
@@ +112,5 @@
>  
>  already_AddRefed<Promise>
>  SubtleCrypto::ExportKey(const nsAString& format,
> +                        CryptoKey& key,
> +                                 ErrorResult& aRv)

Nit: Indentation.

::: dom/bluetooth2/BluetoothAdapter.cpp
@@ +422,5 @@
>    }
> +  nsRefPtr<Promise> promise = Promise::Create(global, aRv);
> +  if (aRv.Failed()) {
> +    return nullptr;
> +  }  

Nit: Trailing ws.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Would it make sense to modify the codegen to force Promise returning methods to be
> annotated with [Throws]?

Hmm.  Maybe, yes...  Seems annoying.  I keep hoping for a better world where we can fail-fatal instead of throwing here.  ;)

Comment 5

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7784c83c72a2

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7784c83c72a2