Closed Bug 917798 Opened 6 years ago Closed 6 years ago

Handlify JS exception APIs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

No description provided.
Attachment #806804 - Flags: review?(terrence)
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.
(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.)
https://hg.mozilla.org/mozilla-central/rev/0874c0c422b1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 919781
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.