Align `instanceof` with the final ES6 spec.

11 files changed, 79 insertions, 18 deletions

diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index 799396a0a..005d2278e 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2154,6 +2154,13 @@ namespace JS {
 extern JS_PUBLIC_API(bool)
 OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* bp);
 
+// Implementation of
+// https://www.ecma-international.org/ecma-262/6.0/#sec-instanceofoperator
+// This is almost identical to JS_HasInstance, except the latter may call a
+// custom hasInstance class op instead of InstanceofOperator.
+extern JS_PUBLIC_API(bool)
+InstanceofOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp);
+
 } // namespace JS
 
 extern JS_PUBLIC_API(void*)
diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp
index bcb0da80b..863871df9 100644
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -690,7 +690,7 @@ js::fun_symbolHasInstance(JSContext* cx, unsigned argc, Value* vp)
 }
 
 /*
- * ES6 (4-25-16) 7.3.19 OrdinaryHasInstance
+ * ES6 7.3.19 OrdinaryHasInstance
  */
 bool
 JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool* bp)
@@ -707,7 +707,7 @@ JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool*
     if (obj->is<JSFunction>() && obj->isBoundFunction()) {
         /* Steps 2a-b. */
         obj = obj->as<JSFunction>().getBoundFunctionTarget();
-        return InstanceOfOperator(cx, obj, v, bp);
+        return InstanceofOperator(cx, obj, v, bp);
     }
 
     /* Step 3. */
@@ -716,12 +716,12 @@ JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool*
         return true;
     }
 
-    /* Step 4. */
+    /* Step 4-5. */
     RootedValue pval(cx);
     if (!GetProperty(cx, obj, obj, cx->names().prototype, &pval))
         return false;
 
-    /* Step 5. */
+    /* Step 6. */
     if (pval.isPrimitive()) {
         /*
          * Throw a runtime error if instanceof is called on a function that
@@ -732,7 +732,7 @@ JS::OrdinaryHasInstance(JSContext* cx, HandleObject objArg, HandleValue v, bool*
         return false;
     }
 
-    /* Step 6. */
+    /* Step 7. */
     RootedObject pobj(cx, &pval.toObject());
     bool isDelegate;
     if (!IsDelegate(cx, pobj, v, &isDelegate))
diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h
index 3c73979f8..84ebe2732 100644
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -270,6 +270,8 @@ class JS_FRIEND_API(OpaqueCrossCompartmentWrapper) : public CrossCompartmentWrap
     virtual bool getBuiltinClass(JSContext* cx, HandleObject wrapper, ESClass* cls) const override;
     virtual bool isArray(JSContext* cx, HandleObject obj,
                          JS::IsArrayAnswer* answer) const override;
+    virtual bool hasInstance(JSContext* cx, HandleObject wrapper,
+                             MutableHandleValue v, bool* bp) const override;
     virtual const char* className(JSContext* cx, HandleObject wrapper) const override;
     virtual JSString* fun_toString(JSContext* cx, HandleObject proxy, unsigned indent) const override;
 
diff --git a/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp b/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp
index ff3f4145c..02bf237ff 100644
--- a/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp
+++ b/js/src/proxy/OpaqueCrossCompartmentWrapper.cpp
@@ -175,6 +175,14 @@ OpaqueCrossCompartmentWrapper::isArray(JSContext* cx, HandleObject obj,
     return true;
 }
 
+bool OpaqueCrossCompartmentWrapper::hasInstance(JSContext* cx,
+                                                HandleObject wrapper,
+                                                MutableHandleValue v,
+                                                bool* bp) const {
+  *bp = false;
+  return true;
+}
+
 const char*
 OpaqueCrossCompartmentWrapper::className(JSContext* cx,
                                          HandleObject proxy) const
diff --git a/js/src/proxy/ScriptedProxyHandler.cpp b/js/src/proxy/ScriptedProxyHandler.cpp
index 776547337..0e25f470c 100644
--- a/js/src/proxy/ScriptedProxyHandler.cpp
+++ b/js/src/proxy/ScriptedProxyHandler.cpp
@@ -8,8 +8,6 @@
 
 #include "jsapi.h"
 
-#include "vm/Interpreter.h" // For InstanceOfOperator
-
 #include "jsobjinlines.h"
 #include "vm/NativeObject-inl.h"
 
@@ -1230,7 +1228,7 @@ bool
 ScriptedProxyHandler::hasInstance(JSContext* cx, HandleObject proxy, MutableHandleValue v,
                                   bool* bp) const
 {
-    return InstanceOfOperator(cx, proxy, v, bp);
+    return InstanceofOperator(cx, proxy, v, bp);
 }
 
 bool
diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp
index b747e4d7a..e6d6630c4 100644
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -718,14 +718,14 @@ js::Execute(JSContext* cx, HandleScript script, JSObject& envChainArg, Value* rv
 }
 
 /*
- * ES6 (4-25-16) 12.10.4 InstanceofOperator
+ * ES6 12.9.4 InstanceofOperator
  */
 extern bool
-js::InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp)
+JS::InstanceofOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp)
 {
     /* Step 1. is handled by caller. */
 
-    /* Step 2. */
+    /* Step 2-3. */
     RootedValue hasInstance(cx);
     RootedId id(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().hasInstance));
     if (!GetProperty(cx, obj, obj, id, &hasInstance))
@@ -735,7 +735,7 @@ js::InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp)
         if (!IsCallable(hasInstance))
             return ReportIsNotFunction(cx, hasInstance);
 
-        /* Step 3. */
+        /* Step 4. */
         RootedValue rval(cx);
         if (!Call(cx, hasInstance, obj, v, &rval))
             return false;
@@ -743,13 +743,13 @@ js::InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp)
         return true;
     }
 
-    /* Step 4. */
+    /* Step 5. */
     if (!obj->isCallable()) {
         RootedValue val(cx, ObjectValue(*obj));
         return ReportIsNotFunction(cx, val);
     }
 
-    /* Step 5. */
+    /* Step 6. */
     return OrdinaryHasInstance(cx, obj, v, bp);
 }
 
@@ -760,7 +760,7 @@ js::HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp)
     RootedValue local(cx, v);
     if (JSHasInstanceOp hasInstance = clasp->getHasInstance())
         return hasInstance(cx, obj, &local, bp);
-    return js::InstanceOfOperator(cx, obj, local, bp);
+    return JS::InstanceofOperator(cx, obj, local, bp);
 }
 
 static inline bool
diff --git a/js/src/vm/Interpreter.h b/js/src/vm/Interpreter.h
index 330dbef5f..9fefd75cc 100644
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -323,9 +323,6 @@ extern JSType
 TypeOfValue(const Value& v);
 
 extern bool
-InstanceOfOperator(JSContext* cx, HandleObject obj, HandleValue v, bool* bp);
-
-extern bool
 HasInstance(JSContext* cx, HandleObject obj, HandleValue v, bool* bp);
 
 // Unwind environment chain and iterator to match the scope corresponding to
diff --git a/js/xpconnect/wrappers/WaiveXrayWrapper.cpp b/js/xpconnect/wrappers/WaiveXrayWrapper.cpp
index 27c010d34..dca3daa58 100644
--- a/js/xpconnect/wrappers/WaiveXrayWrapper.cpp
+++ b/js/xpconnect/wrappers/WaiveXrayWrapper.cpp
@@ -88,6 +88,37 @@ WaiveXrayWrapper::nativeCall(JSContext* cx, JS::IsAcceptableThis test,
 }
 
 bool
+WaiveXrayWrapper::hasInstance(JSContext* cx, HandleObject wrapper,
+                                   MutableHandleValue v, bool* bp) const {
+  if (v.isObject() && WrapperFactory::IsXrayWrapper(&v.toObject())) {
+    // If |v| is an XrayWrapper and in the same compartment as the value
+    // wrapped by |wrapper|, then the Xrays of |v| would be waived upon
+    // calling CrossCompartmentWrapper::hasInstance. This may trigger
+    // getters and proxy traps of unwrapped |v|. To prevent that from
+    // happening, we exit early.
+
+    // |wrapper| is the right operand of "instanceof", and must either be
+    // a function or an object with a @@hasInstance method. We are not going
+    // to call @@hasInstance, so only check whether it is a function.
+    // This check is here for consistency with usual "instanceof" behavior,
+    // which throws if the right operand is not a function. Without this
+    // check, the "instanceof" operator would return false and potentially
+    // hide errors in the code that uses the "instanceof" operator.
+    if (!JS::IsCallable(wrapper)) {
+      RootedValue wrapperv(cx, JS::ObjectValue(*wrapper));
+      js::ReportIsNotFunction(cx, wrapperv);
+      return false;
+    }
+
+    *bp = false;
+    return true;
+  }
+
+  // Both |wrapper| and |v| have no Xrays here.
+  return CrossCompartmentWrapper::hasInstance(cx, wrapper, v, bp);
+}
+
+bool
 WaiveXrayWrapper::getPrototype(JSContext* cx, HandleObject wrapper, MutableHandleObject protop) const
 {
     return CrossCompartmentWrapper::getPrototype(cx, wrapper, protop) &&
diff --git a/js/xpconnect/wrappers/WaiveXrayWrapper.h b/js/xpconnect/wrappers/WaiveXrayWrapper.h
index b0b447796..0f9675c17 100644
--- a/js/xpconnect/wrappers/WaiveXrayWrapper.h
+++ b/js/xpconnect/wrappers/WaiveXrayWrapper.h
@@ -36,6 +36,8 @@ class WaiveXrayWrapper : public js::CrossCompartmentWrapper {
                            JS::MutableHandle<JSObject*> objp) const override;
     virtual bool nativeCall(JSContext* cx, JS::IsAcceptableThis test,
                             JS::NativeImpl impl, const JS::CallArgs& args) const override;
+    virtual bool hasInstance(JSContext* cx, JS::HandleObject wrapper,
+                             JS::MutableHandleValue v, bool* bp) const override;
     virtual bool getPropertyDescriptor(JSContext* cx, JS::Handle<JSObject*> wrapper,
                                        JS::Handle<jsid> id,
                                        JS::MutableHandle<JS::PropertyDescriptor> desc) const override;
diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp
index 48a9fdc68..6e5a2f5e5 100644
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -2309,6 +2309,20 @@ XrayWrapper<Base, Traits>::getBuiltinClass(JSContext* cx, JS::HandleObject wrapp
 }
 
 template <typename Base, typename Traits>
+bool
+XrayWrapper<Base, Traits>::hasInstance(JSContext* cx,
+                                       JS::HandleObject wrapper,
+                                       JS::MutableHandleValue v,
+                                       bool* bp) const {
+  assertEnteredPolicy(cx, wrapper, JSID_VOID, BaseProxyHandler::GET);
+
+  // CrossCompartmentWrapper::hasInstance unwraps |wrapper|'s Xrays and enters
+  // its compartment. Any present XrayWrappers should be preserved, so the
+  // standard "instanceof" implementation is called without unwrapping first.
+  return JS::InstanceofOperator(cx, wrapper, v, bp);
+}
+
+template <typename Base, typename Traits>
 const char*
 XrayWrapper<Base, Traits>::className(JSContext* cx, HandleObject wrapper) const
 {
diff --git a/js/xpconnect/wrappers/XrayWrapper.h b/js/xpconnect/wrappers/XrayWrapper.h
index 5630982c2..038d82390 100644
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -482,6 +482,8 @@ class XrayWrapper : public Base {
                                               JS::AutoIdVector& props) const override;
 
     virtual bool getBuiltinClass(JSContext* cx, JS::HandleObject wapper, js::ESClass* cls) const override;
+    virtual bool hasInstance(JSContext* cx, JS::HandleObject wrapper,
+                             JS::MutableHandleValue v, bool* bp) const override;
     virtual const char* className(JSContext* cx, JS::HandleObject proxy) const override;
 
     static const XrayWrapper singleton;