Closed Bug 1563687 Opened 5 years ago Closed 5 years ago

Unify the various clean/destroy/disconnect methods

Categories

(Remote Protocol :: Agent, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(2 files)

One convention stands out of the current code: most of the cleanup/destruction code operates from a method called destructor, which cleanly matches constructor method used by ES6 classes.

At least one component, ContentProcessSession names it destroy. We should rename it.

Then, we use the word disconnect for something that looks more like a destruction than a disconnection. In the case of Targets, the usage of connect and disconnect is misleading as when these methods are called (connect/disconnect), there is no connection being made. Instead it is about a target being just created and to be registered, and a target being destroyed and to be unregistered and forget about. That looks more like a contructor/destructor pattern.

Assignee: nobody → poirot.alex
Blocks: 1544458

Connect and disconnect in misleading here as a target aren't connected/disconnected,
but instead, being unregistered and destroyed. The fact that they are "disconnected"
is a side effect of this destruction. Also note that a Target is never "connected",
it is only a Connection and its related sessions which really are connected to a remote client.

Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fc54f88b32e0 Renamed ContentProcessSession.destroy to destructor in order to match this project's conventions. r=remote-protocol-reviewers,jdescottes
Keywords: leave-open
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f5a12c2479a1 Rename Targets.disconnect to destructor to better match project's conventions. r=remote-protocol-reviewers,jdescottes,ato

Alexandre, is something left here for landing or can the bug been closed now?

Flags: needinfo?(poirot.alex)

It can be closed, I forgot to remove the leave-open when landing the second last part.

Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
Flags: needinfo?(poirot.alex)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: