See some of the discussion in bug 1036527. Spinning this out, since we can do this without actually fixing bug 1036527.

This is some hackery to try this out. It has at least the following issues: 1) I just disabled the wrap() call in js_CopyErrorObject to make this compile. That's clearly wrong. 2) This is storing the SavedFrame in the slot. That's bogus; we want to either store a string or nothing at all. We definitely shouldn't expose the SavedFrame to script. To make the wrap() thing really work right, we probably need to allow creation of Error objects with a HandleObject and then just assert that it's either a SavedFrame or a security wrapper for one. For the other, we need to decide whether we want to use a getter or a resolve hook (with Jason preferring the latter) to do this, and then rejigger the insides of ErrorObject accordingly.

OK. So can we decide whether we want an accessor or a resolve hook here? I'm happy to try hooking up either one...

Though if resolve hook, I'm not sure how that's supposed to interact with Xrays, exactly...

(In reply to Boris Zbarsky [:bz] from comment #2) > OK. So can we decide whether we want an accessor or a resolve hook here? > I'm happy to try hooking up either one... Slight preference for an accessor, but feel free to do whatever's expedient.

(In reply to Nick Fitzgerald [:fitzgen] from comment #6) > Created attachment 8576990 [details] [diff] [review] > Make Error instances use SavedFrame objects for their stacks So it seems to work locally, for the most part, but I'm getting this strange bug where the error name doesn't prefix the error message in jit-test (seems to work fine for me in the shell): WITHOUT PATCH /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js:9:0 SyntaxError: missing ( before formal parameters: /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js:9:0 })() /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js:9:0 ^ Exit code: 3 [1|0|0|0] 100% ==========================================================>| 0.2s PASSED ALL WITH PATCH /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js:9:0 missing ( before formal parameters: /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js:9:0 })() /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js:9:0 ^ Exit code: 3 FAIL - basic/testBug1126754.js [0|1|0|0] 100% ==========================================================>| 0.2s FAILURES: /Users/fitzgen/src/mozilla-central/js/src/jit-test/tests/basic/testBug1126754.js TIMEOUTS:

(In reply to Nick Fitzgerald [:fitzgen] from comment #8) > Created attachment 8577265 [details] [diff] [review] > Make Error instances use SavedFrame objects for their stacks Alright got it down to two jit-test failures. Getting closer.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=65aee61f2ea2

:gkw, do you think you could fuzz these patches a bit? I'd like to double check that this isn't introducing any weird behavior before landing.

(In reply to Nick Fitzgerald [:fitzgen] from comment #12) > Try push: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=65aee61f2ea2 Oof. That's not a happy try push.

A bunch of those look like the fact that the new getter and setter don't handle "this" being an Xray to an Error object, right?

(In reply to Not doing reviews right now from comment #15) > A bunch of those look like the fact that the new getter and setter don't > handle "this" being an Xray to an Error object, right? Yup

Ok, that should take care of the issue with wrappers, and here is another try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c325d39aeae1

Which patches should be tested? (Part 0 or 1 or both?) It is preferred to have a rollup patch and let us know which m-c rev it is based on, again preferably rebased to tip.

Comment on attachment 8577572 [details] [diff] [review] Part 0: Make js/src/vm/SavedStacks.h use 1-based column numbers, like js::ComputeStackString and other browsers do. Review of attachment 8577572 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/SavedStacks.cpp @@ +1094,5 @@ > return false; > > locationp->line = iter.computeLine(&locationp->column); > + // Make the column 1-based instead of 0-based as in other browsers. > + locationp->column++; As discussed on IRC, please push this ++ down into FrameIter::computeLine and from there down into the functions it calls, and so on until we're 1-based throughout the engine. Reflect.parse can specially -1 things if that's I don't think it's necessary to change source notes, since they purportedly only record column *spans*, i.e. the length of character sequences...? I dunno, your call. Reflect.parse may unfortunately be stuck with 0-indexed column numbers. If so, we can specially subtract 1 in that code. Btw, this comment is ambiguous; it sounds like you're saying "0-based as in other browsers" which isn't what you mean...

Comment on attachment 8577666 [details] [diff] [review] Part 1: Make Error instances use SavedFrame objects for their stacks Review of attachment 8577666 [details] [diff] [review]: ----------------------------------------------------------------- f+ although I'm a little concerned about the amount of unwrapping that seems to surround SavedFrameObjects in the existing code. ::: js/src/vm/ErrorObject.cpp @@ +166,5 @@ > + InformalValueTypeName(thisValue)); > + return false; > + } > + > + RootedObject thisObj(cx, CheckedUnwrap(&thisValue.toObject())); See if you can use CallNonGenericMethod instead of adding a CheckedUnwrap call here. (Search in js/src/builtins and you'll see plenty of examples of how it's used.) @@ +214,5 @@ > + return false; > + } > + > + JSAutoCompartment ac(cx, error); > + error->setReservedSlot(STACK_OVERRIDDEN_SLOT, BooleanValue(true)); I went back and forth about whether this should define a data property on `error` or what you wrote. I guess what you wrote is better. ::: js/src/vm/SavedStacks-inl.h @@ +13,5 @@ > +js::AssertObjectIsSavedFrame(JSContext *cx, HandleObject stack) > +{ > +#ifdef DEBUG > + if (stack) { > + RootedObject savedFrameObj(cx, CheckedUnwrap(stack)); Please add a comment explaining why it is *necessary* to allow wrappers, and what we have to do to make sure it is *safe* to allow wrappers. I mean "safe" in the sense of avoiding accidentally accessing SavedStackObject slots on a wrapper object. @@ +15,5 @@ > +#ifdef DEBUG > + if (stack) { > + RootedObject savedFrameObj(cx, CheckedUnwrap(stack)); > + MOZ_ASSERT(savedFrameObj); > + MOZ_ASSERT(js::SavedFrame::isSavedFrameAndNotProto(*savedFrameObj)); If you can figure out a way to make the proto a PlainObject, please do it and then callers can just say something like MOZ_ASSERT_IF(frameObj, frameObj->is<SavedFrameObject>());

It seems that the Error.prototype.stack accessor is failing on BackstagePass objects (and perhaps others too, this is the first failure that reproduces). Do we need to implement xrays for error objects now? Or something else?

We have Xrays for Error objects. See bug 987669. When you say "failing on BackstagePass objects", do you mean that "this" is a BackstagePass? Or the compartment global is a BackstagePass? Something else?

Looking at the try push from comment 18, the M1 failure is triggered by this bit: Assert.AssertionError = function(options) { ... this.stack = stack; } Assert.AssertionError.prototype = Object.create(Error.prototype, { ... in Assert.jsm. As in, we have people poor-man's-subclassing Error. At some point we'll make Error actually subclassable and they can stop doing that, but since I bet people do this on the web too, it doesn't seem to me like we can have the stack setter assume it's got an Error object. The simplest thing to do would be to have the stack setter define a property on "this" (as suggested in comment 21) which has "stack" as the name and the provided value as a value (akin to how [Replaceable] webidl attributes work). This would shadow the proto setter and it seems like things would just work. And we should probably add an explicit test for this sort of thing, presumably in SpiderMonkey, since it's not Gecko-specific. Looking more at the try run, the M5 orange is the same issue. So is "B". The "bc" failures are something different, since they involve the "stack" getter. Would have to actually check what's throwing in TaskImpl_run and what it's throwing, exactly.

Argh, I misread the proto/parent while debugging. I think that this is likely the same issue as the AssertionErrors. frame #0: 0x000000010768a716 XUL`js::ErrorObject::checkAndUnwrapThis(cx=0x0000000100433c40, args=0x00007fff5fbf06a8, fnName=0x0000000108569654, error=MutableHandle<js::ErrorObject *> at 0x00007fff5fbf0540) + 262 at ErrorObject.cpp:172 169 170 RootedObject thisObj(cx, CheckedUnwrap(&thisValue.toObject())); 171 if (!thisObj || !thisObj->is<ErrorObject>()) { -> 172 JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO, 173 js_Error_str, fnName, 174 thisObj ? thisObj->getClass()->name : "object"); 175 return false; (lldb) p thisObj->dump() object 0x122ad0920 class 0x10acce730 Object flags: proto <Object at 0x11b8f2440> parent <BackstagePass object at 0x1196599c0> properties: ((js::Shape *) 0x11eb9b8d0) enumerate JSString* (0x1196cc418) = Latin1Char * (0x1196cc420) = "operation" : slot 0 = JSString* (0x13064ef70) = Latin1Char * (0x13064ef78) = "open" ((js::Shape *) 0x11eb9b8f8) enumerate JSString* (0x11964cdd8) = Latin1Char * (0x11964cde0) = "path" : slot 1 = JSString* (0x121c999a0) = char16_t * (0x12ecdf9c0) = "/var/folders/14/l4lqpmvs5sncftp1c0ym243w0000gp/T/tmpVPgIbI.mozrunner/signedInUser.json" ((js::Shape *) 0x11eb9b920) enumerate JSString* (0x11b21a5b0) = Latin1Char * (0x11b21a5b8) = "unixErrno" : slot 2 = 2 (lldb) p thisObj.ptr->getTaggedProto().toObjectOrNull()->dump() object 0x11b8f2440 class 0x10acce730 Object flags: delegate proto <Proxy object at 0x11b226560> parent <BackstagePass object at 0x1196599c0> properties: ((js::Shape *) 0x11b22f6a0) enumerate JSString* (0x11961c9a0) = Latin1Char * (0x11961c9a8) = "toString" : slot 0 = <function toString (resource://gre/modules/osfile/osfile_unix_allthreads.jsm:88) at 0x11b8ef180> ((js::Shape *) 0x11b22f6c8) enumerate JSString* (0x1196be838) = Latin1Char * (0x1196be840) = "toMsg" : slot 1 = <function toMsg (resource://gre/modules/osfile/osfile_unix_allthreads.jsm:94) at 0x11b8f1f80> ((js::Shape *) 0x11b2169a8) permanent shared getterValue=0x11b8fa580 JSString* (0x11b21a5f8) = Latin1Char * (0x11b21a600) = "becauseExists" : slot 16777215 ((js::Shape *) 0x11b2169e0) permanent shared getterValue=0x11b8f6c80 JSString* (0x1196e55e0) = Latin1Char * (0x1196e55e8) = "becauseNoSuchFile" : slot 16777215 ((js::Shape *) 0x11b216a18) permanent shared getterValue=0x11c105bc0 JSString* (0x11b21a610) = Latin1Char * (0x11b21a618) = "becauseNotEmpty" : slot 16777215 ((js::Shape *) 0x11b216a50) permanent shared getterValue=0x11b8f4f00 JSString* (0x11b21a640) = Latin1Char * (0x11b21a648) = "becauseClosed" : slot 16777215 ((js::Shape *) 0x11b216a88) permanent shared getterValue=0x11b8f85c0 JSString* (0x1196e5600) = Latin1Char * (0x1196e5608) = "becauseAccessDenied" : slot 16777215 ((js::Shape *) 0x11eba0548) permanent shared getterValue=0x11b8f8040 JSString* (0x1196e5620) = Latin1Char * (0x1196e5628) = "becauseInvalidArgument" : slot 16777215

Ok, with this patch version I was able to successfully run some devtools tests that were previously failing. Fingers crossed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=19134fcc7d15

(In reply to Nick Fitzgerald [:fitzgen] from comment #32) > Created attachment 8578909 [details] [diff] [review] > Part 0: Make js/src/vm/SavedStacks.h use 1-based column numbers, like > js::ComputeStackString and other browsers do As discussed on IRC, pushing the 1-based column numbers down into SpiderMonkey turns out to be a bit of a mess, so we are back to the old version of the patch that had r+ already. See bug 1144340 for future work here.

Ok, here is a new try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=faf34dd7458a

And another try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1483c51d4a32

Ok, those last SM(ggc) failures were most likely fixed by bug 1144108. I've rebased onto the latest m-c and I think everything is good to go now! https://treeherder.mozilla.org/#/jobs?repo=try&revision=19b55498f36f

This is the combined series (plus bug 1144973) which applies cleanly on the latest m-c, which for posterity is: 234403[tip]:234392,234402 cbd0efcd976c 2015-03-19 14:00 +0100 cbook merge fx-team to mozilla-central a=merge Gary, can you fuzz this a little bit? Thanks!

Comment on attachment 8580161 [details] [diff] [review] combined.patch Nothing found after spinning up some extra EC2 instances for testing this.

Comment on attachment 8580156 [details] [diff] [review] Part 1: Make Error instances use SavedFrame objects for their stacks Review of attachment 8580156 [details] [diff] [review]: ----------------------------------------------------------------- This is a very nice patch. Only nits and a few test requests. I don't see where StringifySavedFrameStack is defined, presumably in some previous patch, but if you can change it to BuildStackString() or something, I'd appreciate it. The only other thing using this word is "JS_Stringify()", which is about JSON; it'd be weird to have just those two unrelated things using the same unusual word. ::: js/src/jit-test/tests/basic/shell-principals.js @@ +29,5 @@ > eval('function a() { check("a", Error().stack); b(); }'); > low .eval('function b() { check("b", Error().stack); c(); }'); > mid .eval('function c() { check("cba", Error().stack); d(); }'); > high.eval('function d() { check("dcba", Error().stack); e(); }'); > + eval('function e() { check("ecba", Error().stack); f(); }'); Nice! I think this test could use a one-line comment somewhere noting that the original global has no principal, and that's treated the same as 0xffff. ::: js/src/jit-test/tests/debug/Memory-drainAllocationsLog-08.js @@ +20,5 @@ > let allocs1 = dbg.memory.drainAllocationsLog(); > root.doSecondAlloc(); > let allocs2 = dbg.memory.drainAllocationsLog(); > > +let allocs1Lines = allocs1.filter(x => !!x.frame).map(x => x.frame.line); What changed here? The implicit allocation of a bunch of SavedFrame objects? Could that ever be actually bad in combination with this API? Btw, if you want to change that root.eval() call to just use a multiline string (`...`) rather than an array and .join() and function.toString(), please feel free... ::: js/src/jsexn.cpp @@ +369,5 @@ > +{ > + SuppressErrorsGuard seg(cx); > + if (!CaptureCurrentStack(cx, stack, MAX_REPORTED_STACK_DEPTH)) > + return false; > + return true; return CaptureCurrentStack(...); ::: js/src/vm/ErrorObject.cpp @@ +168,5 @@ > + > + // Walk up the prototype chain until we find the first ErrorObject that has > + // the slots we need. This allows us to support the poor-man's subclassing > + // of error: Object.create(Error.prototype). > + RootedObject target(cx, CheckedUnwrap(&thisValue.toObject())); It's not a good idea to elide error reporting, because errors are stateful in the JS engine. To avoid reporting errors twice, with the second error clobbering the first, please check after this CheckedUnwrap succeeded: if (!target) return false; @@ +173,5 @@ > + RootedObject proto(cx); > + while (target && !target->is<ErrorObject>()) { > + if (!GetPrototype(cx, target, &proto)) > + return false; > + target = CheckedUnwrap(proto); And here. @@ +178,5 @@ > + } > + > + if (!target) { > + JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO, > + js_Error_str, fnName, thisValue.toObject().getClass()->name); Make sure there's a test for this error case. It would have to be something like var obj = {}; var desc = Object.getOwnPropertyDescriptor(Error.prototype, "stack"); Object.defineProperty(obj, "stack", desc); assertThrowsInstanceOf(() => obj.stack, TypeError); @@ +194,5 @@ > + Rooted<ErrorObject*> error(cx); > + if (!checkAndUnwrapThis(cx, args, "(get stack)", &error)) > + return false; > + > + RootedObject savedFrameObj(cx, error->stack()); Please make sure there's a test for when this is null. We should return an empty string, right? (Presumably we already have a test for Error.prototype.stack etc. somewhere, but make sure...) @@ +197,5 @@ > + > + RootedObject savedFrameObj(cx, error->stack()); > + RootedString stackString(cx); > + if (!StringifySavedFrameStack(cx, savedFrameObj, &stackString)) > + return false; Please change the comment on StringifySavedFrameStack in jsapi.h to note two unusual things about it: 1. The 'stack' argument must be a SavedFrame object, a wrapper around a SavedFrame, or null. 2. The 'stack' argument does *not* have to be in the same compartment as cx. @@ +213,5 @@ > +js::ErrorObject::setStack(JSContext *cx, unsigned argc, Value *vp) > +{ > + CallArgs args = CallArgsFromVp(argc, vp); > + // We accept any object here, because of poor-man's subclassing of Error. > + return CallNonGenericMethod<IsObject, setStack_impl>(cx, args); Please also test this with non-object receivers: assertThrowsInstanceOf(desc.set, TypeError); assertThrowsInstanceOf(() => desc.set.call("string"), TypeError); @@ +229,5 @@ > + RootedValue val(cx, args[0]); > + > + if (!DefineProperty(cx, thisObj, cx->names().stack, val)) > + return false; > + return true; Style nit: instead of the if-statement, write a tail call: return DefineProperty(...); ::: js/src/vm/SavedStacks-inl.h @@ +16,5 @@ > +// We allow wrappers here because the JSAPI functions for working with > +// SavedFrame objects and the SavedFrame accessors themselves handle wrappers > +// and use the original caller's compartment's principals to determine what > +// level of data to present. Unwrapping and entering the referent's compartment > +// would mess that up. See the module level documentation in Good comment. ::: js/src/vm/SavedStacks.h @@ +132,1 @@ > class SavedStacks { If it's not too much trouble, please add a comment on this class.

Comment on attachment 8580161 [details] [diff] [review] combined.patch feedback+ from me as well, no bugs found.

100% green try push with these latest versions: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bc353a3eca1 This fixes the errors from this try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=19b55498f36f * Remove SuppressErrorGuard in CaptureStack in jsexn.cpp -- this was causing a test to fail in SM(arm) on basic/bug1118996.js and making the syntax error propogate past the catch! * Set the stack depth limit from 1 << 20 to 1 << 7 -- this fixes the timeouts in SM(compacting nee ggc) for the too-much-recursion + saved-stacks error.

(In reply to Jason Orendorff [:jorendorff] from comment #45) > ::: js/src/jit-test/tests/debug/Memory-drainAllocationsLog-08.js > @@ +20,5 @@ > > let allocs1 = dbg.memory.drainAllocationsLog(); > > root.doSecondAlloc(); > > let allocs2 = dbg.memory.drainAllocationsLog(); > > > > +let allocs1Lines = allocs1.filter(x => !!x.frame).map(x => x.frame.line); > > What changed here? The implicit allocation of a bunch of SavedFrame objects? We were saving a SavedFrame stack for each SavedFrame object we allocated, resulting in accidental O(n^2) behavior. Now, SavedFrame objects just get a null allocation site, but they still show up in the allocations log. > Could that ever be actually bad in combination with this API? We already have to handle null stacks in general because Objects can be allocated with no JS on the stack (for example, DOM Events are often allocated before any JS gets run). > ::: js/src/vm/ErrorObject.cpp > @@ +168,5 @@ > > + > > + // Walk up the prototype chain until we find the first ErrorObject that has > > + // the slots we need. This allows us to support the poor-man's subclassing > > + // of error: Object.create(Error.prototype). > > + RootedObject target(cx, CheckedUnwrap(&thisValue.toObject())); > > It's not a good idea to elide error reporting, because errors are stateful > in the JS engine. To avoid reporting errors twice, with the second error > clobbering the first, please check after this CheckedUnwrap succeeded: > > if (!target) > return false; CheckedUnwrap doesn't take a cx, so it can't set an exception on the cx can it? Am I misunderstanding? > @@ +194,5 @@ > > + Rooted<ErrorObject*> error(cx); > > + if (!checkAndUnwrapThis(cx, args, "(get stack)", &error)) > > + return false; > > + > > + RootedObject savedFrameObj(cx, error->stack()); > > Please make sure there's a test for when this is null. We should return an > empty string, right? Yes, we have tests: https://dxr.mozilla.org/mozilla-central/source/js/src/jsapi-tests/testSavedStacks.cpp (Although, I see the claiming to test for stack string is actually a copy-paste fail and tests the source property... Will fix that here.) > @@ +197,5 @@ > > + > > + RootedObject savedFrameObj(cx, error->stack()); > > + RootedString stackString(cx); > > + if (!StringifySavedFrameStack(cx, savedFrameObj, &stackString)) > > + return false; > > Please change the comment on StringifySavedFrameStack in jsapi.h to note two > unusual things about it: > > 1. The 'stack' argument must be a SavedFrame object, a wrapper around a > SavedFrame, or null. > > 2. The 'stack' argument does *not* have to be in the same compartment as > cx. Both are already noted in jsapi.h just above: https://dxr.mozilla.org/mozilla-central/source/js/src/jsapi.h#5174-5196 > ::: js/src/vm/SavedStacks.h > @@ +132,1 @@ > > class SavedStacks { > > If it's not too much trouble, please add a comment on this class. Did you see part 2 yet? Does that cover what you want here?

(In reply to Nick Fitzgerald [:fitzgen] from comment #50) > > ::: js/src/vm/ErrorObject.cpp > > @@ +168,5 @@ > > > + > > > + // Walk up the prototype chain until we find the first ErrorObject that has > > > + // the slots we need. This allows us to support the poor-man's subclassing > > > + // of error: Object.create(Error.prototype). > > > + RootedObject target(cx, CheckedUnwrap(&thisValue.toObject())); > > > > It's not a good idea to elide error reporting, because errors are stateful > > in the JS engine. To avoid reporting errors twice, with the second error > > clobbering the first, please check after this CheckedUnwrap succeeded: > > > > if (!target) > > return false; > > CheckedUnwrap doesn't take a cx, so it can't set an exception on the cx can > it? Am I misunderstanding? New version does: if (!target) { JS_ReportError(cx, "Permission denied to access object"); return false; }

(In reply to Nick Fitzgerald [:fitzgen] from comment #50) > CheckedUnwrap doesn't take a cx, so it can't set an exception on the cx can > it? Am I misunderstanding? My error. Carry on. > Did you see part 2 yet? Does that cover what you want here? It does, and I need to finish that review. On it now...

Comment on attachment 8580157 [details] [diff] [review] Part 2: Add module level documentation for js/src/vm/SavedStacks.h Review of attachment 8580157 [details] [diff] [review]: ----------------------------------------------------------------- Thank you so much for this. ::: js/src/vm/SavedStacks.h @@ +17,5 @@ > +// > +// The `SavedStacks` class provides a compact way to capture and save JS stacks > +// as `SavedFrame` `JSObject` subclasses. A single `SavedFrame` object > +// represents one frame that was on the stack, and has a strong reference to its > +// parent `SavedFrame` (the next oldest frame). This reference is null when the I think "next oldest" means the opposite of what you intend here. "Apart from Chris, Izzy is the next tallest person in the room" means Izzy is *shorter* than Chris. I don't have a good alternative, though. "(the next older frame)"? Maybe this isn't a problem. Change it if you want.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fadf551562b8 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/550a5c9e8800 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba50784c810d remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7304e101be54 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/b336dc0af92c

https://hg.mozilla.org/mozilla-central/rev/fadf551562b8 https://hg.mozilla.org/mozilla-central/rev/550a5c9e8800 https://hg.mozilla.org/mozilla-central/rev/ba50784c810d https://hg.mozilla.org/mozilla-central/rev/7304e101be54 https://hg.mozilla.org/mozilla-central/rev/b336dc0af92c

We're not caching the stack string in any way anymore, right?

(In reply to Not doing reviews right now from comment #59) > We're not caching the stack string in any way anymore, right? No, we aren't. I imagine if repeatedly stringifying the SavedFrame stack becomes a problem, we can make a per-principals cache of some sort.