Upgrade to Spidermonkey 39

[Ms2ger]

Here it is.

There's two major things that are unfinished here:

• Dealing with the unroot_must_root lint. I'm not sure about the value of this lint with the new rooting API. Done.
• Updating the Cargo.locks to point to the new SM and SM binding. Done.

I also included my fixes for the rust update, but these will disappear in a rebase. A rust update is necessary to support calling Drop on Heap<T> correctly when Heap<T> is inside a Rc<T>. Otherwise &self points to the wrong location.

Incremental GC is disabled here. I'm not sure how to deal with the incremental barriers so that's left for later.

Generational GC works. SM doesn't work without it.

The biggest change here is to the rooting API. Root was made movable, and Temporary and JSRef was removed. Movable Roots means there's no need for Temporary, and JSRefs aren't needed generally since it can be assumed that being able to obtain a reference to a dom object means it's already rooted. References have their lifetime bound to the Roots that provided them. DOM objects that haven't passed through reflect_dom_object don't need to be rooted, and DOM objects that have passed through reflect_dom_object can't be obtained without being rooted through native_from_reflector_jsmanaged or JS::<T>::root().

Support for Heap<T> ended up messier than I expected. It's split into two commits, but only because it's a bit difficult to fold them together. Supporting Heap<T> properly requires that that Heap::<T>::set() be called on something that won't move. I removed the Copy and Clone trait from Heap<T> so Cell can't hold Heap<T> - only UnsafeCell can hold it.

CallbackObject is a bit tricky - I moved all callbacks into Rc<T> in order to make sure that the pointer inside to a *mut JSObject doesn't move. This is necessary for supporting Heap<T>.

RootedCollectionSet is very general purpose now. Anything with JSTraceable can be rooted by RootedCollectionSet/RootedTraceable. Right now, RootedTraceable is only used to hold down dom objects before they're fully attached to their reflector. I had to make a custom mechanism to dispatch the trace call - couldn't figure out how to get trait objects working for this case.

This has been tested with the following zeal settings:

GC after every allocation
JS_GC_ZEAL=2,1

GC after every 100 allocations (important for catching use-after-free bugs)
JS_GC_ZEAL=2,100

Verify pre barriers
JS_GC_ZEAL=4,1

Verify post barriers
JS_GC_ZEAL=11,1

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/5064

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[bors-servo]

☔ The latest upstream changes (presumably #6156) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #6125) made this pull request unmergeable. Please resolve the merge conflicts.

[michaelwu]

Apparently homu/bors doesn't always notice a bitrotted PR?

Bitrotted by #6179 .

[bors-servo]

☔ The latest upstream changes (presumably #6168) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #6186) made this pull request unmergeable. Please resolve the merge conflicts.

[michaelwu]

Bors is now blaming random PRs for bitrot here.

[bors-servo]

☔ The latest upstream changes (presumably #6206) made this pull request unmergeable. Please resolve the merge conflicts.

[jdm]

Review status: all files reviewed, 125 unresolved discussions, some commit checks failed.
Reviewed files:

• components/layout/wrapper.rs @ r3
• components/plugins/lints/unrooted_must_root.rs @ r7
• components/plugins/reflector.rs @ r4
• components/script/devtools.rs @ r3
• components/script/dom/activation.rs @ r3
• components/script/dom/attr.rs @ r3
• components/script/dom/bindings/callback.rs @ r4
• components/script/dom/bindings/codegen/Bindings.conf @ r1
• components/script/dom/bindings/codegen/CodegenRust.py @ r5
• components/script/dom/bindings/codegen/Configuration.py @ r4
• components/script/dom/bindings/conversions.rs @ r4
• components/script/dom/bindings/error.rs @ r3
• components/script/dom/bindings/global.rs @ r3
• components/script/dom/bindings/js.rs @ r3
• components/script/dom/bindings/proxyhandler.rs @ r1
• components/script/dom/bindings/refcounted.rs @ r3
• components/script/dom/bindings/structuredclone.rs @ r1
• components/script/dom/bindings/trace.rs @ r5
• components/script/dom/bindings/utils.rs @ r10
• components/script/dom/blob.rs @ r3
• components/script/dom/browsercontext.rs @ r4
• components/script/dom/canvasgradient.rs @ r3
• components/script/dom/canvasrenderingcontext2d.rs @ r3
• components/script/dom/characterdata.rs @ r3
• components/script/dom/closeevent.rs @ r3
• components/script/dom/comment.rs @ r3
• components/script/dom/console.rs @ r3
• components/script/dom/create.rs @ r3
• components/script/dom/cssstyledeclaration.rs @ r3
• components/script/dom/customevent.rs @ r4
• components/script/dom/dedicatedworkerglobalscope.rs @ r3
• components/script/dom/document.rs @ r10
• components/script/dom/documentfragment.rs @ r3
• components/script/dom/documenttype.rs @ r3
• components/script/dom/domexception.rs @ r3
• components/script/dom/domimplementation.rs @ r3
• components/script/dom/domparser.rs @ r3
• components/script/dom/domrect.rs @ r3
• components/script/dom/domrectlist.rs @ r3
• components/script/dom/domstringmap.rs @ r3
• components/script/dom/domtokenlist.rs @ r3
• components/script/dom/element.rs @ r3
• components/script/dom/errorevent.rs @ r4
• components/script/dom/event.rs @ r3
• components/script/dom/eventdispatcher.rs @ r3
• components/script/dom/eventtarget.rs @ r4
• components/script/dom/file.rs @ r3
• components/script/dom/formdata.rs @ r3
• components/script/dom/htmlanchorelement.rs @ r3
• components/script/dom/htmlappletelement.rs @ r3
• components/script/dom/htmlareaelement.rs @ r3
• components/script/dom/htmlaudioelement.rs @ r3
• components/script/dom/htmlbaseelement.rs @ r3
• components/script/dom/htmlbodyelement.rs @ r10
• components/script/dom/htmlbrelement.rs @ r3
• components/script/dom/htmlbuttonelement.rs @ r3
• components/script/dom/htmlcanvaselement.rs @ r3
• components/script/dom/htmlcollection.rs @ r3
• components/script/dom/htmldataelement.rs @ r3
• components/script/dom/htmldatalistelement.rs @ r3
• components/script/dom/htmldialogelement.rs @ r3
• components/script/dom/htmldirectoryelement.rs @ r3
• components/script/dom/htmldivelement.rs @ r3
• components/script/dom/htmldlistelement.rs @ r3
• components/script/dom/htmlelement.rs @ r4
• components/script/dom/htmlembedelement.rs @ r3
• components/script/dom/htmlfieldsetelement.rs @ r3
• components/script/dom/htmlfontelement.rs @ r3
• components/script/dom/htmlformelement.rs @ r3
• components/script/dom/htmlframeelement.rs @ r3
• components/script/dom/htmlframesetelement.rs @ r3
• components/script/dom/htmlheadelement.rs @ r3
• components/script/dom/htmlheadingelement.rs @ r3
• components/script/dom/htmlhrelement.rs @ r3
• components/script/dom/htmlhtmlelement.rs @ r3
• components/script/dom/htmliframeelement.rs @ r3
• components/script/dom/htmlimageelement.rs @ r3
• components/script/dom/htmlinputelement.rs @ r3
• components/script/dom/htmllabelelement.rs @ r3
• components/script/dom/htmllegendelement.rs @ r3
• components/script/dom/htmllielement.rs @ r3
• components/script/dom/htmllinkelement.rs @ r3
• components/script/dom/htmlmapelement.rs @ r3
• components/script/dom/htmlmediaelement.rs @ r3
• components/script/dom/htmlmetaelement.rs @ r3
• components/script/dom/htmlmeterelement.rs @ r3
• components/script/dom/htmlmodelement.rs @ r3
• components/script/dom/htmlobjectelement.rs @ r3
• components/script/dom/htmlolistelement.rs @ r3
• components/script/dom/htmloptgroupelement.rs @ r3
• components/script/dom/htmloptionelement.rs @ r3
• components/script/dom/htmloutputelement.rs @ r3
• components/script/dom/htmlparagraphelement.rs @ r3
• components/script/dom/htmlparamelement.rs @ r3
• components/script/dom/htmlpreelement.rs @ r3
• components/script/dom/htmlprogresselement.rs @ r3
• components/script/dom/htmlquoteelement.rs @ r3
• components/script/dom/htmlscriptelement.rs @ r3
• components/script/dom/htmlselectelement.rs @ r3
• components/script/dom/htmlsourceelement.rs @ r3
• components/script/dom/htmlspanelement.rs @ r3
• components/script/dom/htmlstyleelement.rs @ r3
• components/script/dom/htmltablecaptionelement.rs @ r3
• components/script/dom/htmltablecellelement.rs @ r3
• components/script/dom/htmltablecolelement.rs @ r3
• components/script/dom/htmltabledatacellelement.rs @ r3
• components/script/dom/htmltableelement.rs @ r3
• components/script/dom/htmltableheadercellelement.rs @ r3
• components/script/dom/htmltablerowelement.rs @ r3
• components/script/dom/htmltablesectionelement.rs @ r3
• components/script/dom/htmltemplateelement.rs @ r3
• components/script/dom/htmltextareaelement.rs @ r3
• components/script/dom/htmltimeelement.rs @ r3
• components/script/dom/htmltitleelement.rs @ r3
• components/script/dom/htmltrackelement.rs @ r3
• components/script/dom/htmlulistelement.rs @ r3
• components/script/dom/htmlunknownelement.rs @ r3
• components/script/dom/htmlvideoelement.rs @ r3
• components/script/dom/imagedata.rs @ r4
• components/script/dom/keyboardevent.rs @ r3
• components/script/dom/location.rs @ r3
• components/script/dom/macros.rs @ r4
• components/script/dom/messageevent.rs @ r4
• components/script/dom/mod.rs @ r3
• components/script/dom/mouseevent.rs @ r3
• components/script/dom/namednodemap.rs @ r3
• components/script/dom/navigator.rs @ r3
• components/script/dom/node.rs @ r3
• components/script/dom/nodeiterator.rs @ r3
• components/script/dom/nodelist.rs @ r3
• components/script/dom/performance.rs @ r3
• components/script/dom/performancetiming.rs @ r3
• components/script/dom/processinginstruction.rs @ r3
• components/script/dom/progressevent.rs @ r3
• components/script/dom/range.rs @ r3
• components/script/dom/screen.rs @ r3
• components/script/dom/servohtmlparser.rs @ r10
• components/script/dom/storage.rs @ r3
• components/script/dom/storageevent.rs @ r3
• components/script/dom/testbinding.rs @ r4
• components/script/dom/text.rs @ r3
• components/script/dom/textdecoder.rs @ r3
• components/script/dom/textencoder.rs @ r3
• components/script/dom/treewalker.rs @ r4
• components/script/dom/uievent.rs @ r3
• components/script/dom/urlsearchparams.rs @ r10
• components/script/dom/userscripts.rs @ r3
• components/script/dom/validitystate.rs @ r3
• components/script/dom/virtualmethods.rs @ r3
• components/script/dom/webglbuffer.rs @ r3
• components/script/dom/webglobject.rs @ r3
• components/script/dom/webglprogram.rs @ r3
• components/script/dom/webglrenderingcontext.rs @ r3
• components/script/dom/webglshader.rs @ r3
• components/script/dom/webgluniformlocation.rs @ r3
• components/script/dom/websocket.rs @ r3
• components/script/dom/window.rs @ r10
• components/script/dom/worker.rs @ r3
• components/script/dom/workerglobalscope.rs @ r4
• components/script/dom/workerlocation.rs @ r3
• components/script/dom/workernavigator.rs @ r3
• components/script/dom/xmlhttprequest.rs @ r10
• components/script/dom/xmlhttprequesteventtarget.rs @ r3
• components/script/dom/xmlhttprequestupload.rs @ r3
• components/script/lib.rs @ r1
• components/script/page.rs @ r3
• components/script/parse/html.rs @ r3
• components/script/script_task.rs @ r10
• components/script/textinput.rs @ r3
• components/script/timers.rs @ r4
• components/script/webdriver_handlers.rs @ r3
• components/servo/Cargo.lock @ r10
• components/servo/lib.rs @ r1
• ports/cef/Cargo.lock @ r10
• ports/gonk/Cargo.lock @ r10

---

^{components/layout/wrapper.rs, line 524 [r10] (raw file):}
Move this up with other imports.

---

^{components/script/dom/bindings/callback.rs, line 52 [r10] (raw file):}
Assert the pointer is null.

---

^{components/script/dom/bindings/callback.rs, line 109 [r10] (raw file):}
Assert the pointer is null.

---

^{components/script/dom/bindings/callback.rs, line 160 [r10] (raw file):}
RootedObject or HandleObject? This is a stack class...

---

^{components/script/dom/bindings/callback.rs, line 191 [r10] (raw file):}
The current code doesn't distinguish between Report and Rethrow because we don't have the ability to rethrow right now and it sucks hitting assertions about pending exceptions/silently suppressing them.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 667 [r10] (raw file):}
0u8?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 993 [r10] (raw file):}
HandleObject

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1291 [r10] (raw file):}
This should either be a MutableHandleValue but in an argument position, or we should be using RootedValue in the caller.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1293 [r10] (raw file):}
This should be a MutableHandleObject but in an argument position, or we should be using RootedObject in the caller.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1410 [r10] (raw file):}
We can use 'b"%s" as *const u8 as *const i*' % m["selfHostedName"] I think.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1763 [r10] (raw file):}
No need for the CGIndenter.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1777 [r10] (raw file):}
This change seems wrong. Why is it necessary?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2058 [r10] (raw file):}
\n

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2214 [r10] (raw file):}
Take a MutableHandleObject instead of returning *mut JSObject?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2275 [r10] (raw file):}
MutableHandleObject arg instead.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2290 [r10] (raw file):}
RootedObject

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2305 [r10] (raw file):}
Make this take a MutableHandleObject instead of returning *mut JSOBject?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2372 [r10] (raw file):}
Pretty sure we want the equivalent of obj_toString here...

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2485 [r10] (raw file):}
Kill it.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2541 [r10] (raw file):}
No need for the branch, and 0u8.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2553 [r10] (raw file):}
This appears to be useless.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2693 [r10] (raw file):}
The cast appears redundant.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2695 [r10] (raw file):}
0u8.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2701 [r10] (raw file):}
Could we just use args.thisv() instead?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2722 [r10] (raw file):}
MutableHandleValue

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2742 [r10] (raw file):}
MutableHandleValue

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2748 [r10] (raw file):}
Prefer explicit casts over transmute.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2799 [r10] (raw file):}
MutableHandleValue

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2815 [r10] (raw file):}
Cast instead of transmute.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2875 [r10] (raw file):}
MutableHandleValue

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2889 [r10] (raw file):}
Unused.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2891 [r10] (raw file):}
Cast instead of transmute.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 3040 [r10] (raw file):}
??

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 3060 [r10] (raw file):}
?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 3098 [r10] (raw file):}
?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4236 [r10] (raw file):}
Comment what 0 means?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4252 [r10] (raw file):}
Comment what 0 means?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4255 [r10] (raw file):}
Comment what 0 means?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4412 [r10] (raw file):}
I'm pretty sure this hook isn't used any more.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4477 [r10] (raw file):}
MutableHandleValue

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4837 [r10] (raw file):}
HandleValue

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4894 [r10] (raw file):}
Can we make get_dictionary_property take a Option<MutableHandleValue> instead and match on rval instead of the return value, and make the return value just Result<(), ...>?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5215 [r10] (raw file):}
HandleObject?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5310 [r10] (raw file):}
HandleObject?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5314 [r10] (raw file):}
MutableHandleObject?

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5509 [r10] (raw file):}
HandleObject

---

^{components/script/dom/bindings/conversions.rs, line 30 [r10] (raw file):}
Rc?

---

^{components/script/dom/bindings/conversions.rs, line 97 [r10] (raw file):}
MutableHandleValue?

---

^{components/script/dom/bindings/conversions.rs, line 337 [r10] (raw file):}
HandleString?

---

^{components/script/dom/bindings/conversions.rs, line 342 [r10] (raw file):}
AutoCheckCannotGC?

---

^{components/script/dom/bindings/conversions.rs, line 348 [r10] (raw file):}
for i in 0..length

---

^{components/script/dom/bindings/conversions.rs, line 357 [r10] (raw file):}
AutpCheckCannotGC?

---

^{components/script/dom/bindings/conversions.rs, line 385 [r10] (raw file):}
RootedString?

---

^{components/script/dom/bindings/conversions.rs, line 406 [r10] (raw file):}
RootedString?

---

^{components/script/dom/bindings/conversions.rs, line 441 [r10] (raw file):}
RootedString?

---

^{components/script/dom/bindings/conversions.rs, line 451 [r10] (raw file):}
AutoCheckCannotGC?

---

^{components/script/dom/bindings/conversions.rs, line 465 [r10] (raw file):}
AutoCheckCannotGC?

---

^{components/script/dom/bindings/conversions.rs, line 498 [r10] (raw file):}
HandleObject?

---

^{components/script/dom/bindings/conversions.rs, line 513 [r10] (raw file):}
HandleObject?

---

^{components/script/dom/bindings/conversions.rs, line 518 [r10] (raw file):}
RootedValue?

---

^{components/script/dom/bindings/error.rs, line 150 [r10] (raw file):}
0u8

---

^{components/script/dom/bindings/js.rs, line 290 [r10] (raw file):}
Note to self: think harder about this.

---

^{components/script/dom/bindings/js.rs, line 417 [r10] (raw file):}
Why would this ever be null?

---

^{components/script/dom/bindings/proxyhandler.rs, line 84 [r10] (raw file):}
???

---

^{components/script/dom/bindings/proxyhandler.rs, line 89 [r10] (raw file):}
1u8

---

^{components/script/dom/bindings/proxyhandler.rs, line 92 [r10] (raw file):}
???

---

^{components/script/dom/bindings/proxyhandler.rs, line 96 [r10] (raw file):}
1u8

---

^{components/script/dom/bindings/proxyhandler.rs, line 114 [r10] (raw file):}
MutableHandleObject instead of returning a raw pointer.

---

^{components/script/dom/bindings/proxyhandler.rs, line 126 [r10] (raw file):}
Why this change? DOMJSProxyHandler::EnsureExpandoObject disagrees.

---

^{components/script/dom/bindings/structuredclone.rs, line 52 [r10] (raw file):}
MutableHandleValue arg instead.

---

^{components/script/dom/bindings/trace.rs, line 94 [r10] (raw file):}
&mut Heap<JSVal>

---

^{components/script/dom/bindings/trace.rs, line 118 [r10] (raw file):}
&mut Heap<*mut JSObject>

---

^{components/script/dom/bindings/trace.rs, line 186 [r10] (raw file):}
Transmuting & to &mut is undefined behaviour.

---

^{components/script/dom/bindings/trace.rs, line 195 [r10] (raw file):}
Transmuting & to &mut is undefined behaviour.

---

^{components/script/dom/bindings/trace.rs, line 349 [r10] (raw file):}
The name doesn't really match any more, or the documentation above it.

---

^{components/script/dom/bindings/trace.rs, line 380 [r10] (raw file):}
let obj: &T = unsafe { &*(obj as *const T) };

---

^{components/script/dom/bindings/utils.rs, line 24 [r10] (raw file):}
std::default::Default

---

^{components/script/dom/bindings/utils.rs, line 84 [r10] (raw file):}
I'm pretty sure this can be b"ArrayValues\0" inline where it's used, instead.

---

^{components/script/dom/bindings/utils.rs, line 194 [r10] (raw file):}
MutHandleValue?

---

^{components/script/dom/bindings/utils.rs, line 206 [r10] (raw file):}
MutHandleObject?

---

^{components/script/dom/bindings/utils.rs, line 243 [r10] (raw file):}
RootedFunction?

---

^{components/script/dom/bindings/utils.rs, line 317 [r10] (raw file):}
MutHandleObject?

---

^{components/script/dom/bindings/utils.rs, line 342 [r10] (raw file):}
MutHandleObject?

---

^{components/script/dom/bindings/utils.rs, line 351 [r10] (raw file):}
HandleObject

---

^{components/script/dom/bindings/utils.rs, line 369 [r10] (raw file):}
Why this default implementation?

---

^{components/script/dom/bindings/utils.rs, line 403 [r10] (raw file):}
MutHandleOBject?

---

^{components/script/dom/bindings/utils.rs, line 408 [r10] (raw file):}
HandleObject?

---

^{components/script/dom/bindings/utils.rs, line 418 [r10] (raw file):}
Casting immutable to mutable is undefined behaviour.

---

^{components/script/dom/bindings/utils.rs, line 490 [r10] (raw file):}
RootedString?

---

^{components/script/dom/bindings/utils.rs, line 497 [r10] (raw file):}
Elaborate? We need to support JSString values that are Latin1, not just UCS-2?

---

^{components/script/dom/bindings/utils.rs, line 499 [r10] (raw file):}
Either debug_assert or get rid of the following if.

---

^{components/script/dom/bindings/utils.rs, line 515 [r10] (raw file):}
HandleObject

---

^{components/script/dom/bindings/utils.rs, line 542 [r10] (raw file):}
This doesn't look right. There's an outparam rval and a return value too? Returning a HandleValue doesn't make sense, either.

---

^{components/script/dom/browsercontext.rs, line 28 [r10] (raw file):}
Not sure if this is necessary any more.

---

^{components/script/dom/browsercontext.rs, line 124 [r10] (raw file):}
return 1

---

^{components/script/dom/browsercontext.rs, line 130 [r10] (raw file):}
return 0

---

^{components/script/dom/customevent.rs, line 24 [r10] (raw file):}
Can we retain MutHeap and make it hide the UnsafeCell from DOM implementors?

---

^{components/script/dom/element.rs, line 189 [r10] (raw file):}
Remove.

---

^{components/script/dom/element.rs, line 519 [r10] (raw file):}
Remove.

---

^{components/script/dom/errorevent.rs, line 32 [r10] (raw file):}
MutHeap

---

^{components/script/dom/eventtarget.rs, line 200 [r10] (raw file):}
HandleObject

---

^{components/script/dom/eventtarget.rs, line 212 [r10] (raw file):}
Maybe we should add scope to this chain?

---

^{components/script/dom/htmlcanvaselement.rs, line 142 [r10] (raw file):}
=> context,

---

^{components/script/dom/htmlinputelement.rs, line 155 [r10] (raw file):}
Why these changes?

---

^{components/script/dom/htmlstyleelement.rs, line 71 [r10] (raw file):}
Why these changes?

---

^{components/script/dom/imagedata.rs, line 29 [r10] (raw file):}
This should be folded in with new, since this can't be used as an inherited value any more.

---

^{components/script/dom/imagedata.rs, line 39 [r10] (raw file):}
RootedObject?

---

^{components/script/dom/messageevent.rs, line 39 [r10] (raw file):}
HandleValue

---

^{components/script/dom/messageevent.rs, line 66 [r10] (raw file):}
HandleValue

---

^{components/script/dom/node.rs, line 1030 [r10] (raw file):}
We lose safety guarantees by removing this.

---

^{components/script/dom/node.rs, line 1166 [r10] (raw file):}
Why these changes?

---

^{components/script/dom/node.rs, line 1533 [r10] (raw file):}
for kid in kids

---

^{components/script/dom/node.rs, line 1539 [r10] (raw file):}
for kid in kids

---

^{components/script/dom/testbinding.rs, line 81 [r10] (raw file):}
We should probably make this take a MutableValueHAndle instead.

---

^{components/script/dom/testbinding.rs, line 127 [r10] (raw file):}
MutableObjectHandle

---

^{components/script/dom/testbinding.rs, line 128 [r10] (raw file):}
ObjectHandle

---

^{components/script/dom/textencoder.rs, line 84 [r10] (raw file):}
RootedObject

---

^{components/script/dom/webglrenderingcontext.rs, line 109 [r10] (raw file):}
Should we replicate the AutoCheckCannotGC API?

---

^{components/script/dom/webglrenderingcontext.rs, line 253 [r10] (raw file):}
Same as previous.

---

^{components/script/dom/window.rs, line 579 [r10] (raw file):}
RootedObject

---

^{components/script/dom/window.rs, line 582 [r10] (raw file):}
RootedObject

---

^{components/script/timers.rs, line 215 [r10] (raw file):}
A comment here explaining why this is necessary wouldn't go amiss.

---

^{components/script/timers.rs, line 223 [r10] (raw file):}
Why this block?

---

^{components/script/timers.rs, line 248 [r10] (raw file):}
Is &* necessary?

---

^{components/script/timers.rs, line 252 [r10] (raw file):}
RootedObject

---

Comments from the review on Reviewable.io

[michaelwu]

Review status: all files reviewed, 94 unresolved discussions, some commit checks failed.

---

^{components/layout/wrapper.rs, line 524 [r10] (raw file):}
I originally had it at the top, but local_name() and namespace() exist in both RawLayoutElementHelpers and ElementHelpers and rustc can't tell which one to use.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 667 [r10] (raw file):}
I've been using "(true|false) as u8" since these u8 return types are actually bool return types but the bindings generator doesn't handle bool. (because I don't know if it's safe to assume rust bool == c++ bool)

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 993 [r10] (raw file):}
This is somewhat awkward - in most cases, we're starting from a HandleValue, so we'd have to reroot in order to get a HandleObject, and not all methods want a rooted object to work with.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 1777 [r10] (raw file):}
Not necessary - removed.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2290 [r10] (raw file):}
This doesn't actually need rooting because it can only trigger GC if cached_object is null. I'll move things around to make that more obvious.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2541 [r10] (raw file):}
Branch killed. I kept false as u8 for the same reason as before.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2553 [r10] (raw file):}
Removed

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2701 [r10] (raw file):}
This function would need to switch to CallArgs first, and then a thisv() function needs to be added to CallArgs. All reasonable, though I think I would prefer to do that afterwards.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2722 [r10] (raw file):}
This function signature is defined by SM

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2742 [r10] (raw file):}
This function signature is defined by SM

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2748 [r10] (raw file):}
I tried, but I couldn't find a way to do it from this.r(). A new function could be added to Root, but given the limited number of valid users, it seems better to transmute on this side.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2799 [r10] (raw file):}
This function signature is defined by SM

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2815 [r10] (raw file):}
Same comment as before - can't figure out how to do it without adding a getter just for this call.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2875 [r10] (raw file):}
This function signature is defined by SM

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 2891 [r10] (raw file):}
Same comment as before - can't figure out how to do it without adding a getter just for this call.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4477 [r10] (raw file):}
Interface defined by SM

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4894 [r10] (raw file):}
Not sure exactly what you mean here

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5215 [r10] (raw file):}
This requires CallbackContainer::new and other callback things to take HandleObject, which I wasn't able to do in a way that made sense to me.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5310 [r10] (raw file):}
Just tried this, but it only seems to make the code more complicated. In many cases, the callback is coming from a HandleValue, so we would have to reroot the obj to get a HandleObject.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 5314 [r10] (raw file):}
Just tried this and it generally seems to make things more complicated without much benefit. When one grabs a callback, it's usually in order to call it, and the jsapi for calling functions requires rooting.

---

^{components/script/dom/bindings/conversions.rs, line 337 [r10] (raw file):}
The actual strings in jsstrings are stored on heap, so the functions used to access data in strings do not cause GC.

---

^{components/script/dom/bindings/conversions.rs, line 342 [r10] (raw file):}
Passing null here because we have no implementation that does anything. AIUI it's in the arg list to ensure that the caller puts it on stack.

---

^{components/script/dom/bindings/conversions.rs, line 357 [r10] (raw file):}
Same comment as before

---

^{components/script/dom/bindings/conversions.rs, line 385 [r10] (raw file):}
No GC can occur here.

---

^{components/script/dom/bindings/conversions.rs, line 406 [r10] (raw file):}
No GC can occur here.

---

^{components/script/dom/bindings/conversions.rs, line 441 [r10] (raw file):}
No GC can occur here.

---

^{components/script/dom/bindings/conversions.rs, line 451 [r10] (raw file):}
Serves no purpose currently.

---

^{components/script/dom/bindings/conversions.rs, line 465 [r10] (raw file):}
Serves no purpose currently.

---

^{components/script/dom/bindings/conversions.rs, line 498 [r10] (raw file):}
Doesn't make sense here.

---

^{components/script/dom/bindings/conversions.rs, line 513 [r10] (raw file):}
Not necessary - nothing should cause GC here.

---

^{components/script/dom/bindings/conversions.rs, line 518 [r10] (raw file):}
These JSVals are private values, so rooting isn't necessary.

---

^{components/script/dom/bindings/error.rs, line 150 [r10] (raw file):}
This is another bool that got converted to u8

---

^{components/script/dom/bindings/js.rs, line 417 [r10] (raw file):}
It can't be right now. At one point I was trying to create a "lightweight" root whose lifetime was bounded by the lifetime of an associated HandleObject. I don't think the current setup allows for that so I'll remove this for now.

---

^{components/script/dom/bindings/proxyhandler.rs, line 84 [r10] (raw file):}
That's a dummy comment I inserted to shut the lint up. I'll write something in there now

---

^{components/script/dom/bindings/proxyhandler.rs, line 89 [r10] (raw file):}
Yet another bool converted to u8

---

^{components/script/dom/bindings/proxyhandler.rs, line 92 [r10] (raw file):}
Another dummy comment. Fixing.

---

^{components/script/dom/bindings/proxyhandler.rs, line 96 [r10] (raw file):}
bool converted to u8

---

^{components/script/dom/bindings/proxyhandler.rs, line 114 [r10] (raw file):}
I don't think that's worth the complexity here.

---

^{components/script/dom/bindings/proxyhandler.rs, line 126 [r10] (raw file):}
It's the same thing. JS_NewObject is basically JS_NewObjectWithGivenProto that defaults to a null prototype. JS_NewObject is a bit more clear for this purpose.

---

^{components/script/dom/bindings/utils.rs, line 243 [r10] (raw file):}
This is getting fed into JS_GetFunctionObject which does not expect a handle, so this would not help.

---

^{components/script/dom/bindings/utils.rs, line 497 [r10] (raw file):}
Calling the two byte string functions on Latin1 strings causes a crash. If we can hit those types of JSString values, then we need to support Latin1.

---

^{components/script/dom/bindings/utils.rs, line 515 [r10] (raw file):}
No GC here either so it doesn't make sense

---

^{components/script/dom/browsercontext.rs, line 124 [r10] (raw file):}
c++ bool as u8

---

^{components/script/dom/browsercontext.rs, line 130 [r10] (raw file):}
c++ bool as u8

---

^{components/script/dom/customevent.rs, line 24 [r10] (raw file):}
I made MutHeapJSVal for this. MutHeap is being used in some places so it can't easily be repurposed.

---

^{components/script/dom/htmlinputelement.rs, line 155 [r10] (raw file):}
The initial batch of changes were made with sed and then manually adjusted. Looks like the Raw*Helpers changes don't make too much sense - I'll undo this.

---

^{components/script/dom/imagedata.rs, line 39 [r10] (raw file):}
No possible GC here and also no consumers of handles.

---

^{components/script/dom/node.rs, line 1030 [r10] (raw file):}
I'm not sure what this can be replaced with.

---

^{components/script/dom/node.rs, line 1166 [r10] (raw file):}
Will undo.

---

^{components/script/dom/node.rs, line 1533 [r10] (raw file):}
rust complains about moved values without this.

---

^{components/script/dom/node.rs, line 1539 [r10] (raw file):}
rust complains about moved values without this.

---

^{components/script/dom/textencoder.rs, line 84 [r10] (raw file):}
There's nothing that can cause a GC here or any handle consumers.

---

^{components/script/dom/webglrenderingcontext.rs, line 109 [r10] (raw file):}
Probably. The bindgen I was using at the time wasn't good enough to access the necessary symbols.

---

^{components/script/dom/webglrenderingcontext.rs, line 253 [r10] (raw file):}
Ditto. Need new bindings to implement.

---

^{components/script/dom/window.rs, line 579 [r10] (raw file):}
Can't root without a context, and this is being used to grab a context.

---

^{components/script/timers.rs, line 215 [r10] (raw file):}
Done

---

^{components/script/timers.rs, line 223 [r10] (raw file):}
Removed

---

^{components/script/timers.rs, line 248 [r10] (raw file):}
Removed

---

^{components/script/timers.rs, line 252 [r10] (raw file):}
Can't root without a context, and we're grabbing this object in order to get a context.

---

Comments from the review on Reviewable.io

[larsbergstrom]

@bors-servo r+

[bors-servo]

📌 Commit 3cfecdd has been approved by larsbergstrom

[bors-servo]

⌛ Testing commit 3cfecdd with merge 4f05ca4...

[bors-servo]

💔 Test failed - gonk

[zmike]

Confirming my r+ regarding CEF linkage hacks.

[michaelwu]

Fixing servo/rust-mozjs#158 should let us get rid of the CEF linkage hacks

[mbrubeck]

@bors-servo r+

[bors-servo]

📌 Commit 675267b has been approved by mbrubeck

[bors-servo]

⌛ Testing commit 675267b with merge e7808c5...

[bors-servo]

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

[Manishearth]

👯

[emilio]

\o/