From 25d491b7924493b5d4bedc07841a1cd92f271100 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Sun, 14 Jun 2020 02:41:45 +0000 Subject: [PATCH] Bug 1601179 - Enable async stacks but limit captured async stacks to debuggees. r=jorendorff,smaug The 'asyncStack' flag on JS execution contexts is used as a general switch to enable async stack capture across all locations in SpiderMonkey, but this causes problems because it can at times be too much of a performance burden to general and track all of these stacks. Since the introduction of this option, we have only enabled it on Nightly and DevEdition for non-mobile builds, which has left a lot of users unable to take advantage of this data while debugging. This patch enables async stack traces across all of Firefox, but introduces a new pref to toggle the scope of the actual expensive part of async stacks, which is _capturing_ them and keeping them alive in memory. The new pref limits the capturing of async stack traces to only debuggees, unless an explicit pref is flipped to capture async traces for all cases. This means that while async stacks are technically enabled, and code could manually capture a stack and pass it back to SpiderMonkey and see that stack reflected in later captured stacks, SpiderMonkey itself and related async DOM APIs, among others, will not capture stacks or pass them to SpiderMonkey, so there should be no general change in performance by enabling the broader feature itself, unless the user is actively debugging the page. One effect of this patch is that if you have the debugger open and then close it, objects that have async stacks associated with them will retain those stacks and they will continue to show up in stack traces, no _new_ stacks will be captured. jorendorff and I have decided that this is okay because the expectation that the debugger fully revert every possible effect that it could have on a page is a nice goal but not a strict requirement. Differential Revision: https://phabricator.services.mozilla.com/D68503 --- .../test/mochitest/browser_dbg-asyncstacks.js | 1 - .../client/netmonitor/test/browser_net_frame.js | 5 ----- .../netmonitor/test/browser_net_initiator.js | 5 ----- .../test/browser_net_view-source-debugger.js | 3 --- devtools/client/netmonitor/test/head.js | 8 ++++++++ .../browser/browser_webconsole_async_stack.js | 2 +- ...t_menu_copy_message_with_async_stacktrace.js | 2 +- ...rowser_webconsole_promise_rejected_object.js | 2 +- .../browser_webconsole_stubs_page_error.js | 2 +- .../browser/browser_webconsole_worker_error.js | 3 ++- .../tests/xpcshell/test_client_request.js | 5 ----- .../tests/xpcshell/test_protocol_stack.js | 5 ----- .../shared/tests/xpcshell/test_executeSoon.js | 13 ------------- .../browser/browser_timelineMarkers-frame-04.js | 6 +++++- .../browser/browser_timelineMarkers-frame-05.js | 6 +++++- dom/base/test/test_async_setTimeout_stack.html | 2 +- ...t_async_setTimeout_stack_across_globals.html | 2 +- dom/bindings/CallbackObject.cpp | 2 +- dom/bindings/CallbackObject.h | 2 +- dom/bindings/test/test_async_stacks.html | 2 +- ...st_exception_options_from_jsimplemented.html | 2 +- ...t_promise_rejections_from_jsimplemented.html | 2 +- dom/ipc/JSActor.cpp | 2 +- .../tests/JSProcessActor/browser_sendQuery.js | 6 +++++- .../tests/JSWindowActor/browser_sendQuery.js | 6 +++++- dom/workers/RuntimeService.cpp | 4 +++- js/public/ContextOptions.h | 10 ++++++++++ js/src/builtin/Promise.cpp | 10 ++-------- js/src/jsapi.cpp | 9 +++++++++ js/src/jsapi.h | 10 ++++++++++ js/src/shell/js.cpp | 8 +++++++- js/src/shell/jsshell.h | 1 + js/xpconnect/src/XPCJSContext.cpp | 3 +++ js/xpconnect/tests/chrome/test_scripterror.html | 5 +++-- modules/libpref/init/all.js | 17 ++++++----------- 35 files changed, 96 insertions(+), 77 deletions(-) diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js b/devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js index 904b40fd1186..35fdd1059203 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-asyncstacks.js @@ -6,7 +6,6 @@ add_task(async function() { pushPref("devtools.debugger.features.async-captured-stacks", true); - pushPref("javascript.options.asyncstack", true); const dbg = await initDebugger("doc-frames-async.html"); invokeInTab("main"); diff --git a/devtools/client/netmonitor/test/browser_net_frame.js b/devtools/client/netmonitor/test/browser_net_frame.js index f79de57e5bb7..8df9428f39c8 100644 --- a/devtools/client/netmonitor/test/browser_net_frame.js +++ b/devtools/client/netmonitor/test/browser_net_frame.js @@ -166,11 +166,6 @@ const REQUEST_COUNT = EXPECTED_REQUESTS_TOP.length + EXPECTED_REQUESTS_SUB.length; add_task(async function() { - // Async stacks aren't on by default in all builds - await SpecialPowers.pushPrefEnv({ - set: [["javascript.options.asyncstack", true]], - }); - // the initNetMonitor function clears the network request list after the // page is loaded. That's why we first load a bogus page from SIMPLE_URL, // and only then load the real thing from TOP_URL - we want to catch diff --git a/devtools/client/netmonitor/test/browser_net_initiator.js b/devtools/client/netmonitor/test/browser_net_initiator.js index 435c05ab225e..99c2a705f26f 100644 --- a/devtools/client/netmonitor/test/browser_net_initiator.js +++ b/devtools/client/netmonitor/test/browser_net_initiator.js @@ -133,11 +133,6 @@ const EXPECTED_REQUESTS = [ ]; add_task(async function() { - // Async stacks aren't on by default in all builds - await SpecialPowers.pushPrefEnv({ - set: [["javascript.options.asyncstack", true]], - }); - // the initNetMonitor function clears the network request list after the // page is loaded. That's why we first load a bogus page from SIMPLE_URL, // and only then load the real thing from INITIATOR_URL - we want to catch diff --git a/devtools/client/netmonitor/test/browser_net_view-source-debugger.js b/devtools/client/netmonitor/test/browser_net_view-source-debugger.js index 324cbca1621c..8a41b9b52b59 100644 --- a/devtools/client/netmonitor/test/browser_net_view-source-debugger.js +++ b/devtools/client/netmonitor/test/browser_net_view-source-debugger.js @@ -18,9 +18,6 @@ add_task(async function() { // Set a higher panel height in order to get full CodeMirror content await pushPref("devtools.toolbox.footer.height", 400); - // Async stacks aren't on by default in all builds - await pushPref("javascript.options.asyncstack", true); - const { tab, monitor, toolbox } = await initNetMonitor(POST_DATA_URL, { requestCount: 1, }); diff --git a/devtools/client/netmonitor/test/head.js b/devtools/client/netmonitor/test/head.js index 204593ebaa30..06616c0b9e59 100644 --- a/devtools/client/netmonitor/test/head.js +++ b/devtools/client/netmonitor/test/head.js @@ -298,6 +298,14 @@ function initNetMonitor(url, { requestCount, enableCache = false }) { } return (async function() { + await SpecialPowers.pushPrefEnv({ + set: [ + // Capture all stacks so that the timing of devtools opening + // doesn't affect the stack trace results. + ["javascript.options.asyncstack_capture_debuggee_only", false], + ], + }); + const tab = await addTab(url); info("Net tab added successfully: " + url); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_async_stack.js b/devtools/client/webconsole/test/browser/browser_webconsole_async_stack.js index 44c7fbae5fac..e7568254d1bd 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_async_stack.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_async_stack.js @@ -25,7 +25,7 @@ promiseThen(onPromiseThen); `; add_task(async function() { - await pushPref("javascript.options.asyncstack", true); + await pushPref("javascript.options.asyncstack_capture_debuggee_only", false); const hud = await openNewTabAndConsole(TEST_URI); // Cached messages stacktrace are missing "promise callback" frames, so we reload diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_async_stacktrace.js b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_async_stacktrace.js index 7cf1aafac06f..bf978f957017 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_async_stacktrace.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_context_menu_copy_message_with_async_stacktrace.js @@ -35,7 +35,7 @@ httpServer.registerPathHandler("/test.js", function(_, response) { const TEST_URI = `http://localhost:${httpServer.identity.primaryPort}/`; add_task(async function() { - await pushPref("javascript.options.asyncstack", true); + await pushPref("javascript.options.asyncstack_capture_debuggee_only", false); const hud = await openNewTabAndConsole(TEST_URI); info("Call the log function defined in the test page"); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_promise_rejected_object.js b/devtools/client/webconsole/test/browser/browser_webconsole_promise_rejected_object.js index 5c51fc492228..f7f320d6dc6b 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_promise_rejected_object.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_promise_rejected_object.js @@ -37,7 +37,7 @@ const TEST_URI = `data:text/html;charset=utf-8, `; add_task(async function() { - await pushPref("javascript.options.asyncstack", true); + await pushPref("javascript.options.asyncstack_capture_debuggee_only", false); const hud = await openNewTabAndConsole(TEST_URI); const expectedErrors = [ diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_stubs_page_error.js b/devtools/client/webconsole/test/browser/browser_webconsole_stubs_page_error.js index 0b89dd1935ef..39255a32c269 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_stubs_page_error.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_stubs_page_error.js @@ -17,7 +17,7 @@ const TEST_URI = const STUB_FILE = "pageError.js"; add_task(async function() { - await pushPref("javascript.options.asyncstack", true); + await pushPref("javascript.options.asyncstack_capture_debuggee_only", false); const isStubsUpdate = env.get(STUBS_UPDATE_ENV) == "true"; info(`${isStubsUpdate ? "Update" : "Check"} ${STUB_FILE}`); diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_worker_error.js b/devtools/client/webconsole/test/browser/browser_webconsole_worker_error.js index d6a23119fb15..2467a2df34a3 100644 --- a/devtools/client/webconsole/test/browser/browser_webconsole_worker_error.js +++ b/devtools/client/webconsole/test/browser/browser_webconsole_worker_error.js @@ -11,7 +11,8 @@ const TEST_URI = "test/browser/test-error-worker.html"; add_task(async function() { - await pushPref("javascript.options.asyncstack", true); + await pushPref("javascript.options.asyncstack_capture_debuggee_only", false); + const hud = await openNewTabAndConsole(TEST_URI); await checkMessageStack(hud, "hello", [13, 4, 3]); diff --git a/devtools/server/tests/xpcshell/test_client_request.js b/devtools/server/tests/xpcshell/test_client_request.js index c72043a1a59b..7d85a00efcf7 100644 --- a/devtools/server/tests/xpcshell/test_client_request.js +++ b/devtools/server/tests/xpcshell/test_client_request.js @@ -64,11 +64,6 @@ function init() { } function checkStack(expectedName) { - if (!Services.prefs.getBoolPref("javascript.options.asyncstack")) { - info("Async stacks are disabled."); - return; - } - let stack = Components.stack; while (stack) { info(stack.name); diff --git a/devtools/shared/protocol/tests/xpcshell/test_protocol_stack.js b/devtools/shared/protocol/tests/xpcshell/test_protocol_stack.js index b64005ce8aea..e8f2544a3e8f 100644 --- a/devtools/shared/protocol/tests/xpcshell/test_protocol_stack.js +++ b/devtools/shared/protocol/tests/xpcshell/test_protocol_stack.js @@ -58,11 +58,6 @@ class RootFront extends protocol.FrontClassWithSpec(rootSpec) { protocol.registerFront(RootFront); function run_test() { - if (!Services.prefs.getBoolPref("javascript.options.asyncstack")) { - info("Async stacks are disabled."); - return; - } - DevToolsServer.createRootActor = RootActor; DevToolsServer.init(); diff --git a/devtools/shared/tests/xpcshell/test_executeSoon.js b/devtools/shared/tests/xpcshell/test_executeSoon.js index 507df8254476..7c6648de105d 100644 --- a/devtools/shared/tests/xpcshell/test_executeSoon.js +++ b/devtools/shared/tests/xpcshell/test_executeSoon.js @@ -13,20 +13,7 @@ var { executeSoon } = require("devtools/shared/DevToolsUtils"); var defer = require("devtools/shared/defer"); -var asyncStackEnabled = Services.prefs.getBoolPref( - "javascript.options.asyncstack" -); - -registerCleanupFunction(() => { - Services.prefs.setBoolPref( - "javascript.options.asyncstack", - asyncStackEnabled - ); -}); - add_task(async function() { - Services.prefs.setBoolPref("javascript.options.asyncstack", true); - await waitForTick(); let stack = Components.stack; diff --git a/docshell/test/browser/browser_timelineMarkers-frame-04.js b/docshell/test/browser/browser_timelineMarkers-frame-04.js index 29972be84ba9..a05804c5b38c 100644 --- a/docshell/test/browser/browser_timelineMarkers-frame-04.js +++ b/docshell/test/browser/browser_timelineMarkers-frame-04.js @@ -41,7 +41,11 @@ var TESTS = [ }, ]; -if (Services.prefs.getBoolPref("javascript.options.asyncstack")) { +if ( + !Services.prefs.getBoolPref( + "javascript.options.asyncstack_capture_debuggee_only" + ) +) { TESTS.push( { desc: "Async stack trace on Javascript marker", diff --git a/docshell/test/browser/browser_timelineMarkers-frame-05.js b/docshell/test/browser/browser_timelineMarkers-frame-05.js index 266b2b5dd322..b5c245e4517d 100644 --- a/docshell/test/browser/browser_timelineMarkers-frame-05.js +++ b/docshell/test/browser/browser_timelineMarkers-frame-05.js @@ -97,7 +97,11 @@ var TESTS = [ }, ]; -if (Services.prefs.getBoolPref("javascript.options.asyncstack")) { +if ( + !Services.prefs.getBoolPref( + "javascript.options.asyncstack_capture_debuggee_only" + ) +) { TESTS.push({ desc: "Async stack trace on Promise", searchFor: "ConsoleTime", diff --git a/dom/base/test/test_async_setTimeout_stack.html b/dom/base/test/test_async_setTimeout_stack.html index 2866f4f636eb..773a923be389 100644 --- a/dom/base/test/test_async_setTimeout_stack.html +++ b/dom/base/test/test_async_setTimeout_stack.html @@ -53,7 +53,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1142577 } SpecialPowers.pushPrefEnv( - {"set": [['javascript.options.asyncstack', true]]}, + {"set": [['javascript.options.asyncstack_capture_debuggee_only', false]]}, a); diff --git a/dom/base/test/test_async_setTimeout_stack_across_globals.html b/dom/base/test/test_async_setTimeout_stack_across_globals.html index 201ed2c37913..358aa21abe8a 100644 --- a/dom/base/test/test_async_setTimeout_stack_across_globals.html +++ b/dom/base/test/test_async_setTimeout_stack_across_globals.html @@ -53,7 +53,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1142577 } SpecialPowers.pushPrefEnv( - {"set": [['javascript.options.asyncstack', true]]}, + {"set": [['javascript.options.asyncstack_capture_debuggee_only', false]]}, a); diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp index 5a0ad5cf07a4..209308ba9356 100644 --- a/dom/bindings/CallbackObject.cpp +++ b/dom/bindings/CallbackObject.cpp @@ -100,7 +100,7 @@ void CallbackObject::FinishSlowJSInitIfMoreThanOneOwner(JSContext* aCx) { MOZ_ASSERT(mRefCnt.get() > 0); if (mRefCnt.get() > 1) { mozilla::HoldJSObjects(this); - if (JS::ContextOptionsRef(aCx).asyncStack()) { + if (JS::IsAsyncStackCaptureEnabledForRealm(aCx)) { JS::RootedObject stack(aCx); if (!JS::CaptureCurrentStack(aCx, &stack)) { JS_ClearPendingException(aCx); diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h index 4b692166563f..649c0ba1cb87 100644 --- a/dom/bindings/CallbackObject.h +++ b/dom/bindings/CallbackObject.h @@ -65,7 +65,7 @@ class CallbackObject : public nsISupports { explicit CallbackObject(JSContext* aCx, JS::Handle aCallback, JS::Handle aCallbackGlobal, nsIGlobalObject* aIncumbentGlobal) { - if (aCx && JS::ContextOptionsRef(aCx).asyncStack()) { + if (aCx && JS::IsAsyncStackCaptureEnabledForRealm(aCx)) { JS::RootedObject stack(aCx); if (!JS::CaptureCurrentStack(aCx, &stack)) { JS_ClearPendingException(aCx); diff --git a/dom/bindings/test/test_async_stacks.html b/dom/bindings/test/test_async_stacks.html index 4d5c0484e2b2..fe761e783bba 100644 --- a/dom/bindings/test/test_async_stacks.html +++ b/dom/bindings/test/test_async_stacks.html @@ -92,7 +92,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1148593 addLoadEvent(function() { SpecialPowers.pushPrefEnv( - {"set": [["javascript.options.asyncstack", true]]}, + {"set": [["javascript.options.asyncstack_capture_debuggee_only", false]]}, runTests); }); diff --git a/dom/bindings/test/test_exception_options_from_jsimplemented.html b/dom/bindings/test/test_exception_options_from_jsimplemented.html index 3bba95d831c1..d7d243b0d08a 100644 --- a/dom/bindings/test/test_exception_options_from_jsimplemented.html +++ b/dom/bindings/test/test_exception_options_from_jsimplemented.html @@ -19,7 +19,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592 var asyncFrame; /* Async parent frames from pushPrefEnv don't show up in e10s. */ - if (SpecialPowers.getBoolPref("javascript.options.asyncstack")) { + if (!SpecialPowers.getBoolPref("javascript.options.asyncstack_capture_debuggee_only")) { asyncFrame = `Async*@${file}:153:17 `; } else { diff --git a/dom/bindings/test/test_promise_rejections_from_jsimplemented.html b/dom/bindings/test/test_promise_rejections_from_jsimplemented.html index e21117199753..ab7e15dc57eb 100644 --- a/dom/bindings/test/test_promise_rejections_from_jsimplemented.html +++ b/dom/bindings/test/test_promise_rejections_from_jsimplemented.html @@ -39,7 +39,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592 var t = new TestInterfaceJS(); - var asyncStack = SpecialPowers.getBoolPref("javascript.options.asyncstack"); + var asyncStack = !SpecialPowers.getBoolPref("javascript.options.asyncstack_capture_debuggee_only"); var ourFile = location.href; var unwrapError = "Promise rejection value is a non-unwrappable cross-compartment wrapper."; var parentFrame = asyncStack ? `Async*@${ourFile}:130:17 diff --git a/dom/ipc/JSActor.cpp b/dom/ipc/JSActor.cpp index b87c061856cd..07f8cfa6b4d0 100644 --- a/dom/ipc/JSActor.cpp +++ b/dom/ipc/JSActor.cpp @@ -195,7 +195,7 @@ static ipc::StructuredCloneData CloneJSStack(JSContext* aCx, static ipc::StructuredCloneData CaptureJSStack(JSContext* aCx) { JS::Rooted stack(aCx, nullptr); - if (JS::ContextOptionsRef(aCx).asyncStack() && + if (JS::IsAsyncStackCaptureEnabledForRealm(aCx) && !JS::CaptureCurrentStack(aCx, &stack)) { JS_ClearPendingException(aCx); } diff --git a/dom/ipc/tests/JSProcessActor/browser_sendQuery.js b/dom/ipc/tests/JSProcessActor/browser_sendQuery.js index 9b7b9d9ff4ba..99b7209f494d 100644 --- a/dom/ipc/tests/JSProcessActor/browser_sendQuery.js +++ b/dom/ipc/tests/JSProcessActor/browser_sendQuery.js @@ -3,7 +3,11 @@ "use strict"; function maybeAsyncStack(offset, column) { - if (!Services.prefs.getBoolPref("javascript.options.asyncstack")) { + if ( + Services.prefs.getBoolPref( + "javascript.options.asyncstack_capture_debuggee_only" + ) + ) { return ""; } diff --git a/dom/ipc/tests/JSWindowActor/browser_sendQuery.js b/dom/ipc/tests/JSWindowActor/browser_sendQuery.js index 7b1f5230d351..e6a32961e507 100644 --- a/dom/ipc/tests/JSWindowActor/browser_sendQuery.js +++ b/dom/ipc/tests/JSWindowActor/browser_sendQuery.js @@ -3,7 +3,11 @@ "use strict"; function maybeAsyncStack(offset, column) { - if (!Services.prefs.getBoolPref("javascript.options.asyncstack")) { + if ( + Services.prefs.getBoolPref( + "javascript.options.asyncstack_capture_debuggee_only" + ) + ) { return ""; } diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp index ae60068da62f..7fdf93c8ffc9 100644 --- a/dom/workers/RuntimeService.cpp +++ b/dom/workers/RuntimeService.cpp @@ -302,7 +302,9 @@ void LoadContextOptions(const char* aPrefName, void* /* aClosure */) { NS_LITERAL_CSTRING("throw_on_asmjs_validation_failure"))) .setSourcePragmas( GetWorkerPref(NS_LITERAL_CSTRING("source_pragmas"))) - .setAsyncStack(GetWorkerPref(NS_LITERAL_CSTRING("asyncstack"))); + .setAsyncStack(GetWorkerPref(NS_LITERAL_CSTRING("asyncstack"))) + .setAsyncStackCaptureDebuggeeOnly(GetWorkerPref( + NS_LITERAL_CSTRING("asyncstack_capture_debuggee_only"))); nsCOMPtr xr = do_GetService("@mozilla.org/xre/runtime;1"); if (xr) { diff --git a/js/public/ContextOptions.h b/js/public/ContextOptions.h index 9abcdaf64e85..48483e8d5408 100644 --- a/js/public/ContextOptions.h +++ b/js/public/ContextOptions.h @@ -34,6 +34,7 @@ class JS_PUBLIC_API ContextOptions { disableIon_(false), disableEvalSecurityChecks_(false), asyncStack_(true), + asyncStackCaptureDebuggeeOnly_(false), sourcePragmas_(true), throwOnDebuggeeWouldRun_(true), dumpStackOnDebuggeeWouldRun_(false), @@ -152,6 +153,14 @@ class JS_PUBLIC_API ContextOptions { return *this; } + bool asyncStackCaptureDebuggeeOnly() const { + return asyncStackCaptureDebuggeeOnly_; + } + ContextOptions& setAsyncStackCaptureDebuggeeOnly(bool flag) { + asyncStackCaptureDebuggeeOnly_ = flag; + return *this; + } + // Enable/disable support for parsing '//(#@) source(Mapping)?URL=' pragmas. bool sourcePragmas() const { return sourcePragmas_; } ContextOptions& setSourcePragmas(bool flag) { @@ -232,6 +241,7 @@ class JS_PUBLIC_API ContextOptions { bool disableIon_ : 1; bool disableEvalSecurityChecks_ : 1; bool asyncStack_ : 1; + bool asyncStackCaptureDebuggeeOnly_ : 1; bool sourcePragmas_ : 1; bool throwOnDebuggeeWouldRun_ : 1; bool dumpStackOnDebuggeeWouldRun_ : 1; diff --git a/js/src/builtin/Promise.cpp b/js/src/builtin/Promise.cpp index e8188eb43462..abb05b1aa750 100644 --- a/js/src/builtin/Promise.cpp +++ b/js/src/builtin/Promise.cpp @@ -386,10 +386,6 @@ namespace { mozilla::Atomic gIDGenerator(0); } // namespace -static MOZ_ALWAYS_INLINE bool ShouldCaptureDebugInfo(JSContext* cx) { - return cx->options().asyncStack() || cx->realm()->isDebuggee(); -} - class PromiseDebugInfo : public NativeObject { private: enum Slots { @@ -479,7 +475,7 @@ class PromiseDebugInfo : public NativeObject { MOZ_ASSERT_IF(unwrappedRejectionStack, promise->state() == JS::PromiseState::Rejected); - if (!ShouldCaptureDebugInfo(cx)) { + if (!JS::IsAsyncStackCaptureEnabledForRealm(cx)) { return; } @@ -2255,7 +2251,7 @@ CreatePromiseObjectInternal(JSContext* cx, HandleObject proto /* = nullptr */, // Step 7. // Implicit, the handled flag is unset by default. - if (MOZ_LIKELY(!ShouldCaptureDebugInfo(cx))) { + if (MOZ_LIKELY(!JS::IsAsyncStackCaptureEnabledForRealm(cx))) { return promise; } @@ -5160,8 +5156,6 @@ static MOZ_ALWAYS_INLINE bool IsPromiseThenOrCatchRetValImplicitlyUsed( // used explicitly in the script, the stack info is observable in devtools // and profilers. We shouldn't apply the optimization not to allocate the // returned Promise object if the it's implicitly used by them. - // - // FIXME: Once bug 1280819 gets fixed, we can use ShouldCaptureDebugInfo. if (!cx->options().asyncStack()) { return false; } diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index dcf0462592f2..92a4e2199eb9 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -5861,6 +5861,15 @@ JS_PUBLIC_API bool JS::CaptureCurrentStack( return true; } +JS_PUBLIC_API bool JS::IsAsyncStackCaptureEnabledForRealm(JSContext* cx) { + if (!cx->options().asyncStack()) { + return false; + } + + return !cx->options().asyncStackCaptureDebuggeeOnly() || + cx->realm()->isDebuggee(); +} + JS_PUBLIC_API bool JS::CopyAsyncStack(JSContext* cx, JS::HandleObject asyncStack, JS::HandleString asyncCause, diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 65f3aac3b3c5..c940d18405df 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -3026,6 +3026,16 @@ extern JS_PUBLIC_API bool CaptureCurrentStack( JSContext* cx, MutableHandleObject stackp, StackCapture&& capture = StackCapture(AllFrames())); +/** + * Returns true if capturing stack trace data to associate with an asynchronous + * operation is currently enabled for the current context realm. + * + * Users should check this state before capturing a stack that will be passed + * back to AutoSetAsyncStackForNewCalls later, in order to avoid capturing a + * stack for async use when we don't actually want to capture it. + */ +extern JS_PUBLIC_API bool IsAsyncStackCaptureEnabledForRealm(JSContext* cx); + /* * This is a utility function for preparing an async stack to be used * by some other object. This may be used when you need to treat a diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index d895d5153a5f..3021f08be0d4 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -506,6 +506,7 @@ bool shell::enableWasmVerbose = false; bool shell::enableTestWasmAwaitTier2 = false; bool shell::enableSourcePragmas = true; bool shell::enableAsyncStacks = false; +bool shell::enableAsyncStackCaptureDebuggeeOnly = false; bool shell::enableStreams = false; bool shell::enableReadableByteStreams = false; bool shell::enableBYOBStreamReaders = false; @@ -10303,6 +10304,8 @@ static bool SetContextOptions(JSContext* cx, const OptionParser& op) { enableTestWasmAwaitTier2 = op.getBoolOption("test-wasm-await-tier2"); enableSourcePragmas = !op.getBoolOption("no-source-pragmas"); enableAsyncStacks = !op.getBoolOption("no-async-stacks"); + enableAsyncStackCaptureDebuggeeOnly = + op.getBoolOption("async-stacks-capture-debuggee-only"); enableStreams = !op.getBoolOption("no-streams"); enableReadableByteStreams = op.getBoolOption("enable-readable-byte-streams"); enableBYOBStreamReaders = op.getBoolOption("enable-byob-stream-readers"); @@ -10337,7 +10340,8 @@ static bool SetContextOptions(JSContext* cx, const OptionParser& op) { .setWasmVerbose(enableWasmVerbose) .setTestWasmAwaitTier2(enableTestWasmAwaitTier2) .setSourcePragmas(enableSourcePragmas) - .setAsyncStack(enableAsyncStacks); + .setAsyncStack(enableAsyncStacks) + .setAsyncStackCaptureDebuggeeOnly(enableAsyncStackCaptureDebuggeeOnly); if (op.getBoolOption("no-ion-for-main-context")) { JS::ContextOptionsRef(cx).setDisableIon(); @@ -11372,6 +11376,8 @@ int main(int argc, char** argv, char** envp) { !op.addBoolOption('\0', "no-source-pragmas", "Disable source(Mapping)URL pragma parsing") || !op.addBoolOption('\0', "no-async-stacks", "Disable async stacks") || + !op.addBoolOption('\0', "async-stacks-capture-debuggee-only", + "Limit async stack capture to only debuggees") || !op.addMultiStringOption('\0', "dll", "LIBRARY", "Dynamically load LIBRARY") || !op.addBoolOption('\0', "suppress-minidump", diff --git a/js/src/shell/jsshell.h b/js/src/shell/jsshell.h index 096d8fa7634e..2e41ad6a091a 100644 --- a/js/src/shell/jsshell.h +++ b/js/src/shell/jsshell.h @@ -130,6 +130,7 @@ extern bool enableWasmVerbose; extern bool enableTestWasmAwaitTier2; extern bool enableSourcePragmas; extern bool enableAsyncStacks; +extern bool enableAsyncStackCaptureDebuggeeOnly; extern bool enableStreams; extern bool enableReadableByteStreams; extern bool enableBYOBStreamReaders; diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index f61756986acc..54461fb88c1d 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -947,6 +947,8 @@ static void ReloadPrefsCallback(const char* pref, void* aXpccx) { bool useSourcePragmas = Preferences::GetBool(JS_OPTIONS_DOT_STR "source_pragmas"); bool useAsyncStack = Preferences::GetBool(JS_OPTIONS_DOT_STR "asyncstack"); + bool useAsyncStackCaptureDebuggeeOnly = Preferences::GetBool( + JS_OPTIONS_DOT_STR "asyncstack_capture_debuggee_only"); bool throwOnDebuggeeWouldRun = Preferences::GetBool(JS_OPTIONS_DOT_STR "throw_on_debuggee_would_run"); @@ -1006,6 +1008,7 @@ static void ReloadPrefsCallback(const char* pref, void* aXpccx) { .setThrowOnAsmJSValidationFailure(throwOnAsmJSValidationFailure) .setSourcePragmas(useSourcePragmas) .setAsyncStack(useAsyncStack) + .setAsyncStackCaptureDebuggeeOnly(useAsyncStackCaptureDebuggeeOnly) .setThrowOnDebuggeeWouldRun(throwOnDebuggeeWouldRun) .setDumpStackOnDebuggeeWouldRun(dumpStackOnDebuggeeWouldRun); diff --git a/js/xpconnect/tests/chrome/test_scripterror.html b/js/xpconnect/tests/chrome/test_scripterror.html index f69da85a7f41..cdb10d9f24ef 100644 --- a/js/xpconnect/tests/chrome/test_scripterror.html +++ b/js/xpconnect/tests/chrome/test_scripterror.html @@ -43,8 +43,9 @@ function hasExpectedProperties(message, exception) { } add_task(async () => { - await SpecialPowers.pushPrefEnv({"set": [["javascript.options.asyncstack", - true]]}); + await SpecialPowers.pushPrefEnv({"set": [ + ["javascript.options.asyncstack_capture_debuggee_only", false], + ]}); const TESTS = [ "abc", diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 4e3e616281e4..52119e547bdb 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1115,17 +1115,12 @@ pref("javascript.options.wasm_reftypes", true); pref("javascript.options.native_regexp", true); pref("javascript.options.parallel_parsing", true); pref("javascript.options.source_pragmas", true); -// Async stacks instrumentation adds overhead that is only -// advisable for developers, so we limit it to Nightly and DevEdition -#if defined(ANDROID) || defined(XP_IOS) - pref("javascript.options.asyncstack", false); -#else - #if defined(NIGHTLY_BUILD) || defined(MOZ_DEV_EDITION) - pref("javascript.options.asyncstack", true); - #else - pref("javascript.options.asyncstack", false); - #endif -#endif + +pref("javascript.options.asyncstack", true); +// Broadly capturing async stack data adds overhead that is only advisable for +// developers, so we only enable it when the devtools are open, by default. +pref("javascript.options.asyncstack_capture_debuggee_only", true); + pref("javascript.options.throw_on_asmjs_validation_failure", false); pref("javascript.options.ion.offthread_compilation", true); #ifdef DEBUG