Bug 1270977 - Remove JS::CallReceiver. r=efaust
authorJeff Walden <jwalden@mit.edu>
Thu, 26 May 2016 20:14:39 -0700
changeset 338450 5482d7b5c18f5337f3ff4037c3601da4780b89c0
parent 338449 b4938a38f3c0c840b81f6988d53dc25f30acf6c7
child 338451 b086d922b1a91965e19234ffec85cf811cc14409
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1270977
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1270977 - Remove JS::CallReceiver. r=efaust
js/public/CallArgs.h
js/src/NamespaceImports.h
js/src/jsapi-tests/moz.build
js/src/jsapi-tests/testCallArgs.cpp
--- a/js/public/CallArgs.h
+++ b/js/public/CallArgs.h
@@ -1,26 +1,61 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*
  * Helper classes encapsulating access to the callee, |this| value, arguments,
- * and argument count for a function call.
+ * and argument count for a call/construct operation.
+ *
+ * JS::CallArgs encapsulates access to a JSNative's un-abstracted
+ * |unsigned argc, Value* vp| arguments.  The principal way to create a
+ * JS::CallArgs is using JS::CallArgsFromVp:
+ *
+ *   // If provided no arguments or a non-numeric first argument, return zero.
+ *   // Otherwise return |this| exactly as given, without boxing.
+ *   static bool
+ *   Func(JSContext* cx, unsigned argc, JS::Value* vp)
+ *   {
+ *       JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+ *
+ *       // Guard against no arguments or a non-numeric arg0.
+ *       if (args.length() == 0 || !args[0].isNumber()) {
+ *           args.rval().setInt32(0);
+ *           return true;
+ *       }
  *
- * The intent of JS::CallArgs and JS::CallReceiver is that they be used to
- * encapsulate access to the un-abstracted |unsigned argc, Value* vp| arguments
- * to a function.  It's possible (albeit deprecated) to manually index into
- * |vp| to access the callee, |this|, and arguments of a function, and to set
- * its return value.  It's also possible to use the supported API of JS_CALLEE,
- * JS_THIS, JS_ARGV, JS_RVAL and JS_SET_RVAL to the same ends.  But neither API
- * has the error-handling or moving-GC correctness of CallArgs or CallReceiver.
- * New code should use CallArgs and CallReceiver instead whenever possible.
+ *       // Access to the callee must occur before accessing/setting
+ *       // the return value.
+ *       JSObject& callee = args.callee();
+ *       args.rval().setObject(callee);
+ *
+ *       // callee() and calleev() will now assert.
+ *
+ *       // It's always fine to access thisv().
+ *       HandleValue thisv = args.thisv();
+ *       args.rval().set(thisv);
+ *
+ *       // As the return value was last set to |this|, returns |this|.
+ *       return true;
+ *   }
+ *
+ * CallArgs is exposed publicly and used internally.  Not all parts of its
+ * public interface are meant to be used by embedders!  See inline comments to
+ * for details.
+ *
+ * It's possible (albeit deprecated) to manually index into |vp| to access the
+ * callee, |this|, and arguments of a function, and to set its return value.
+ * It's also possible to use the supported API of JS_CALLEE, JS_THIS, JS_ARGV,
+ * JS_RVAL, and JS_SET_RVAL to the same ends.
+ *
+ * But neither API has the error-handling or moving-GC correctness of CallArgs.
+ * New code should use CallArgs instead whenever possible.
  *
  * The eventual plan is to change JSNative to take |const CallArgs&| directly,
  * for automatic assertion of correct use and to make calling functions more
  * efficient.  Embedders should start internally switching away from using
  * |argc| and |vp| directly, except to create a |CallArgs|.  Then, when an
  * eventual release making that change occurs, porting efforts will require
  * changing methods' signatures but won't require invasive changes to the
  * methods' implementations, potentially under time pressure.
@@ -41,53 +76,16 @@
 /* Typedef for native functions called by the JS VM. */
 typedef bool
 (* JSNative)(JSContext* cx, unsigned argc, JS::Value* vp);
 
 namespace JS {
 
 extern JS_PUBLIC_DATA(const HandleValue) UndefinedHandleValue;
 
-/*
- * JS::CallReceiver encapsulates access to the callee, |this|, and eventual
- * return value for a function call.  The principal way to create a
- * CallReceiver is using JS::CallReceiverFromVp:
- *
- *   static bool
- *   FunctionReturningThis(JSContext* cx, unsigned argc, JS::Value* vp)
- *   {
- *       JS::CallReceiver rec = JS::CallReceiverFromVp(vp);
- *
- *       // Access to the callee must occur before accessing/setting
- *       // the return value.
- *       JSObject& callee = rec.callee();
- *       rec.rval().set(JS::ObjectValue(callee));
- *
- *       // callee() and calleev() will now assert.
- *
- *       // It's always fine to access thisv().
- *       HandleValue thisv = rec.thisv();
- *       rec.rval().set(thisv);
- *
- *       // As the return value was last set to |this|, returns |this|.
- *       return true;
- *   }
- *
- * A note on JS_THIS_OBJECT: no equivalent method is part of the CallReceiver
- * interface, and we're unlikely to add one (because this style of function is
- * disfavored -- functions shouldn't be implicitly exposing the global object
- * to arbitrary callers).  JS_THIS_OBJECT will eventually be removed, and
- * callers should switch to |args.thisv()| supplemented with |JS::ToObject| and
- * |JS::CurrentGlobalObjectOrNull|.
- *
- * CallReceiver is exposed publicly and used internally.  Not all parts of its
- * public interface are meant to be used by embedders!  See inline comments to
- * for details.
- */
-
 namespace detail {
 
 /*
  * Compute |this| for the |vp| inside a JSNative, either boxing primitives or
  * replacing with the global object as necessary.
  */
 extern JS_PUBLIC_API(Value)
 ComputeThis(JSContext* cx, JS::Value* vp);
@@ -104,58 +102,83 @@ class MOZ_STACK_CLASS UsedRvalBase;
 
 template<>
 class MOZ_STACK_CLASS UsedRvalBase<IncludeUsedRval>
 {
   protected:
     mutable bool usedRval_;
     void setUsedRval() const { usedRval_ = true; }
     void clearUsedRval() const { usedRval_ = false; }
+    void assertUnusedRval() const { MOZ_ASSERT(!usedRval_); }
 };
 
 template<>
 class MOZ_STACK_CLASS UsedRvalBase<NoUsedRval>
 {
   protected:
     void setUsedRval() const {}
     void clearUsedRval() const {}
+    void assertUnusedRval() const {}
 };
 
 template<UsedRval WantUsedRval>
-class MOZ_STACK_CLASS CallReceiverBase : public UsedRvalBase<
+class MOZ_STACK_CLASS CallArgsBase
+  : public UsedRvalBase<
 #ifdef JS_DEBUG
         WantUsedRval
 #else
         NoUsedRval
 #endif
     >
 {
   protected:
     Value* argv_;
+    unsigned argc_;
+    bool constructing_;
 
   public:
-    /*
-     * Returns the function being called, as an object.  Must not be called
-     * after rval() has been used!
-     */
-    JSObject& callee() const {
-        MOZ_ASSERT(!this->usedRval_);
-        return argv_[-2].toObject();
-    }
+    // CALLEE ACCESS
 
     /*
      * Returns the function being called, as a value.  Must not be called after
      * rval() has been used!
      */
     HandleValue calleev() const {
-        MOZ_ASSERT(!this->usedRval_);
+        this->assertUnusedRval();
         return HandleValue::fromMarkedLocation(&argv_[-2]);
     }
 
     /*
+     * Returns the function being called, as an object.  Must not be called
+     * after rval() has been used!
+     */
+    JSObject& callee() const {
+        return calleev().toObject();
+    }
+
+    // CALLING/CONSTRUCTING-DIFFERENTIATIONS
+
+    bool isConstructing() const {
+        if (!argv_[-1].isMagic())
+            return false;
+
+#ifdef JS_DEBUG
+        if (!this->usedRval_)
+            CheckIsValidConstructible(calleev());
+#endif
+
+        return true;
+    }
+
+    MutableHandleValue newTarget() const {
+        MOZ_ASSERT(constructing_);
+        return MutableHandleValue::fromMarkedLocation(&this->argv_[argc_]);
+    }
+
+    /*
      * Returns the |this| value passed to the function.  This method must not
      * be called when the function is being called as a constructor via |new|.
      * The value may or may not be an object: it is the individual function's
      * responsibility to box the value if needed.
      */
     HandleValue thisv() const {
         // Some internal code uses thisv() in constructing cases, so don't do
         // this yet.
@@ -165,132 +188,19 @@ class MOZ_STACK_CLASS CallReceiverBase :
 
     Value computeThis(JSContext* cx) const {
         if (thisv().isObject())
             return thisv();
 
         return ComputeThis(cx, base());
     }
 
-    bool isConstructing() const {
-#ifdef JS_DEBUG
-        if (this->usedRval_)
-            CheckIsValidConstructible(calleev());
-#endif
-        return argv_[-1].isMagic();
-    }
-
-    /*
-     * Returns the currently-set return value.  The initial contents of this
-     * value are unspecified.  Once this method has been called, callee() and
-     * calleev() can no longer be used.  (If you're compiling against a debug
-     * build of SpiderMonkey, these methods will assert to aid debugging.)
-     *
-     * If the method you're implementing succeeds by returning true, you *must*
-     * set this.  (SpiderMonkey doesn't currently assert this, but it will do
-     * so eventually.)  You don't need to use or change this if your method
-     * fails.
-     */
-    MutableHandleValue rval() const {
-        this->setUsedRval();
-        return MutableHandleValue::fromMarkedLocation(&argv_[-2]);
-    }
-
-  public:
-    // These methods are only intended for internal use.  Embedders shouldn't
-    // use them!
-
-    Value* base() const { return argv_ - 2; }
-
-    Value* spAfterCall() const {
-        this->setUsedRval();
-        return argv_ - 1;
-    }
-
-  public:
-    // These methods are publicly exposed, but they are *not* to be used when
-    // implementing a JSNative method and encapsulating access to |vp| within
-    // it.  You probably don't want to use these!
-
-    void setCallee(Value aCalleev) const {
-        this->clearUsedRval();
-        argv_[-2] = aCalleev;
-    }
-
-    void setThis(Value aThisv) const {
-        argv_[-1] = aThisv;
-    }
-
-    MutableHandleValue mutableThisv() const {
-        return MutableHandleValue::fromMarkedLocation(&argv_[-1]);
-    }
-};
-
-} // namespace detail
+    // ARGUMENTS
 
-class MOZ_STACK_CLASS CallReceiver : public detail::CallReceiverBase<detail::IncludeUsedRval>
-{
-  private:
-    friend CallReceiver CallReceiverFromVp(Value* vp);
-    friend CallReceiver CallReceiverFromArgv(Value* argv);
-};
-
-MOZ_ALWAYS_INLINE CallReceiver
-CallReceiverFromArgv(Value* argv)
-{
-    CallReceiver receiver;
-    receiver.clearUsedRval();
-    receiver.argv_ = argv;
-    return receiver;
-}
-
-MOZ_ALWAYS_INLINE CallReceiver
-CallReceiverFromVp(Value* vp)
-{
-    return CallReceiverFromArgv(vp + 2);
-}
-
-/*
- * JS::CallArgs encapsulates everything JS::CallReceiver does, plus access to
- * the function call's arguments.  The principal way to create a CallArgs is
- * like so, using JS::CallArgsFromVp:
- *
- *   static bool
- *   FunctionReturningArgcTimesArg0(JSContext* cx, unsigned argc, JS::Value* vp)
- *   {
- *       JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
- *
- *       // Guard against no arguments or a non-numeric arg0.
- *       if (args.length() == 0 || !args[0].isNumber()) {
- *           args.rval().setInt32(0);
- *           return true;
- *       }
- *
- *       args.rval().set(JS::NumberValue(args.length() * args[0].toNumber()));
- *       return true;
- *   }
- *
- * CallArgs is exposed publicly and used internally.  Not all parts of its
- * public interface are meant to be used by embedders!  See inline comments to
- * for details.
- */
-namespace detail {
-
-template<UsedRval WantUsedRval>
-class MOZ_STACK_CLASS CallArgsBase :
-        public mozilla::Conditional<WantUsedRval == detail::IncludeUsedRval,
-                                    CallReceiver,
-                                    CallReceiverBase<NoUsedRval> >::Type
-{
-  protected:
-    unsigned argc_;
-    bool constructing_;
-
-  public:
-    /* Returns the number of arguments. */
+        /* Returns the number of arguments. */
     unsigned length() const { return argc_; }
 
     /* Returns the i-th zero-indexed argument. */
     MutableHandleValue operator[](unsigned i) const {
         MOZ_ASSERT(i < argc_);
         return MutableHandleValue::fromMarkedLocation(&this->argv_[i]);
     }
 
@@ -307,28 +217,70 @@ class MOZ_STACK_CLASS CallArgsBase :
     /*
      * Returns true if the i-th zero-indexed argument is present and is not
      * |undefined|.
      */
     bool hasDefined(unsigned i) const {
         return i < argc_ && !this->argv_[i].isUndefined();
     }
 
-    MutableHandleValue newTarget() const {
-        MOZ_ASSERT(constructing_);
-        return MutableHandleValue::fromMarkedLocation(&this->argv_[argc_]);
+    // RETURN VALUE
+
+    /*
+     * Returns the currently-set return value.  The initial contents of this
+     * value are unspecified.  Once this method has been called, callee() and
+     * calleev() can no longer be used.  (If you're compiling against a debug
+     * build of SpiderMonkey, these methods will assert to aid debugging.)
+     *
+     * If the method you're implementing succeeds by returning true, you *must*
+     * set this.  (SpiderMonkey doesn't currently assert this, but it will do
+     * so eventually.)  You don't need to use or change this if your method
+     * fails.
+     */
+    MutableHandleValue rval() const {
+        this->setUsedRval();
+        return MutableHandleValue::fromMarkedLocation(&argv_[-2]);
     }
 
   public:
-    // These methods are publicly exposed, but we're less sure of the interface
-    // here than we'd like (because they're hackish and drop assertions).  Try
-    // to avoid using these if you can.
+    // These methods are publicly exposed, but they are *not* to be used when
+    // implementing a JSNative method and encapsulating access to |vp| within
+    // it.  You probably don't want to use these!
+
+    void setCallee(Value aCalleev) const {
+        this->clearUsedRval();
+        argv_[-2] = aCalleev;
+    }
+
+    void setThis(Value aThisv) const {
+        argv_[-1] = aThisv;
+    }
+
+    MutableHandleValue mutableThisv() const {
+        return MutableHandleValue::fromMarkedLocation(&argv_[-1]);
+    }
 
-    Value* array() const { return this->argv_; }
-    Value* end() const { return this->argv_ + argc_ + constructing_; }
+  public:
+    // These methods are publicly exposed, but we're unsure of the interfaces
+    // (because they're hackish and drop assertions).  Avoid using these if you
+    // can.
+
+    Value* array() const { return argv_; }
+    Value* end() const { return argv_ + argc_ + constructing_; }
+
+  public:
+    // These methods are only intended for internal use.  Embedders shouldn't
+    // use them!
+
+    Value* base() const { return argv_ - 2; }
+
+    Value* spAfterCall() const {
+        this->setUsedRval();
+        return argv_ - 1;
+    }
 };
 
 } // namespace detail
 
 class MOZ_STACK_CLASS CallArgs : public detail::CallArgsBase<detail::IncludeUsedRval>
 {
   private:
     friend CallArgs CallArgsFromVp(unsigned argc, Value* vp);
@@ -371,29 +323,40 @@ CallArgsFromSp(unsigned stackSlots, Valu
 
 /*
  * Macros to hide interpreter stack layout details from a JSNative using its
  * JS::Value* vp parameter.  DO NOT USE THESE!  Instead use JS::CallArgs and
  * friends, above.  These macros will be removed when we change JSNative to
  * take a const JS::CallArgs&.
  */
 
-#define JS_THIS_OBJECT(cx,vp)   (JS_THIS(cx,vp).toObjectOrNull())
-
 /*
+ * Return |this| if |this| is an object.  Otherwise, return the global object
+ * if |this| is null or undefined, and finally return a boxed version of any
+ * other primitive.
+ *
  * Note: if this method returns null, an error has occurred and must be
  * propagated or caught.
  */
 MOZ_ALWAYS_INLINE JS::Value
 JS_THIS(JSContext* cx, JS::Value* vp)
 {
     return vp[1].isPrimitive() ? JS::detail::ComputeThis(cx, vp) : vp[1];
 }
 
 /*
+ * A note on JS_THIS_OBJECT: no equivalent method is part of the CallArgs
+ * interface, and we're unlikely to add one (functions shouldn't be implicitly
+ * exposing the global object to arbitrary callers).  Continue using |vp|
+ * directly for this case, but be aware this API will eventually be replaced
+ * with a function that operates directly upon |args.thisv()|.
+ */
+#define JS_THIS_OBJECT(cx,vp)   (JS_THIS(cx,vp).toObjectOrNull())
+
+/*
  * |this| is passed to functions in ES5 without change.  Functions themselves
  * do any post-processing they desire to box |this|, compute the global object,
  * &c.  This macro retrieves a function's unboxed |this| value.
  *
  * This macro must not be used in conjunction with JS_THIS or JS_THIS_OBJECT,
  * or vice versa.  Either use the provided this value with this macro, or
  * compute the boxed |this| value using those.  JS_THIS_VALUE must not be used
  * if the function is being called as a constructor.
--- a/js/src/NamespaceImports.h
+++ b/js/src/NamespaceImports.h
@@ -96,17 +96,16 @@ using JS::AutoHashMapRooter;
 using JS::AutoHashSetRooter;
 
 using JS::GCVector;
 using JS::GCHashMap;
 using JS::GCHashSet;
 
 using JS::CallArgs;
 using JS::CallNonGenericMethod;
-using JS::CallReceiver;
 using JS::CompileOptions;
 using JS::IsAcceptableThis;
 using JS::NativeImpl;
 using JS::OwningCompileOptions;
 using JS::ReadOnlyCompileOptions;
 using JS::SourceBufferHolder;
 using JS::TransitiveCompileOptions;
 
--- a/js/src/jsapi-tests/moz.build
+++ b/js/src/jsapi-tests/moz.build
@@ -8,16 +8,17 @@ GeckoProgram('jsapi-tests', linkage=None
 
 UNIFIED_SOURCES += [
     'selfTest.cpp',
     'testAddPropertyPropcache.cpp',
     'testArgumentsObject.cpp',
     'testArrayBuffer.cpp',
     'testArrayBufferView.cpp',
     'testBug604087.cpp',
+    'testCallArgs.cpp',
     'testCallNonGenericMethodOnProxy.cpp',
     'testChromeBuffer.cpp',
     'testClassGetter.cpp',
     'testCloneScript.cpp',
     'testContexts.cpp',
     'testDateToLocaleString.cpp',
     'testDebugger.cpp',
     'testDeepFreeze.cpp',
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/testCallArgs.cpp
@@ -0,0 +1,106 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "jsapi-tests/tests.h"
+
+static bool
+CustomNative(JSContext* cx, unsigned argc, JS::Value* vp)
+{
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    MOZ_RELEASE_ASSERT(!JS_IsExceptionPending(cx));
+
+    MOZ_RELEASE_ASSERT(!args.isConstructing());
+    args.rval().setUndefined();
+    MOZ_RELEASE_ASSERT(!args.isConstructing());
+
+    return true;
+}
+
+static bool
+TryConstruct(JSContext* cx, const char* code, const char* filename, decltype(__LINE__) lineno,
+             JS::MutableHandleValue vp)
+{
+    JS::CompileOptions opts(cx);
+    opts.setFileAndLine(filename, lineno);
+    return JS::Evaluate(cx, opts, code, strlen(code), vp);
+}
+
+BEGIN_TEST(testCallArgs_isConstructing_native)
+{
+    CHECK(JS_DefineFunction(cx, global, "customNative", CustomNative, 0, 0));
+
+    JS::RootedValue result(cx);
+
+    CHECK(!TryConstruct(cx, "new customNative();", __FILE__, __LINE__, &result));
+    CHECK(JS_IsExceptionPending(cx));
+    JS_ClearPendingException(cx);
+
+    EVAL("customNative();", &result);
+    CHECK(result.isUndefined());
+
+    return true;
+}
+
+virtual bool init() override
+{
+    if (!JSAPITest::init())
+        return false;
+
+    JS::ContextOptionsRef(cx).setAutoJSAPIOwnsErrorReporting(true);
+
+    return true;
+}
+END_TEST(testCallArgs_isConstructing_native)
+
+static bool
+CustomConstructor(JSContext* cx, unsigned argc, JS::Value* vp)
+{
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    MOZ_RELEASE_ASSERT(!JS_IsExceptionPending(cx));
+
+    if (args.isConstructing()) {
+        JSObject* obj = JS_NewPlainObject(cx);
+        if (!obj)
+            return false;
+
+        args.rval().setObject(*obj);
+
+        MOZ_RELEASE_ASSERT(args.isConstructing());
+    } else {
+        args.rval().setUndefined();
+
+        MOZ_RELEASE_ASSERT(!args.isConstructing());
+    }
+
+    return true;
+}
+
+BEGIN_TEST(testCallArgs_isConstructing_constructor)
+{
+    CHECK(JS_DefineFunction(cx, global, "customConstructor", CustomConstructor, 0,
+                            JSFUN_CONSTRUCTOR));
+
+    JS::RootedValue result(cx);
+
+    EVAL("new customConstructor();", &result);
+    CHECK(result.isObject());
+
+    EVAL("customConstructor();", &result);
+    CHECK(result.isUndefined());
+
+    return true;
+}
+
+virtual bool init() override
+{
+    if (!JSAPITest::init())
+        return false;
+
+    JS::ContextOptionsRef(cx).setAutoJSAPIOwnsErrorReporting(true);
+
+    return true;
+}
+END_TEST(testCallArgs_isConstructing_constructor)