911258 - Make exceptions thread agnostic

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-1b806960-41fb-4715-bb22-d162d2130830.
=============================================================

Top frames (see report for more):
0 	xul.dll 	nsXPCException::NewException(char const *,tag_nsresult,nsIStackFrame *,nsISupports *,nsIException * *) 	js/xpconnect/src/XPCException.cpp
1 	xul.dll 	XPCThrower::BuildAndThrowException(JSContext *,tag_nsresult,char const *) 	js/xpconnect/src/XPCThrower.cpp
2 	xul.dll 	XPCThrower::Throw(tag_nsresult,JSContext *) 	js/xpconnect/src/XPCThrower.cpp
3 	xul.dll 	xpc::Throw(JSContext *,tag_nsresult) 	js/xpconnect/src/XPCThrower.cpp
4 	xul.dll 	mozilla::dom::Throw<1>(JSContext *,tag_nsresult) 	obj-firefox/dist/include/mozilla/dom/BindingUtils.h
5 	xul.dll 	mozilla::dom::ThrowMethodFailedWithDetails<1>(JSContext *,mozilla::ErrorResult &,char const *,char const *) 	obj-firefox/dist/include/mozilla/dom/BindingUtils.h
6 	xul.dll 	mozilla::dom::TextEncoderBinding::encode 	obj-firefox/dom/bindings/TextEncoderBinding.cpp
7 	xul.dll 	mozilla::dom::TextEncoderBinding::genericMethod 	obj-firefox/dom/bindings/TextEncoderBinding.cpp
8 	mozjs.dll 	js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) 	js/src/vm/Interpreter.cpp
9 	mozjs.dll 	Interpret 	js/src/vm/Interpreter.cpp
10 	mozjs.dll 	js::RunScript(JSContext *,js::RunState &) 	js/src/vm/Interpreter.cpp


Not sure if that belongs in XPConnet or actually in whatever component that mozilla::dom::TextEncoderBinding::encode belongs in, or something else entirely.

This is a new crash that started happening on 2013-08-25 and since has instances on 26.0a1 Nightly every day. It ranges at #18 over the last 7 days of builds, so far happens on Windows only, seems like something to look into as it has a clear regression day, though.

https://crash-stats.mozilla.com/report/list?signature=nsXPCException%3A%3ANewException%28char%20const%2A%2C%20tag_nsresult%2C%20nsIStackFrame%2A%2C%20nsISupports%2A%2C%20nsIException%2A%2A%29

Comment 1

[Andrew McCreight [:mccr8]]

I looked at three of these stacks, and 2 out of 3 were happening on worker threads, so this could be a regression from the worker text encoder stuff.  I don't have the bug number for that handy so I'm not sure what it landed.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Ah, yes, TextEncoder can actually throw.

Crap.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 1 - Stop using nsIExceptionManager

It's not doing anything useful, so let's get rid of it.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 2: Kill the exception service

Now it has no consumers.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 3 - Fiddle with NS_NewDOMException signature

Not really sure why I made this separate ... it should be folded into the following patch.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 4 - Convert exceptions to WebIDL

This converts both regular XPConnect exceptions and DOM exceptions to webidl, and makes the DOM exception impl inherit from the XPConnect impl.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 5 - Refactor exceptions

This moves the exception implementations to mozilla::dom and pushes the pending exception handling down into CycleCollectedJSRuntime.

Longer term I think we want to figure out what to do with code that is shared between XPConnect and workers.  dom/ seems like a fine home to me but bholley may disagree.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 6 - Use thread-agnostic exception handling everywhere

Finish making exception handling thread-agnostic and use it everywhere.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 798935 [details] [diff] [review]
Part 1 - Stop using nsIExceptionManager

Review of attachment 798935 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCThrower.cpp
@@ +182,5 @@
> +    XPCJSRuntime* runtime = XPCJSRuntime::Get();
> +    nsCOMPtr<nsIException> existingException = runtime->GetPendingException();
> +    if (existingException) {
> +        nsresult nr;
> +        if (NS_SUCCEEDED(existingException->GetResult(&nr)) && 

whitespace error

@@ +196,5 @@
> +                                 getter_AddRefs(defaultException));
> +
> +    // Do we use DOM exceptions for this error code?
> +    switch (NS_ERROR_GET_MODULE(rv)) {
> +    case NS_ERROR_MODULE_DOM:

Indentation.

Comment 10

[Bobby Holley (:bholley)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #7)
> Longer term I think we want to figure out what to do with code that is
> shared between XPConnect and workers.  dom/ seems like a fine home to me but
> bholley may disagree.

xpcom/ for non-dom machinery, dom/ for dom machinery IMO.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 798941 [details] [diff] [review]
Part 4 - Convert exceptions to WebIDL

So the constructor expects static strings, right?  Please document that.

>+nsDOMException::GetName(nsString& retval)
>+  retval = NS_ConvertUTF8toUTF16(mName);

  NS_CopyUTF8toUTF16(mName, retval);

and similar lots of other places in this patch.

Is there a reason nsXPCException doesn't use NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0?

We may want to keep the GetMessageMoz binaryname, to avoid Windows troubles.

>+  readonly attribute nsISupports             location;
>+  readonly attribute nsISupports             inner;
>+  readonly attribute nsISupports             data;

All three should be nullable, no?

>@@ -190,10 +191,7 @@ XPCThrower::BuildAndThrowException(JSContext* cx,
>-            finalException = NS_NewDOMException(rv, defaultException);

Didn't that use to give the DOMException the right filename/lineno, etc?  How is that working now?

This patch seems to be assuming that Initialize() never returns error, nor do the various nsXPCException XPCOM methods.  I assume you've audited that?  Could we at least add asserts or something?

r=me with that.  Followup to remove the classinfo?

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 7 - Fixes to pass tests

The following fixes are required to pass tests:

1. Don't use nsContentUtils::GetCurrentJSContext, since nsContentUtils is not initialized in xpcshell but XPConnect still needs to throw exceptions there.
2. Avoid clobbering an existing exception in Throw to allow ancient components that throw custom exception strings (such as plugins and crypto) to continue to do so.
3. Use new binding unwrapping to make instanceof Components.Exception continue to work.
4. Use new binding unwrapping when looking to see if an XPCWrappedJS threw NS_NOINTERFACE so that we continue to properly swallow NS_NOINTERFACE exceptions.
5. Change a few tests to use SpecialPowers.wrap on exception objects now that exceptions have a canonical wrapper.
6. Explicitly clear mPendingException in ~CycleCollectedJSRuntime so that if a pending exception is cycle collected we clear it before we shut down the cycle collector (since exceptions are now cycle collected, not doing this crashes).

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 798943 [details] [diff] [review]
Part 5 - Refactor exceptions

Please post a patch with "hg mv" instead of delete and readd.  That would also incidentally be simpler to review....

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 798945 [details] [diff] [review]
Part 6 - Use thread-agnostic exception handling everywhere

This needs a way better checkin comment.

Assuming exceptions::CreateStack is threadsafe, r=me

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky [:bz] from comment #13)
> Comment on attachment 798943 [details] [diff] [review]
> Part 5 - Refactor exceptions
> 
> Please post a patch with "hg mv" instead of delete and readd.  That would
> also incidentally be simpler to review....

mmm, git doesn't really do that :-/  I'll see what I can do

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 799560 [details] [diff] [review]
Part 7 - Fixes to pass tests

>+    // We can't call nsContentUtils::ThreadsafeGetCurrentJSContext

Because it doesn't exist after this patch.  Did you mean nsContentUtils::GetCurrentJSContext?

>+        *bp = UNWRAP_OBJECT(Exception, cx, obj, e) ||
>+              JSValIsInterfaceOfType(cx, v, NS_GET_IID(nsIException));

UNWRAP_OBJECT returns nsresult.  So you want NS_SUCCEEDED() around it.

>+    e = SpecialPowers.wrap(e);

Maybe file a followup to have the specialpowers wrappers auto-do that when exceptions happen?

Can you fold the pieces of this into the relevant patches, please, insofar as that's possible?

r=me with that.

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky [:bz] from comment #16)
> Comment on attachment 799560 [details] [diff] [review]
> Part 7 - Fixes to pass tests
> 
> >+    // We can't call nsContentUtils::ThreadsafeGetCurrentJSContext
> 
> Because it doesn't exist after this patch.  Did you mean
> nsContentUtils::GetCurrentJSContext?

Yes.

> >+        *bp = UNWRAP_OBJECT(Exception, cx, obj, e) ||
> >+              JSValIsInterfaceOfType(cx, v, NS_GET_IID(nsIException));
> 
> UNWRAP_OBJECT returns nsresult.  So you want NS_SUCCEEDED() around it.

Yeah, try pointed that out to me too.

> >+    e = SpecialPowers.wrap(e);
> 
> Maybe file a followup to have the specialpowers wrappers auto-do that when
> exceptions happen?

Ok.

> Can you fold the pieces of this into the relevant patches, please, insofar
> as that's possible?

Yes, I will.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 5 - Refactor exceptions

With moves, but since I reformatted everything I moved out of XPConnect to gecko style I'm not sure it helps that much :-/

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky [:bz] from comment #11)
> Comment on attachment 798941 [details] [diff] [review]
> Part 4 - Convert exceptions to WebIDL
> 
> So the constructor expects static strings, right?  Please document that.

Done.

> >+nsDOMException::GetName(nsString& retval)
> >+  retval = NS_ConvertUTF8toUTF16(mName);
> 
>   NS_CopyUTF8toUTF16(mName, retval);
> 
> and similar lots of other places in this patch.

Fixed.  (didn't know that signature existed).

> Is there a reason nsXPCException doesn't use
> NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0?

It gets a jsval it has to trace later in the series.

> We may want to keep the GetMessageMoz binaryname, to avoid Windows troubles.

Changed.

> >+  readonly attribute nsISupports             location;
> >+  readonly attribute nsISupports             inner;
> >+  readonly attribute nsISupports             data;
> 
> All three should be nullable, no?

Indeed.  Fixed.

> >@@ -190,10 +191,7 @@ XPCThrower::BuildAndThrowException(JSContext* cx,
> >-            finalException = NS_NewDOMException(rv, defaultException);
> 
> Didn't that use to give the DOMException the right filename/lineno, etc? 
> How is that working now?

Inheritance.  The Exception ctor grabs the necessary stuff.

> This patch seems to be assuming that Initialize() never returns error, nor
> do the various nsXPCException XPCOM methods.  I assume you've audited that? 
> Could we at least add asserts or something?

Added.

> r=me with that.  Followup to remove the classinfo?

Will do.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky [:bz] from comment #14)
> Comment on attachment 798945 [details] [diff] [review]
> Part 6 - Use thread-agnostic exception handling everywhere
> 
> This needs a way better checkin comment.

So do all the other patches ;-)

> Assuming exceptions::CreateStack is threadsafe, r=me

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #19)
> > >+nsDOMException::GetName(nsString& retval)
> > >+  retval = NS_ConvertUTF8toUTF16(mName);
> > 
> >   NS_CopyUTF8toUTF16(mName, retval);
> > 
> > and similar lots of other places in this patch.
> 
> Fixed.  (didn't know that signature existed).

Ah, it doesn't.  But there is a CopyUTF8toUTF16 that does what I want.

Comment 22

[Boris Zbarsky [:bzbarsky]]

> but since I reformatted everything I moved out of XPConnect to gecko style

diff -w?  ;)

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 5: diff -w

Doesn't shrink it much :-/

Comment 24

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Part 5

This won't compile but I think it's easier to review.

Comment 25

[Eric Shepherd [:sheppy]]

This will affect documentation.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 800244 [details] [diff] [review]
Part 5 - Refactor exceptions

>+++ b/dom/base/DOMException.h
>+#pragma GCC diagnostic push
>+#pragma GCC diagnostic ignored "-Woverloaded-virtual"

Please add a comment explaining why.

>+#define MOZILLA_DOM_EXCEPTION_IID \

Should that perhaps be called MOZILLA_EXCEPTION_IID?

>+++ b/dom/bindings/Exceptions.cpp
>+ThrowExceptionObject(JSContext* aCx, nsIException* aException)

>+  JS::RootedObject glob(aCx, JS::CurrentGlobalOrNull(aCx));

JS::Rooted<JSObject*>, please; I think that's current bindings style.

>+  nsresult rv = nsContentUtils::XPConnect()->WrapNative(aCx, glob, aException,

How about:
  
  JS::Rooted<JS::Value> val(aCx);
  if (!WrapObject(aCx, glob, aException, NS_GET_IID(nsIException), &val)) {
    return false;
  }

and skipping the explicit xpconnect bits?

>+ThrowExceptionObject(JSContext* aCx, Exception* aException)
>+  JS::RootedValue thrown(aCx);

JS::Rooted<JS::Value>.

>+  JS::RootedObject glob(aCx, JS::CurrentGlobalOrNull(aCx));

JS::Rooted<JSObject*>.

>+  JS::RootedValue val(aCx);

Why not keep using |thrown|?

>+  if (!mozilla::dom::WrapNewBindingObject(aCx, glob, aException, &val)) {

You're already in namespace mozilla::dom.

>+Throw(JSContext* cx, nsresult rv, const char* sz)

Give that last argument a sane name, please.  

>+  if (finalException == nullptr) {

if (!finalException)

>+NS_IMETHODIMP JSStackFrame::ToString(char** _retval)
>+            sizeof(format) + 6 /* space for lineno */;

I know you just copied this, but mind doing +10 or something, since lineno is 32-bit?  Or better yet +3*sizeof(mLineNo) to be very safe even if mLineNo goes 64-bit?

>+    self = frame.forget();

self.swap(frame) might be clearer.

>+++ b/js/xpconnect/src/XPCConvert.cpp
>+    if (cx && jsExceptionPtr) {
>+    e.forget(exceptn);

These two changes should have been in part 4, right?

>+++ b/dom/base/DOMException.cpp
>+Exception::~Exception()
>+    NS_DROP_JS_OBJECTS(this, Exception);

Will need merging.

>+Exception::StealJSVal(JS::Value* aVp)
>+    mThrownJSVal = JSVAL_NULL;

  mThrownJSVal.setNull();

Followup to make this take a MutableHandle, perhaps?

>+    NS_DROP_JS_OBJECTS(this, Exception);

Will need merging.

>+Exception::StowJSVal(JS::Value& aVp)

And maybe followup to take a Handle here?

>+    NS_HOLD_JS_OBJECTS(this, Exception);

Will need merging.

>+++ b/js/xpconnect/src/XPCQuickStubs.cpp
>-    XPCThrower::BuildAndThrowException(cx, rv, sz);
>+    dom::Throw(cx, rv, sz);

The old thing wouldn't clobber an existing exception but it looks like the new one does.  Why the change?  There are several places where this comment applies, both in this file and in XPCThrower.

>+++ b/xpcom/base/CycleCollectedJSRuntime.h
>+  already_AddRefed<nsIException> GetPendingException() const;

Why not return nsIException*?  Worth documenting if there is a reason.

>+  static CycleCollectedJSRuntime*
>+  Get();

Put that on one line?

r=me with those nits picked

Comment 27

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Please double-check the bug numbers in your commit messages.

Comment 28

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yeah that's fixed locally.

Comment 29

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Boris Zbarsky [:bz] from comment #26)
> Comment on attachment 800244 [details] [diff] [review]
> Part 5 - Refactor exceptions
> 
> >+++ b/dom/base/DOMException.h
> >+#pragma GCC diagnostic push
> >+#pragma GCC diagnostic ignored "-Woverloaded-virtual"
> 
> Please add a comment explaining why.

Done.

> >+#define MOZILLA_DOM_EXCEPTION_IID \
> 
> Should that perhaps be called MOZILLA_EXCEPTION_IID?

Yeah, I suppose so, since there's a DOMException too.

> >+++ b/dom/bindings/Exceptions.cpp
> >+ThrowExceptionObject(JSContext* aCx, nsIException* aException)
> 
> >+  JS::RootedObject glob(aCx, JS::CurrentGlobalOrNull(aCx));
> 
> JS::Rooted<JSObject*>, please; I think that's current bindings style.

Changed.

> >+  nsresult rv = nsContentUtils::XPConnect()->WrapNative(aCx, glob, aException,
> 
> How about:
>   
>   JS::Rooted<JS::Value> val(aCx);
>   if (!WrapObject(aCx, glob, aException, NS_GET_IID(nsIException), &val)) {
>     return false;
>   }
> 
> and skipping the explicit xpconnect bits?

AFAICT WrapObject only accepts things inheriting from nsWrapperCache, and nsIException doesn't (and we might have an XPCWrappedJS here, so there's no guarantee that we really do inherit from nsWrapperCache).  If I missed something please file a followup and I'll fix it.

> >+ThrowExceptionObject(JSContext* aCx, Exception* aException)
> >+  JS::RootedValue thrown(aCx);
> 
> JS::Rooted<JS::Value>.

Changed.

> >+  JS::RootedObject glob(aCx, JS::CurrentGlobalOrNull(aCx));
> 
> JS::Rooted<JSObject*>.

Same.

> >+  JS::RootedValue val(aCx);
> 
> Why not keep using |thrown|?

No reason.  Fixed.

> >+  if (!mozilla::dom::WrapNewBindingObject(aCx, glob, aException, &val)) {
> 
> You're already in namespace mozilla::dom.

Indeed.

> >+Throw(JSContext* cx, nsresult rv, const char* sz)
> 
> Give that last argument a sane name, please. 

Fixed. 
> 
> >+  if (finalException == nullptr) {
> 
> if (!finalException)
> 
> >+NS_IMETHODIMP JSStackFrame::ToString(char** _retval)
> >+            sizeof(format) + 6 /* space for lineno */;
> 
> I know you just copied this, but mind doing +10 or something, since lineno
> is 32-bit?  Or better yet +3*sizeof(mLineNo) to be very safe even if mLineNo
> goes 64-bit?

Done.

> >+    self = frame.forget();
> 
> self.swap(frame) might be clearer.

Fixed.

> >+++ b/js/xpconnect/src/XPCConvert.cpp
> >+    if (cx && jsExceptionPtr) {
> >+    e.forget(exceptn);
> 
> These two changes should have been in part 4, right?

Not really.  I didn't remove these methods from the interface until this patch.  Although these could have been in part 4 if I had changed the concrete type of the pointer.

> >+++ b/dom/base/DOMException.cpp
> >+Exception::~Exception()
> >+    NS_DROP_JS_OBJECTS(this, Exception);
> 
> Will need merging.
> 
> >+Exception::StealJSVal(JS::Value* aVp)
> >+    mThrownJSVal = JSVAL_NULL;
> 
>   mThrownJSVal.setNull();
> 
> Followup to make this take a MutableHandle, perhaps?

Ok.

> >+    NS_DROP_JS_OBJECTS(this, Exception);
> 
> Will need merging.
> 
> >+Exception::StowJSVal(JS::Value& aVp)
> 
> And maybe followup to take a Handle here?
> 
> >+    NS_HOLD_JS_OBJECTS(this, Exception);
> 
> Will need merging.
> 
> >+++ b/js/xpconnect/src/XPCQuickStubs.cpp
> >-    XPCThrower::BuildAndThrowException(cx, rv, sz);
> >+    dom::Throw(cx, rv, sz);
> 
> The old thing wouldn't clobber an existing exception but it looks like the
> new one does.  Why the change?  There are several places where this comment
> applies, both in this file and in XPCThrower.

This is fixed in part 7 and folded back in appropriately.

> >+++ b/xpcom/base/CycleCollectedJSRuntime.h
> >+  already_AddRefed<nsIException> GetPendingException() const;
> 
> Why not return nsIException*?  Worth documenting if there is a reason.

All the callers wanted a strong reference.  *shrug*

> >+  static CycleCollectedJSRuntime*
> >+  Get();
> 
> Put that on one line?

Fixed.

> r=me with those nits picked

Thanks.

Comment 30

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a9cc2c01bd0
https://hg.mozilla.org/integration/mozilla-inbound/rev/c04e927b631b
https://hg.mozilla.org/integration/mozilla-inbound/rev/16cf9da0d242
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f4223fab461
https://hg.mozilla.org/integration/mozilla-inbound/rev/4127ebc547fd
https://hg.mozilla.org/integration/mozilla-inbound/rev/9846f07d5867

I guess I should probably add a test for TextEncoder throwing.

Comment 31

[Boris Zbarsky [:bzbarsky]]

> AFAICT WrapObject only accepts things inheriting from nsWrapperCache

That's incorrect.  I'll call into the normal xpconnect wrapping code via XPCOMObjectToJsval.  Make sure to call this signature:

1037 template<class T>
1038 inline bool
1039 WrapObject(JSContext* cx, JS::Handle<JSObject*> scope, T* p, const nsIID* iid,
1040            JS::MutableHandle<JS::Value> rval)

and note no wrapper cache requirement there.  I filed bug 914014 on this.

> Not really.  I didn't remove these methods from the interface until this patch. 

I'm not sure what you mean.  If I'm reading the code right, part 4 as-attached (part 3 as-landed) never sets *exceptn.  It then tests *exceptn which is either null (probably) or uninitialized (less likely).  If, during a bisect, you happen to build that changeset it won't behave right if this code is ever hit.  Not much we can do about it now that it's landed.  :(

Comment 32

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a9cc2c01bd0
https://hg.mozilla.org/mozilla-central/rev/c04e927b631b
https://hg.mozilla.org/mozilla-central/rev/16cf9da0d242
https://hg.mozilla.org/mozilla-central/rev/3f4223fab461
https://hg.mozilla.org/mozilla-central/rev/4127ebc547fd
https://hg.mozilla.org/mozilla-central/rev/9846f07d5867

Comment 33

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

KaiRo can you watch crashstats here and make sure this goes away?

Comment 34

[Robert Kaiser]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #33)
> KaiRo can you watch crashstats here and make sure this goes away?

The "Table" tab in https://crash-stats.mozilla.com/report/list?signature=nsXPCException%3A%3ANewException(char+const*%2C+tag_nsresult%2C+nsIStackFrame*%2C+nsISupports*%2C+nsIException**) says the last crashes with that were in the build from September 9, so the fix seems effective in stopping this signature.

Comment 35

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

\o/