Closed Bug 1564168 Opened 5 years ago Closed 5 years ago

[jsdbg2] Debugger.prototype.enabled should be removed

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla70

Tracking Flags:

Tracking

Status

firefox70

---

fixed

People

(Reporter: jimb, Assigned: jimb)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [debugger-mvp])

Attachments

(2 files, 2 obsolete files)

Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb 5 years ago Jason Laster [:jlast] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed (part 1). r=jimb 5 years ago Jason Laster [:jlast] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb 5 years ago Jason Laster [:jlast] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1564168 - [jsdbg2] Debugger.prototype.enabled should be removed. r=jimb 5 years ago Jason Laster [:jlast] 47 bytes, text/x-phabricator-request		Details \| Review

Jim Blandy :jimb

Assignee

Description

•

5 years ago

In the Debugger API, every Debugger object has an enabled property, to which one can assign true or false to enable or disable the debugger. The idea was that, to make closing the debugger panel more robust, Debugger should have a single cut-off point guaranteed to prevent it from interfering with further execution or consuming any resources.

However, this turned out to be more complicated than expected. Two examples:

Even if a Debugger has hooks set that would prevent it from being GC'd (because the hooks may cause observable side effects), if the Debugger is disabled and not reachable from JavaScript, then it's okay to GC it anyway, since none of those hooks will ever fire. The GC logic actually has a special case for this; maybe it's correct? But who cares?
When there are multiple Debuggers observing a realm, our hooks often construct a list of Debuggers to notify about some interesting event, and then iterate over that list. But Debuggers earlier in the list can disable those later in the list. We've forgotten to handle this case several times.

But in the end, the enabled property isn't really necessary: just calling removeAllDebuggees, or some custom method that does that along with some other debuggee-independent tear down, should be just as useful, and uses only functionality that we have to implement anyway for other reasons (like removing debuggees). The only advantage of the enabled property is that, if you set it back to true, you get back the original set of debuggees; that's kind of nice, but not important enough to justify the complexity it ends up causing.