1174486 - Stop using JS_ReportPendingException in the javascript: protocol handler and nsJSUtils

Status: RESOLVED FIXED

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Stop using JS_ReportPendingException in the javascript: protocol handler. Just let AutoJSAPI handle things for us

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, this is riduculous.  Resummarizing to the whole ball of string involved in fixing this.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Btw, the try run from that first patch shows it failing in the expected way: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4882c4d823e3

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add a way to explicitly report a pending exception on an AutoJSAPI that has taken ownership of error reporting

This is needed for some cases that currently report an exception and then do
some other work on the same cx (specifically, nsXBLProtoImplField::InstallField).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Eliminate the unused codepath on which EvaluateOptions::reportUncaught is false

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting

Comment 10

[Bobby Holley (:bholley)]

Comment on attachment 8622182 [details] [diff] [review]
part 4.  Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead

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

::: dom/base/nsJSUtils.cpp
@@ -98,5 @@
>    return innerWindowID;
>  }
>  
> -void
> -nsJSUtils::ReportPendingException(JSContext *aContext)

\o/ So glad to see this gone.

Comment 11

[Bobby Holley (:bholley)]

Comment on attachment 8622183 [details] [diff] [review]
part 3.  Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting

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

Nit - Make this part 0?

Thanks for working on this stuff!

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c8ef16615ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c609ab7716c
https://hg.mozilla.org/integration/mozilla-inbound/rev/77406f36aa86
https://hg.mozilla.org/integration/mozilla-inbound/rev/026ce2bd75de

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5c8ef16615ca
https://hg.mozilla.org/mozilla-central/rev/4c609ab7716c
https://hg.mozilla.org/mozilla-central/rev/77406f36aa86
https://hg.mozilla.org/mozilla-central/rev/026ce2bd75de