Closed Bug 1255817 Opened 8 years ago Closed 8 years ago

Make AutoJSAPI always take ownership of error reporting

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(8 files, 1 obsolete file)

13.75 KB, text/plain
Details
31.66 KB, patch
bholley
: review+
Details | Diff | Splinter Review
1.35 KB, patch
bholley
: review+
Details | Diff | Splinter Review
2.63 KB, patch
bholley
: review+
Details | Diff | Splinter Review
4.82 KB, patch
bholley
: review+
Details | Diff | Splinter Review
2.51 KB, patch
bholley
: review+
Details | Diff | Splinter Review
6.23 KB, patch
bholley
: review+
Details | Diff | Splinter Review
7.72 KB, text/plain
Details
I audited all our callsites and will post the results in a bit once I file the bugs tracking things that need fixing.  But once those things are fixed, we can do this.
Depends on: 1255818
Depends on: 1255830
Depends on: 1255832
Depends on: 1255840
Depends on: 1255849
Note that line numbers might be slightly off because of other code changes, of course.
Note that things that are particularly interesting in the audit are marked with "XXX" comments in that attachment.
Depends on: 1255867
Blocks: 1255874
Attachment #8729785 - Attachment is obsolete: true
Attachment #8729785 - Flags: review?(bobbyholley)
Attachment #8729809 - Flags: review?(bobbyholley) → review+
Attachment #8729786 - Flags: review?(bobbyholley) → review+
Attachment #8729788 - Flags: review?(bobbyholley) → review+
Attachment #8729789 - Flags: review?(bobbyholley) → review+
Comment on attachment 8729790 [details] [diff] [review]
part 5.  Remove the now-unused xpc::SystemErrorReporter

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

\o/ \o/ \o/
Attachment #8729790 - Flags: review?(bobbyholley) → review+
Attachment #8729791 - Flags: review?(bobbyholley) → review+
Depends on: 1256424
Depends on: 1257306
So I realized there are three more sources of AutoJSAPI that I had not audited yet: AutoSafeJSContext, AutoJSContext, ThreadSafeAutoJSContext (these last two only on mainthread when there is nothing on the JSContext stack).  I think my plan is as follows:

1)  Audit AutoSafeJSContext uses.  I will attach that audit here.
2)  Get rid of ThreadSafeAutoJSContext in favor of passing JSContext* directly.
3)  Assume that any AutoJSContext consumer who is running when the JSContext stack is empty really does want
    to report the error, since there is nowhere else for it to go.
Depends on: 1257335
Depends on: 1257422
Depends on: 1257568
Depends on: 1257725
Blocks: 1277278
Blocks: 1493811
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.