Closed Bug 1486125 Opened 1 year ago Closed 1 year ago

JSContext::noGCOrAllocationCheck and its helper methods are unused

Categories

(Core :: JavaScript: GC, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: anba, Assigned: dpino, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

There doesn't seem to be a single caller to JSContext::allowAlloc() and JSContext::disallowAlloc(), so it may make sense to simply remove JSContext::noGCOrAllocationCheck and its helper methods.
Mentor: jcoppeard
Keywords: good-first-bug
Priority: -- → P3
Here's a patch that removes JSContext::noGCOrAllocationCheck and its helper methods.
Attachment #9005885 - Flags: review?(jcoppeard)
Comment on attachment 9005885 [details] [diff] [review]
Bug-1486125-Remove-JSContext-noGCOrAllocationCheck-a.patch

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

You also need to remove the initialisation of noGCOrAllocationCheck in the JSContext constructor in JSContext.cpp.

r=me with that change.
Attachment #9005885 - Flags: review?(jcoppeard) → review+
I amended the patch to include the necessary change in JSContext.cpp. I also modified the patch message to include r=jonco. I don't have commit access, could you help me run the try-server?
Attachment #9005885 - Attachment is obsolete: true
Attachment #9005980 - Flags: review+
It seems the try-server was OK. I would need to add the keyword "checkin-needed" to land it, but I think I cannot do that unless I have permissions to edit the bug.
Done.  Thanks for the patch!
Keywords: checkin-needed
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e99f6ed6c2f
Remove JSContext::noGCOrAllocationCheck and its helper methods. r=jonco
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5e99f6ed6c2f
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Target Milestone: mozilla64 → mozilla63
Target Milestone: mozilla63 → mozilla64
Assignee: nobody → dpino
You need to log in before you can comment on or make changes to this bug.