From 623be0b606292cca5e659b75d63dc1cdb199a22d Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Mon, 9 Mar 2015 15:07:16 -0600 Subject: [PATCH] Bug 1135491: Part 2 - Changes to nsJSNPRuntime to ensure that JS exceptions aren't thrown when plugin destruction is pending; r=josh,bholley --HG-- extra : rebase_source : 450766689e0c0bf1cfa8c52f4e4051e2214715f2 --- dom/plugins/base/nsJSNPRuntime.cpp | 59 +++++++++++++++++++++++------- dom/plugins/base/nsJSNPRuntime.h | 2 + 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/dom/plugins/base/nsJSNPRuntime.cpp b/dom/plugins/base/nsJSNPRuntime.cpp index 98bccd0a5e10..7c5135e4c29f 100644 --- a/dom/plugins/base/nsJSNPRuntime.cpp +++ b/dom/plugins/base/nsJSNPRuntime.cpp @@ -122,21 +122,26 @@ NPObjectIsOutOfProcessProxy(NPObject *obj) // 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; }; @@ -746,7 +751,7 @@ nsJSObjWrapper::NP_HasMethod(NPObject *npobj, NPIdentifier id) JSAutoCompartment ac(cx, npjsobj->mJSObj); - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(jsapi, npjsobj); JS::Rooted v(cx); bool ok = GetProperty(cx, npjsobj->mJSObj, id, &v); @@ -786,7 +791,7 @@ doInvoke(NPObject *npobj, NPIdentifier method, const NPVariant *args, JSAutoCompartment ac(cx, jsobj); JS::Rooted fv(cx); - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(aes, npjsobj); if (method != NPIdentifier_VOID) { if (!GetProperty(cx, jsobj, method, &fv) || @@ -871,7 +876,7 @@ nsJSObjWrapper::NP_HasProperty(NPObject *npobj, NPIdentifier npid) nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj; bool found, ok = false; - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(jsapi, npjsobj); JS::Rooted jsobj(cx, npjsobj->mJSObj); JSAutoCompartment ac(cx, jsobj); @@ -908,7 +913,7 @@ nsJSObjWrapper::NP_GetProperty(NPObject *npobj, NPIdentifier id, nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj; - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(aes, npjsobj); JSAutoCompartment ac(cx, npjsobj->mJSObj); JS::Rooted v(cx); @@ -943,7 +948,7 @@ nsJSObjWrapper::NP_SetProperty(NPObject *npobj, NPIdentifier npid, nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj; bool ok = false; - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(aes, npjsobj); JS::Rooted jsObj(cx, npjsobj->mJSObj); JSAutoCompartment ac(cx, jsObj); @@ -977,7 +982,7 @@ nsJSObjWrapper::NP_RemoveProperty(NPObject *npobj, NPIdentifier npid) nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj; - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(jsapi, npjsobj); JS::ObjectOpResult result; JS::Rooted obj(cx, npjsobj->mJSObj); JSAutoCompartment ac(cx, obj); @@ -1029,7 +1034,7 @@ nsJSObjWrapper::NP_Enumerate(NPObject *npobj, NPIdentifier **idarray, nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj; - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(jsapi, npjsobj); JS::Rooted jsobj(cx, npjsobj->mJSObj); JSAutoCompartment ac(cx, jsobj); @@ -1120,6 +1125,17 @@ nsJSObjWrapper::GetNewOrUsed(NPP npp, JSContext *cx, JS::Handle obj) 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(npp->ndata); + if (inst->GetPlugin()->GetLibrary()->IsOOP()) { + PluginAsyncSurrogate* surrogate = PluginAsyncSurrogate::Cast(npp); + if (surrogate && surrogate->IsDestroyPending()) { + return nullptr; + } + } + if (!cx) { cx = GetJSContext(npp); @@ -2029,6 +2045,23 @@ nsJSNPRuntime::OnPluginDestroy(NPP npp) } } +// 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 @@ -2291,7 +2324,7 @@ nsJSObjWrapper::HasOwnProperty(NPObject *npobj, NPIdentifier npid) nsJSObjWrapper *npjsobj = (nsJSObjWrapper *)npobj; bool found, ok = false; - AutoJSExceptionReporter reporter(cx); + AutoJSExceptionReporter reporter(jsapi, npjsobj); JS::Rooted jsobj(cx, npjsobj->mJSObj); JSAutoCompartment ac(cx, jsobj); diff --git a/dom/plugins/base/nsJSNPRuntime.h b/dom/plugins/base/nsJSNPRuntime.h index e361b94e1879..97eac6e69489 100644 --- a/dom/plugins/base/nsJSNPRuntime.h +++ b/dom/plugins/base/nsJSNPRuntime.h @@ -15,6 +15,7 @@ class nsJSNPRuntime { public: static void OnPluginDestroy(NPP npp); + static void OnPluginDestroyPending(NPP npp); }; class nsJSObjWrapperKey @@ -41,6 +42,7 @@ class nsJSObjWrapper : public NPObject public: JS::Heap mJSObj; const NPP mNpp; + bool mDestroyPending; static NPObject *GetNewOrUsed(NPP npp, JSContext *cx, JS::Handle obj);