Closed Bug 977287 Opened 6 years ago Closed 6 years ago
Rename the operation callback
"operation callback" is meaningless. It should be called the interrupt callback.
Riskiest thing going on here is moving js_HandleExecutionInterrupt into a namespace and removing the `extern` keyword from its declaration.
Assignee: general → jorendorff
Attachment #8383355 - Flags: review?(luke)
Not-quite-as-trivial odds and ends found during renaming. For example, it looks like JSShellContextData::startTime isn't used from the interrupt callback anymore, for years now.
Attachment #8383357 - Flags: review?(luke)
I'd love to delete the following paragraph from jsapi.h. It basically says, "Note: we might call this callback randomly. No reason. We just do sometimes." Maybe I'm the only person bothered by this. > /* > * These functions allow setting an interrupt callback that will be called > * from the JS thread some time after any thread triggered the callback using > * JS_RequestInterruptCallback(rt). > * >- * To schedule the GC and for other activities the engine internally triggers >- * interrupt callbacks. The embedding should thus not rely on callbacks being >- * triggered through the external API only. >- * > * Important note: Additional callbacks can occur inside the callback handler > * if it re-enters the JS engine. The embedding must ensure that the callback > * is disconnected before attempting such re-entry. > */ It would be an easy fix, since all the interrupt state is already protected by a lock. We'd still need a comment to the effect that interrupts can be coalesced; you might request 2 and only get 1.
Comment on attachment 8383355 [details] [diff] [review] bug-977287-part-1-renaming-v1.patch Lovely
Attachment #8383355 - Flags: review?(luke) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.