From cca591c20a05b663b7a576cb04f3f15e23c3d16f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 2 Dec 2013 19:08:07 -0500 Subject: [PATCH] Bug 936056. Be consistent about the thisobj we pass to getters. r=jorendorff --- dom/bindings/test/mochitest.ini | 1 + .../test/test_barewordGetsWindow.html | 60 +++++++++++++++++++ js/src/jit/IonCaches.cpp | 15 ++++- js/src/jsfriendapi.h | 8 +++ js/src/vm/Interpreter.cpp | 18 ++++-- 5 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 dom/bindings/test/test_barewordGetsWindow.html diff --git a/dom/bindings/test/mochitest.ini b/dom/bindings/test/mochitest.ini index 3b019305bd7d..e6fbde9ea04a 100644 --- a/dom/bindings/test/mochitest.ini +++ b/dom/bindings/test/mochitest.ini @@ -16,6 +16,7 @@ support-files = [test_bug788369.html] [test_bug852846.html] [test_bug862092.html] +[test_barewordGetsWindow.html] [test_cloneAndImportNode.html] [test_defineProperty.html] [test_enums.html] diff --git a/dom/bindings/test/test_barewordGetsWindow.html b/dom/bindings/test/test_barewordGetsWindow.html new file mode 100644 index 000000000000..a9986503c75d --- /dev/null +++ b/dom/bindings/test/test_barewordGetsWindow.html @@ -0,0 +1,60 @@ + + + + + + Test for Bug 936056 + + + + + +Mozilla Bug 936056 +

+ +
+
+ + diff --git a/js/src/jit/IonCaches.cpp b/js/src/jit/IonCaches.cpp index d1c34c99e804..a47f0b7d3174 100644 --- a/js/src/jit/IonCaches.cpp +++ b/js/src/jit/IonCaches.cpp @@ -611,8 +611,19 @@ IsCacheableGetPropCallNative(JSObject *obj, JSObject *holder, Shape *shape) if (!shape->hasGetterValue() || !shape->getterValue().isObject()) return false; - return shape->getterValue().toObject().is() && - shape->getterValue().toObject().as().isNative(); + if (!shape->getterValue().toObject().is()) + return false; + + JSFunction& getter = shape->getterValue().toObject().as(); + if (!getter.isNative()) + return false; + + // Check for a DOM method; those are OK with both inner and outer objects. + if (getter.jitInfo()) + return true; + + // For non-DOM methods, don't cache if obj has an outerObject hook. + return !obj->getClass()->ext.outerObject; } static bool diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 4c5d113d2d34..84f6f23fd110 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -1496,6 +1496,11 @@ struct JSJitInfo { AliasEverything }; + bool isDOMJitInfo() const + { + return type != OpType_None; + } + union { JSJitGetterOp getter; JSJitSetterOp setter; @@ -1504,6 +1509,9 @@ struct JSJitInfo { uint32_t protoID; uint32_t depth; + // type not being OpType_None means this is a DOM method. If you + // change that, come up with a different way of implementing + // isDOMJitInfo(). OpType type; bool isInfallible; /* Is op fallible? False in setters. */ bool isMovable; /* Is op movable? To be movable the op must not diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 09acf0431ff3..8d28cddd55fa 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -501,13 +501,19 @@ js::Invoke(JSContext *cx, const Value &thisv, const Value &fval, unsigned argc, /* * We must call the thisObject hook in case we are not called from the * interpreter, where a prior bytecode has computed an appropriate - * |this| already. + * |this| already. But don't do that if fval is a DOM function. */ - RootedObject thisObj(cx, &args.thisv().toObject()); - JSObject *thisp = JSObject::thisObject(cx, thisObj); - if (!thisp) - return false; - args.setThis(ObjectValue(*thisp)); + if (!fval.isObject() || !fval.toObject().is() || + !fval.toObject().as().isNative() || + !fval.toObject().as().jitInfo() || + !fval.toObject().as().jitInfo()->isDOMJitInfo()) + { + RootedObject thisObj(cx, &args.thisv().toObject()); + JSObject *thisp = JSObject::thisObject(cx, thisObj); + if (!thisp) + return false; + args.setThis(ObjectValue(*thisp)); + } } if (!Invoke(cx, args))