Closed Bug 1144973 Opened 7 years ago Closed 7 years ago

js::StringifySavedFrameStack doesn't put the string in the cx's compartment like it promises

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

Details

Attachments

(1 file, 2 obsolete files)

It's documentation says:

> The stringified stack out parameter is placed in the cx's compartment.

*Sigh*, but that just is not true.

The first thing it does is maybe enter the SavedFrame's compartment and then the string is built in that possibly different compartment.
Sorry, I know you aren't doing reviews right now, but you broke this in 04f34e86aee1 and this is a pretty small patch :P
Comment on attachment 8579711 [details] [diff] [review]
Ensure that StringifySavedFrameStack puts its stack string in the cx's compartment

This works, but couldn't we actually just let the AutoMaybeEnterFrameCompartment go out of scope before calling finishString()?  Seems to me like that would work fine (albeit with a bit more dependency on StringBuffer internals) and avoid the wrap() call.  Maybe I'm just allergic to wrap() calls: they show up in profiles too much.  :(

Either way should document near the opening curly of the scope we're adding what the scope is there for.
Attachment #8579711 - Flags: review?(bzbarsky) → review+
(In reply to Nick Fitzgerald [:fitzgen] from comment #6)
> Try push:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=17c29e1ad7d1

Those failures are fixed in bug 1144108.
https://hg.mozilla.org/mozilla-central/rev/f3988f82aefc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.