From f07754c85a159c418b9189134c995859f411279f Mon Sep 17 00:00:00 2001
From: Bryce Tham <brycetham@users.noreply.github.com>
Date: Fri, 19 Jan 2024 11:39:41 -0500
Subject: [PATCH] fix: check muted property for mute getter (#69)

* fix: check muted property for mute getter

* fix: update mute event conditions

* test: update unit tests

---------

Co-authored-by: Bryce Tham <btham@cisco.com>
---
 src/media/local-stream.spec.ts | 43 ++++++++++++++++++++++++++++++++--
 src/media/local-stream.ts      | 27 +++++++++++++++++++--
 src/media/stream.ts            |  4 ++--
 3 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/src/media/local-stream.spec.ts b/src/media/local-stream.spec.ts
index 1d6bb05..071b985 100644
--- a/src/media/local-stream.spec.ts
+++ b/src/media/local-stream.spec.ts
@@ -1,6 +1,7 @@
 import { WebrtcCoreError } from '../errors';
 import { createMockedStream } from '../util/test-utils';
 import { LocalStream, LocalStreamEventNames, TrackEffect } from './local-stream';
+import { StreamEventNames } from './stream';
 
 /**
  * A dummy LocalStream implementation so we can instantiate it for testing.
@@ -10,20 +11,58 @@ class TestLocalStream extends LocalStream {}
 describe('LocalStream', () => {
   const mockStream = createMockedStream();
   let localStream: LocalStream;
+
   beforeEach(() => {
     localStream = new TestLocalStream(mockStream);
   });
 
   describe('setMuted', () => {
-    it('should change the input track state based on being muted & unmuted', () => {
-      expect.assertions(2);
+    let emitSpy: jest.SpyInstance;
+
+    beforeEach(() => {
+      localStream = new TestLocalStream(mockStream);
+      emitSpy = jest.spyOn(localStream[StreamEventNames.MuteStateChange], 'emit');
+    });
+
+    it('should change the input track enabled state and fire an event', () => {
+      expect.assertions(6);
+
       // Simulate the default state of the track's enabled state.
       mockStream.getTracks()[0].enabled = true;
 
       localStream.setMuted(true);
       expect(mockStream.getTracks()[0].enabled).toBe(false);
+      expect(emitSpy).toHaveBeenCalledTimes(1);
+      expect(emitSpy).toHaveBeenLastCalledWith(true);
+
       localStream.setMuted(false);
       expect(mockStream.getTracks()[0].enabled).toBe(true);
+      expect(emitSpy).toHaveBeenCalledTimes(2);
+      expect(emitSpy).toHaveBeenLastCalledWith(false);
+    });
+
+    it('should not fire an event if the same mute state is set twice', () => {
+      expect.assertions(1);
+
+      // Simulate the default state of the track's enabled state.
+      mockStream.getTracks()[0].enabled = true;
+
+      localStream.setMuted(false);
+      expect(emitSpy).toHaveBeenCalledTimes(0);
+    });
+
+    it('should not fire an event if the track has been muted by the browser', () => {
+      expect.assertions(2);
+
+      // Simulate the default state of the track's enabled state.
+      mockStream.getTracks()[0].enabled = true;
+
+      // Simulate the track being muted by the browser.
+      Object.defineProperty(mockStream.getTracks()[0], 'muted', { value: true });
+
+      localStream.setMuted(true);
+      expect(mockStream.getTracks()[0].enabled).toBe(false);
+      expect(emitSpy).toHaveBeenCalledTimes(0);
     });
   });
 
diff --git a/src/media/local-stream.ts b/src/media/local-stream.ts
index 190053e..62d7526 100644
--- a/src/media/local-stream.ts
+++ b/src/media/local-stream.ts
@@ -57,11 +57,32 @@ abstract class _LocalStream extends Stream {
     return this.inputStream.getTracks()[0];
   }
 
+  /**
+   * @inheritdoc
+   */
+  protected handleTrackMuted() {
+    if (this.inputTrack.enabled) {
+      super.handleTrackMuted();
+    }
+  }
+
+  /**
+   * @inheritdoc
+   */
+  protected handleTrackUnmuted() {
+    if (this.inputTrack.enabled) {
+      super.handleTrackUnmuted();
+    }
+  }
+
   /**
    * @inheritdoc
    */
   get muted(): boolean {
-    return !this.inputTrack.enabled;
+    // Calls to `setMuted` will only affect the "enabled" state, but there are specific cases in
+    // which the browser may mute the track, which will affect the "muted" state but not the
+    // "enabled" state, e.g. minimizing a shared window or unplugging a shared screen.
+    return !this.inputTrack.enabled || this.inputTrack.muted;
   }
 
   /**
@@ -73,7 +94,9 @@ abstract class _LocalStream extends Stream {
     if (this.inputTrack.enabled === isMuted) {
       this.inputTrack.enabled = !isMuted;
       // setting `enabled` will not automatically emit MuteStateChange, so we emit it here
-      this[StreamEventNames.MuteStateChange].emit(isMuted);
+      if (!this.inputTrack.muted) {
+        this[StreamEventNames.MuteStateChange].emit(isMuted);
+      }
     }
   }
 
diff --git a/src/media/stream.ts b/src/media/stream.ts
index 826870c..7040d0a 100644
--- a/src/media/stream.ts
+++ b/src/media/stream.ts
@@ -39,14 +39,14 @@ abstract class _Stream {
   /**
    * Handler which is called when a track's mute event fires.
    */
-  private handleTrackMuted() {
+  protected handleTrackMuted() {
     this[StreamEventNames.MuteStateChange].emit(true);
   }
 
   /**
    * Handler which is called when a track's unmute event fires.
    */
-  private handleTrackUnmuted() {
+  protected handleTrackUnmuted() {
     this[StreamEventNames.MuteStateChange].emit(false);
   }