Closed Bug 660441 Opened 8 years ago Closed 8 years ago

removal of cx parameter from IsAboutToBeFinalized

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: igor, Assigned: igor)

Details

Attachments

(1 file, 2 obsolete files)

IsAboutToBeFinalized takes cx to access the runtime and get the associated gcCurrentCompartment. Howerever, the runtime can be accessed from the cpmpartment for the thing making the cx parameter unnecessary.
Do you also want to change the JS_IsAboutToBeFinalized function? Whats the procedure to change an API function?
Attached patch v1 (obsolete) — Splinter Review
Here is the patch that removes the unneeded cx parameter.
Assignee: general → igor
Attachment #535871 - Flags: review?(anygregor)
(In reply to comment #1)
> Do you also want to change the JS_IsAboutToBeFinalized function?

Thanks for remainding about it! I will update the patch to always call IsAboutToBeFinalized from xpconnect. After that we would not have JS_IsAboutToBeFinalized calls in the browser and we can wait with any changes to JS_IsAboutToBeFinalized until we have a generational GC or something like that that disrupts the GC logic significantly. 

> Whats the
> procedure to change an API function?

Some discussion on internal and public mail lists is in due.
Attachment #535871 - Attachment is obsolete: true
Attachment #535871 - Flags: review?(anygregor)
Attached patch v2 (obsolete) — Splinter Review
The patch removes the cx from all internal and public API - per resent discussion on the mailing list this is OK and desirable.
Attachment #595170 - Flags: review?(anygregor)
Attached patch v3Splinter Review
Here is the right patch!
Attachment #595170 - Attachment is obsolete: true
Attachment #595170 - Flags: review?(anygregor)
Attachment #595186 - Flags: review?(anygregor)
Comment on attachment 595186 [details] [diff] [review]
v3


>-{
>-    JS_ASSERT(thing);
>-    JS_ASSERT(!cx->runtime->gcIncrementalTracer);
>-    return IsAboutToBeFinalized(cx, (gc::Cell *)thing);
>+JS_IsAboutToBeFinalized(void *thing)
>+{
>+    gc::Cell * t = static_cast<gc::Cell *>(thing);

Space around *?

nice!
Attachment #595186 - Flags: review?(anygregor) → review+
https://hg.mozilla.org/mozilla-central/rev/d6f1d1fbe13a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.