Skip to content

Commit

Permalink
Changed hasReceivers_ into a class variable.
Browse files Browse the repository at this point in the history
If you call chrome.cast.initialize a second time, it does not error but it
also does not fire off receiverStatusChanged in order to signal the
initial receiver status. This can result in problems if the CastProxy is
destroyed and then re-created; specifically, it will erroneously claim to be
unable to cast until the receiver status next changes.

This makes hasReceivers_ into a class variable, so that a new sender will use
the hasReceivers_ of previous ones.

The original bug report was kind of confusing, so I cannot say for sure if
this actually solves their problem or not. Hopefully it does.

Issue #768

Change-Id: I7839ed99a8c48c69567bbcaeb1f9b6728265d63b
  • Loading branch information
theodab authored and joeyparrish committed Aug 31, 2017
1 parent 30d32b1 commit 7c60e5c
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 30 deletions.
21 changes: 14 additions & 7 deletions lib/cast/cast_sender.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ shaka.cast.CastSender =
/** @private {boolean} */
this.apiReady_ = false;

/** @private {boolean} */
this.hasReceivers_ = false;

/** @private {boolean} */
this.isCasting_ = false;

Expand Down Expand Up @@ -103,6 +100,10 @@ shaka.cast.CastSender =
};


/** @private {boolean} */
shaka.cast.CastSender.hasReceivers_ = false;


/** @override */
shaka.cast.CastSender.prototype.destroy = function() {
this.rejectAllPromises_();
Expand All @@ -115,7 +116,6 @@ shaka.cast.CastSender.prototype.destroy = function() {
this.onRemoteEvent_ = null;
this.onResumeLocal_ = null;
this.apiReady_ = false;
this.hasReceivers_ = false;
this.isCasting_ = false;
this.appData_ = null;
this.cachedProperties_ = null;
Expand All @@ -138,7 +138,7 @@ shaka.cast.CastSender.prototype.apiReady = function() {
* @return {boolean} True if there are receivers.
*/
shaka.cast.CastSender.prototype.hasReceivers = function() {
return this.hasReceivers_;
return shaka.cast.CastSender.hasReceivers_;
};


Expand Down Expand Up @@ -197,6 +197,13 @@ shaka.cast.CastSender.prototype.init = function() {
chrome.cast.initialize(apiConfig,
function() { shaka.log.debug('CastSender: init'); },
function(error) { shaka.log.error('CastSender: init error', error); });
if (shaka.cast.CastSender.hasReceivers_) {
// Fire a fake cast status change, to simulate the update that
// would be fired normally.
// This is after a brief delay, to give users a chance to add event
// listeners.
setTimeout(this.onStatusChanged_.bind(this), 20);
}
};


Expand Down Expand Up @@ -229,7 +236,7 @@ shaka.cast.CastSender.prototype.cast = function(initState) {
shaka.util.Error.Category.CAST,
shaka.util.Error.Code.CAST_API_UNAVAILABLE));
}
if (!this.hasReceivers_) {
if (!shaka.cast.CastSender.hasReceivers_) {
return Promise.reject(new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.CAST,
Expand Down Expand Up @@ -477,7 +484,7 @@ shaka.cast.CastSender.prototype.onReceiverStatusChanged_ =
// The cast extension is telling us whether there are any cast receiver
// devices available.
shaka.log.debug('CastSender: receiver status', availability);
this.hasReceivers_ = availability == 'available';
shaka.cast.CastSender.hasReceivers_ = availability == 'available';
this.onStatusChanged_();
};

Expand Down
78 changes: 55 additions & 23 deletions test/cast/cast_sender_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('CastSender', function() {
video: null
};

/** @type {!jasmine.Spy} */
var onStatusChanged;
/** @type {!jasmine.Spy} */
var onFirstCastStateUpdate;
Expand Down Expand Up @@ -69,6 +70,7 @@ describe('CastSender', function() {
fakeAppId, Util.spyFunc(onStatusChanged),
Util.spyFunc(onFirstCastStateUpdate), Util.spyFunc(onRemoteEvent),
Util.spyFunc(onResumeLocal), Util.spyFunc(onInitStateRequired));
resetHasReceivers();
});

afterEach(function(done) {
Expand Down Expand Up @@ -126,6 +128,27 @@ describe('CastSender', function() {
fakeReceiverAvailability(false);
expect(sender.hasReceivers()).toBe(false);
});

it('remembers status from previous senders', function(done) {
sender.init();
fakeReceiverAvailability(true);
sender.destroy().then(function() {
sender = new CastSender(
fakeAppId, Util.spyFunc(onStatusChanged),
Util.spyFunc(onFirstCastStateUpdate), Util.spyFunc(onRemoteEvent),
Util.spyFunc(onResumeLocal), Util.spyFunc(onInitStateRequired));
sender.init();
// You get an initial call to onStatusChanged when it initializes.
expect(onStatusChanged).toHaveBeenCalledTimes(3);

return Util.delay(0.25);
}).then(function() {
// And then you get another call after it has 'discovered' the
// existing receivers.
expect(sender.hasReceivers()).toBe(true);
expect(onStatusChanged).toHaveBeenCalledTimes(4);
}).catch(fail).then(done);
});
});

describe('cast', function() {
Expand Down Expand Up @@ -230,7 +253,7 @@ describe('CastSender', function() {
fakeReceiverAvailability(true);
fakeJoinExistingSession();

shaka.test.Util.delay(0.1).then(function() {
Util.delay(0.1).then(function() {
expect(onStatusChanged).toHaveBeenCalled();
expect(sender.isCasting()).toBe(true);
expect(onInitStateRequired).toHaveBeenCalled();
Expand Down Expand Up @@ -292,7 +315,7 @@ describe('CastSender', function() {
fakeReceiverAvailability(true);
fakeJoinExistingSession();

shaka.test.Util.delay(0.1).then(function() {
Util.delay(0.1).then(function() {
expect(onFirstCastStateUpdate).not.toHaveBeenCalled();

fakeSessionMessage({
Expand Down Expand Up @@ -321,7 +344,7 @@ describe('CastSender', function() {
fakeReceiverAvailability(true);
fakeJoinExistingSession();

shaka.test.Util.delay(0.1).then(function() {
Util.delay(0.1).then(function() {
fakeSessionMessage({
type: 'update',
update: {video: {currentTime: 12}, player: {isLive: false}}
Expand All @@ -338,7 +361,7 @@ describe('CastSender', function() {

// Disconnect and then connect to another existing session.
fakeJoinExistingSession();
return shaka.test.Util.delay(0.1);
return Util.delay(0.1);
}).then(function() {
fakeSessionMessage({
type: 'update',
Expand Down Expand Up @@ -506,10 +529,10 @@ describe('CastSender', function() {

it('resolve when "asyncComplete" messages are received', function(done) {
var p = method(123, 'abc');
shaka.test.Util.capturePromiseStatus(p);
Util.capturePromiseStatus(p);

// Wait a tick for the Promise status to be set.
shaka.test.Util.delay(0.1).then(function() {
Util.delay(0.1).then(function() {
expect(p.status).toBe('pending');
var id = mockSession.messages[mockSession.messages.length - 1].id;
fakeSessionMessage({
Expand All @@ -519,7 +542,7 @@ describe('CastSender', function() {
});

// Wait a tick for the Promise status to change.
return shaka.test.Util.delay(0.1);
return Util.delay(0.1);
}).then(function() {
expect(p.status).toBe('resolved');
}).catch(fail).then(done);
Expand All @@ -532,10 +555,10 @@ describe('CastSender', function() {
shaka.util.Error.Code.UNABLE_TO_GUESS_MANIFEST_TYPE,
'foo://bar');
var p = method(123, 'abc');
shaka.test.Util.capturePromiseStatus(p);
Util.capturePromiseStatus(p);

// Wait a tick for the Promise status to be set.
shaka.test.Util.delay(0.1).then(function() {
Util.delay(0.1).then(function() {
expect(p.status).toBe('pending');
var id = mockSession.messages[mockSession.messages.length - 1].id;
fakeSessionMessage({
Expand All @@ -545,30 +568,30 @@ describe('CastSender', function() {
});

// Wait a tick for the Promise status to change.
return shaka.test.Util.delay(0.1);
return Util.delay(0.1);
}).then(function() {
expect(p.status).toBe('rejected');
return p.catch(function(error) {
shaka.test.Util.expectToEqualError(error, originalError);
Util.expectToEqualError(error, originalError);
});
}).catch(fail).then(done);
});

it('reject when disconnected remotely', function(done) {
var p = method(123, 'abc');
shaka.test.Util.capturePromiseStatus(p);
Util.capturePromiseStatus(p);

// Wait a tick for the Promise status to be set.
shaka.test.Util.delay(0.1).then(function() {
Util.delay(0.1).then(function() {
expect(p.status).toBe('pending');
fakeRemoteDisconnect();

// Wait a tick for the Promise status to change.
return shaka.test.Util.delay(0.1);
return Util.delay(0.1);
}).then(function() {
expect(p.status).toBe('rejected');
return p.catch(function(error) {
shaka.test.Util.expectToEqualError(error, new shaka.util.Error(
Util.expectToEqualError(error, new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.PLAYER,
shaka.util.Error.Code.LOAD_INTERRUPTED));
Expand Down Expand Up @@ -653,21 +676,21 @@ describe('CastSender', function() {

var method = sender.get('player', 'load');
var p = method();
shaka.test.Util.capturePromiseStatus(p);
Util.capturePromiseStatus(p);

// Wait a tick for the Promise status to be set.
return shaka.test.Util.delay(0.1).then(function() {
return Util.delay(0.1).then(function() {
expect(p.status).toBe('pending');
sender.forceDisconnect();
expect(mockSession.leave).not.toHaveBeenCalled();
expect(mockSession.stop).toHaveBeenCalled();

// Wait a tick for the Promise status to change.
return shaka.test.Util.delay(0.1);
return Util.delay(0.1);
}).then(function() {
expect(p.status).toBe('rejected');
return p.catch(function(error) {
shaka.test.Util.expectToEqualError(error, new shaka.util.Error(
Util.expectToEqualError(error, new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.PLAYER,
shaka.util.Error.Code.LOAD_INTERRUPTED));
Expand All @@ -689,21 +712,21 @@ describe('CastSender', function() {

var method = sender.get('player', 'load');
var p = method();
shaka.test.Util.capturePromiseStatus(p);
Util.capturePromiseStatus(p);

// Wait a tick for the Promise status to be set.
return shaka.test.Util.delay(0.1).then(function() {
return Util.delay(0.1).then(function() {
expect(p.status).toBe('pending');
sender.destroy().catch(fail);
expect(mockSession.leave).toHaveBeenCalled();
expect(mockSession.stop).not.toHaveBeenCalled();

// Wait a tick for the Promise status to change.
return shaka.test.Util.delay(0.1);
return Util.delay(0.1);
}).then(function() {
expect(p.status).toBe('rejected');
return p.catch(function(error) {
shaka.test.Util.expectToEqualError(error, new shaka.util.Error(
Util.expectToEqualError(error, new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.PLAYER,
shaka.util.Error.Code.LOAD_INTERRUPTED));
Expand Down Expand Up @@ -811,4 +834,13 @@ describe('CastSender', function() {
onJoinExistingSession(mockSession);
}
}

/**
* @suppress {visibility}
*/
function resetHasReceivers() {
// @suppress visibility has function scope, so this is a mini-function that
// exists solely to suppress visibility for this call.
CastSender.hasReceivers_ = false;
}
});

0 comments on commit 7c60e5c

Please sign in to comment.