Bug 834732 - Get rid of footgun bool param for nsCxPusher and use an explicit enum. r=mrbkap

The goal here is to get rid of this crap entirely, and make nsCxPusher always
push. But that's a scary change, so we do it in chunks. This patch, in particular,
should have zero behavioral change. This means preserving some very wrong behavior.
For instance, currently SafeAutoJSContext never pushes a damn thing, because the
safe JSContext doesn't have an associated nsIScriptContext. We preserve this
behavior, and in fact convert various similarly-buggy consumers to
SafeAutoJSContext, so that we can hoist the behavioral change into a subsequent
patch.

content/base/public/nsContentUtils.h

@@ -2195,6 +2195,7 @@ typedef nsCharSeparatedTokenizerTemplate<nsContentUtils::IsHTMLWhitespace>
 class NS_STACK_CLASS nsCxPusher
 {
 public:
+  enum PushBehavior { ALWAYS_PUSH, REQUIRE_SCRIPT_CONTEXT, ASSERT_SCRIPT_CONTEXT };
   nsCxPusher();
   ~nsCxPusher(); // Calls Pop();
 
@@ -2205,7 +2206,7 @@ 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, bool aRequiresScriptContext = true);
+  bool Push(JSContext *cx, PushBehavior behavior);
   // Explicitly push a null JSContext on the the stack
   bool PushNull();
 

content/base/src/nsContentUtils.cpp

@@ -3015,19 +3015,13 @@ nsCxPusher::Push(nsIDOMEventTarget *aCurrentTarget)
     return true;
   }
 
-  JSContext* cx = nullptr;
-
-  if (scx) {
-    cx = scx->GetNativeContext();
-    // Bad, no JSContext from script context!
-    NS_ENSURE_TRUE(cx, false);
-  }
+  JSContext* cx = scx ? scx->GetNativeContext() : nullptr;
 
   // If there's no native context in the script context it must be
   // 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);
+  return Push(cx, ASSERT_SCRIPT_CONTEXT);
 }
 
 bool
@@ -3059,7 +3053,7 @@ nsCxPusher::RePush(nsIDOMEventTarget *aCurrentTarget)
 }
 
 bool
-nsCxPusher::Push(JSContext *cx, bool aRequiresScriptContext)
+nsCxPusher::Push(JSContext *cx, PushBehavior behavior)
 {
   if (mPushedSomething) {
     NS_ERROR("Whaaa! No double pushing with nsCxPusher::Push()!");
@@ -3075,7 +3069,8 @@ nsCxPusher::Push(JSContext *cx, bool aRequiresScriptContext)
   // 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);
-  if (!mScx && aRequiresScriptContext) {
+  MOZ_ASSERT_IF(behavior == ASSERT_SCRIPT_CONTEXT, mScx);
+  if (!mScx && behavior == REQUIRE_SCRIPT_CONTEXT) {
     // Should probably return false. See bug 416916.
     return true;
   }
@@ -6860,7 +6855,9 @@ AutoJSContext::Init(bool aSafe MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
 
   if (!mCx) {
     mCx = nsContentUtils::GetSafeJSContext();
-    bool result = mPusher.Push(mCx);
+    // XXXbholley - This is totally wrong, but necessary to make this patch
+    // not change any behavior. We'll fix it in an upcoming patch.
+    bool result = mPusher.Push(mCx, nsCxPusher::REQUIRE_SCRIPT_CONTEXT);
     if (!result || !mCx) {
       MOZ_CRASH();
     }

content/base/src/nsFrameMessageManager.cpp

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

content/events/src/nsEventListenerManager.cpp

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

content/xbl/src/nsXBLBinding.cpp

@@ -943,7 +943,7 @@ nsXBLBinding::ChangeDocument(nsIDocument* aOldDocument, nsIDocument* aNewDocumen
             JSContext *cx = context->GetNativeContext();
  
             nsCxPusher pusher;
-            pusher.Push(cx);
+            pusher.Push(cx, nsCxPusher::ASSERT_SCRIPT_CONTEXT);
 
             JSObject* scriptObject = mBoundElement->GetWrapper();
             if (scriptObject) {

dom/base/nsDOMClassInfo.cpp

@@ -4765,7 +4765,7 @@ BaseStubConstructor(nsIWeakReference* aWeakOwner,
       }
 
       nsCxPusher pusher;
-      NS_ENSURE_STATE(pusher.Push(cx, false));
+      NS_ENSURE_STATE(pusher.Push(cx, nsCxPusher::ALWAYS_PUSH));
 
       JSAutoRequest ar(cx);
       JSAutoCompartment ac(cx, object);
@@ -8546,7 +8546,7 @@ nsHTMLPluginObjElementSH::SetupProtoChain(nsIXPConnectWrappedNative *wrapper,
                "Shouldn't have gotten in here");
 
   nsCxPusher cxPusher;
-  if (!cxPusher.Push(cx)) {
+  if (!cxPusher.Push(cx, nsCxPusher::REQUIRE_SCRIPT_CONTEXT)) {
     return NS_OK;
   }
 

dom/base/nsGlobalWindow.cpp

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

dom/base/nsJSEnvironment.cpp

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

dom/base/nsStructuredCloneContainer.cpp

@@ -55,7 +55,7 @@ nsStructuredCloneContainer::InitFromVariant(nsIVariant *aData, JSContext *aCx)
   JS_WrapValue(aCx, &jsData);
 
   nsCxPusher cxPusher;
-  cxPusher.Push(aCx);
+  cxPusher.Push(aCx, nsCxPusher::REQUIRE_SCRIPT_CONTEXT);
 
   uint64_t* jsBytes = nullptr;
   bool success = JS_WriteStructuredClone(aCx, jsData, &jsBytes, &mSize,

dom/bindings/CallbackObject.cpp

@@ -87,7 +87,7 @@ CallbackObject::CallSetup::CallSetup(JSObject* const aCallback)
   mAr.construct(cx);
 
   // Make sure our JSContext is pushed on the stack.
-  if (!mCxPusher.Push(cx, false)) {
+  if (!mCxPusher.Push(cx, nsCxPusher::ALWAYS_PUSH)) {
     return;
   }
 

dom/indexedDB/IDBFactory.cpp

@@ -214,15 +214,7 @@ IDBFactory::Create(ContentParent* aContentParent,
     do_CreateInstance("@mozilla.org/nullprincipal;1");
   NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE);
 
-  JSContext* cx = nsContentUtils::GetSafeJSContext();
-  NS_ENSURE_TRUE(cx, NS_ERROR_FAILURE);
-
-  nsCxPusher pusher;
-  if (!pusher.Push(cx)) {
-    NS_WARNING("Failed to push safe JS context!");
-    return NS_ERROR_FAILURE;
-  }
-
+  SafeAutoJSContext cx;
   JSAutoRequest ar(cx);
 
   nsIXPConnect* xpc = nsContentUtils::XPConnect();

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, false));
+  NS_ENSURE_TRUE_VOID(pusher.Push(cx, nsCxPusher::ALWAYS_PUSH));
 
   JSAutoRequest ar(cx);
   JSAutoCompartment ac(cx, ownerObj);

dom/system/gonk/SystemWorkerManager.cpp

@@ -342,13 +342,7 @@ SystemWorkerManager::Init()
   NS_ASSERTION(NS_IsMainThread(), "We can only initialize on the main thread");
   NS_ASSERTION(!mShutdown, "Already shutdown!");
 
-  JSContext* cx = nsContentUtils::ThreadJSContextStack()->GetSafeJSContext();
-  NS_ENSURE_TRUE(cx, NS_ERROR_FAILURE);
-
-  nsCxPusher pusher;
-  if (!pusher.Push(cx, false)) {
-    return NS_ERROR_FAILURE;
-  }
+  AutoSafeJSContext cx;
 
   nsresult rv = InitWifi(cx);
   if (NS_FAILED(rv)) {

js/ipc/ObjectWrapperChild.cpp

@@ -42,7 +42,7 @@ namespace {
                                                JSOPTION_DONT_REPORT_UNCAUGHT)))
         {
             MOZ_GUARD_OBJECT_NOTIFIER_INIT;
-            mStack.Push(cx, false);
+            mStack.Push(cx, nsCxPusher::ALWAYS_PUSH);
         }
 
         ~AutoContextPusher() {

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, false));\n"
+             "  NS_ENSURE_STATE(pusher.Push(aCx, nsCxPusher::ALWAYS_PUSH));\n"
              "  JSAutoRequest ar(aCx);\n"
              "  JSAutoCompartment ac(aCx, obj);\n")