From 8afaa93a256ae01b9bd2c7a435a9b616d19fb8dd Mon Sep 17 00:00:00 2001 From: Yoshi Cheng-Hao Huang Date: Thu, 4 Apr 2019 14:55:44 +0800 Subject: [PATCH] Bug 1534967 - Part 2: Use RootedIdVector in rust binding. r=jonco,fitzgen - Replace AutoIdVector with RootedIdVector. - Replace JS::MutableHandleIdVector through rustbindgen in wrapper.hpp, otherwise MutableHandleIdVector will be converted to u8 in rust, and causes only 1 byte is passed to props in GetPropertyKeys. - Add another glue function GetMutableHandleIdVector to get the object. Differential Revision: https://phabricator.services.mozilla.com/D25360 --- js/rust/build.rs | 2 +- js/rust/etc/wrapper.hpp | 5 +++++ js/rust/src/glue.rs | 13 ++++++------ js/rust/src/jsglue.cpp | 28 ++++++++++++++----------- js/rust/src/rust.rs | 42 ++++++++++++++++++++++---------------- js/rust/tests/enumerate.rs | 6 +++--- 6 files changed, 56 insertions(+), 40 deletions(-) diff --git a/js/rust/build.rs b/js/rust/build.rs index 13d843029bdc..124a97e9c690 100644 --- a/js/rust/build.rs +++ b/js/rust/build.rs @@ -152,7 +152,7 @@ const EXTRA_CLANG_FLAGS: &'static [&'static str] = &[ /// transitively use). const WHITELIST_TYPES: &'static [&'static str] = &[ "JS::AutoCheckCannotGC", - "JS::AutoIdVector", + "JS::RootedIdVector", "JS::CallArgs", "js::Class", "JS::RealmOptions", diff --git a/js/rust/etc/wrapper.hpp b/js/rust/etc/wrapper.hpp index 94a7405060fe..4c2cf8e81bbe 100644 --- a/js/rust/etc/wrapper.hpp +++ b/js/rust/etc/wrapper.hpp @@ -32,3 +32,8 @@ typedef uint32_t HashNumber; ///
template using replaces_MaybeWrapped = T; + +///
+struct MutableHandleIdVector_Simple { + uintptr_t handle_mut; +}; diff --git a/js/rust/src/glue.rs b/js/rust/src/glue.rs index 4d65e6f52a38..a998fec20374 100644 --- a/js/rust/src/glue.rs +++ b/js/rust/src/glue.rs @@ -30,7 +30,7 @@ pub struct ProxyTraps { -> bool>, pub ownPropertyKeys: ::std::option::Option bool>, pub delete_: ::std::option::Option bool>, pub nativeCall: ::std::option::Option bool; pub fn UnwrapObjectStatic(obj: *mut JSObject) -> *mut JSObject; pub fn UncheckedUnwrapObject(obj: *mut JSObject, stopAtOuter: u8) -> *mut JSObject; - pub fn CreateAutoIdVector(cx: *mut JSContext) -> *mut JS::AutoIdVector; - pub fn AppendToAutoIdVector(v: *mut JS::AutoIdVector, id: jsid) -> bool; - pub fn SliceAutoIdVector(v: *const JS::AutoIdVector, length: *mut usize) -> *const jsid; - pub fn DestroyAutoIdVector(v: *mut JS::AutoIdVector); + pub fn CreateRootedIdVector(cx: *mut JSContext) -> *mut JS::RootedIdVector; + pub fn AppendToRootedIdVector(v: *mut JS::RootedIdVector, id: jsid) -> bool; + pub fn SliceRootedIdVector(v: *const JS::RootedIdVector, length: *mut usize) -> *const jsid; + pub fn DestroyRootedIdVector(v: *mut JS::RootedIdVector); + pub fn GetMutableHandleIdVector(v: *mut JS::RootedIdVector)-> JS::MutableHandleIdVector; pub fn CreateRootedObjectVector(aCx: *mut JSContext) -> *mut JS::RootedObjectVector; pub fn AppendToRootedObjectVector(v: *mut JS::RootedObjectVector, obj: *mut JSObject) -> bool; pub fn DeleteRootedObjectVector(v: *mut JS::RootedObjectVector); diff --git a/js/rust/src/jsglue.cpp b/js/rust/src/jsglue.cpp index 2dca2f711cc4..636ea3e0bc30 100644 --- a/js/rust/src/jsglue.cpp +++ b/js/rust/src/jsglue.cpp @@ -34,12 +34,12 @@ struct ProxyTraps { JS::Handle desc, JS::ObjectOpResult& result); bool (*ownPropertyKeys)(JSContext* cx, JS::HandleObject proxy, - JS::AutoIdVector& props); + JS::MutableHandleIdVector props); bool (*delete_)(JSContext* cx, JS::HandleObject proxy, JS::HandleId id, JS::ObjectOpResult& result); bool (*enumerate)(JSContext* cx, JS::HandleObject proxy, - JS::AutoIdVector& props); + JS::MutableHandleIdVector props); bool (*getPrototypeIfOrdinary)(JSContext* cx, JS::HandleObject proxy, bool* isOrdinary, @@ -67,7 +67,7 @@ struct ProxyTraps { bool (*hasOwn)(JSContext* cx, JS::HandleObject proxy, JS::HandleId id, bool* bp); bool (*getOwnEnumerablePropertyKeys)(JSContext* cx, JS::HandleObject proxy, - JS::AutoIdVector& props); + JS::MutableHandleIdVector props); bool (*nativeCall)(JSContext* cx, JS::IsAcceptableThis test, JS::NativeImpl impl, JS::CallArgs args); bool (*hasInstance)(JSContext* cx, JS::HandleObject proxy, @@ -101,7 +101,7 @@ static int HandlerFamily; \ /* Standard internal methods. */ \ virtual bool enumerate(JSContext* cx, JS::HandleObject proxy, \ - JS::AutoIdVector& props) const override { \ + JS::MutableHandleIdVector props) const override { \ return mTraps.enumerate ? mTraps.enumerate(cx, proxy, props) \ : _base::enumerate(cx, proxy, props); \ } \ @@ -146,7 +146,7 @@ static int HandlerFamily; } \ \ virtual bool getOwnEnumerablePropertyKeys( \ - JSContext* cx, JS::HandleObject proxy, JS::AutoIdVector& props) \ + JSContext* cx, JS::HandleObject proxy, JS::MutableHandleIdVector props) \ const override { \ return mTraps.getOwnEnumerablePropertyKeys \ ? mTraps.getOwnEnumerablePropertyKeys(cx, proxy, props) \ @@ -240,7 +240,7 @@ class WrapperProxyHandler : public js::Wrapper { } virtual bool ownPropertyKeys(JSContext* cx, JS::HandleObject proxy, - JS::AutoIdVector& props) const override { + JS::MutableHandleIdVector props) const override { return mTraps.ownPropertyKeys ? mTraps.ownPropertyKeys(cx, proxy, props) : js::Wrapper::ownPropertyKeys(cx, proxy, props); @@ -322,7 +322,7 @@ class ForwardingProxyHandler : public js::BaseProxyHandler { } virtual bool ownPropertyKeys(JSContext* cx, JS::HandleObject proxy, - JS::AutoIdVector& props) const override { + JS::MutableHandleIdVector props) const override { return mTraps.ownPropertyKeys(cx, proxy, props); } @@ -538,20 +538,24 @@ JSObject* UncheckedUnwrapObject(JSObject* obj, bool stopAtOuter) { return js::UncheckedUnwrap(obj, stopAtOuter); } -JS::AutoIdVector* CreateAutoIdVector(JSContext* cx) { - return new JS::AutoIdVector(cx); +JS::RootedIdVector* CreateRootedIdVector(JSContext* cx) { + return new JS::RootedIdVector(cx); } -bool AppendToAutoIdVector(JS::AutoIdVector* v, jsid id) { +bool AppendToRootedIdVector(JS::RootedIdVector* v, jsid id) { return v->append(id); } -const jsid* SliceAutoIdVector(const JS::AutoIdVector* v, size_t* length) { +const jsid* SliceRootedIdVector(const JS::RootedIdVector* v, size_t* length) { *length = v->length(); return v->begin(); } -void DestroyAutoIdVector(JS::AutoIdVector* v) { delete v; } +void DestroyRootedIdVector(JS::RootedIdVector* v) { delete v; } + +JS::MutableHandleIdVector GetMutableHandleIdVector(JS::RootedIdVector* v) { + return JS::MutableHandleIdVector(v); +} JS::RootedObjectVector* CreateRootedObjectVector(JSContext* aCx) { JS::RootedObjectVector* vec = new JS::RootedObjectVector(aCx); diff --git a/js/rust/src/rust.rs b/js/rust/src/rust.rs index 336f01dce6e3..a54182bad198 100644 --- a/js/rust/src/rust.rs +++ b/js/rust/src/rust.rs @@ -23,7 +23,7 @@ use std::thread; use jsapi::root::*; use jsval::{self, UndefinedValue}; use glue::{CreateRootedObjectVector, CreateCallArgsFromVp, AppendToRootedObjectVector, DeleteRootedObjectVector, IsDebugBuild}; -use glue::{CreateAutoIdVector, SliceAutoIdVector, DestroyAutoIdVector}; +use glue::{CreateRootedIdVector, SliceRootedIdVector, DestroyRootedIdVector, GetMutableHandleIdVector}; use glue::{NewCompileOptions, DeleteCompileOptions}; use panic; @@ -934,35 +934,41 @@ impl JSNativeWrapper { } } -pub struct IdVector(*mut JS::AutoIdVector); - -impl IdVector { - pub unsafe fn new(cx: *mut JSContext) -> IdVector { - let vector = CreateAutoIdVector(cx); - assert!(!vector.is_null()); - IdVector(vector) - } - - pub fn get(&self) -> *mut JS::AutoIdVector { - self.0 - } +pub struct RootedIdVectorWrapper { + pub ptr: *mut JS::RootedIdVector } -impl Drop for IdVector { - fn drop(&mut self) { +impl RootedIdVectorWrapper { + pub fn new(cx: *mut JSContext) -> RootedIdVectorWrapper { + RootedIdVectorWrapper { + ptr: unsafe { + CreateRootedIdVector(cx) + } + } + } + + pub fn handle_mut(&self) -> JS::MutableHandleIdVector { unsafe { - DestroyAutoIdVector(self.0) + GetMutableHandleIdVector(self.ptr) } } } -impl Deref for IdVector { +impl Drop for RootedIdVectorWrapper { + fn drop(&mut self) { + unsafe { + DestroyRootedIdVector(self.ptr) + } + } +} + +impl Deref for RootedIdVectorWrapper { type Target = [jsid]; fn deref(&self) -> &[jsid] { unsafe { let mut length = 0; - let pointer = SliceAutoIdVector(self.0 as *const _, &mut length); + let pointer = SliceRootedIdVector(self.ptr as *const _, &mut length); slice::from_raw_parts(pointer, length) } } diff --git a/js/rust/tests/enumerate.rs b/js/rust/tests/enumerate.rs index 3e7e02a99cc3..2bb993f73e90 100644 --- a/js/rust/tests/enumerate.rs +++ b/js/rust/tests/enumerate.rs @@ -14,7 +14,7 @@ use js::jsapi::root::JS_NewGlobalObject; use js::jsapi::root::JS_StringEqualsAscii; use js::jsapi::root::JS::OnNewGlobalHookOption; use js::jsval::UndefinedValue; -use js::rust::IdVector; +use js::rust::RootedIdVectorWrapper; use js::rust::Runtime; use js::rust::SIMPLE_GLOBAL_CLASS; use std::ptr; @@ -37,8 +37,8 @@ fn enumerate() { assert!(rval.is_object()); rooted!(in(cx) let object = rval.to_object()); - let ids = IdVector::new(cx); - assert!(GetPropertyKeys(cx, object.handle(), JSITER_OWNONLY, ids.get())); + let ids = RootedIdVectorWrapper::new(cx); + assert!(GetPropertyKeys(cx, object.handle(), JSITER_OWNONLY, ids.handle_mut())); assert_eq!(ids.len(), 1); rooted!(in(cx) let id = ids[0]);