Skip to content
This repository has been archived by the owner on Jul 23, 2024. It is now read-only.

Commit

Permalink
Bug 1148593 - Create async stack in callback objects. r=bz, r=fitzgen
Browse files Browse the repository at this point in the history
--HG--
extra : rebase_source : f9b507d8f005dbca6f40f510ca41a0cbb03aebf9
  • Loading branch information
Tom Tromey committed Jul 24, 2015
1 parent a37b7fa commit 9f3b16b
Show file tree
Hide file tree
Showing 23 changed files with 421 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ function* ifTestingSupported() {

isnot($(".call-item-stack", callItem.target), null,
"There should be a stack container available now for the draw call.");
is($all(".call-item-stack-fn", callItem.target).length, 4,
"There should be 4 functions on the stack for the draw call.");
// We may have more than 4 functions, depending on whether async
// stacks are available.
ok($all(".call-item-stack-fn", callItem.target).length >= 4,
"There should be at least 4 functions on the stack for the draw call.");

ok($all(".call-item-stack-fn-name", callItem.target)[0].getAttribute("value")
.includes("C()"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ function* ifTestingSupported() {

isnot($(".call-item-stack", callItem.target), null,
"There should be a stack container available now for the draw call.");
is($all(".call-item-stack-fn", callItem.target).length, 4,
"There should be 4 functions on the stack for the draw call.");
// We may have more than 4 functions, depending on whether async
// stacks are available.
ok($all(".call-item-stack-fn", callItem.target).length >= 4,
"There should be at least 4 functions on the stack for the draw call.");

let jumpedToSource = once(window, EVENTS.SOURCE_SHOWN_IN_JS_DEBUGGER);
EventUtils.sendMouseEvent({ type: "mousedown" }, $(".call-item-location", callItem.target));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ function* ifTestingSupported() {
"There should be a stack container available now for the draw call.");
is($(".call-item-stack", callItem.target).hidden, false,
"The stack container should now be visible.");
is($all(".call-item-stack-fn", callItem.target).length, 4,
"There should be 4 functions on the stack for the draw call.");
// We may have more than 4 functions, depending on whether async
// stacks are available.
ok($all(".call-item-stack-fn", callItem.target).length >= 4,
"There should be at least 4 functions on the stack for the draw call.");

EventUtils.sendMouseEvent({ type: "dblclick" }, contents, window);

Expand All @@ -53,8 +55,10 @@ function* ifTestingSupported() {
"There should still be a stack container available for the draw call.");
is($(".call-item-stack", callItem.target).hidden, true,
"The stack container should now be hidden.");
is($all(".call-item-stack-fn", callItem.target).length, 4,
"There should still be 4 functions on the stack for the draw call.");
// We may have more than 4 functions, depending on whether async
// stacks are available.
ok($all(".call-item-stack-fn", callItem.target).length >= 4,
"There should still be at least 4 functions on the stack for the draw call.");

yield teardown(panel);
finish();
Expand Down
2 changes: 1 addition & 1 deletion docshell/test/browser/browser_timelineMarkers-frame-05.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ if (Services.prefs.getBoolPref("javascript.options.asyncstack")) {

let frame = markers[0].endStack;
ok(frame.parent.asyncParent !== null, "Parent frame has async parent");
is(frame.parent.asyncParent.asyncCause, "Promise",
is(frame.parent.asyncParent.asyncCause, "promise callback",
"Async parent has correct cause");
is(frame.parent.asyncParent.functionDisplayName, "makePromise",
"Async parent has correct function name");
Expand Down
2 changes: 2 additions & 0 deletions dom/base/test/mochitest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ support-files =
[test_anonymousContent_insert.html]
[test_anonymousContent_manipulate_content.html]
[test_appname_override.html]
[test_async_setTimeout_stack.html]
[test_async_setTimeout_stack_across_globals.html]
[test_audioWindowUtils.html]
[test_audioNotification.html]
skip-if = buildapp == 'mulet'
Expand Down
60 changes: 60 additions & 0 deletions dom/base/test/test_async_setTimeout_stack.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<!DOCTYPE HTML>
<html>
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=1142577
-->
<head>
<meta charset="utf-8">
<title>Test for Bug 1142577 - Async stacks for setTimeout</title>
<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1142577">Mozilla Bug 1142577</a>
<pre id="stack"></pre>
<script type="application/javascript">
SimpleTest.waitForExplicitFinish();
SimpleTest.requestFlakyTimeout("Testing async stacks across setTimeout");

function getFunctionName(frame) {
return frame.slice(0, frame.indexOf("@"));
}

function a() { b() }
function b() { c() }
function c() { setTimeout(d, 1) }
function d() { e() }
function e() { f() }
function f() { setTimeout(g, 1) }
function g() { h() }
function h() { i() }
function i() {
var stackString = Error().stack;
document.getElementById("stack").textContent = stackString;

var frames = stackString
.split("\n")
.map(getFunctionName)
.filter(function (name) { return !!name; });

is(frames[0], "i");
is(frames[1], "h");
is(frames[2], "g");
is(frames[3], "setTimeout handler*SimpleTest_setTimeoutShim");
is(frames[4], "f");
is(frames[5], "e");
is(frames[6], "d");
is(frames[7], "setTimeout handler*SimpleTest_setTimeoutShim");
is(frames[8], "c");
is(frames[9], "b");
is(frames[10], "a");

SimpleTest.finish();
}

SpecialPowers.pushPrefEnv(
{"set": [['javascript.options.asyncstack', true]]},
a);
</script>
</body>
</html>
60 changes: 60 additions & 0 deletions dom/base/test/test_async_setTimeout_stack_across_globals.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<!DOCTYPE HTML>
<html>
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=1142577
-->
<head>
<meta charset="utf-8">
<title>Test for Bug 1142577 - Async stacks for setTimeout</title>
<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1142577">Mozilla Bug 1142577</a>
<pre id="stack"></pre>
<iframe id="iframe"></iframe>
<script type="application/javascript">
SimpleTest.waitForExplicitFinish();

var otherGlobal = document.getElementById("iframe").contentWindow;

function getFunctionName(frame) {
return frame.slice(0, frame.indexOf("@"));
}

function a() { b() }
function b() { c() }
function c() { otherGlobal.setTimeout(d, 1) }
function d() { e() }
function e() { f() }
function f() { otherGlobal.setTimeout(g, 1) }
function g() { h() }
function h() { i() }
function i() {
var stackString = Error().stack;
document.getElementById("stack").textContent = stackString;

var frames = stackString
.split("\n")
.map(getFunctionName)
.filter(function (name) { return !!name; });

is(frames[0], "i");
is(frames[1], "h");
is(frames[2], "g");
is(frames[3], "setTimeout handler*f");
is(frames[4], "e");
is(frames[5], "d");
is(frames[6], "setTimeout handler*c");
is(frames[7], "b");
is(frames[8], "a");

SimpleTest.finish();
}

SpecialPowers.pushPrefEnv(
{"set": [['javascript.options.asyncstack', true]]},
a);
</script>
</body>
</html>
11 changes: 11 additions & 0 deletions dom/bindings/CallbackObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(CallbackObject)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(CallbackObject)
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mCallback)
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mCreationStack)
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mIncumbentJSGlobal)
NS_IMPL_CYCLE_COLLECTION_TRACE_END

Expand Down Expand Up @@ -169,6 +170,16 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
}
}

mAsyncStack.emplace(cx, aCallback->GetCreationStack());
if (*mAsyncStack) {
mAsyncCause.emplace(cx, JS_NewStringCopyZ(cx, aExecutionReason));
if (*mAsyncCause) {
mAsyncStackSetter.emplace(cx, *mAsyncStack, *mAsyncCause);
} else {
JS_ClearPendingException(cx);
}
}

// Enter the compartment of our callback, so we can actually work with it.
//
// Note that if the callback is a wrapper, this will not be the same
Expand Down
34 changes: 31 additions & 3 deletions dom/bindings/CallbackObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "nsWrapperCache.h"
#include "nsJSEnvironment.h"
#include "xpcpublic.h"
#include "jsapi.h"

namespace mozilla {
namespace dom {
Expand All @@ -56,7 +57,15 @@ class CallbackObject : public nsISupports
explicit CallbackObject(JSContext* aCx, JS::Handle<JSObject*> aCallback,
nsIGlobalObject *aIncumbentGlobal)
{
Init(aCallback, aIncumbentGlobal);
if (aCx && JS::RuntimeOptionsRef(aCx).asyncStack()) {
JS::RootedObject stack(aCx);
if (!JS::CaptureCurrentStack(aCx, &stack)) {
JS_ClearPendingException(aCx);
}
Init(aCallback, stack, aIncumbentGlobal);
} else {
Init(aCallback, nullptr, aIncumbentGlobal);
}
}

JS::Handle<JSObject*> Callback() const
Expand All @@ -65,6 +74,15 @@ class CallbackObject : public nsISupports
return CallbackPreserveColor();
}

JSObject* GetCreationStack() const
{
JSObject* result = mCreationStack;
if (result) {
JS::ExposeObjectToActiveJS(result);
}
return result;
}

/*
* This getter does not change the color of the JSObject meaning that the
* object returned is not guaranteed to be kept alive past the next CC.
Expand Down Expand Up @@ -112,7 +130,8 @@ class CallbackObject : public nsISupports

explicit CallbackObject(CallbackObject* aCallbackObject)
{
Init(aCallbackObject->mCallback, aCallbackObject->mIncumbentGlobal);
Init(aCallbackObject->mCallback, aCallbackObject->mCreationStack,
aCallbackObject->mIncumbentGlobal);
}

bool operator==(const CallbackObject& aOther) const
Expand All @@ -125,12 +144,14 @@ class CallbackObject : public nsISupports
}

private:
inline void Init(JSObject* aCallback, nsIGlobalObject* aIncumbentGlobal)
inline void Init(JSObject* aCallback, JSObject* aCreationStack,
nsIGlobalObject* aIncumbentGlobal)
{
MOZ_ASSERT(aCallback && !mCallback);
// Set script objects before we hold, on the off chance that a GC could
// somehow happen in there... (which would be pretty odd, granted).
mCallback = aCallback;
mCreationStack = aCreationStack;
if (aIncumbentGlobal) {
mIncumbentGlobal = aIncumbentGlobal;
mIncumbentJSGlobal = aIncumbentGlobal->GetGlobalJSObject();
Expand All @@ -147,12 +168,14 @@ class CallbackObject : public nsISupports
MOZ_ASSERT_IF(mIncumbentJSGlobal, mCallback);
if (mCallback) {
mCallback = nullptr;
mCreationStack = nullptr;
mIncumbentJSGlobal = nullptr;
mozilla::DropJSObjects(this);
}
}

JS::Heap<JSObject*> mCallback;
JS::Heap<JSObject*> mCreationStack;
// Ideally, we'd just hold a reference to the nsIGlobalObject, since that's
// what we need to pass to AutoIncumbentScript. Unfortunately, that doesn't
// hold the actual JS global alive. So we maintain an additional pointer to
Expand Down Expand Up @@ -213,6 +236,11 @@ class CallbackObject : public nsISupports
// always within a request during its lifetime.
Maybe<JS::Rooted<JSObject*> > mRootedCallable;

// Members which are used to set the async stack.
Maybe<JS::Rooted<JSObject*>> mAsyncStack;
Maybe<JS::Rooted<JSString*>> mAsyncCause;
Maybe<JS::AutoSetAsyncStackForNewCalls> mAsyncStackSetter;

// Can't construct a JSAutoCompartment without a JSContext either. Also,
// Put mAc after mAutoEntryScript so that we exit the compartment before
// we pop the JSContext. Though in practice we'll often manually order
Expand Down
1 change: 1 addition & 0 deletions dom/bindings/test/mochitest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ support-files =
file_proxies_via_xray.html
forOf_iframe.html

[test_async_stacks.html]
[test_ByteString.html]
[test_InstanceOf.html]
[test_bug560072.html]
Expand Down
Loading

0 comments on commit 9f3b16b

Please sign in to comment.