Closed Bug 1040263 Opened 6 years ago Closed 6 years ago

Make promises always have a wrapper

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Then we can use it in their "report that we have no catch handlers" code.
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
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8458170 - Flags: review?(khuey)
Attachment #8458170 - Flags: review?(bobbyholley)
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.
Attachment #8458170 - Flags: review?(bobbyholley) → review+
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.
Attachment #8458170 - Flags: review?(khuey)
Attachment #8458170 - Flags: review?(bobbyholley)
Attachment #8458170 - Flags: review+
> 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.  ;)
https://hg.mozilla.org/mozilla-central/rev/7784c83c72a2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.