Remove listeners before destroying CastSender.

Beforehand, calling chrome.cast.session_.destroy() would typically cause an error, as the message or update listener was called after the the CastSender was destroyed. This removes those listeners before destroying. Issue #768 Change-Id: I7889adce7b829c3f24dac7a178c9be26e2fdc887
shaka-project · Oct 2, 2017 · 84bfdd3 · 84bfdd3
1 parent f45868b
commit 84bfdd3
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 2 deletions.
diff --git a/externs/chromecast.js b/externs/chromecast.js
@@ -283,12 +283,26 @@ chrome.cast.Session.prototype.addMessageListener = function(
     namespace, listener) {};
 
 
+/**
+ * @param {string} namespace
+ * @param {Function} listener
+ */
+chrome.cast.Session.prototype.removeMessageListener = function(
+    namespace, listener) {};
+
+
 /**
  * @param {Function} listener
  */
 chrome.cast.Session.prototype.addUpdateListener = function(listener) {};
 
 
+/**
+ * @param {Function} listener
+ */
+chrome.cast.Session.prototype.removeUpdateListener = function(listener) {};
+
+
 /**
  * @param {Function} successCallback
  * @param {Function} errorCallback

diff --git a/lib/cast/cast_sender.js b/lib/cast/cast_sender.js
@@ -83,6 +83,13 @@ shaka.cast.CastSender =
   /** @private {chrome.cast.Session} */
   this.session_ = null;
 
+  /** @private {?function()} */
+  this.onConnectionStatusChangedBound_ =
+      this.onConnectionStatusChanged_.bind(this);
+
+  /** @private {?function(string, string)} */
+  this.onMessageReceivedBound_ = this.onMessageReceived_.bind(this);
+
   /** @private {Object} */
   this.cachedProperties_ = {
     'video': {},
@@ -108,6 +115,7 @@ shaka.cast.CastSender.hasReceivers_ = false;
 shaka.cast.CastSender.prototype.destroy = function() {
   this.rejectAllPromises_();
   if (this.session_) {
+    this.removeListeners_();
     this.session_.leave(function() {}, function() {});
     this.session_ = null;
   }
@@ -121,6 +129,8 @@ shaka.cast.CastSender.prototype.destroy = function() {
   this.cachedProperties_ = null;
   this.asyncCallPromises_ = null;
   this.castPromise_ = null;
+  this.onConnectionStatusChangedBound_ = null;
+  this.onMessageReceivedBound_ = null;
 
   return Promise.resolve();
 };
@@ -285,6 +295,7 @@ shaka.cast.CastSender.prototype.forceDisconnect = function() {
 
   this.rejectAllPromises_();
   if (this.session_) {
+    this.removeListeners_();
     this.session_.stop(function() {}, function() {});
     this.session_ = null;
   }
@@ -495,14 +506,25 @@ shaka.cast.CastSender.prototype.onReceiverStatusChanged_ =
  */
 shaka.cast.CastSender.prototype.onSessionCreated_ = function(session) {
   this.session_ = session;
-  this.session_.addUpdateListener(this.onConnectionStatusChanged_.bind(this));
+  this.session_.addUpdateListener(this.onConnectionStatusChangedBound_);
   this.session_.addMessageListener(
       shaka.cast.CastUtils.SHAKA_MESSAGE_NAMESPACE,
-      this.onMessageReceived_.bind(this));
+      this.onMessageReceivedBound_);
   this.onConnectionStatusChanged_();
 };
 
 
+/**
+ * @private
+ */
+shaka.cast.CastSender.prototype.removeListeners_ = function() {
+  this.session_.removeUpdateListener(this.onConnectionStatusChangedBound_);
+  this.session_.removeMessageListener(
+      shaka.cast.CastUtils.SHAKA_MESSAGE_NAMESPACE,
+      this.onMessageReceivedBound_);
+};
+
+
 /**
  * @private
  */

diff --git a/test/cast/cast_sender_unit.js b/test/cast/cast_sender_unit.js
@@ -673,6 +673,8 @@ describe('CastSender', function() {
         expect(sender.isCasting()).toBe(true);
         expect(mockSession.leave).not.toHaveBeenCalled();
         expect(mockSession.stop).not.toHaveBeenCalled();
+        expect(mockSession.removeUpdateListener).not.toHaveBeenCalled();
+        expect(mockSession.removeMessageListener).not.toHaveBeenCalled();
 
         var method = sender.get('player', 'load');
         var p = method();
@@ -684,6 +686,8 @@ describe('CastSender', function() {
           sender.forceDisconnect();
           expect(mockSession.leave).not.toHaveBeenCalled();
           expect(mockSession.stop).toHaveBeenCalled();
+          expect(mockSession.removeUpdateListener).toHaveBeenCalled();
+          expect(mockSession.removeMessageListener).toHaveBeenCalled();
 
           // Wait a tick for the Promise status to change.
           return Util.delay(0.1);
@@ -709,6 +713,8 @@ describe('CastSender', function() {
         expect(sender.isCasting()).toBe(true);
         expect(mockSession.leave).not.toHaveBeenCalled();
         expect(mockSession.stop).not.toHaveBeenCalled();
+        expect(mockSession.removeUpdateListener).not.toHaveBeenCalled();
+        expect(mockSession.removeMessageListener).not.toHaveBeenCalled();
 
         var method = sender.get('player', 'load');
         var p = method();
@@ -720,6 +726,8 @@ describe('CastSender', function() {
           sender.destroy().catch(fail);
           expect(mockSession.leave).toHaveBeenCalled();
           expect(mockSession.stop).not.toHaveBeenCalled();
+          expect(mockSession.removeUpdateListener).toHaveBeenCalled();
+          expect(mockSession.removeMessageListener).toHaveBeenCalled();
 
           // Wait a tick for the Promise status to change.
           return Util.delay(0.1);
@@ -753,7 +761,9 @@ describe('CastSender', function() {
       status: 'connected',
       receiver: { friendlyName: 'SomeDevice' },
       addUpdateListener: jasmine.createSpy('Session.addUpdateListener'),
+      removeUpdateListener: jasmine.createSpy('Session.removeUpdateListener'),
       addMessageListener: jasmine.createSpy('Session.addMessageListener'),
+      removeMessageListener: jasmine.createSpy('Session.removeMessageListener'),
       leave: jasmine.createSpy('Session.leave'),
       sendMessage: jasmine.createSpy('Session.sendMessage'),
       stop: jasmine.createSpy('Session.stop')