791322 - GC: Unexport Rooted classes that are not part of the public API

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: v0

The fact that these are exposed has allowed us to be somewhat sloppy with what gets used in publicly exported headers.  This also has the potential to be majorly troublesome down the road.  Lets put the cat back in the bag before things get seriously messy.

Comment 1

[Terrence Cole [:terrence]]

Comment on attachment 661287 [details] [diff] [review]
v0

Okay, this is going to take some more hacking: I shouldn't have just assumed that Rooted isn't already used all over the browser.  New patch in a bit.

Comment 2

[Terrence Cole [:terrence]]

Attachment: v1: The cat is back in the bag.

The browser builds locally and a try run is up at:
https://tbpl.mozilla.org/?tree=Try&rev=e9eef8698885

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 661430 [details] [diff] [review]
v1: The cat is back in the bag.

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

The good thing about this patch is that it hides Rooted from the API, which we're not sure we want exposed. The bad thing is that, to satisfy the compiler, it adds fromMarkedLocation in 20 places that are not guaranteed to be marked. That seems likely to really confuse people. Overall, I think we're better off sticking with the status quo. It's possible that people will add Rooted in more places, but it will be easy to fix those when we have a coherent strategy.

Comment 4

[Terrence Cole [:terrence]]

Perhaps it would make more sense to move Rooted from JS:: to js:: to better signal our intent?

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Yeah, I think that would be a good idea.

Comment 6

[Terrence Cole [:terrence]]

Attachment: v2: trivial

The browser and jsapi files were changed with a sed script.  Try run up at:
https://tbpl.mozilla.org/?tree=Try&rev=1b59c7fcb34e

Comment 7

[Terrence Cole [:terrence]]

Green try run at:
https://tbpl.mozilla.org/?tree=Try&rev=1b59c7fcb34e

Pushed at:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9721197c221

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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