From de14085d8c947b740b6df1b5c0e88835fcacea34 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Wed, 5 Mar 2014 07:50:55 -0500 Subject: [PATCH 1/6] Bug 979067 - Stop exporting the guard object classes; r=froydnj --- mfbt/GuardObjects.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mfbt/GuardObjects.h b/mfbt/GuardObjects.h index aeae7dcbc08..4f5467d2844 100644 --- a/mfbt/GuardObjects.h +++ b/mfbt/GuardObjects.h @@ -68,7 +68,7 @@ namespace detail { * For more details, and examples of using these macros, see * https://developer.mozilla.org/en/Using_RAII_classes_in_Mozilla */ -class MOZ_EXPORT GuardObjectNotifier +class GuardObjectNotifier { private: bool* statementDone; @@ -85,7 +85,7 @@ class MOZ_EXPORT GuardObjectNotifier } }; -class MOZ_EXPORT GuardObjectNotificationReceiver +class GuardObjectNotificationReceiver { private: bool statementDone; From 02010883fb0e49a18c565aebbd26733cf1e615c8 Mon Sep 17 00:00:00 2001 From: Honza Bambas Date: Wed, 5 Mar 2014 14:01:09 +0100 Subject: [PATCH 2/6] Bug 977339 - Do GECKO_SEPARATE_NSPR_LOGS=1 by default, r=jduell --- ipc/glue/GeckoChildProcessHost.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index 807f8b01130..2fd7c69cbc1 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -396,11 +396,9 @@ int32_t GeckoChildProcessHost::mChildCounter = 0; bool GeckoChildProcessHost::PerformAsyncLaunch(std::vector aExtraOpts, base::ProcessArchitecture arch) { - // If separate NSPR log files are not requested, we're done. + // If NSPR log files are not requested, we're done. const char* origLogName = PR_GetEnv("NSPR_LOG_FILE"); - const char* separateLogs = PR_GetEnv("GECKO_SEPARATE_NSPR_LOGS"); - if (!origLogName || !separateLogs || !*separateLogs || - *separateLogs == '0' || *separateLogs == 'N' || *separateLogs == 'n') { + if (!origLogName) { return PerformAsyncLaunchInternal(aExtraOpts, arch); } From 093484e91562ed305abe0a6d61b82e10cd45845f Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Wed, 5 Mar 2014 15:16:04 +0200 Subject: [PATCH 3/6] Backout the previous landing (15229e2a01b1) for Bug 978862, since it had wrong bug number, r=backout --- dom/events/nsEventListenerManager.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dom/events/nsEventListenerManager.cpp b/dom/events/nsEventListenerManager.cpp index fc367734646..1c0d4435070 100644 --- a/dom/events/nsEventListenerManager.cpp +++ b/dom/events/nsEventListenerManager.cpp @@ -809,8 +809,7 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS JS::Rooted scope(cx, listener->GetEventScope()); - nsCOMPtr typeAtom = aListenerStruct->mTypeAtom; - nsIAtom* attrName = typeAtom; + nsIAtom* attrName = aListenerStruct->mTypeAtom; if (aListenerStruct->mHandlerIsString) { // OK, we didn't find an existing compiled event handler. Flag us @@ -851,7 +850,6 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS body = &handlerBody; aElement = element; } - aListenerStruct = nullptr; uint32_t lineNo = 0; nsAutoCString url (NS_LITERAL_CSTRING("-moz-evil:lying-event-listener")); @@ -866,7 +864,7 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS uint32_t argCount; const char **argNames; nsContentUtils::GetEventArgNames(aElement->GetNameSpaceID(), - typeAtom, + aListenerStruct->mTypeAtom, &argCount, &argNames); JSAutoCompartment ac(cx, context->GetWindowProxy()); @@ -896,13 +894,11 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS JS::Rooted handlerFun(cx); result = nsJSUtils::CompileFunction(cx, JS::NullPtr(), options, - nsAtomCString(typeAtom), + nsAtomCString(aListenerStruct->mTypeAtom), argCount, argNames, *body, handlerFun.address()); NS_ENSURE_SUCCESS(result, result); handler = handlerFun; NS_ENSURE_TRUE(handler, NS_ERROR_FAILURE); - } else { - aListenerStruct = nullptr; } if (handler) { @@ -910,6 +906,7 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS // Bind it JS::Rooted boundHandler(cx); context->BindCompiledEventHandler(mTarget, scope, handler, &boundHandler); + aListenerStruct = nullptr; // Note - We pass null for aIncumbentGlobal below. We could also pass the // compilation global, but since the handler is guaranteed to be scripted, // there's no need to use an override, since the JS engine will always give From 15a962965681591c7ea6fb6d04525bbd98b0e7e7 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Wed, 5 Mar 2014 15:16:42 +0200 Subject: [PATCH 4/6] Bug 978862, make sure to use the right event type atom throughout CompileEventHandlerInternal, r=jst --- dom/events/nsEventListenerManager.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dom/events/nsEventListenerManager.cpp b/dom/events/nsEventListenerManager.cpp index 1c0d4435070..fc367734646 100644 --- a/dom/events/nsEventListenerManager.cpp +++ b/dom/events/nsEventListenerManager.cpp @@ -809,7 +809,8 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS JS::Rooted scope(cx, listener->GetEventScope()); - nsIAtom* attrName = aListenerStruct->mTypeAtom; + nsCOMPtr typeAtom = aListenerStruct->mTypeAtom; + nsIAtom* attrName = typeAtom; if (aListenerStruct->mHandlerIsString) { // OK, we didn't find an existing compiled event handler. Flag us @@ -850,6 +851,7 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS body = &handlerBody; aElement = element; } + aListenerStruct = nullptr; uint32_t lineNo = 0; nsAutoCString url (NS_LITERAL_CSTRING("-moz-evil:lying-event-listener")); @@ -864,7 +866,7 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS uint32_t argCount; const char **argNames; nsContentUtils::GetEventArgNames(aElement->GetNameSpaceID(), - aListenerStruct->mTypeAtom, + typeAtom, &argCount, &argNames); JSAutoCompartment ac(cx, context->GetWindowProxy()); @@ -894,11 +896,13 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS JS::Rooted handlerFun(cx); result = nsJSUtils::CompileFunction(cx, JS::NullPtr(), options, - nsAtomCString(aListenerStruct->mTypeAtom), + nsAtomCString(typeAtom), argCount, argNames, *body, handlerFun.address()); NS_ENSURE_SUCCESS(result, result); handler = handlerFun; NS_ENSURE_TRUE(handler, NS_ERROR_FAILURE); + } else { + aListenerStruct = nullptr; } if (handler) { @@ -906,7 +910,6 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS // Bind it JS::Rooted boundHandler(cx); context->BindCompiledEventHandler(mTarget, scope, handler, &boundHandler); - aListenerStruct = nullptr; // Note - We pass null for aIncumbentGlobal below. We could also pass the // compilation global, but since the handler is guaranteed to be scripted, // there's no need to use an override, since the JS engine will always give From 7b32b1e627bf59027f62299c2b00474d1c59bfa2 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 5 Mar 2014 08:32:27 -0500 Subject: [PATCH 5/6] Bug 978646. Properly cycle-collect document.styleSheetSets. r=smaug --- content/base/crashtests/978646.html | 15 +++++++++++++++ content/base/crashtests/crashtests.list | 1 + content/base/src/nsDocument.cpp | 8 ++++++++ 3 files changed, 24 insertions(+) create mode 100644 content/base/crashtests/978646.html diff --git a/content/base/crashtests/978646.html b/content/base/crashtests/978646.html new file mode 100644 index 00000000000..09274ed16c3 --- /dev/null +++ b/content/base/crashtests/978646.html @@ -0,0 +1,15 @@ + + + + + + + + diff --git a/content/base/crashtests/crashtests.list b/content/base/crashtests/crashtests.list index 68a589962b8..14ae8345789 100644 --- a/content/base/crashtests/crashtests.list +++ b/content/base/crashtests/crashtests.list @@ -147,3 +147,4 @@ skip-if(Android) load 851353-1.html load 863950.html load 864448.html load 942979.html +load 978646.html diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index be3c5b020c6..1aa222a76fd 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -1904,6 +1904,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsDocument) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScriptGlobalObject) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mListenerManager) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDOMStyleSheets) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStyleSheetSetList) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScriptLoader) tmp->mRadioGroups.EnumerateRead(RadioGroupsTraverser, &cb); @@ -2042,6 +2043,13 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDocument) tmp->mListenerManager = nullptr; } + NS_IMPL_CYCLE_COLLECTION_UNLINK(mDOMStyleSheets) + + if (tmp->mStyleSheetSetList) { + tmp->mStyleSheetSetList->Disconnect(); + tmp->mStyleSheetSetList = nullptr; + } + if (tmp->mSubDocuments) { PL_DHashTableDestroy(tmp->mSubDocuments); tmp->mSubDocuments = nullptr; From 4b3ac033b1587f8af0b328059fb5a7519c90f535 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 5 Mar 2014 08:32:27 -0500 Subject: [PATCH 6/6] Bug 978618. Fix error reporting for unintended XPConnect exceptions thrown by JS-implemented webidl to actually work correctly. r=bholley --- dom/base/nsJSUtils.cpp | 11 +++++++++ dom/bindings/CallbackObject.cpp | 41 +++++++++++++++++++++++++++------ dom/bindings/ErrorResult.h | 4 ++++ js/src/jscntxt.cpp | 2 ++ 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/dom/base/nsJSUtils.cpp b/dom/base/nsJSUtils.cpp index b152956a4df..b346c0ef296 100644 --- a/dom/base/nsJSUtils.cpp +++ b/dom/base/nsJSUtils.cpp @@ -105,6 +105,17 @@ nsJSUtils::ReportPendingException(JSContext *aContext) if (JS_IsExceptionPending(aContext)) { bool saved = JS_SaveFrameChain(aContext); { + // JS_SaveFrameChain set the compartment of aContext to null, so we need + // to enter a compartment. The question is, which one? We don't want to + // enter the original compartment of aContext (or the compartment of the + // current exception on aContext, for that matter) because when we + // JS_ReportPendingException the JS engine can try to duck-type the + // exception and produce a JSErrorReport. It will then pass that + // JSErrorReport to the error reporter on aContext, which might expose + // information from it to script via onerror handlers. So it's very + // important that the duck typing happen in the same compartment as the + // onerror handler. In practice, that's the compartment of the window (or + // otherwise default global) of aContext, so use that here. nsIScriptContext* scx = GetScriptContextFromJSContext(aContext); JS::Rooted scope(aContext); scope = scx ? scx->GetWindowProxy() diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp index 4884fe93615..4b0aa1c8d99 100644 --- a/dom/bindings/CallbackObject.cpp +++ b/dom/bindings/CallbackObject.cpp @@ -207,8 +207,15 @@ CallbackObject::CallSetup::ShouldRethrowException(JS::Handle aExcepti CallbackObject::CallSetup::~CallSetup() { - // First things first: if we have a JSContext, report any pending - // errors on it, unless we were told to re-throw them. + // To get our nesting right we have to destroy our JSAutoCompartment first. + // In particular, we want to do this before we try reporting any exceptions, + // so we end up reporting them while in the compartment of our entry point, + // not whatever cross-compartment wrappper mCallback might be. + // Be careful: the JSAutoCompartment might not have been constructed at all! + mAc.destroyIfConstructed(); + + // Now, if we have a JSContext, report any pending errors on it, unless we + // were told to re-throw them. if (mCx) { bool dealtWithPendingException = false; if ((mCompartment && mExceptionHandling == eRethrowContentExceptions) || @@ -231,14 +238,34 @@ CallbackObject::CallSetup::~CallSetup() // Either we're supposed to report our exceptions, or we're supposed to // re-throw them but we failed to JS_GetPendingException. Either way, // just report the pending exception, if any. - nsJSUtils::ReportPendingException(mCx); + // + // We don't use nsJSUtils::ReportPendingException here because all it + // does at this point is JS_SaveFrameChain and enter a compartment around + // a JS_ReportPendingException call. But our mAutoEntryScript should + // already do a JS_SaveFrameChain and we are already in the compartment + // we want to be in, so all nsJSUtils::ReportPendingException would do is + // screw up our compartment, which is exactly what we do not want. + // + // XXXbz FIXME: bug 979525 means we don't always JS_SaveFrameChain here, + // so we need to go ahead and do that. + JS::Rooted oldGlobal(mCx, JS::CurrentGlobalOrNull(mCx)); + MOZ_ASSERT(oldGlobal, "How can we not have a global here??"); + bool saved = JS_SaveFrameChain(mCx); + // Make sure the JSAutoCompartment goes out of scope before the + // JS_RestoreFrameChain call! + { + JSAutoCompartment ac(mCx, oldGlobal); + MOZ_ASSERT(!JS::DescribeScriptedCaller(mCx), + "Our comment above about JS_SaveFrameChain having been " + "called is a lie?"); + JS_ReportPendingException(mCx); + } + if (saved) { + JS_RestoreFrameChain(mCx); + } } } - // To get our nesting right we have to destroy our JSAutoCompartment first. - // But be careful: it might not have been constructed at all! - mAc.destroyIfConstructed(); - mAutoIncumbentScript.destroyIfConstructed(); mAutoEntryScript.destroyIfConstructed(); diff --git a/dom/bindings/ErrorResult.h b/dom/bindings/ErrorResult.h index 636a5a3cfcc..0d5e185269a 100644 --- a/dom/bindings/ErrorResult.h +++ b/dom/bindings/ErrorResult.h @@ -78,6 +78,10 @@ public: // is being thrown. Code that would call ReportJSException* or // StealJSException as needed must first call WouldReportJSException even if // this ErrorResult has not failed. + // + // The exn argument to ThrowJSException can be in any compartment. It does + // not have to be in the compartment of cx. If someone later uses it, they + // will wrap it into whatever compartment they're working in, as needed. void ThrowJSException(JSContext* cx, JS::Handle exn); void ReportJSException(JSContext* cx); // Used to implement throwing exceptions from the JS implementation of diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index 0d91f9b2f44..b04b697e633 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -1172,6 +1172,8 @@ JSContext::saveFrameChain() void JSContext::restoreFrameChain() { + JS_ASSERT(enterCompartmentDepth_ == 0); // We're about to clobber it, and it + // will be wrong forevermore. SavedFrameChain sfc = savedFrameChains_.popCopy(); setCompartment(sfc.compartment); enterCompartmentDepth_ = sfc.enterCompartmentCount;