Bug 936056 - Be consistent about the thisobj we pass to getters. r=jorendorff, a=bajaj
☠☠ backed out by e682394a24a8 ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 02 Dec 2013 19:08:07 -0500
changeset 167816 0414fb5b7ceba48ba14aa60d6711599ebcb7ae3e
parent 167815 e33d85e6e12a8652fd4443ae2dfd501c1cd69165
child 167817 2debd43d9b9efcf74a2fb56b1a47d54e3ea64627
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, bajaj
bugs936056
milestone27.0
Bug 936056 - Be consistent about the thisobj we pass to getters. r=jorendorff, a=bajaj
dom/bindings/test/mochitest.ini
dom/bindings/test/test_barewordGetsWindow.html
js/src/jit/IonCaches.cpp
js/src/jsfriendapi.h
js/src/vm/Interpreter.cpp
--- a/dom/bindings/test/mochitest.ini
+++ b/dom/bindings/test/mochitest.ini
@@ -11,16 +11,17 @@ support-files =
 [test_bug560072.html]
 [test_bug707564.html]
 [test_bug742191.html]
 [test_bug759621.html]
 [test_bug773326.html]
 [test_bug788369.html]
 [test_bug852846.html]
 [test_bug862092.html]
+[test_barewordGetsWindow.html]
 [test_defineProperty.html]
 [test_enums.html]
 [test_exceptionThrowing.html]
 [test_exception_messages.html]
 [test_forOf.html]
 [test_integers.html]
 [test_interfaceToString.html]
 [test_exceptions_from_jsimplemented.html]
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_barewordGetsWindow.html
@@ -0,0 +1,60 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=936056
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 936056</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 936056 **/
+  SimpleTest.waitForExplicitFinish();
+  window.onload = function() {
+    var desc = Object.getOwnPropertyDescriptor(frames[0], "document");
+    if (!desc || !desc.get) {
+      todo(false, "This test does nothing so far, but will once Window is on WebIDL bindings");
+      SimpleTest.finish();
+      return;
+    }
+    get = desc.get;
+    ok(get, "Couldn't find document getter");
+    Object.defineProperty(frames[0], "foo", { get: get });
+
+    var barewordFunc = frames[0].eval("(function (count) { var doc; for (var i = 0; i < count; ++i) doc = foo; return doc.documentElement; })");
+    var qualifiedFunc = frames[0].eval("(function (count) { var doc; for (var i = 0; i < count; ++i) doc = window.document; return doc.documentElement; })");
+    document.querySelector("iframe").onload = function () {
+      // interp
+      is(barewordFunc(1).textContent, "OLD", "Bareword should see own inner 1");
+      is(qualifiedFunc(1).textContent, "NEW",
+         "Qualified should see current inner 1");
+      // baseline
+      is(barewordFunc(100).textContent, "OLD", "Bareword should see own inner 2");
+      is(qualifiedFunc(100).textContent, "NEW",
+         "Qualified should see current inner 2");
+      // ion
+      is(barewordFunc(10000).textContent, "OLD", "Bareword should see own inner 3");
+      is(qualifiedFunc(10000).textContent, "NEW",
+         "Qualified should see current inner 2");
+      SimpleTest.finish();
+    }
+    frames[0].location = "data:text/plain,NEW";
+  }
+
+
+
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=936056">Mozilla Bug 936056</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe src="data:text/plain,OLD"></iframe>
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -606,18 +606,29 @@ static bool
 IsCacheableGetPropCallNative(JSObject *obj, JSObject *holder, Shape *shape)
 {
     if (!shape || !IsCacheableProtoChain(obj, holder))
         return false;
 
     if (!shape->hasGetterValue() || !shape->getterValue().isObject())
         return false;
 
-    return shape->getterValue().toObject().is<JSFunction>() &&
-           shape->getterValue().toObject().as<JSFunction>().isNative();
+    if (!shape->getterValue().toObject().is<JSFunction>())
+        return false;
+
+    JSFunction& getter = shape->getterValue().toObject().as<JSFunction>();
+    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
 IsCacheableGetPropCallPropertyOp(JSObject *obj, JSObject *holder, Shape *shape)
 {
     if (!shape || !IsCacheableProtoChain(obj, holder))
         return false;
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -1436,23 +1436,31 @@ typedef bool
 struct JSJitInfo {
     enum OpType {
         Getter,
         Setter,
         Method,
         OpType_None
     };
 
+    bool isDOMJitInfo() const
+    {
+        return type != OpType_None;
+    }
+
     union {
         JSJitGetterOp getter;
         JSJitSetterOp setter;
         JSJitMethodOp method;
     };
     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 isConstant;        /* Getting a construction-time constant? */
     bool isPure;            /* As long as no non-pure DOM things happen, will
                                keep returning the same value for the given
                                "this" object" */
     JSValueType returnType; /* The return type tag.  Might be JSVAL_TYPE_UNKNOWN */
 
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -495,23 +495,29 @@ js::Invoke(JSContext *cx, const Value &t
     args.setCallee(fval);
     args.setThis(thisv);
     PodCopy(args.array(), argv, argc);
 
     if (args.thisv().isObject()) {
         /*
          * 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<JSFunction>() ||
+            !fval.toObject().as<JSFunction>().isNative() ||
+            !fval.toObject().as<JSFunction>().jitInfo() ||
+            !fval.toObject().as<JSFunction>().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))
         return false;
 
     rval.set(args.rval());
     return true;
 }