Auto merge of #12980 - jdm:proxychanges, r=nox

Fix ridiculous DOM proxy getter performance

This implements the missing shadowing checks that were causing us to take many slow paths when dealing with proxy objects. Verified by running `tests/html/binding_perf.html` before and after and observing a 12x improvement.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12357
- [X] These changes do not require tests because we can't test performance yet.

<!-- 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/12980)
<!-- Reviewable:end -->

components/script/dom/bindings/proxyhandler.rs

@@ -10,21 +10,51 @@ use dom::bindings::conversions::is_dom_proxy;
 use dom::bindings::utils::delete_property_by_id;
 use js::glue::GetProxyExtra;
 use js::glue::InvokeGetOwnPropertyDescriptor;
-use js::glue::{GetProxyHandler, SetProxyExtra};
+use js::glue::{GetProxyHandler, SetProxyExtra, GetProxyHandlerFamily};
 use js::jsapi::GetObjectProto;
 use js::jsapi::GetStaticPrototype;
 use js::jsapi::JS_GetPropertyDescriptorById;
 use js::jsapi::MutableHandleObject;
 use js::jsapi::{Handle, HandleId, HandleObject, MutableHandle, ObjectOpResult};
-use js::jsapi::{JSContext, JSObject, JSPROP_GETTER, PropertyDescriptor};
+use js::jsapi::{JSContext, JSObject, JSPROP_GETTER, PropertyDescriptor, DOMProxyShadowsResult};
-use js::jsapi::{JSErrNum, JS_StrictPropertyStub};
+use js::jsapi::{JSErrNum, JS_StrictPropertyStub, JS_AlreadyHasOwnPropertyById};
-use js::jsapi::{JS_DefinePropertyById, JS_NewObjectWithGivenProto};
+use js::jsapi::{JS_DefinePropertyById, JS_NewObjectWithGivenProto, SetDOMProxyInformation};
 use js::jsval::ObjectValue;
 use libc;
 use std::{mem, ptr};
 
 static JSPROXYSLOT_EXPANDO: u32 = 0;
 
+/// Determine if this id shadows any existing properties for this proxy.
+pub unsafe extern "C" fn shadow_check_callback(cx: *mut JSContext,
+                                               object: HandleObject,
+                                               id: HandleId)
+                                               -> DOMProxyShadowsResult {
+    // TODO: support OverrideBuiltins when #12978 is fixed.
+
+    rooted!(in(cx) let expando = get_expando_object(object));
+    if !expando.get().is_null() {
+        let mut has_own = false;
+        if !JS_AlreadyHasOwnPropertyById(cx, expando.handle(), id, &mut has_own) {
+            return DOMProxyShadowsResult::ShadowCheckFailed;
+        }
+
+        if has_own {
+            return DOMProxyShadowsResult::ShadowsViaDirectExpando;
+        }
+    }
+
+    // Our expando, if any, didn't shadow, so we're not shadowing at all.
+    DOMProxyShadowsResult::DoesntShadow
+}
+
+/// Initialize the infrastructure for DOM proxy objects.
+pub unsafe fn init() {
+    SetDOMProxyInformation(GetProxyHandlerFamily(),
+                           JSPROXYSLOT_EXPANDO,
+                           Some(shadow_check_callback));
+}
+
 /// Invoke the [[GetOwnProperty]] trap (`getOwnPropertyDescriptor`) on `proxy`,
 /// with argument `id` and return the result, if it is not `undefined`.
 /// Otherwise, walk along the prototype chain to find a property with that

components/script/lib.rs

@@ -114,10 +114,10 @@ mod unpremultiplytable;
 mod webdriver_handlers;
 
 use dom::bindings::codegen::RegisterBindings;
-use js::jsapi::{Handle, JSContext, JSObject, SetDOMProxyInformation};
+use dom::bindings::proxyhandler;
+use js::jsapi::{Handle, JSContext, JSObject};
 use script_traits::SWManagerSenders;
 use serviceworker_manager::ServiceWorkerManager;
-use std::ptr;
 use util::opts;
 
 #[cfg(target_os = "linux")]
@@ -164,7 +164,7 @@ fn perform_platform_specific_initialization() {}
 #[allow(unsafe_code)]
 pub fn init(sw_senders: SWManagerSenders) {
     unsafe {
-        SetDOMProxyInformation(ptr::null(), 0, Some(script_thread::shadow_check_callback));
+        proxyhandler::init();
     }
 
     // Spawn the service worker manager passing the constellation sender

components/script/script_thread.rs

@@ -59,7 +59,6 @@ use hyper_serde::Serde;
 use ipc_channel::ipc::{self, IpcSender};
 use ipc_channel::router::ROUTER;
 use js::glue::GetWindowProxyClass;
-use js::jsapi::{DOMProxyShadowsResult, HandleId, HandleObject};
 use js::jsapi::{JSAutoCompartment, JSContext, JS_SetWrapObjectCallbacks};
 use js::jsapi::{JSTracer, SetWindowProxyClass};
 use js::jsval::UndefinedValue;
@@ -484,12 +483,6 @@ impl ScriptThreadFactory for ScriptThread {
     }
 }
 
-pub unsafe extern "C" fn shadow_check_callback(_cx: *mut JSContext,
-    _object: HandleObject, _id: HandleId) -> DOMProxyShadowsResult {
-    // XXX implement me
-    DOMProxyShadowsResult::ShadowCheckFailed
-}
-
 impl ScriptThread {
     pub fn page_headers_available(id: &PipelineId, subpage: Option<&SubpageId>, metadata: Option<Metadata>)
                                   -> Option<ParserRoot> {

components/servo/Cargo.lock

@@ -1086,7 +1086,7 @@ dependencies = [
 [[package]]
 name = "js"
 version = "0.1.3"
-source = "git+https://github.com/servo/rust-mozjs#daffcfb1e0568b3e67dda331b487cc9fd8a202cb"
+source = "git+https://github.com/servo/rust-mozjs#a4a693bf41fc67e839f3a5e8219d872f5c675b5f"
 dependencies = [
  "cmake 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",

ports/cef/Cargo.lock

@@ -994,7 +994,7 @@ dependencies = [
 [[package]]
 name = "js"
 version = "0.1.3"
-source = "git+https://github.com/servo/rust-mozjs#daffcfb1e0568b3e67dda331b487cc9fd8a202cb"
+source = "git+https://github.com/servo/rust-mozjs#a4a693bf41fc67e839f3a5e8219d872f5c675b5f"
 dependencies = [
  "cmake 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",

tests/html/bindings_perf.html

@@ -1,6 +1,7 @@
 <button onclick="void_method()">measure void method</button>
 <button onclick="int_getter()">measure int getter</button>
 <button onclick="firstChild_getter()">measure firstChild getter</button>
+<button onclick="proxy_firstChild_getter()">measure proxy firstChild getter</button>
 <script>
 var t = 'TestBinding' in window ? (new TestBinding()) : (new TextEncoder());
 function void_method() {
@@ -33,4 +34,15 @@ function firstChild_getter() {
   var stop = new Date();
   console.log('firstChild getter: ' + ((stop - start) / count * 1e6) + 'ns');
 }
+
+function proxy_firstChild_getter() {
+  var n = document;
+  var start = new Date();
+  var count = 1000000;
+  for (var i = 0; i < count; i++) {
+    var a = n.firstChild;
+  }
+  var stop = new Date();
+  console.log('proxy firstChild getter: ' + ((stop - start) / count * 1e6) + 'ns');
+}
 </script>