From 655d4654c80f030c247112e6c5418859e2ab9f05 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 22 Mar 2016 13:50:31 -0400 Subject: [PATCH] Bug 1257919 part 4. Stop returning StackFrame instances from exceptions::CreateStack. C++ callers of GetCurrentJSStack or exceptions::CreateStack always check for null anyway, and none of them seem to want this non-JS thing. r=khuey --- dom/base/Console.cpp | 53 +++++------------------- dom/base/DOMException.cpp | 43 +++---------------- dom/base/nsHostObjectProtocolHandler.cpp | 13 +++--- dom/bindings/Exceptions.cpp | 27 ++++-------- dom/bindings/Exceptions.h | 8 +++- js/xpconnect/idl/nsIXPConnect.idl | 1 + js/xpconnect/idl/xpccomponents.idl | 1 + 7 files changed, 38 insertions(+), 108 deletions(-) diff --git a/dom/base/Console.cpp b/dom/base/Console.cpp index 778e719d4df2..520abe442619 100644 --- a/dom/base/Console.cpp +++ b/dom/base/Console.cpp @@ -1034,8 +1034,7 @@ Console::NoopMethod() static nsresult StackFrameToStackEntry(nsIStackFrame* aStackFrame, - ConsoleStackEntry& aStackEntry, - uint32_t aLanguage) + ConsoleStackEntry& aStackEntry) { MOZ_ASSERT(aStackFrame); @@ -1064,7 +1063,7 @@ StackFrameToStackEntry(nsIStackFrame* aStackFrame, aStackEntry.mAsyncCause.Construct(cause); } - aStackEntry.mLanguage = aLanguage; + aStackEntry.mLanguage = nsIProgrammingLanguage::JAVASCRIPT; return NS_OK; } @@ -1075,16 +1074,10 @@ ReifyStack(nsIStackFrame* aStack, nsTArray& aRefiedStack) nsCOMPtr stack(aStack); while (stack) { - uint32_t language; - nsresult rv = stack->GetLanguage(&language); + ConsoleStackEntry& data = *aRefiedStack.AppendElement(); + nsresult rv = StackFrameToStackEntry(stack, data); NS_ENSURE_SUCCESS(rv, rv); - if (language == nsIProgrammingLanguage::JAVASCRIPT) { - ConsoleStackEntry& data = *aRefiedStack.AppendElement(); - rv = StackFrameToStackEntry(stack, data, language); - NS_ENSURE_SUCCESS(rv, rv); - } - nsCOMPtr caller; rv = stack->GetCaller(getter_AddRefs(caller)); NS_ENSURE_SUCCESS(rv, rv); @@ -1129,39 +1122,15 @@ Console::Method(JSContext* aCx, MethodName aMethodName, DEFAULT_MAX_STACKTRACE_DEPTH : 1; nsCOMPtr stack = CreateStack(aCx, maxDepth); - if (!stack) { - return; + if (stack) { + callData->mTopStackFrame.emplace(); + nsresult rv = StackFrameToStackEntry(stack, + *callData->mTopStackFrame); + if (NS_FAILED(rv)) { + return; + } } - // Walk up to the first JS stack frame and save it if we find it. - do { - uint32_t language; - nsresult rv = stack->GetLanguage(&language); - if (NS_FAILED(rv)) { - return; - } - - if (language == nsIProgrammingLanguage::JAVASCRIPT) { - callData->mTopStackFrame.emplace(); - nsresult rv = StackFrameToStackEntry(stack, - *callData->mTopStackFrame, - language); - if (NS_FAILED(rv)) { - return; - } - - break; - } - - nsCOMPtr caller; - rv = stack->GetCaller(getter_AddRefs(caller)); - if (NS_FAILED(rv)) { - return; - } - - stack.swap(caller); - } while (stack); - if (NS_IsMainThread()) { callData->mStack = stack; } else { diff --git a/dom/base/DOMException.cpp b/dom/base/DOMException.cpp index 80142ebac391..5f9a1b15459b 100644 --- a/dom/base/DOMException.cpp +++ b/dom/base/DOMException.cpp @@ -218,37 +218,7 @@ Exception::Exception(const nsACString& aMessage, sEverMadeOneFromFactory = true; } - nsCOMPtr location; - if (aLocation) { - location = aLocation; - } else { - location = GetCurrentJSStack(); - // it is legal for there to be no active JS stack, if C++ code - // is operating on a JS-implemented interface pointer without - // having been called in turn by JS. This happens in the JS - // component loader, and will become more common as additional - // components are implemented in JS. - } - // We want to trim off any leading native 'dataless' frames - if (location) { - while (1) { - uint32_t language; - int32_t lineNumber; - if (NS_FAILED(location->GetLanguage(&language)) || - language == nsIProgrammingLanguage::JAVASCRIPT || - NS_FAILED(location->GetLineNumber(&lineNumber)) || - lineNumber) { - break; - } - nsCOMPtr caller; - if (NS_FAILED(location->GetCaller(getter_AddRefs(caller))) || !caller) { - break; - } - location = caller; - } - } - - Initialize(aMessage, aResult, aName, location, aData); + Initialize(aMessage, aResult, aName, aLocation, aData); } Exception::Exception() @@ -454,12 +424,11 @@ Exception::Initialize(const nsACString& aMessage, nsresult aResult, if (aLocation) { mLocation = aLocation; } else { - nsresult rv; - nsXPConnect* xpc = nsXPConnect::XPConnect(); - rv = xpc->GetCurrentJSStack(getter_AddRefs(mLocation)); - if (NS_FAILED(rv)) { - return rv; - } + mLocation = GetCurrentJSStack(); + // it is legal for there to be no active JS stack, if C++ code + // is operating on a JS-implemented interface pointer without + // having been called in turn by JS. This happens in the JS + // component loader. } mData = aData; diff --git a/dom/base/nsHostObjectProtocolHandler.cpp b/dom/base/nsHostObjectProtocolHandler.cpp index 9614bb822e92..db97d7974d88 100644 --- a/dom/base/nsHostObjectProtocolHandler.cpp +++ b/dom/base/nsHostObjectProtocolHandler.cpp @@ -7,6 +7,7 @@ #include "nsHostObjectProtocolHandler.h" #include "DOMMediaStream.h" +#include "mozilla/dom/Exceptions.h" #include "mozilla/dom/File.h" #include "mozilla/dom/MediaSource.h" #include "mozilla/LoadInfo.h" @@ -201,11 +202,7 @@ class BlobURLsReporter final : public nsIMemoryReporter return; } - nsresult rv; - nsIXPConnect* xpc = nsContentUtils::XPConnect(); - nsCOMPtr frame; - rv = xpc->GetCurrentJSStack(getter_AddRefs(frame)); - NS_ENSURE_SUCCESS_VOID(rv); + nsCOMPtr frame = dom::GetCurrentJSStack(maxFrames); nsAutoCString origin; nsCOMPtr principalURI; @@ -214,7 +211,7 @@ class BlobURLsReporter final : public nsIMemoryReporter principalURI->GetPrePath(origin); } - for (uint32_t i = 0; i < maxFrames && frame; ++i) { + for (uint32_t i = 0; frame; ++i) { nsString fileNameUTF16; int32_t lineNumber = 0; @@ -246,8 +243,10 @@ class BlobURLsReporter final : public nsIMemoryReporter stack += ")/"; } - rv = frame->GetCaller(getter_AddRefs(frame)); + nsCOMPtr caller; + nsresult rv = frame->GetCaller(getter_AddRefs(caller)); NS_ENSURE_SUCCESS_VOID(rv); + caller.swap(frame); } } diff --git a/dom/bindings/Exceptions.cpp b/dom/bindings/Exceptions.cpp index 120190588411..58fa22db98df 100644 --- a/dom/bindings/Exceptions.cpp +++ b/dom/bindings/Exceptions.cpp @@ -175,7 +175,7 @@ CreateException(JSContext* aCx, nsresult aRv, const nsACString& aMessage) } already_AddRefed -GetCurrentJSStack() +GetCurrentJSStack(int32_t aMaxDepth) { // is there a current context available? JSContext* cx = nullptr; @@ -191,13 +191,7 @@ GetCurrentJSStack() return nullptr; } - nsCOMPtr stack = exceptions::CreateStack(cx); - if (!stack) { - return nullptr; - } - - // Note that CreateStack only returns JS frames, so we're done here. - return stack.forget(); + return exceptions::CreateStack(cx, aMaxDepth); } AutoForceSetExceptionOnContext::AutoForceSetExceptionOnContext(JSContext* aCx) @@ -795,8 +789,8 @@ NS_IMETHODIMP StackFrame::ToString(JSContext* aCx, nsACString& _retval) return NS_OK; } -/* static */ already_AddRefed -JSStackFrame::CreateStack(JSContext* aCx, int32_t aMaxDepth) +already_AddRefed +CreateStack(JSContext* aCx, int32_t aMaxDepth) { static const unsigned MAX_FRAMES = 100; if (aMaxDepth < 0) { @@ -808,19 +802,12 @@ JSStackFrame::CreateStack(JSContext* aCx, int32_t aMaxDepth) return nullptr; } - nsCOMPtr first; if (!stack) { - first = new StackFrame(); - } else { - first = new JSStackFrame(stack); + return nullptr; } - return first.forget(); -} -already_AddRefed -CreateStack(JSContext* aCx, int32_t aMaxDepth) -{ - return JSStackFrame::CreateStack(aCx, aMaxDepth); + nsCOMPtr frame = new JSStackFrame(stack); + return frame.forget(); } } // namespace exceptions diff --git a/dom/bindings/Exceptions.h b/dom/bindings/Exceptions.h index b9cdc46f0ad1..4ff8c6964259 100644 --- a/dom/bindings/Exceptions.h +++ b/dom/bindings/Exceptions.h @@ -48,8 +48,11 @@ already_AddRefed CreateException(JSContext* aCx, nsresult aRv, const nsACString& aMessage = EmptyCString()); +// aMaxDepth can be used to define a maximal depth for the stack trace. If the +// value is -1, a default maximal depth will be selected. Will return null if +// there is no JS stack right now. already_AddRefed -GetCurrentJSStack(); +GetCurrentJSStack(int32_t aMaxDepth = -1); // Throwing a TypeError on an ErrorResult may result in SpiderMonkey using its // own error reporting mechanism instead of just setting the exception on the @@ -68,7 +71,8 @@ public: namespace exceptions { // aMaxDepth can be used to define a maximal depth for the stack trace. If the -// value is -1, a default maximal depth will be selected. +// value is -1, a default maximal depth will be selected. Will return null if +// there is no JS stack right now. already_AddRefed CreateStack(JSContext* aCx, int32_t aMaxDepth = -1); diff --git a/js/xpconnect/idl/nsIXPConnect.idl b/js/xpconnect/idl/nsIXPConnect.idl index 0edde65a4da3..1670a3cb7139 100644 --- a/js/xpconnect/idl/nsIXPConnect.idl +++ b/js/xpconnect/idl/nsIXPConnect.idl @@ -412,6 +412,7 @@ interface nsIXPConnect : nsISupports [noscript,notxpcom,nostdcall] JSContextPtr getCurrentJSContext(); [noscript,notxpcom,nostdcall] JSContextPtr getSafeJSContext(); + // Will return null if there is no JS stack right now. readonly attribute nsIStackFrame CurrentJSStack; readonly attribute nsAXPCNativeCallContextPtr CurrentNativeCallContext; diff --git a/js/xpconnect/idl/xpccomponents.idl b/js/xpconnect/idl/xpccomponents.idl index 66cd7027ce57..37bb266aa6f1 100644 --- a/js/xpconnect/idl/xpccomponents.idl +++ b/js/xpconnect/idl/xpccomponents.idl @@ -710,6 +710,7 @@ interface nsIXPCComponents : nsIXPCComponentsBase { readonly attribute nsIXPCComponents_Classes classes; readonly attribute nsIXPCComponents_ClassesByID classesByID; + // Will return null if there is no JS stack right now. readonly attribute nsIStackFrame stack; readonly attribute nsIComponentManager manager; readonly attribute nsIXPCComponents_Utils utils;