Bug 1135491 - Part 2: Changes to nsJSNPRuntime to ensure that JS exceptions aren't thrown when plugin destruction is pending. r=josh, r=bholley, a=lsblakk
authorAaron Klotz <aklotz@mozilla.com>
Mon, 09 Mar 2015 15:07:16 -0600
changeset 257807 b1dc2c9553d47c55aff21a0b5bc81cec3b33fe2d
parent 257806 538258785927031872a033e0e0c1e839bf505edc
child 257808 b742cde7beed0876d006b993d9814f3e537537f5
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjosh, bholley, lsblakk
bugs1135491
milestone38.0a2
Bug 1135491 - Part 2: Changes to nsJSNPRuntime to ensure that JS exceptions aren't thrown when plugin destruction is pending. r=josh, r=bholley, a=lsblakk
dom/plugins/base/nsJSNPRuntime.cpp
dom/plugins/base/nsJSNPRuntime.h
--- a/dom/plugins/base/nsJSNPRuntime.cpp
+++ b/dom/plugins/base/nsJSNPRuntime.cpp
@@ -117,31 +117,36 @@ NPObjectIsOutOfProcessProxy(NPObject *ob
          obj->_class == PluginAsyncSurrogate::GetClass();
 }
 
 } // anonymous namespace
 
 // Helper class that reports any JS exceptions that were thrown while
 // the plugin executed JS.
 
-class AutoJSExceptionReporter
+class MOZ_STACK_CLASS AutoJSExceptionReporter
 {
 public:
-  explicit AutoJSExceptionReporter(JSContext* aCx)
-    : mCx(aCx)
+  AutoJSExceptionReporter(dom::AutoJSAPI& jsapi, nsJSObjWrapper* aWrapper)
+    : mJsapi(jsapi)
+    , mIsDestroyPending(aWrapper->mDestroyPending)
   {
+    jsapi.TakeOwnershipOfErrorReporting();
   }
 
   ~AutoJSExceptionReporter()
   {
-    JS_ReportPendingException(mCx);
+    if (mIsDestroyPending) {
+      mJsapi.ClearException();
+    }
   }
 
 protected:
-  JSContext *mCx;
+  dom::AutoJSAPI& mJsapi;
+  bool mIsDestroyPending;
 };
 
 
 NPClass nsJSObjWrapper::sJSObjWrapperNPClass =
   {
     NP_CLASS_STRUCT_VERSION,
     nsJSObjWrapper::NP_Allocate,
     nsJSObjWrapper::NP_Deallocate,
@@ -740,17 +745,17 @@ nsJSObjWrapper::NP_HasMethod(NPObject *n
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(jsapi, npjsobj);
 
   JS::Rooted<JS::Value> v(cx);
   bool ok = GetProperty(cx, npjsobj->mJSObj, id, &v);
 
   return ok && !v.isPrimitive() &&
     ::JS_ObjectIsFunction(cx, v.toObjectOrNull());
 }
 
@@ -780,17 +785,17 @@ doInvoke(NPObject *npobj, NPIdentifier m
   VOID_TO_NPVARIANT(*result);
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
   JS::Rooted<JSObject*> jsobj(cx, npjsobj->mJSObj);
   JSAutoCompartment ac(cx, jsobj);
   JS::Rooted<JS::Value> fv(cx);
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(aes, npjsobj);
 
   if (method != NPIdentifier_VOID) {
     if (!GetProperty(cx, jsobj, method, &fv) ||
         ::JS_TypeOfValue(cx, fv) != JSTYPE_FUNCTION) {
       return false;
     }
   } else {
     fv.setObject(*jsobj);
@@ -865,17 +870,17 @@ nsJSObjWrapper::NP_HasProperty(NPObject 
                      "Null npobj in nsJSObjWrapper::NP_HasProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   bool found, ok = false;
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(jsapi, npjsobj);
   JS::Rooted<JSObject*> jsobj(cx, npjsobj->mJSObj);
   JSAutoCompartment ac(cx, jsobj);
 
   NS_ASSERTION(NPIdentifierIsInt(npid) || NPIdentifierIsString(npid),
                "id must be either string or int!\n");
   JS::Rooted<jsid> id(cx, NPIdentifierToJSId(npid));
   ok = ::JS_HasPropertyById(cx, jsobj, id, &found);
   return ok && found;
@@ -902,17 +907,17 @@ nsJSObjWrapper::NP_GetProperty(NPObject 
     ThrowJSException(cx,
                      "Null npobj in nsJSObjWrapper::NP_GetProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(aes, npjsobj);
   JSAutoCompartment ac(cx, npjsobj->mJSObj);
 
   JS::Rooted<JS::Value> v(cx);
   return (GetProperty(cx, npjsobj->mJSObj, id, &v) &&
           JSValToNPVariant(npp, cx, v, result));
 }
 
 // static
@@ -937,17 +942,17 @@ nsJSObjWrapper::NP_SetProperty(NPObject 
                      "Null npobj in nsJSObjWrapper::NP_SetProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   bool ok = false;
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(aes, npjsobj);
   JS::Rooted<JSObject*> jsObj(cx, npjsobj->mJSObj);
   JSAutoCompartment ac(cx, jsObj);
 
   JS::Rooted<JS::Value> v(cx, NPVariantToJSVal(npp, cx, value));
 
   NS_ASSERTION(NPIdentifierIsInt(npid) || NPIdentifierIsString(npid),
                "id must be either string or int!\n");
   JS::Rooted<jsid> id(cx, NPIdentifierToJSId(npid));
@@ -972,17 +977,17 @@ nsJSObjWrapper::NP_RemoveProperty(NPObje
                      "Null npobj in nsJSObjWrapper::NP_RemoveProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   bool ok = false;
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(jsapi, npjsobj);
   bool deleted = false;
   JS::Rooted<JSObject*> obj(cx, npjsobj->mJSObj);
   JSAutoCompartment ac(cx, obj);
 
   NS_ASSERTION(NPIdentifierIsInt(npid) || NPIdentifierIsString(npid),
                "id must be either string or int!\n");
   JS::Rooted<jsid> id(cx, NPIdentifierToJSId(npid));
   ok = ::JS_DeletePropertyById2(cx, obj, id, &deleted);
@@ -1023,17 +1028,17 @@ nsJSObjWrapper::NP_Enumerate(NPObject *n
     ThrowJSException(cx,
                      "Null npobj in nsJSObjWrapper::NP_Enumerate!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(jsapi, npjsobj);
   JS::Rooted<JSObject*> jsobj(cx, npjsobj->mJSObj);
   JSAutoCompartment ac(cx, jsobj);
 
   JS::AutoIdArray ida(cx, JS_Enumerate(cx, jsobj));
   if (!ida) {
     return false;
   }
 
@@ -1114,16 +1119,27 @@ NPObject *
 nsJSObjWrapper::GetNewOrUsed(NPP npp, JSContext *cx, JS::Handle<JSObject*> obj)
 {
   if (!npp) {
     NS_ERROR("Null NPP passed to nsJSObjWrapper::GetNewOrUsed()!");
 
     return nullptr;
   }
 
+  // If we're running out-of-process and initializing asynchronously, and if
+  // the plugin has been asked to destroy itself during initialization,
+  // don't return any new NPObjects.
+  nsNPAPIPluginInstance* inst = static_cast<nsNPAPIPluginInstance*>(npp->ndata);
+  if (inst->GetPlugin()->GetLibrary()->IsOOP()) {
+    PluginAsyncSurrogate* surrogate = PluginAsyncSurrogate::Cast(npp);
+    if (surrogate && surrogate->IsDestroyPending()) {
+      return nullptr;
+    }
+  }
+
   if (!cx) {
     cx = GetJSContext(npp);
 
     if (!cx) {
       NS_ERROR("Unable to find a JSContext in nsJSObjWrapper::GetNewOrUsed()!");
 
       return nullptr;
     }
@@ -2019,16 +2035,33 @@ nsJSNPRuntime::OnPluginDestroy(NPP npp)
   AutoSafeJSContext cx;
   if (sNPObjWrappers.IsInitialized()) {
     NppAndCx nppcx = { npp, cx };
     PL_DHashTableEnumerate(&sNPObjWrappers,
                            NPObjWrapperPluginDestroyedCallback, &nppcx);
   }
 }
 
+// static
+void
+nsJSNPRuntime::OnPluginDestroyPending(NPP npp)
+{
+  if (sJSObjWrappersAccessible) {
+    // Prevent modification of sJSObjWrappers table if we go reentrant.
+    sJSObjWrappersAccessible = false;
+    for (JSObjWrapperTable::Enum e(sJSObjWrappers); !e.empty(); e.popFront()) {
+      nsJSObjWrapper *npobj = e.front().value();
+      MOZ_ASSERT(npobj->_class == &nsJSObjWrapper::sJSObjWrapperNPClass);
+      if (npobj->mNpp == npp) {
+        npobj->mDestroyPending = true;
+      }
+    }
+    sJSObjWrappersAccessible = true;
+  }
+}
 
 // Find the NPP for a NPObject.
 static NPP
 LookupNPP(NPObject *npobj)
 {
   if (npobj->_class == &nsJSObjWrapper::sJSObjWrapperNPClass) {
     nsJSObjWrapper* o = static_cast<nsJSObjWrapper*>(npobj);
     return o->mNpp;
@@ -2281,17 +2314,17 @@ nsJSObjWrapper::HasOwnProperty(NPObject 
                      "Null npobj in nsJSObjWrapper::NP_HasOwnProperty!");
 
     return false;
   }
 
   nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj;
   bool found, ok = false;
 
-  AutoJSExceptionReporter reporter(cx);
+  AutoJSExceptionReporter reporter(jsapi, npjsobj);
   JS::Rooted<JSObject*> jsobj(cx, npjsobj->mJSObj);
   JSAutoCompartment ac(cx, jsobj);
 
   NS_ASSERTION(NPIdentifierIsInt(npid) || NPIdentifierIsString(npid),
                "id must be either string or int!\n");
   JS::Rooted<jsid> id(cx, NPIdentifierToJSId(npid));
   ok = ::JS_AlreadyHasOwnPropertyById(cx, jsobj, id, &found);
   return ok && found;
--- a/dom/plugins/base/nsJSNPRuntime.h
+++ b/dom/plugins/base/nsJSNPRuntime.h
@@ -10,16 +10,17 @@
 #include "npapi.h"
 #include "npruntime.h"
 #include "pldhash.h"
 
 class nsJSNPRuntime
 {
 public:
   static void OnPluginDestroy(NPP npp);
+  static void OnPluginDestroyPending(NPP npp);
 };
 
 class nsJSObjWrapperKey
 {
 public:
   nsJSObjWrapperKey(JSObject *obj, NPP npp)
     : mJSObj(obj), mNpp(npp)
   {
@@ -36,16 +37,17 @@ public:
   const NPP mNpp;
 };
 
 class nsJSObjWrapper : public NPObject
 {
 public:
   JS::Heap<JSObject *> mJSObj;
   const NPP mNpp;
+  bool mDestroyPending;
 
   static NPObject *GetNewOrUsed(NPP npp, JSContext *cx,
                                 JS::Handle<JSObject*> obj);
   static bool HasOwnProperty(NPObject* npobj, NPIdentifier npid);
 
 protected:
   explicit nsJSObjWrapper(NPP npp);
   ~nsJSObjWrapper();