SPARK-532493 webrtc core should expose separate events for ice and co…

…nnection state (#78) * feat: spark-532493 expose separate events for connection state and ice connection state * feat: spark-532493 expose separate events for connection state and ice connection state * feat: use native webrtc states * feat: correct check in tests --------- Co-authored-by: kwasniow <[email protected]>
webex · Jun 12, 2024 · 9df9bd2 · 9df9bd2
1 parent ab8c6f5
commit 9df9bd2
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 53 deletions.
diff --git a/cspell.json b/cspell.json
@@ -12,6 +12,8 @@
     "codecov",
     "commitlint",
     "cpaas",
+    "createansweronsuccess",
+    "createofferonsuccess",
     "dbaeumer",
     "dependabot",
     "eamodio",
@@ -25,22 +27,21 @@
     "libauth",
     "mkdir",
     "negotiatedneeded",
+    "peerconnectionstatechange",
     "preprocessors",
     "prettierignore",
     "rohit",
     "sandboxed",
     "saucelabs",
+    "setlocaldescriptiononsuccess",
+    "setremotedescriptiononsuccess",
     "transpiled",
     "typedoc",
     "untracked",
     "Wcme",
     "WCME",
     "webex",
-    "webrtc",
-    "createofferonsuccess",
-    "createansweronsuccess",
-    "setlocaldescriptiononsuccess",
-    "setremotedescriptiononsuccess"
+    "webrtc"
   ],
   "flagWords": [],
   "ignorePaths": [

diff --git a/src/connection-state-handler.spec.ts b/src/connection-state-handler.spec.ts
@@ -17,39 +17,53 @@ describe('ConnectionStateHandler', () => {
     fakeConnectionState = 'new';
   });
 
+  it('reads initial peer connection state', () => {
+    expect.assertions(1);
+    const connStateHandler = new ConnectionStateHandler(fakeCallback);
+
+    expect(connStateHandler.getPeerConnectionState()).toBe('new');
+  });
+
+  it('reads initial ice connection state', () => {
+    expect.assertions(1);
+    const connStateHandler = new ConnectionStateHandler(fakeCallback);
+
+    expect(connStateHandler.getIceConnectionState()).toBe('new');
+  });
+
   it('reads initial connection state', () => {
     expect.assertions(1);
     const connStateHandler = new ConnectionStateHandler(fakeCallback);
 
     expect(connStateHandler.getConnectionState()).toStrictEqual(ConnectionState.New);
   });
 
-  it('updates connection state on ice connection state change and emits the event', () => {
+  it('updates ice connection state on ice connection state change and emits the event', () => {
     expect.assertions(2);
     const connStateHandler = new ConnectionStateHandler(fakeCallback);
 
-    connStateHandler.on(ConnectionStateHandler.Events.ConnectionStateChanged, (state) => {
-      expect(state).toStrictEqual(ConnectionState.Connecting);
+    connStateHandler.on(ConnectionStateHandler.Events.IceConnectionStateChanged, (state) => {
+      expect(state).toBe('checking');
     });
 
     fakeIceState = 'checking';
     connStateHandler.onIceConnectionStateChange();
 
-    expect(connStateHandler.getConnectionState()).toStrictEqual(ConnectionState.Connecting);
+    expect(connStateHandler.getIceConnectionState()).toBe('checking');
   });
 
   it("updates connection state on RTCPeerConnection's connection state change", () => {
     expect.assertions(2);
     const connStateHandler = new ConnectionStateHandler(fakeCallback);
 
-    connStateHandler.on(ConnectionStateHandler.Events.ConnectionStateChanged, (state) => {
-      expect(state).toStrictEqual(ConnectionState.Connecting);
+    connStateHandler.on(ConnectionStateHandler.Events.PeerConnectionStateChanged, (state) => {
+      expect(state).toBe('connecting');
     });
 
     fakeConnectionState = 'connecting';
-    connStateHandler.onConnectionStateChange();
+    connStateHandler.onPeerConnectionStateChange();
 
-    expect(connStateHandler.getConnectionState()).toStrictEqual(ConnectionState.Connecting);
+    expect(connStateHandler.getPeerConnectionState()).toBe('connecting');
   });
 
   // test matrix for all possible combinations of iceConnectionState and connectionState
@@ -118,9 +132,6 @@ describe('ConnectionStateHandler', () => {
       fakeConnectionState = connState;
       fakeIceState = iceState;
 
-      // it's sufficient to trigger just one of the callbacks
-      connStateHandler.onConnectionStateChange();
-
       expect(connStateHandler.getConnectionState()).toStrictEqual(expected);
     })
   );

diff --git a/src/connection-state-handler.ts b/src/connection-state-handler.ts
@@ -12,11 +12,13 @@ export enum ConnectionState {
 }
 
 enum ConnectionStateEvents {
-  ConnectionStateChanged = 'ConnectionStateChanged',
+  PeerConnectionStateChanged = 'PeerConnectionStateChanged',
+  IceConnectionStateChanged = 'IceConnectionStateChanged',
 }
 
 interface ConnectionStateEventHandlers extends EventMap {
-  [ConnectionStateEvents.ConnectionStateChanged]: (state: ConnectionState) => void;
+  [ConnectionStateEvents.PeerConnectionStateChanged]: (state: RTCPeerConnectionState) => void;
+  [ConnectionStateEvents.IceConnectionStateChanged]: (state: RTCIceConnectionState) => void;
 }
 
 type GetCurrentStatesCallback = () => {
@@ -31,8 +33,6 @@ type GetCurrentStatesCallback = () => {
 export class ConnectionStateHandler extends EventEmitter<ConnectionStateEventHandlers> {
   static Events = ConnectionStateEvents;
 
-  private mediaConnectionState: ConnectionState;
-
   private getCurrentStatesCallback: GetCurrentStatesCallback;
 
   /**
@@ -44,33 +44,24 @@ export class ConnectionStateHandler extends EventEmitter<ConnectionStateEventHan
   constructor(getCurrentStatesCallback: GetCurrentStatesCallback) {
     super();
     this.getCurrentStatesCallback = getCurrentStatesCallback;
-    this.mediaConnectionState = this.evaluateMediaConnectionState();
   }
 
   /**
    * Handler for connection state change.
    */
-  public onConnectionStateChange(): void {
-    this.handleAnyConnectionStateChange();
+  public onPeerConnectionStateChange(): void {
+    const state = this.getPeerConnectionState();
+
+    this.emit(ConnectionStateEvents.PeerConnectionStateChanged, state);
   }
 
   /**
    * Handler for ice connection state change.
    */
   public onIceConnectionStateChange(): void {
-    this.handleAnyConnectionStateChange();
-  }
-
-  /**
-   * Method to be called whenever ice connection or dtls connection state is changed.
-   */
-  private handleAnyConnectionStateChange() {
-    const newConnectionState = this.evaluateMediaConnectionState();
+    const state = this.getIceConnectionState();
 
-    if (newConnectionState !== this.mediaConnectionState) {
-      this.mediaConnectionState = newConnectionState;
-      this.emit(ConnectionStateEvents.ConnectionStateChanged, this.mediaConnectionState);
-    }
+    this.emit(ConnectionStateEvents.IceConnectionStateChanged, state);
   }
 
   /**
@@ -79,12 +70,12 @@ export class ConnectionStateHandler extends EventEmitter<ConnectionStateEventHan
    *
    * @returns Current overall connection state.
    */
-  private evaluateMediaConnectionState() {
+  private evaluateMediaConnectionState(): ConnectionState {
     const { connectionState, iceState } = this.getCurrentStatesCallback();
 
     const connectionStates = [connectionState, iceState];
 
-    let mediaConnectionState;
+    let mediaConnectionState: ConnectionState;
 
     if (connectionStates.every((value) => value === 'new')) {
       mediaConnectionState = ConnectionState.New;
@@ -112,7 +103,29 @@ export class ConnectionStateHandler extends EventEmitter<ConnectionStateEventHan
    *
    * @returns Current connection state.
    */
+  public getPeerConnectionState(): RTCPeerConnectionState {
+    const { connectionState } = this.getCurrentStatesCallback();
+
+    return connectionState;
+  }
+
+  /**
+   * Gets current ice connection state.
+   *
+   * @returns Current ice connection state.
+   */
+  public getIceConnectionState(): RTCIceConnectionState {
+    const { iceState } = this.getCurrentStatesCallback();
+
+    return iceState;
+  }
+
+  /**
+   * Gets current overall connection state.
+   *
+   * @returns Current overall connection state.
+   */
   public getConnectionState(): ConnectionState {
-    return this.mediaConnectionState;
+    return this.evaluateMediaConnectionState();
   }
 }
diff --git a/src/peer-connection.spec.ts b/src/peer-connection.spec.ts
@@ -1,6 +1,6 @@
 import { BrowserInfo } from '@webex/web-capabilities';
 import { MockedObjectDeep } from 'ts-jest';
-import { ConnectionState, ConnectionStateHandler } from './connection-state-handler';
+import { ConnectionStateHandler } from './connection-state-handler';
 import { mocked } from './mocks/mock';
 import { RTCPeerConnectionStub } from './mocks/rtc-peer-connection-stub';
 import { PeerConnection } from './peer-connection';
@@ -219,32 +219,49 @@ describe('PeerConnection', () => {
 
       mockPc.onconnectionstatechange();
 
-      expect(connectionStateHandler.onConnectionStateChange).toHaveBeenCalledTimes(1);
+      expect(connectionStateHandler.onPeerConnectionStateChange).toHaveBeenCalledTimes(1);
     });
     it('returns connection state from connection state handler when geConnectionState() is called', () => {
       expect.assertions(2);
       const connectionStateHandler = getInstantiatedConnectionStateHandler();
-      connectionStateHandler.getConnectionState.mockReturnValueOnce(ConnectionState.Connected);
+      connectionStateHandler.getConnectionState.mockReturnValueOnce('connected');
 
-      expect(pc.getConnectionState()).toStrictEqual(ConnectionState.Connected);
+      expect(pc.getConnectionState()).toBe('connected');
       expect(connectionStateHandler.getConnectionState).toHaveBeenCalledTimes(1);
     });
-    it("listens on ConnectionStateHandler's ConnectionStateChange event and emits it", () => {
+    it("listens on ConnectionStateHandler's PeerConnectionStateChange event and emits it", () => {
       expect.assertions(2);
       const connectionStateHandler = getInstantiatedConnectionStateHandler();
 
-      pc.on(PeerConnection.Events.ConnectionStateChange, (state) => {
-        expect(state).toStrictEqual(ConnectionState.Connecting);
+      pc.on(PeerConnection.Events.PeerConnectionStateChange, (state) => {
+        expect(state).toBe('connecting');
       });
 
       // verify that PeerConnection listens for the right event
       expect(connectionStateHandler.on.mock.calls[0][0]).toStrictEqual(
-        ConnectionStateHandler.Events.ConnectionStateChanged
+        ConnectionStateHandler.Events.PeerConnectionStateChanged
       );
 
       // trigger the fake event from ConnectionStateHandler
       const connectionStateHandlerListener = connectionStateHandler.on.mock.calls[0][1];
-      connectionStateHandlerListener(ConnectionState.Connecting);
+      connectionStateHandlerListener('connecting');
+    });
+    it("listens on ConnectionStateHandler's IceConnectionStateChange event and emits it", () => {
+      expect.assertions(2);
+      const connectionStateHandler = getInstantiatedConnectionStateHandler();
+
+      pc.on(PeerConnection.Events.IceConnectionStateChange, (state) => {
+        expect(state).toBe('checking');
+      });
+
+      // verify that PeerConnection listens for the right event
+      expect(connectionStateHandler.on.mock.calls[1][0]).toStrictEqual(
+        ConnectionStateHandler.Events.IceConnectionStateChanged
+      );
+
+      // trigger the fake event from ConnectionStateHandler
+      const connectionStateHandlerListener = connectionStateHandler.on.mock.calls[1][1];
+      connectionStateHandlerListener('checking');
     });
   });
   describe('createAnswer', () => {

diff --git a/src/peer-connection.ts b/src/peer-connection.ts
@@ -28,7 +28,8 @@ type IceGatheringStateChangeEvent = {
 enum PeerConnectionEvents {
   IceGatheringStateChange = 'icegatheringstatechange',
   IceCandidate = 'icecandidate',
-  ConnectionStateChange = 'connectionstatechange',
+  PeerConnectionStateChange = 'peerconnectionstatechange',
+  IceConnectionStateChange = 'iceconnectionstatechange',
   CreateOfferOnSuccess = 'createofferonsuccess',
   CreateAnswerOnSuccess = 'createansweronsuccess',
   SetLocalDescriptionOnSuccess = 'setlocaldescriptiononsuccess',
@@ -38,7 +39,8 @@ enum PeerConnectionEvents {
 interface PeerConnectionEventHandlers extends EventMap {
   [PeerConnectionEvents.IceGatheringStateChange]: (ev: IceGatheringStateChangeEvent) => void;
   [PeerConnectionEvents.IceCandidate]: (ev: RTCPeerConnectionIceEvent) => void;
-  [PeerConnectionEvents.ConnectionStateChange]: (state: ConnectionState) => void;
+  [PeerConnectionEvents.PeerConnectionStateChange]: (state: RTCPeerConnectionState) => void;
+  [PeerConnectionEvents.IceConnectionStateChange]: (state: RTCIceConnectionState) => void;
   [PeerConnectionEvents.CreateOfferOnSuccess]: (offer: RTCSessionDescriptionInit) => void;
   [PeerConnectionEvents.CreateAnswerOnSuccess]: (answer: RTCSessionDescriptionInit) => void;
   [PeerConnectionEvents.SetLocalDescriptionOnSuccess]: (
@@ -79,9 +81,16 @@ class PeerConnection extends EventEmitter<PeerConnectionEventHandlers> {
     });
 
     this.connectionStateHandler.on(
-      ConnectionStateHandler.Events.ConnectionStateChanged,
-      (state: ConnectionState) => {
-        this.emit(PeerConnection.Events.ConnectionStateChange, state);
+      ConnectionStateHandler.Events.PeerConnectionStateChanged,
+      (state: RTCPeerConnectionState) => {
+        this.emit(PeerConnection.Events.PeerConnectionStateChange, state);
+      }
+    );
+
+    this.connectionStateHandler.on(
+      ConnectionStateHandler.Events.IceConnectionStateChanged,
+      (state: RTCIceConnectionState) => {
+        this.emit(PeerConnection.Events.IceConnectionStateChange, state);
       }
     );
 
@@ -91,7 +100,8 @@ class PeerConnection extends EventEmitter<PeerConnectionEventHandlers> {
       this.connectionStateHandler.onIceConnectionStateChange();
 
     // eslint-disable-next-line jsdoc/require-jsdoc
-    this.pc.onconnectionstatechange = () => this.connectionStateHandler.onConnectionStateChange();
+    this.pc.onconnectionstatechange = () =>
+      this.connectionStateHandler.onPeerConnectionStateChange();
 
     // Subscribe to underlying PeerConnection events and emit them via the EventEmitter
     /* eslint-disable jsdoc/require-jsdoc */
@@ -122,6 +132,24 @@ class PeerConnection extends EventEmitter<PeerConnectionEventHandlers> {
     return this.connectionStateHandler.getConnectionState();
   }
 
+  /**
+   * Gets the connection state of the underlying RTCPeerConnection.
+   *
+   * @returns The underlying RTCPeerConnection connection state.
+   */
+  getPeerConnectionState(): RTCPeerConnectionState {
+    return this.connectionStateHandler.getPeerConnectionState();
+  }
+
+  /**
+   * Gets the ICE connection state of the underlying RTCPeerConnection.
+   *
+   * @returns The underlying RTCPeerConnection ICE connection state.
+   */
+  getIceConnectionState(): RTCIceConnectionState {
+    return this.connectionStateHandler.getIceConnectionState();
+  }
+
   /**
    * Adds a new media track to the set of tracks which will be transmitted to the other peer.
    *