Bug 834732 - Make nsCxPusher.Push(JSContext*) infallible. r=mrbkap

We leave the nsIDOMEventTarget* versions fallible for now, but this makes the
common case a lot simpler. Note that this means that pushing a null JSContext,
a bug, is no longer handled at runtime. But I think we should just assert
against it, since there are already callers that don't check the return value.

content/base/public/nsContentUtils.h

@@ -2205,9 +2205,9 @@ public:
   bool RePush(nsIDOMEventTarget *aCurrentTarget);
   // If a null JSContext is passed to Push(), that will cause no
   // push to happen and false to be returned.
-  bool Push(JSContext *cx);
+  void Push(JSContext *cx);
   // Explicitly push a null JSContext on the the stack
-  bool PushNull();
+  void PushNull();
 
   // Pop() will be a no-op if Push() or PushNull() fail
   void Pop();
@@ -2215,7 +2215,7 @@ public:
   nsIScriptContext* GetCurrentScriptContext() { return mScx; }
 private:
   // Combined code for PushNull() and Push(JSContext*)
-  bool DoPush(JSContext* cx);
+  void DoPush(JSContext* cx);
 
   nsCOMPtr<nsIScriptContext> mScx;
   bool mScriptIsRunning;

content/base/src/nsContentUtils.cpp

@@ -3021,7 +3021,8 @@ nsCxPusher::Push(nsIDOMEventTarget *aCurrentTarget)
   // in the process or being torn down. We don't want to notify the
   // script context about scripts having been evaluated in such a
   // case, calling with a null cx is fine in that case.
-  return Push(cx);
+  Push(cx);
+  return true;
 }
 
 bool
@@ -3052,33 +3053,26 @@ nsCxPusher::RePush(nsIDOMEventTarget *aCurrentTarget)
   return Push(aCurrentTarget);
 }
 
-bool
+void
 nsCxPusher::Push(JSContext *cx)
 {
-  if (mPushedSomething) {
-    NS_ERROR("Whaaa! No double pushing with nsCxPusher::Push()!");
-
-    return false;
-  }
-
-  if (!cx) {
-    return false;
-  }
+  MOZ_ASSERT(!mPushedSomething, "No double pushing with nsCxPusher::Push()!");
+  MOZ_ASSERT(cx);
 
   // Hold a strong ref to the nsIScriptContext, just in case
   // XXXbz do we really need to?  If we don't get one of these in Pop(), is
   // that really a problem?  Or do we need to do this to effectively root |cx|?
   mScx = GetScriptContextFromJSContext(cx);
 
-  return DoPush(cx);
+  DoPush(cx);
 }
 
-bool
+void
 nsCxPusher::DoPush(JSContext* cx)
 {
   nsIThreadJSContextStack* stack = nsContentUtils::ThreadJSContextStack();
   if (!stack) {
-    return true;
+    return;
   }
 
   if (cx && IsContextOnStack(stack, cx)) {
@@ -3088,9 +3082,7 @@ nsCxPusher::DoPush(JSContext* cx)
   }
 
   if (NS_FAILED(stack->Push(cx))) {
-    mScriptIsRunning = false;
-    mScx = nullptr;
-    return false;
+    MOZ_CRASH();
   }
 
   mPushedSomething = true;
@@ -3099,13 +3091,12 @@ nsCxPusher::DoPush(JSContext* cx)
   if (cx)
     mCompartmentDepthOnEntry = js::GetEnterCompartmentDepth(cx);
 #endif
-  return true;
 }
 
-bool
+void
 nsCxPusher::PushNull()
 {
-  return DoPush(nullptr);
+  DoPush(nullptr);
 }
 
 void
@@ -6850,10 +6841,7 @@ AutoJSContext::Init(bool aSafe MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
 
   if (!mCx) {
     mCx = nsContentUtils::GetSafeJSContext();
-    bool result = mPusher.Push(mCx);
-    if (!result || !mCx) {
-      MOZ_CRASH();
-    }
+    mPusher.Push(mCx);
   }
 }
 

content/base/src/nsFrameMessageManager.cpp

@@ -651,7 +651,7 @@ nsFrameMessageManager::ReceiveMessage(nsISupports* aTarget,
           continue;
         }
         nsCxPusher pusher;
-        NS_ENSURE_STATE(pusher.Push(ctx));
+        pusher.Push(ctx);
 
         JSAutoRequest ar(ctx);
         JSAutoCompartment ac(ctx, object);

content/events/src/nsEventListenerManager.cpp

@@ -818,8 +818,8 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS
     }
 
     nsCxPusher pusher;
-    if (aNeedsCxPush && !pusher.Push(cx)) {
-      return NS_ERROR_FAILURE;
+    if (aNeedsCxPush) {
+      pusher.Push(cx);
     }
 
     uint32_t argCount;

dom/base/nsDOMClassInfo.cpp

@@ -4765,7 +4765,7 @@ BaseStubConstructor(nsIWeakReference* aWeakOwner,
       }
 
       nsCxPusher pusher;
-      NS_ENSURE_STATE(pusher.Push(cx));
+      pusher.Push(cx);
 
       JSAutoRequest ar(cx);
       JSAutoCompartment ac(cx, object);

dom/base/nsGlobalWindow.cpp

@@ -2085,9 +2085,7 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
   bool thisChrome = IsChromeWindow();
 
   nsCxPusher cxPusher;
-  if (!cxPusher.Push(cx)) {
-    return NS_ERROR_FAILURE;
-  }
+  cxPusher.Push(cx);
 
   XPCAutoRequest ar(cx);
 

dom/base/nsJSEnvironment.cpp

@@ -1260,8 +1260,7 @@ nsJSContext::EvaluateString(const nsAString& aScript,
   }
 
   nsCxPusher pusher;
-  if (!pusher.Push(mContext))
-    return NS_ERROR_FAILURE;
+  pusher.Push(mContext);
 
   xpc_UnmarkGrayObject(&aScopeObject);
   nsAutoMicroTask mt;
@@ -1515,8 +1514,7 @@ nsJSContext::CallEventHandler(nsISupports* aTarget, JSObject* aScope,
   // all now, and never were in some cases.
 
   nsCxPusher pusher;
-  if (!pusher.Push(mContext))
-    return NS_ERROR_FAILURE;
+  pusher.Push(mContext);
 
   // check if the event handler can be run on the object in question
   rv = sSecurityManager->CheckFunctionAccess(mContext, aHandler, target);

dom/bindings/CallbackObject.cpp

@@ -87,9 +87,7 @@ CallbackObject::CallSetup::CallSetup(JSObject* const aCallback)
   mAr.construct(cx);
 
   // Make sure our JSContext is pushed on the stack.
-  if (!mCxPusher.Push(cx)) {
-    return;
-  }
+  mCxPusher.Push(cx);
 
   // After this point we guarantee calling ScriptEvaluated() if we
   // have an nsIScriptContext.

dom/sms/src/SmsRequest.cpp

@@ -523,7 +523,7 @@ SmsRequest::NotifyThreadList(const InfallibleTArray<ThreadListItem>& aItems)
   NS_ENSURE_TRUE_VOID(ownerObj);
 
   nsCxPusher pusher;
-  NS_ENSURE_TRUE_VOID(pusher.Push(cx));
+  pusher.Push(cx);
 
   JSAutoRequest ar(cx);
   JSAutoCompartment ac(cx, ownerObj);

js/xpconnect/src/dictionary_helper_gen.py

@@ -428,7 +428,7 @@ def write_cpp(iface, fd):
              "  }\n\n"
              "  JSObject* obj = &aVal->toObject();\n"
              "  nsCxPusher pusher;\n"
-             "  NS_ENSURE_STATE(pusher.Push(aCx));\n"
+             "  pusher.Push(aCx);\n"
              "  JSAutoRequest ar(aCx);\n"
              "  JSAutoCompartment ac(aCx, obj);\n")
 

layout/base/nsRefreshDriver.cpp

@@ -555,10 +555,8 @@ nsRefreshDriver::AdvanceTimeAndRefresh(int64_t aMilliseconds)
   mMostRecentRefresh += TimeDuration::FromMilliseconds((double) aMilliseconds);
 
   nsCxPusher pusher;
-  if (pusher.PushNull()) {
-    DoTick();
-    pusher.Pop();
-  }
+  pusher.PushNull();
+  DoTick();
 }
 
 void

xpfe/appshell/src/nsXULWindow.cpp

@@ -1800,8 +1800,7 @@ NS_IMETHODIMP nsXULWindow::CreateNewContentWindow(int32_t aChromeFlags,
   // it to make things work right, so push a null cx. See bug 799348 comment 13
   // for a description of what happens when we don't.
   nsCxPusher pusher;
-  if (!pusher.PushNull())
-    return NS_ERROR_FAILURE;
+  pusher.PushNull();
   nsCOMPtr<nsIXULWindow> newWindow;
   appShell->CreateTopLevelWindow(this, uri,
                                  aChromeFlags, 615, 480,