Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WindowProxy::set_window doesn't transplant the proxy properly #20605

Open
asajeffrey opened this issue Apr 10, 2018 · 1 comment
Open

WindowProxy::set_window doesn't transplant the proxy properly #20605

asajeffrey opened this issue Apr 10, 2018 · 1 comment
Labels
A-content/script Related to the script thread

Comments

@asajeffrey
Copy link
Member

It looks like when WindowProxy::set_window calls JS_TransplantObject at

fn set_window(&self, window: &GlobalScope, traps: &ProxyTraps) {
unsafe {
debug!("Setting window of {:p}.", self);
let handler = CreateWrapperProxyHandler(traps);
assert!(!handler.is_null());
let cx = window.get_cx();
let window_jsobject = window.reflector().get_jsobject();
let old_js_proxy = self.reflector.get_jsobject();
assert!(!window_jsobject.get().is_null());
assert_ne!(((*get_object_class(window_jsobject.get())).flags & JSCLASS_IS_GLOBAL), 0);
let _ac = JSAutoCompartment::new(cx, window_jsobject.get());
// The old window proxy no longer owns this browsing context.
SetProxyExtra(old_js_proxy.get(), 0, &PrivateValue(ptr::null_mut()));
// Brain transpant the window proxy.
// We need to do this, because the Window and WindowProxy
// objects need to be in the same compartment.
// JS_TransplantObject does this by copying the contents
// of the old window proxy to the new window proxy, then
// making the old window proxy a cross-compartment wrapper
// pointing to the new window proxy.
rooted!(in(cx) let new_js_proxy = NewWindowProxy(cx, window_jsobject, handler));
debug!("Transplanting proxy from {:p} to {:p}.", old_js_proxy.get(), new_js_proxy.get());
rooted!(in(cx) let new_js_proxy = JS_TransplantObject(cx, old_js_proxy, new_js_proxy.handle()));
debug!("Transplanted proxy is {:p}.", new_js_proxy.get());
// Transfer ownership of this browsing context from the old window proxy to the new one.
SetProxyExtra(new_js_proxy.get(), 0, &PrivateValue(self.as_void_ptr()));
// Notify the JS engine about the new window proxy binding.
SetWindowProxy(cx, window_jsobject, new_js_proxy.handle());
// Update the reflector.
debug!("Setting reflector of {:p} to {:p}.", self, new_js_proxy.get());
self.reflector.rootable().set(new_js_proxy.get());
}
}
this can end up with SM thinking it's a same-origin transplant, so swapping the pointers rather than actually transplanting.

Thread 20 "ScriptThread Pi" hit Breakpoint 2, JS_TransplantObject (cx=0x7fffa02da4a0, origobj=..., target=...)
    at /home/ajeffrey/.cargo/registry/src/github.com-1ecc6299db9ec823/mozjs_sys-0.50.1/mozjs/js/src/jsapi.cpp:855
855	{
(gdb) n
861	    RootedValue origv(cx, ObjectValue(*origobj));
(gdb) 
855	{
(gdb) 
861	    RootedValue origv(cx, ObjectValue(*origobj));
(gdb) 
862	    RootedObject newIdentity(cx);
(gdb) 
861	    RootedValue origv(cx, ObjectValue(*origobj));
(gdb) 
862	    RootedObject newIdentity(cx);
(gdb) 
861	    RootedValue origv(cx, ObjectValue(*origobj));
(gdb) 
862	    RootedObject newIdentity(cx);
(gdb) 
865	    AutoDisableCompactingGC nocgc(cx->runtime());
(gdb) 
862	    RootedObject newIdentity(cx);
(gdb) 
865	    AutoDisableCompactingGC nocgc(cx->runtime());
(gdb) 
862	    RootedObject newIdentity(cx);
(gdb) 
865	    AutoDisableCompactingGC nocgc(cx->runtime());
(gdb) 
869	    JSCompartment* destination = target->compartment();
(gdb) 
871	    if (origobj->compartment() == destination) {
(gdb) 
869	    JSCompartment* destination = target->compartment();
(gdb) 
871	    if (origobj->compartment() == destination) {
(gdb) 
876	        if (!JSObject::swap(cx, origobj, target))
(gdb) 
878	        newIdentity = origobj;
@asajeffrey
Copy link
Member Author

IRC chat in the context of #20604: https://mozilla.logbot.info/servo/20180410#c14585942

cc @cbrewster

@asajeffrey asajeffrey added the A-content/script Related to the script thread label Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/script Related to the script thread
Projects
None yet
Development

No branches or pull requests

1 participant