Bug 860438 - Remove plugin-specific cx pusher. r=bsmedberg
authorBobby Holley <bobbyholley@gmail.com>
Thu, 18 Apr 2013 11:36:04 -0400
changeset 129214 c14d7d33f3e7871120be9e1f6f5a2d3ac3f2fe06
parent 129213 9fbf3dcab5154ba583cf194cca1db95def925224
child 129215 dd382d615402a519130ac52d50cfebed5cec8041
push id24562
push userryanvm@gmail.com
push dateFri, 19 Apr 2013 01:24:04 +0000
treeherdermozilla-central@f8d27fe5d7c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs860438, 841312
milestone23.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 860438 - Remove plugin-specific cx pusher. r=bsmedberg The old code does two little bits of special sauce that are worth mentioning: 1 - It calls OnWrapper{Created,Destroyed} to maintain the lifetime of the addref'd context stack pointer. But that whole thing is gone now. 2 - It calls ScriptEvaluated to potentially invoke termination functions in certain cases. nsCxPusher does this too, but with slightly different logic. In particular, nsCxPusher checks whether the given JSContext was already on the stack, whereas AutoCXPusher checked whether there was another cx on the stack above this one. As far as I can tell from my investigations of this stuff, this is all total voodoo, and I think it should probably be fine to just use an nsCxPusher here. Also, termination functions are going away soon in bug 841312.
dom/plugins/base/nsJSNPRuntime.cpp
--- a/dom/plugins/base/nsJSNPRuntime.cpp
+++ b/dom/plugins/base/nsJSNPRuntime.cpp
@@ -14,17 +14,16 @@
 #include "nsNPAPIPlugin.h"
 #include "nsNPAPIPluginInstance.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsIScriptContext.h"
 #include "nsDOMJSUtils.h"
 #include "nsContentUtils.h"
 #include "nsIDocument.h"
 #include "nsIJSRuntimeService.h"
-#include "nsIJSContextStack.h"
 #include "nsIXPConnect.h"
 #include "nsIDOMElement.h"
 #include "prmem.h"
 #include "nsIContent.h"
 #include "nsPluginInstanceOwner.h"
 #include "mozilla/HashFunctions.h"
 #include "nsWrapperCacheInlines.h"
 
@@ -53,20 +52,16 @@ static PLDHashTable sNPObjWrappers;
 // NPObject wrappers. When this count goes to zero, there are no more
 // wrappers and we can kill off hash tables etc.
 static int32_t sWrapperCount;
 
 // The JSRuntime. Used to unroot JSObjects when no JSContext is
 // reachable.
 static JSRuntime *sJSRuntime;
 
-// The JS context stack, we use this to push a plugin's JSContext onto
-// while executing JS on the context.
-static nsIJSContextStack *sContextStack;
-
 static nsTArray<NPObject*>* sDelayedReleases;
 
 namespace {
 
 inline bool
 NPObjectIsOutOfProcessProxy(NPObject *obj)
 {
   return obj->_class == PluginScriptableObjectParent::GetClass();
@@ -229,18 +224,16 @@ OnWrapperCreated()
       return;
 
     rtsvc->GetRuntime(&sJSRuntime);
     NS_ASSERTION(sJSRuntime != nullptr, "no JSRuntime?!");
 
     // Register our GC callback to perform delayed destruction of finalized
     // NPObjects. Leave this callback around and don't ever unregister it.
     rtsvc->RegisterGCCallback(DelayedReleaseGCCallback);
-
-    CallGetService("@mozilla.org/js/xpc/ContextStack;1", &sContextStack);
   }
 }
 
 static void
 OnWrapperDestroyed()
 {
   NS_ASSERTION(sWrapperCount, "Whaaa, unbalanced created/destroyed calls!");
 
@@ -262,61 +255,19 @@ OnWrapperDestroyed()
       // hash to prevent leaking it.
       PL_DHashTableFinish(&sNPObjWrappers);
 
       sNPObjWrappers.ops = nullptr;
     }
 
     // No more need for this.
     sJSRuntime = nullptr;
-
-    NS_IF_RELEASE(sContextStack);
   }
 }
 
-struct AutoCXPusher
-{
-  AutoCXPusher(JSContext *cx)
-  {
-    // Precondition explaining why we don't need to worry about errors
-    // in OnWrapperCreated.
-    NS_PRECONDITION(sWrapperCount > 0,
-                    "must have live wrappers when using AutoCXPusher");
-
-    // Call OnWrapperCreated and OnWrapperDestroyed to ensure that the
-    // last OnWrapperDestroyed doesn't happen while we're on the stack
-    // and null out sContextStack.
-    OnWrapperCreated();
-
-    sContextStack->Push(cx);
-  }
-
-  ~AutoCXPusher()
-  {
-    JSContext *cx = nullptr;
-    sContextStack->Pop(&cx);
-
-    JSContext *currentCx = nullptr;
-    sContextStack->Peek(&currentCx);
-
-    if (!currentCx) {
-      // No JS is running, tell the context we're done executing
-      // script.
-
-      nsIScriptContext *scx = GetScriptContextFromJSContext(cx);
-
-      if (scx) {
-        scx->ScriptEvaluated(true);
-      }
-    }
-
-    OnWrapperDestroyed();
-  }
-};
-
 namespace mozilla {
 namespace plugins {
 namespace parent {
 
 JSContext *
 GetJSContext(NPP npp)
 {
   NS_ENSURE_TRUE(npp, nullptr);
@@ -610,17 +561,18 @@ nsJSObjWrapper::NP_HasMethod(NPObject *n
     ThrowJSException(cx,
                      "Null npobj in nsJSObjWrapper::NP_HasMethod!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   AutoJSExceptionReporter reporter(cx);
 
   JS::Value v;
   JSBool ok = GetProperty(cx, npjsobj->mJSObj, id, &v);
 
@@ -646,17 +598,18 @@ doInvoke(NPObject *npobj, NPIdentifier m
   }
 
   // Initialize *result
   VOID_TO_NPVARIANT(*result);
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   JS::Value fv;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   AutoJSExceptionReporter reporter(cx);
 
   if (method != NPIdentifier_VOID) {
     if (!GetProperty(cx, npjsobj->mJSObj, method, &fv) ||
         ::JS_TypeOfValue(cx, fv) != JSTYPE_FUNCTION) {
@@ -757,17 +710,18 @@ nsJSObjWrapper::NP_HasProperty(NPObject 
                      "Null npobj in nsJSObjWrapper::NP_HasProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   JSBool found, ok = JS_FALSE;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   AutoJSExceptionReporter reporter(cx);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   NS_ASSERTION(NPIdentifierIsInt(id) || NPIdentifierIsString(id),
                "id must be either string or int!\n");
   ok = ::JS_HasPropertyById(cx, npjsobj->mJSObj, NPIdentifierToJSId(id), &found);
   return ok && found;
@@ -789,17 +743,18 @@ nsJSObjWrapper::NP_GetProperty(NPObject 
     ThrowJSException(cx,
                      "Null npobj in nsJSObjWrapper::NP_GetProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   AutoJSExceptionReporter reporter(cx);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   JS::Value v;
   return (GetProperty(cx, npjsobj->mJSObj, id, &v) &&
           JSValToNPVariant(npp, cx, v, result));
 }
@@ -821,17 +776,18 @@ nsJSObjWrapper::NP_SetProperty(NPObject 
                      "Null npobj in nsJSObjWrapper::NP_SetProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   JSBool ok = JS_FALSE;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   AutoJSExceptionReporter reporter(cx);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   JS::Value v = NPVariantToJSVal(npp, cx, value);
   JS::AutoValueRooter tvr(cx, v);
 
   NS_ASSERTION(NPIdentifierIsInt(id) || NPIdentifierIsString(id),
@@ -859,17 +815,18 @@ nsJSObjWrapper::NP_RemoveProperty(NPObje
                      "Null npobj in nsJSObjWrapper::NP_RemoveProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   JSBool ok = JS_FALSE;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   AutoJSExceptionReporter reporter(cx);
   JS::Value deleted = JSVAL_FALSE;
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   NS_ASSERTION(NPIdentifierIsInt(id) || NPIdentifierIsString(id),
                "id must be either string or int!\n");
   ok = ::JS_DeletePropertyById2(cx, npjsobj->mJSObj, NPIdentifierToJSId(id), &deleted);
@@ -912,17 +869,18 @@ nsJSObjWrapper::NP_Enumerate(NPObject *n
     ThrowJSException(cx,
                      "Null npobj in nsJSObjWrapper::NP_Enumerate!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
-  AutoCXPusher pusher(cx);
+  nsCxPusher pusher;
+  pusher.Push(cx);
   JSAutoRequest ar(cx);
   AutoJSExceptionReporter reporter(cx);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   JS::AutoIdArray ida(cx, JS_Enumerate(cx, npjsobj->mJSObj));
   if (!ida) {
     return false;
   }