Closed
Bug 1140472
Opened 10 years ago
Closed 10 years ago
Set an async stack when invoking promise handlers
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: Paolo, Assigned: Paolo)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
1.86 KB,
patch
|
Paolo
:
review+
jimb
:
feedback+
|
Details | Diff | Splinter Review |
We can now set async stacks from bug 1083359 when calling DOM Promise handlers.
Assignee | ||
Comment 1•10 years ago
|
||
Carrying over r+ from bug 1083359 comment 44 for the change. Since this is the first use of async stacks, there might be adjustments needed in some tests that make assumptions about the stack structure, before this can land. I think there is one instance where this (correctly) masks a stack from a nested event loop, and this triggers a bug in an xpcshell assertion stack capture.
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8574029 [details] [diff] [review] The patch In the meantime, asking more feedback on the actual code. For example, this uses JS_NewStringCopyZ every time, maybe there is a better way?
Attachment #8574029 -
Flags: feedback?(jimb)
Comment 3•10 years ago
|
||
Comment on attachment 8574029 [details] [diff] [review] The patch Review of attachment 8574029 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/promise/Promise.cpp @@ +77,5 @@ > return NS_OK; > } > > + JS::Rooted<JSObject*> asyncStack(cx, mPromise->mAllocationStack); > + JS::Rooted<JSString*> asyncCause(cx, JS_NewStringCopyZ(cx, "Promise")); Short of calling JS_NewStringCopyZ at startup and stashing it somewhere rooted, I don't know of any way to avoid the string copy every time, outside the JS engine. @@ +79,5 @@ > > + JS::Rooted<JSObject*> asyncStack(cx, mPromise->mAllocationStack); > + JS::Rooted<JSString*> asyncCause(cx, JS_NewStringCopyZ(cx, "Promise")); > + if (!asyncCause) > + return NS_ERROR_OUT_OF_MEMORY; That's going to leave an exception set on cx. Should we call JS_ClearPendingException before returning NS_ERROR_OUT_OF_MEMORY? I don't know how errors are propagated out here.
Attachment #8574029 -
Flags: feedback?(jimb) → feedback+
Assignee | ||
Comment 4•10 years ago
|
||
I've added JS_ClearPendingException and left the string creation in place in this tryserver build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d2bd0233b92
Assignee | ||
Comment 5•10 years ago
|
||
And fixed one more test where we did actually get an async stack now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8a7e253b2a2 Pushed to fx-team: https://hg.mozilla.org/integration/fx-team/rev/c5fc760d1401
https://hg.mozilla.org/mozilla-central/rev/c5fc760d1401
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•