Closed Bug 972319 Opened 6 years ago Closed 6 years ago

New API JS::AutoSaveExceptionState, replacement for JS_SaveExceptionState

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(2 files)

JS_SaveExceptionState is crazy. If nothing else, it does a tiny malloc which could fail. LOL.

It has a handful of uses and should probably have more. A class-based API would be pretty much tolerable. Patches coming.
Assignee: general → jorendorff
Attachment #8377655 - Flags: review?(luke)
With this, the only remaining user of JS_SaveExceptionState is JSD1.
Attachment #8377661 - Flags: review?(bobbyholley)
Attachment #8377661 - Flags: review?(bobbyholley) → review+
Comment on attachment 8377655 [details] [diff] [review]
bug-972319-part-1-AutoSaveExceptionState-v1.patch

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

::: js/src/jsapi.cpp
@@ +4590,4 @@
>      AssertHeapIsIdle(cx);
>      CHECK_REQUEST(cx);
>      assertSameCompartment(cx, obj);
> +    cx->clearPendingException();

It might be nice to have a \n before the clearPendingException() this to visually separate it from the preceding assertion block (which eyes are inclined to skip over).

::: js/src/jsapi.h
@@ +4636,2 @@
>  extern JS_PUBLIC_API(JSExceptionState *)
>  JS_SaveExceptionState(JSContext *cx);

Any reason not to just remove this now?  I doubt anyone using this will read the comment before whenever we do finally remove it.
Attachment #8377655 - Flags: review?(luke) → review+
It can be removed when JSD is removed.
https://hg.mozilla.org/mozilla-central/rev/8aed2eaed137
https://hg.mozilla.org/mozilla-central/rev/aacc99e7c1e5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.