Closed Bug 1174486 Opened 9 years ago Closed 9 years ago

Stop using JS_ReportPendingException in the javascript: protocol handler and nsJSUtils

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla41

Tracking Flags:

Tracking

Status

firefox41

---

fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(4 files, 3 obsolete files)

Stop using JS_ReportPendingException in the javascript: protocol handler. Just let AutoJSAPI handle things for us 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch		Details \| Diff \| Splinter Review
part 1. Add a way to explicitly report a pending exception on an AutoJSAPI that has taken ownership of error reporting 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch	bholley : review+	Details \| Diff \| Splinter Review
part 2. Eliminate the unused codepath on which EvaluateOptions::reportUncaught is false 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch	bholley : review+	Details \| Diff \| Splinter Review
part 3. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch		Details \| Diff \| Splinter Review
part 3. Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch		Details \| Diff \| Splinter Review
part 4. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch	bholley : review+	Details \| Diff \| Splinter Review
part 3. Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting 9 years ago Boris Zbarsky [:bzbarsky] (deleted), patch	bholley : review+	Details \| Diff \| Splinter Review

Boris Zbarsky [:bzbarsky]

Assignee

Description

•

9 years ago

No description provided.

Boris Zbarsky [:bzbarsky]

Assignee

Comment 1

•

9 years ago

Attached patch Stop using JS_ReportPendingException in the javascript: protocol handler. Just let AutoJSAPI handle things for us (obsolete) (deleted) — Details — Splinter Review

Attachment #8622027 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Assignee: nobody → bzbarsky

Status: NEW → ASSIGNED

Boris Zbarsky [:bzbarsky]

Assignee

Comment 2

•

9 years ago

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

Summary: Stop using JS_ReportPendingException in the javascript: protocol handler → Stop using JS_ReportPendingException in the javascript: protocol handler and nsJSUtils

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Attachment #8622027 - Attachment is obsolete: true

Attachment #8622027 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 3

•

9 years ago

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

Boris Zbarsky [:bzbarsky]

Assignee

Comment 4

•

9 years ago

Attached patch part 1. Add a way to explicitly report a pending exception on an AutoJSAPI that has taken ownership of error reporting (deleted) — Details — Splinter Review

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

Attachment #8622032 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 5

•

9 years ago

Attached patch part 2. Eliminate the unused codepath on which EvaluateOptions::reportUncaught is false (deleted) — Details — Splinter Review

Attachment #8622033 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 6

•

9 years ago

Attached patch part 3. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead (obsolete) (deleted) — Details — Splinter Review

Attachment #8622034 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 7

•

9 years ago

Attached patch part 3. Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting (obsolete) (deleted) — Details — Splinter Review

Attachment #8622181 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 8

•

9 years ago

Attached patch part 4. Stop reporting exceptions in nsJSUtils::EvaluateString and have its consumers use AutoJSAPIs that take ownership of error reporting instead (deleted) — Details — Splinter Review

Attachment #8622182 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Attachment #8622034 - Attachment is obsolete: true

Attachment #8622034 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Comment 9

•

9 years ago

Attached patch part 3. Make sure that warnings do still get reported even if our AutoJSAPI has taken over error reporting (deleted) — Details — Splinter Review

Attachment #8622183 - Flags: review?(bobbyholley)

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Attachment #8622181 - Attachment is obsolete: true

Attachment #8622181 - Flags: review?(bobbyholley)

Bobby Holley (:bholley)

Updated

•

9 years ago

Attachment #8622032 - Flags: review?(bobbyholley) → review+

Bobby Holley (:bholley)

Updated

•

9 years ago

Attachment #8622033 - Flags: review?(bobbyholley) → review+

Bobby Holley (:bholley)

Comment 10

•

9 years ago

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.

Attachment #8622182 - Flags: review?(bobbyholley) → review+

Bobby Holley (:bholley)

Comment 11

•

9 years ago

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!

Attachment #8622183 - Flags: review?(bobbyholley) → review+

Pulsebot

Comment 12

•

9 years ago

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

Carsten Book [:Tomcat]

Comment 13

•

9 years ago

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

Status: ASSIGNED → RESOLVED

Closed: 9 years ago

status-firefox41: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla41

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Depends on: 1181619

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Blocks: 1070049

Boris Zbarsky [:bzbarsky]

Assignee

Updated

•

9 years ago

Blocks: 1099425

Nobody; OK to take it and work on it

Updated

•

6 years ago

Component: DOM → DOM: Core & HTML

You need to log in before you can comment on or make changes to this bug.