Bug 1563690 - Make Target class save a list of Connection rather than Session. r=remote-protocol-reviewers,jdescottes

Connection already saves the list of Session, so it is more natural
to save it only once there and instead directly close the connections
from Target. Each connection is going to cleanup all related sessions.

I also stop automatically registering the session to the connection from Session constructor,
it felt not explicit enough.

Differential Revision: https://phabricator.services.mozilla.com/D37045

--HG--
extra : moz-landing-system : lando

remote/Connection.jsm

@@ -244,9 +244,13 @@ class Connection {
     this.onPacket(packet);
   }
 
+  /**
+   * Instruct the connection to close.
+   * This will ask the transport to shutdown the WebSocket connection
+   * and destroy all active sessions.
+   */
   close() {
     this.transport.close();
-    this.sessions.clear();
 
     // In addition to the WebSocket transport, we also have to close the Connection
     // used internaly within httpd.js. Otherwise the server doesn't shut down correctly
@@ -254,7 +258,16 @@ class Connection {
     this.httpdConnection.close();
   }
 
-  onClosed(status) {}
+  /**
+   * This is called by the `transport` when the connection is closed.
+   * Cleanup all the registered sessions.
+   */
+  onClosed(status) {
+    for (const session of this.sessions.values()) {
+      session.destructor();
+    }
+    this.sessions.clear();
+  }
 
   /**
    * Splits a method, e.g. "Browser.getVersion",

remote/domains/parent/Target.jsm

@@ -105,20 +105,21 @@ class Target extends Domain {
       return new Error(`Unable to find target with id '${targetId}'`);
     }
 
-    const session = new TabSession(
+    const tabSession = new TabSession(
       this.session.connection,
       target,
       sessionIds++
     );
+    this.session.connection.registerSession(tabSession);
     this.emit("Target.attachedToTarget", {
       targetInfo: {
         type: "page",
       },
-      sessionId: session.id,
+      sessionId: tabSession.id,
     });
 
     return {
-      sessionId: session.id,
+      sessionId: tabSession.id,
     };
   }
 

remote/sessions/Session.jsm

@@ -43,11 +43,6 @@ class Session {
     this.target = target;
     this.id = id;
 
-    this.destructor = this.destructor.bind(this);
-
-    this.connection.registerSession(this);
-    this.connection.transport.on("close", this.destructor);
-
     this.domains = new Domains(this, ParentProcessDomains);
   }
 

remote/targets/Target.jsm

@@ -25,16 +25,25 @@ class Target {
    * @param Class sessionClass
    */
   constructor(targets, sessionClass) {
+    // Save a reference to Targets instance in order to expose it to:
+    // domains/parent/Target.jsm
     this.targets = targets;
+
+    // When a new connection is made to this target,
+    // we will instantiate a new session based on this given class.
+    // The session class is specific to each target's kind and is passed
+    // by the inheriting class.
     this.sessionClass = sessionClass;
-    this.sessions = new Map();
+
+    // There can be more than one connection if multiple clients connect to the remote agent.
+    this.connections = new Set();
   }
 
   /**
    * Close all active connections made to this target.
    */
   destructor() {
-    for (const [conn] of this.sessions) {
+    for (const conn of this.connections) {
       conn.close();
     }
   }
@@ -45,7 +54,9 @@ class Target {
     const so = await WebSocketHandshake.upgrade(request, response);
     const transport = new WebSocketTransport(so);
     const conn = new Connection(transport, response._connection);
-    this.sessions.set(conn, new this.sessionClass(conn, this));
+    const session = new this.sessionClass(conn, this);
+    conn.registerSession(session);
+    this.connections.add(conn);
   }
 
   // XPCOM