777219 - Prepare for converting the JSAPI to JSObject handles

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

As a step towards converting the JSAPI to use Handles everywhere necessary, I:

step 1: converted (almost) all JSObject pointers to JSHandleObject or JSMutableHandleObject

step 2: fixed up everything within SpiderMonkey that uses the public (or friend) API

step 3: partially reverted the change to maintain compatibility with Gecko

The end result is an unmodified JSAPI that properly roots everything, but it does the rooting just underneath the public API so that it will be straightforward to backout step 3 (all at once or incrementally) to have a fully handle-based JSAPI.

Note that this is not a complete Handle-ification; it only does JSObject pointers. In particular, jsval/Value is not done. That will be the next step. I haven't decided whether to update the patches or add another similar series on top.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Make RawObject and RawConstObject typedefs for documenting parameters that do not require rooting

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Use JSHandleObject in the JSAPI (all but jsapi-tests)

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Use JSHandleObject in jsapi-tests (it works out differently than the rest of SM)

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Automated portion of step 3 (un-handlifying JSAPI)

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Manual portion of step 3 (un-handlifying the JSAPI)

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: patch1 - Make RawObject typedef for documenting parameters that do not require rooting

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: patch 2 - Use JSHandleObject in the JSAPI (all but jsapi-tests)

Comment 8

[Steve Fink [:sfink] [:s:]]

Attachment: patch 3 - Use JSHandleObject in jsapi-tests

The changes in jsapi-tests are a little weird, so separating out. This must land together with patch 2, though, or it won't compile.

Comment 9

[Steve Fink [:sfink] [:s:]]

Attachment: patch 4 - Automated portion of step 3 (un-handlifying JSAPI)

Comment 10

[Steve Fink [:sfink] [:s:]]

Attachment: patch 5 - Manual portion of step 3 (un-handlifying the JSAPI)

Comment 11

[Steve Fink [:sfink] [:s:]]

Note that all jstests, jit-tests, and jsapi-tests pass after this patch series, or after just patches 1-3.

Comment 12

[Steve Fink [:sfink] [:s:]]

Attachment: patch 4 - Automated portion of step 3 (un-handlifying JSAPI)

After reading billm's review of bug 776579, I updated the naming from obj_ => objArg.

Comment 13

[Steve Fink [:sfink] [:s:]]

Attachment: Manual portion of step 3 (un-handlifying the JSAPI)

...and rebased the manual portion on top.

Comment 14

[Steve Fink [:sfink] [:s:]]

Ug. Also note that my automatic api reversion script only caught things named JS_ something, so the browser (eg xpconnect) won't compile with this. I'll either have to make the script more flexible or manually revert several dozen things. Doh!

Comment 15

[Steve Fink [:sfink] [:s:]]

Attachment: patch 2 - Use JSHandleObject in the JSAPI (all but jsapi-tests)

Updated, mostly to add JSRawObject in more places discovered while getting things to build with the browser.

Comment 16

[Steve Fink [:sfink] [:s:]]

Attachment: patch 3 - Use JSHandleObject in jsapi-tests (it works out differently than the rest of SM)

this probably didn't change much at all, but updating just in case.

Comment 17

[Steve Fink [:sfink] [:s:]]

Attachment: patch 4 - Automated portion of step 3 (un-handlifying JSAPI)

Added a bunch of things in for jsfriendapi.h, which doesn't use a leading JS_ so I was missing it before.

Note that this patch is generated automatically, and so shouldn't require much more than a cursory review.

Comment 18

[Steve Fink [:sfink] [:s:]]

Attachment: patch 5 - Manual portion of step 3 (un-handlifying the JSAPI)

This is mainly what changed in order to get the browser building. I still have not run this through try; will do that next.

Comment 19

[Steve Fink [:sfink] [:s:]]

Attachment: script for automatically un-handlifying jsapi functions

just in case it's helpful to some poor sucker needing to rebase this stuff (hi terrence!), this is the script I use to generate the automated portion. Run it with no arguments from within js/src.

Comment 20

[Terrence Cole [:terrence]]

Comment on attachment 646376 [details] [diff] [review]
patch1 - Make RawObject typedef for documenting parameters that do not require rooting

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

I've already done half of this, so I'm going to steal the r+.

::: js/src/gc/Root.h
@@ +224,5 @@
>  /*
> + * Raw pointer used as documentation that a parameter does not need to be
> + * rooted.
> + */
> +typedef JSObject *                  RawObject;

I've already got this and equivalent comment checked into central, farther down in the file.

Comment 21

[Steve Fink [:sfink] [:s:]]

Attachment: patch 2 - Use JSHandleObject in the JSAPI (all but jsapi-tests)

Sorry for the patch churn, but try server found one stupid thing I messed up in CTypes.cpp. This incorporates it. This is also rebased onto the latest greatest m-i.

Comment 22

[Brian Hackett [Laid off!]]

Comment on attachment 646682 [details] [diff] [review]
patch 4 - Automated portion of step 3 (un-handlifying JSAPI)

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

::: js/src/jstypedarray.cpp
@@ +3302,2 @@
>  {
> +    RootedObject obj_(cx, obj_Arg);

Pre-existing it seems, but the script kind of made a hash of these obj_ args, which should be renamed I think.

Comment 23

[Terrence Cole [:terrence]]

And we have a green try run:
https://tbpl.mozilla.org/?tree=Try&rev=950a3d974aa6

And we are pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a91040f69ea3

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a91040f69ea3