1276400 - Eliminate Gecko consumers of the dontReportUncaught JSContext option

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

This option is only checked in one place: ~AutoLastFrameCheck().  This also checks autoJSAPIOwnsErrorReporting.  Since AutoJSAPI/AutoEntryScript always set this latter option, there is no point setting dontReportUncaught anywhere we're using AutoJSAPI/AutoEntryScript... which should be everywhere.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Get rid of AutoDontReportUncaught and its one consumer

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Stop setting the dontReportUncaught option on the worker JSContext, since workers should be using AutoJSAPI/AutoEntryScript for everything

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Stop setting the dontReportUncaught option in IPC code, since we're working with an AutoJSAPI there

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Stop setting dontReportUncaught in XPConnect code, since all consumers got their JSContext from an AutoEntryScript or AutoJSAPI

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8757556 [details] [diff] [review]
part 3.  Stop setting the dontReportUncaught option in IPC code, since we're working with an AutoJSAPI there

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

::: js/ipc/WrapperAnswer.cpp
@@ -410,5 @@
>  
>      RootedValue rval(cx);
>      {
> -        AutoSaveContextOptions asco(cx);
> -        ContextOptionsRef(cx).setDontReportUncaught(true);

Should this assert that its an AutoJSAPI like we did in Promise.cpp?  Or is there a test for this? I know that the context is defined in the same method here, but someone might not realize that we're depending on the AutoJSAPI to avoid error reporting.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> but someone might not realize that we're depending on the AutoJSAPI to avoid error reporting.

Really, we're working on removing this "owns error reporting" flag, because it's about to become the only possible behavior.  I can add an assert, but it wouldn't hang out for very long...

Comment 7

[Boris Zbarsky [:bzbarsky]]

I did add the assert.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ae227cd2b5d
part 1.  Get rid of AutoDontReportUncaught and its one consumer.  r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c74e35ddd1e
part 2.  Stop setting the dontReportUncaught option on the worker JSContext, since workers should be using AutoJSAPI/AutoEntryScript for everything.  r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/2abd1b50f673
part 3.  Stop setting the dontReportUncaught option in IPC code, since we're working with an AutoJSAPI there.  r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d46836853cd
part 4.  Stop setting dontReportUncaught in XPConnect code, since all consumers got their JSContext from an AutoEntryScript or AutoJSAPI.  r=bkelly

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5ae227cd2b5d
https://hg.mozilla.org/mozilla-central/rev/2c74e35ddd1e
https://hg.mozilla.org/mozilla-central/rev/2abd1b50f673
https://hg.mozilla.org/mozilla-central/rev/7d46836853cd