Issue #1442 Follow-up: Stop pretending proxies have a JSNative call/construct hook.

https://bugzilla.mozilla.org/show_bug.cgi?id=1471924 Part 1
Also remove an erroneous debug assert and guard against future issues.
This fixes crashes on vk.com but still does not behave correctly.

7 files changed, 30 insertions, 65 deletions

diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp
index 3b3de7e3b5..51ab67f708 100644
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -6226,8 +6226,6 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx)
   // Guard against recursion.
   mRunningExpiredTimeouts = true;
 
-  MOZ_DIAGNOSTIC_ASSERT(data->mCurrentTimerNestingLevel == 0);
-
   // Run expired timeouts.
   for (uint32_t index = 0; index < expiredTimeouts.Length(); index++) {
     TimeoutInfo*& info = expiredTimeouts[index];
diff --git a/js/src/builtin/Stream.cpp b/js/src/builtin/Stream.cpp
index c8d8e3e324..26457709d8 100644
--- a/js/src/builtin/Stream.cpp
+++ b/js/src/builtin/Stream.cpp
@@ -2790,7 +2790,7 @@ ReadableStreamControllerCallPullIfNeeded(JSContext* cx, HandleNativeObject contr
     } else {
         pullPromise = PromiseInvokeOrNoop(cx, underlyingSource, cx->names().pull, controllerVal);
     }
-    if (!pullPromise)
+    if (!pullPromise || !pullPromise->is<PromiseObject>())
         return false;
 
     RootedObject onPullFulfilled(cx, NewHandler(cx, ControllerPullHandler, controller));
diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp
index b657359d3e..a6adc122c7 100644
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -5593,9 +5593,6 @@ TryAttachCallStub(JSContext* cx, ICCall_Fallback* stub, HandleScript script, jsb
     RootedObject obj(cx, &callee.toObject());
     if (!obj->is<JSFunction>()) {
         // Try to attach a stub for a call/construct hook on the object.
-        // Ignore proxies, which are special cased by callHook/constructHook.
-        if (obj->is<ProxyObject>())
-            return true;
         if (JSNative hook = constructing ? obj->constructHook() : obj->callHook()) {
             if (op != JSOP_FUNAPPLY && !isSpread && !createSingleton) {
                 RootedObject templateObject(cx);
diff --git a/js/src/jscntxtinlines.h b/js/src/jscntxtinlines.h
index c89ef86ec2..d5bd0dc46e 100644
--- a/js/src/jscntxtinlines.h
+++ b/js/src/jscntxtinlines.h
@@ -278,22 +278,9 @@ CallJSNativeConstructor(JSContext* cx, Native native, const CallArgs& args)
      * constructor to return the callee, the assertion can be removed or
      * (another) conjunct can be added to the antecedent.
      *
-     * Exceptions:
-     *
-     * - Proxies are exceptions to both rules: they can return primitives and
-     *   they allow content to return the callee.
-     *
-     * - CallOrConstructBoundFunction is an exception as well because we might
-     *   have used bind on a proxy function.
-     *
-     * - new Iterator(x) is user-hookable; it returns x.__iterator__() which
-     *   could be any object.
-     *
-     * - (new Object(Object)) returns the callee.
+     * Exception: (new Object(Object)) returns the callee.
      */
-    MOZ_ASSERT_IF(native != js::proxy_Construct &&
-                  native != js::IteratorConstructor &&
-                  (!callee->is<JSFunction>() || callee->as<JSFunction>().native() != obj_construct),
+    MOZ_ASSERT_IF((!callee->is<JSFunction>() || callee->as<JSFunction>().native() != obj_construct),
                   args.rval().isObject() && callee != &args.rval().toObject());
 
     return true;
diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp
index 730805e038..46f89e547e 100644
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -2066,6 +2066,10 @@ JSObject::isCallable() const
 {
     if (is<JSFunction>())
         return true;
+    if (is<js::ProxyObject>()) {
+        const js::ProxyObject& p = as<js::ProxyObject>();
+        return p.handler()->isCallable(const_cast<JSObject*>(this));
+    }
     return callHook() != nullptr;
 }
 
@@ -2076,39 +2080,23 @@ JSObject::isConstructor() const
         const JSFunction& fun = as<JSFunction>();
         return fun.isConstructor();
     }
+    if (is<js::ProxyObject>()) {
+        const js::ProxyObject& p = as<js::ProxyObject>();
+        return p.handler()->isConstructor(const_cast<JSObject*>(this));
+    }
     return constructHook() != nullptr;
 }
 
 JSNative
 JSObject::callHook() const
 {
-    const js::Class* clasp = getClass();
-
-    if (JSNative call = clasp->getCall())
-        return call;
-
-    if (is<js::ProxyObject>()) {
-        const js::ProxyObject& p = as<js::ProxyObject>();
-        if (p.handler()->isCallable(const_cast<JSObject*>(this)))
-            return js::proxy_Call;
-    }
-    return nullptr;
+    return getClass()->getCall();
 }
 
 JSNative
 JSObject::constructHook() const
 {
-    const js::Class* clasp = getClass();
-
-    if (JSNative construct = clasp->getConstruct())
-        return construct;
-
-    if (is<js::ProxyObject>()) {
-        const js::ProxyObject& p = as<js::ProxyObject>();
-        if (p.handler()->isConstructor(const_cast<JSObject*>(this)))
-            return js::proxy_Construct;
-    }
-    return nullptr;
+    return getClass()->getConstruct();
 }
 
 bool
diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp
index 8f6a262106..984e1f411d 100644
--- a/js/src/proxy/Proxy.cpp
+++ b/js/src/proxy/Proxy.cpp
@@ -666,24 +666,6 @@ js::proxy_HasInstance(JSContext* cx, HandleObject proxy, MutableHandleValue v, b
 }
 
 bool
-js::proxy_Call(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    RootedObject proxy(cx, &args.callee());
-    MOZ_ASSERT(proxy->is<ProxyObject>());
-    return Proxy::call(cx, proxy, args);
-}
-
-bool
-js::proxy_Construct(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    RootedObject proxy(cx, &args.callee());
-    MOZ_ASSERT(proxy->is<ProxyObject>());
-    return Proxy::construct(cx, proxy, args);
-}
-
-bool
 js::proxy_GetElements(JSContext* cx, HandleObject proxy, uint32_t begin, uint32_t end,
                       ElementAdder* adder)
 {
diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp
index f97ba99272..ad52234a31 100644
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -457,10 +457,18 @@ js::InternalCallOrConstruct(JSContext* cx, const CallArgs& args, MaybeConstruct
 
     /* Invoke non-functions. */
     if (MOZ_UNLIKELY(!args.callee().is<JSFunction>())) {
-        MOZ_ASSERT_IF(construct, !args.callee().constructHook());
-        JSNative call = args.callee().callHook();
-        if (!call)
+        MOZ_ASSERT_IF(construct, !args.callee().isConstructor());
+
+        if (!args.callee().isCallable())
             return ReportIsNotFunction(cx, args.calleev(), skipForCallee, construct);
+
+        if (args.callee().is<ProxyObject>()) {
+            RootedObject proxy(cx, &args.callee());
+            return Proxy::call(cx, proxy, args);
+        }
+
+        JSNative call = args.callee().callHook();
+        MOZ_ASSERT(call, "isCallable without a callHook?");
         return CallJSNative(cx, call, args);
     }
 
@@ -579,6 +587,11 @@ InternalConstruct(JSContext* cx, const AnyConstructArgs& args)
         return true;
     }
 
+    if (callee.is<ProxyObject>()) {
+        RootedObject proxy(cx, &callee);
+        return Proxy::construct(cx, proxy, args);
+    }
+
     JSNative construct = callee.constructHook();
     MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?");
 
@@ -4846,7 +4859,7 @@ js::SpreadCallOperation(JSContext* cx, HandleScript script, jsbytecode* pc, Hand
                                    constructing ? CONSTRUCT : NO_CONSTRUCT);
     }
 
-    if (MOZ_UNLIKELY(!callee.toObject().is<JSFunction>()) && !callee.toObject().callHook()) {
+    if (!callee.toObject().isCallable()) {
         return ReportIsNotFunction(cx, callee, 2 + constructing,
                                    constructing ? CONSTRUCT : NO_CONSTRUCT);
     }