Auto merge of #20399 - Xanewok:remove-heap-handle-mut, r=jdm

Sanitize Heap::handle(_mut) functions

<!-- Please describe your changes on the following line: -->
Complementary to https://github.com/servo/rust-mozjs/pull/404.

Removing `Heap::handle_mut` didn't warrant any changes on Servo side, and so the changes here are only to fix compilation with `Heap::handle` being now marked as `unsafe`.

The main idea is that we can't hand out handles to heap values themselves, since they're not guaranteed to be rooted, but it's safe to do when we are - hence why the safe impl on `RootedTraceableBox<Heap<T>>` and why it's safe to use inside structs that hold a Heap and are `#[must_root]`.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because the compiler forces correctness here.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20399)
<!-- Reviewable:end -->

Cargo.lock

@@ -1631,7 +1631,7 @@ dependencies = [
  "cssparser 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.17.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "hashglobe 0.1.0",
- "mozjs 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "mozjs 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "selectors 0.19.0",
  "servo_arc 0.1.1",
  "smallbitvec 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1808,7 +1808,7 @@ dependencies = [
 
 [[package]]
 name = "mozjs"
-version = "0.3.1"
+version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "cmake 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2496,7 +2496,7 @@ dependencies = [
  "mime_guess 1.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "mitochondria 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "mozangle 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
- "mozjs 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "mozjs 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "msg 0.0.1",
  "net_traits 0.0.1",
  "num-traits 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3896,7 +3896,7 @@ dependencies = [
 "checksum miow 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "8c1f2f3b1cf331de6896aabf6e9d55dca90356cc9960cca7eaaf408a355ae919"
 "checksum mitochondria 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9de3eca27871df31c33b807f834b94ef7d000956f57aa25c5aed9c5f0aae8f6f"
 "checksum mozangle 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "1f0583e6792917f498bb3a7440f777a59353102063445ab7f5e9d1dc4ed593aa"
-"checksum mozjs 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "896b93aaf26a4cbdcd878b6a9e3b4b90ac018dccebaaac1fe67d2d0724f6a711"
+"checksum mozjs 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4463834f4d4ffcf2a54706c8a202e25ddac3f142e3eccc4aebade58fa62bc1ff"
 "checksum mozjs_sys 0.50.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e61a792a125b1364c5ec50255ed8343ce02dc56098f8868dd209d472c8de006a"
 "checksum mp3-metadata 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4ab5f1d2693586420208d1200ce5a51cd44726f055b635176188137aff42c7de"
 "checksum mp4parse 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f821e3799bc0fd16d9b861fb02fa7ee1b5fba29f45ad591dade105c48ca9a1a0"

components/malloc_size_of/Cargo.toml

@@ -16,7 +16,7 @@ app_units = "0.6"
 cssparser = "0.23.0"
 euclid = "0.17"
 hashglobe = { path = "../hashglobe" }
-mozjs = { version = "0.3", features = ["promises"], optional = true }
+mozjs = { version = "0.4", features = ["promises"], optional = true }
 selectors = { path = "../selectors" }
 servo_arc = { path = "../servo_arc" }
 smallbitvec = "1.0.3"

components/script/Cargo.toml

@@ -62,7 +62,7 @@ metrics = {path = "../metrics"}
 mitochondria = "1.1.2"
 mime = "0.2.1"
 mime_guess = "1.8.0"
-mozjs = { version = "0.3", features = ["promises"]}
+mozjs = { version = "0.4", features = ["promises"]}
 msg = {path = "../msg"}
 net_traits = {path = "../net_traits"}
 num-traits = "0.1.32"

components/script/dom/bindings/reflector.rs

@@ -46,7 +46,8 @@ impl Reflector {
     /// Get the reflector.
     #[inline]
     pub fn get_jsobject(&self) -> HandleObject {
-        self.object.handle()
+        // We're rooted, so it's safe to hand out a handle to object in Heap
+        unsafe { self.object.handle() }
     }
 
     /// Initialize the reflector. (May be called only once.)

components/script/dom/bindings/trace.rs

@@ -59,9 +59,9 @@ use hyper::mime::Mime;
 use hyper::status::StatusCode;
 use ipc_channel::ipc::{IpcReceiver, IpcSender};
 use js::glue::{CallObjectTracer, CallValueTracer};
-use js::jsapi::{GCTraceKindToAscii, Heap, JSObject, JSTracer, TraceKind};
+use js::jsapi::{GCTraceKindToAscii, Heap, Handle, JSObject, JSTracer, TraceKind};
 use js::jsval::JSVal;
-use js::rust::Runtime;
+use js::rust::{GCMethods, Runtime};
 use js::typedarray::TypedArray;
 use js::typedarray::TypedArrayElement;
 use metrics::{InteractiveMetrics, InteractiveWindow};
@@ -788,6 +788,16 @@ impl<T: JSTraceable + 'static> RootedTraceableBox<T> {
     }
 }
 
+impl<T> RootedTraceableBox<Heap<T>>
+    where
+        Heap<T>: JSTraceable + 'static,
+        T: GCMethods + Copy,
+{
+    pub fn handle(&self) -> Handle<T> {
+        unsafe { (*self.ptr).handle() }
+    }
+}
+
 impl<T: JSTraceable + Default> Default for RootedTraceableBox<T> {
     fn default() -> RootedTraceableBox<T> {
         RootedTraceableBox::new(T::default())

components/script/dom/customelementregistry.rs

@@ -604,7 +604,8 @@ impl CustomElementReaction {
         match *self {
             CustomElementReaction::Upgrade(ref definition) => upgrade_element(definition.clone(), element),
             CustomElementReaction::Callback(ref callback, ref arguments) => {
-                let arguments = arguments.iter().map(|arg| arg.handle()).collect();
+                // We're rooted, so it's safe to hand out a handle to objects in Heap
+                let arguments = arguments.iter().map(|arg| unsafe { arg.handle() }).collect();
                 let _ = callback.Call_(&*element, arguments, ExceptionHandling::Report);
             }
         }

components/script/timers.rs

@@ -500,7 +500,7 @@ impl JsTimerTask {
                     code_str, rval.handle_mut());
             },
             InternalTimerCallback::FunctionTimerCallback(ref function, ref arguments) => {
-                let arguments = arguments.iter().map(|arg| arg.handle()).collect();
+                let arguments = self.collect_heap_args(arguments);
                 let _ = function.Call_(this, arguments, Report);
             },
         };
@@ -516,4 +516,11 @@ impl JsTimerTask {
             timers.initialize_and_schedule(&this.global(), self);
         }
     }
+
+    // Returning Handles directly from Heap values is inherently unsafe, but here it's
+    // always done via rooted JsTimers, which is safe.
+    #[allow(unsafe_code)]
+    fn collect_heap_args(&self, args: &[Heap<JSVal>]) -> Vec<HandleValue> {
+        args.iter().map(|arg| unsafe { arg.handle() }).collect()
+    }
 }