If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Handlify JS exception APIs

RESOLVED FIXED in mozilla27

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

({dev-doc-complete})

unspecified
mozilla27
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

4 years ago
Created attachment 806804 [details] [diff] [review]
handlify-exception-apis
Attachment #806804 - Flags: review?(terrence)
(Assignee)

Updated

4 years ago
Attachment #806804 - Flags: review?(bzbarsky)
Comment on attachment 806804 [details] [diff] [review]
handlify-exception-apis

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

r=me on the JS and JSD changes.
Attachment #806804 - Flags: review?(terrence) → review+
Comment on attachment 806804 [details] [diff] [review]
handlify-exception-apis

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

::: dom/bindings/Exceptions.cpp
@@ +70,5 @@
>    if (!obj) {
>      return false;
>    }
>  
> +  JS::Rooted<JS::Value> exn(aCx, OBJECT_TO_JSVAL(obj));

I think we're now officially using JS::RootedValue everywhere.

This should also be JS::ObjectValue(obj).

::: dom/plugins/base/nsJSNPRuntime.cpp
@@ +456,5 @@
>      JSString *str = ::JS_NewUCStringCopyN(cx, (jschar *)ucex.get(),
>                                            ucex.Length());
>  
>      if (str) {
> +      JS::Rooted<JS::Value> exn(cx, STRING_TO_JSVAL(str));

RootedValue, StringValue(str)

::: js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ +61,5 @@
>  
>  static nsresult
>  ReportError(JSContext *cx, const char *msg)
>  {
> +    RootedValue exn(cx, STRING_TO_JSVAL(JS_NewStringCopyZ(cx, msg)));

StringValue
Comment on attachment 806804 [details] [diff] [review]
handlify-exception-apis

>+      if (aRetValue)

Curlies, please.  r=me
Attachment #806804 - Flags: review?(bzbarsky) → review+
> I think we're now officially using JS::RootedValue everywhere.

Er, we are?  In non-JS code?  Where was that decided, exactly?  I saw nothing about it on .platform.

Comment 6

4 years ago
(In reply to Boris Zbarsky [:bz] from comment #5)
Well, what was "decided" (insofar as njn asking for a module owner decision and me providing one in https://groups.google.com/d/msg/mozilla.dev.tech.js-engine.internals/meWx5yxofYw/NKl_wsFCvL4J) was on using typedefs inside the JS engine.  A corollary is that it would be weird to use a different style inside and outside of the engine, so I expressed my preference for that.  I guess I should have followed up with a dev.platform post asking if we could adopt typedefs-only convention outside JS too; I mostly just forgot about the thread as comments died down.  (Note: the Handle/Rooted templates would still be exposed and used in exceptional cases like generic algorithms.)

Comment 7

4 years ago
m.d.platform thread: https://groups.google.com/forum/#!topic/mozilla.dev.platform/DtN9842JqRo
(Assignee)

Comment 8

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0874c0c422b1

Comment 9

4 years ago
https://hg.mozilla.org/mozilla-central/rev/0874c0c422b1
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 919781
(Assignee)

Updated

4 years ago
Keywords: dev-doc-needed
Docs updated (thanks :arai):

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_GetPendingException
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_SetPendingException
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_ErrorFromException

Technical review appreciated.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.