Fix ghost iceConnectionState changes when remote client has left.

Replace event system with TargetObserver as proposed in hotwired/stimulus#409 and teardown clients when the remote video is removed

Author: domchristie

README.md

@@ -4,7 +4,7 @@ An basic video chat app using the [WebRTC Perfect Negotiation pattern](https://w
 ## How does it work?
 The Stimulus room controller handles Enter button click. It gets the user's local audio and video and feeds them into the local video element. It also starts Action Cable subscriptions specific to the current room: one for communicating WebRTC messages: the `Signaller`; and one for clients to ping others: the `RoomSubscription`. Once connected, the room channel broadcasts the presence of this new client. Each connected client then "greets" this newcomer by calling the `greet` action on the room channel and specifying `to` and `from` fields.
 
-The `greet` action broadcasts specifically to the newcomer, a Turbo Stream update which appends a media element representing the remote client. It's important we do this first so that it's ready for incoming streams. The media controller broadcasts it's connected (using a snippet taken from the hey.com source code, which will be simplified once [we can listen for adding/removing Stimulus targets](https://github.com/hotwired/stimulus/pull/367)). This begins the WebRTC negotiation to form a connection between the two clients.
+The `greet` action broadcasts specifically to the newcomer, a Turbo Stream update which appends a media element representing the remote client. It's important we do this first so that it's ready for incoming streams. The room controller detects that a remote video has been added (by patching Stimulus with code from: https://github.com/hotwired/stimulus/pull/409). This begins the WebRTC negotiation to form a connection between the two clients.
 
 The WebRTC negotiation is quite complex, and even though things are required to happen in a particular order, responses are triggered asynchronously, making it tricky to get right. This is where the [WebRTC Perfect Negotiation pattern](https://w3c.github.io/webrtc-pc/#perfect-negotiation-example) comes in. We won't go into it too much here, as it's covered well elsewhere; but for the purpose of this description, (_deep breath_), Session Description Protocol (SDP) descriptions and Interactive Connectivity Establishment (ICE) candidates are exchanged over the `Signaller`. The negotiation is "perfect" as it ensures that the `RTCPeerConnection` is in the correct state for setting descriptions, and avoids collision errors by having one client be "polite" and other "impolite"—the polite one backs down in the case of a collision. Anyway, the code for this has been mostly lifted from the spec and handled by the `Signaller`/`WebrtcNegotiation`. When creating the negotition, the room controller listens for `track` events on the `RTCPeerConnection`, so it can start streaming from the remote client.
 

app/assets/javascripts/controllers/medium_controller.js

@@ -3,32 +3,15 @@ import { Controller } from 'stimulus'
 export default class MediumController extends Controller {
   connect () {
     this.reRenderMediaElement()
-    this.dispatch('connect', {
-      detail: { clientId: this.clientIdValue }
-    })
   }
 
-
   // Fix potentially blank videos due to autoplay rules?
   reRenderMediaElement () {
     const mediaElement = this.mediaElementTarget
     const clone = mediaElement.cloneNode(true)
     mediaElement.parentNode.insertBefore(clone, mediaElement)
     mediaElement.remove()
   }
-
-  disconnect () {
-    this.dispatch('disconnect', {
-      detail: { clientId: this.clientIdValue }
-    })
-  }
-
-  dispatch (eventName, { target = this.element, detail = {}, bubbles = true, cancelable = true } = {}) {
-    const type = `${this.identifier}:${eventName}`
-    const event = new CustomEvent(type, { detail, bubbles, cancelable })
-    target.dispatchEvent(event)
-    return event
-  }
 }
 
 MediumController.values = { clientId: String }

app/assets/javascripts/controllers/room_controller.js

@@ -1,11 +1,17 @@
 import { Controller } from 'stimulus'
+import TargetObserver from 'target_observer'
 import Client from 'client'
 import WebrtcNegotiation from 'webrtc_negotiation'
 import RoomSubscription from 'room_subscription'
 import Signaller from 'webrtc_session_subscription'
 
 export default class RoomController extends Controller {
+  initialize () {
+    this.targetObserver = new TargetObserver(this.context, this.context)
+  }
   connect() {
+    this.targetObserver.start()
+
     this.clients = {}
     this.client = new Client(this.clientIdValue)
 
@@ -26,6 +32,10 @@ export default class RoomController extends Controller {
     })
   }
 
+  disconnect () {
+    this.targetObserver.stop()
+  }
+
   async enter () {
     try {
       const constraints = { audio: true, video: true }
@@ -46,7 +56,17 @@ export default class RoomController extends Controller {
     this.subscription.greet({ to: otherClient.id, from: this.client.id })
   }
 
-  negotiateConnection ({ detail: { clientId } }) {
+  remoteMediumTargetConnected (element) {
+    const clientId = element.id.replace('medium_', '')
+    this.negotiateConnection(clientId)
+  }
+
+  remoteMediumTargetDisconnected (element) {
+    const clientId = element.id.replace('medium_', '')
+    this.teardownClient(clientId)
+  }
+
+  negotiateConnection (clientId) {
     const otherClient = this.findOrCreateClient(clientId)
 
     // Be polite to newcomers!
@@ -63,6 +83,11 @@ export default class RoomController extends Controller {
     }
   }
 
+  teardownClient (clientId) {
+    this.clients[clientId].stop()
+    delete this.clients[clientId]
+  }
+
   createNegotiation ({ otherClient, polite }) {
     const negotiation = new WebrtcNegotiation({
       signaller: this.signaller,
@@ -89,13 +114,6 @@ export default class RoomController extends Controller {
     }
   }
 
-  removeClient ({ from }) {
-    if (this.clients[from]) {
-      this.clients[from].stop()
-      delete this.clients[from]
-    }
-  }
-
   findOrCreateClient (id) {
     return this.clients[id] || (this.clients[id] = new Client(id))
   }

app/assets/javascripts/importmap.json.erb

@@ -1,11 +1,14 @@
 {
   "imports": {
     "turbo": "<%= asset_path "turbo" %>",
+    "@stimulus/multimap": "https://cdn.skypack.dev/@stimulus/multimap",
+    "@stimulus/mutation-observers": "https://cdn.skypack.dev/@stimulus/mutation-observers",
     <%= importmap_list_with_stimulus_from(
       "app/assets/javascripts/models",
       "app/assets/javascripts/controllers",
       "app/assets/javascripts/subscriptions",
-      "app/assets/javascripts/libraries"
+      "app/assets/javascripts/libraries",
+      "app/assets/javascripts/patches"
     ) %>
   }
 }

app/assets/javascripts/models/webrtc_negotiation.js

@@ -52,6 +52,7 @@ export default class WebrtcNegotiation {
         this.initiateManualRollback()
         this.retryCount++
       } else {
+        this.stop()
         console.error(`Negotiation failed after ${this.retryCount} retries`)
       }
     }

app/assets/javascripts/patches/target_observer.js

@@ -0,0 +1,96 @@
+import { Context } from "stimulus"
+import { Multimap } from "@stimulus/multimap"
+import { TokenListObserver } from "@stimulus/mutation-observers"
+
+export default class TargetObserver {
+  constructor(context, delegate) {
+    this.context = context
+    this.delegate = delegate
+    this.targetsByName = new Multimap
+  }
+
+  start() {
+    if (!this.tokenListObserver) {
+      this.tokenListObserver = new TokenListObserver(this.element, this.attributeName, this)
+      this.tokenListObserver.start()
+    }
+  }
+
+  stop() {
+    if (this.tokenListObserver) {
+      this.disconnectAllTargets()
+      this.tokenListObserver.stop()
+      delete this.tokenListObserver
+    }
+  }
+
+  // Token list observer delegate
+
+  tokenMatched({ element, content: name }) {
+    if (this.scope.containsElement(element)) {
+      this.connectTarget(element, name)
+    }
+  }
+
+  tokenUnmatched({ element, content: name }) {
+    this.disconnectTarget(element, name)
+  }
+
+  // Target management
+
+  connectTarget(element, name) {
+    if (!this.targetsByName.has(name, element)) {
+      this.targetsByName.add(name, element)
+      this.delegate.targetConnected(element, name)
+    }
+  }
+
+  disconnectTarget(element, name) {
+    if (this.targetsByName.has(name, element)) {
+      this.targetsByName.delete(name, element)
+      this.delegate.targetDisconnected(element, name)
+    }
+  }
+
+  disconnectAllTargets() {
+    for (const name of this.targetsByName.keys) {
+      for (const element of this.targetsByName.getValuesForKey(name)) {
+        this.disconnectTarget(element, name)
+      }
+    }
+  }
+
+  // Private
+
+  get attributeName() {
+    return `data-${this.context.identifier}-target`
+  }
+
+  get element() {
+    return this.context.element
+  }
+
+  get scope() {
+    return this.context.scope
+  }
+}
+
+// Monkey Patch Context
+// Target observer delegate
+
+Context.prototype.targetConnected = function (element, name) {
+  this.invokeControllerMethod(`${name}TargetConnected`, element)
+}
+
+Context.prototype.targetDisconnected = function (element, name) {
+  this.invokeControllerMethod(`${name}TargetDisconnected`, element)
+}
+
+// Private
+
+Context.prototype.invokeControllerMethod = function (methodName, ...args) {
+  const controller = this.controller
+  if (typeof controller[methodName] == "function") {
+    controller[methodName](...args)
+  }
+}