Skip to content

Commit

Permalink
Stopped proxying getManifest()
Browse files Browse the repository at this point in the history
getManifest() hugely increased the message size when casting, to the
point where we were having message size problems.
This CL stops that property from being proxied.

This also adds an integration test that makes sure update messages do not get
too large.

Closes #1128

Change-Id: I3c4bfabb4d35ee870a603c38f784cb226366a28b
  • Loading branch information
theodab committed Dec 13, 2017
1 parent e7c5a3a commit 31b2984
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 23 deletions.
10 changes: 10 additions & 0 deletions lib/cast/cast_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,16 @@ shaka.cast.CastProxy.prototype.playerProxyGet_ = function(name) {
return this.localPlayer_.getNetworkingEngine.bind(this.localPlayer_);
}

if (name == 'getManifest') {
if (this.sender_.isCasting()) {
return function() {
shaka.log.error('getManifest() does not work while casting!');
return null;
};
}
return this.localPlayer_.getManifest.bind(this.localPlayer_);
}

// If we are casting, but the first update has not come in yet, use local
// getters, but not local methods.
if (this.sender_.isCasting() && !this.sender_.hasRemoteProperties()) {
Expand Down
2 changes: 1 addition & 1 deletion lib/cast/cast_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ shaka.cast.CastUtils.PlayerGetterMethods = [
'getBufferedInfo',
'getConfiguration',
'getExpiration',
'getManifest',
// Note that the 'getManifest' property is not proxied, as it is very large.
'getManifestUri',
'getPlaybackRate',
'getTextLanguages',
Expand Down
94 changes: 72 additions & 22 deletions test/cast/cast_receiver_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,20 @@ describe('CastReceiver', function() {
var isChrome;
/** @type {boolean} */
var isChromecast;
/** @type {!Object.<string, ?shakaExtern.DrmSupportType>} */
var support = {};

var fakeInitState;

function checkKeySystems() {
// Our test asset for this suite can use any of these key systems:
if (!support['com.widevine.alpha']) {
// pending() throws a special exception that Jasmine uses to skip a test.
// It can only be used from inside it(), not describe() or beforeEach().
pending('Skipping DrmEngine tests.');
// The rest of the test will not run.
}
}

function checkChromeOrChromecast() {
if (!isChromecast && !isChrome) {
Expand All @@ -57,6 +71,10 @@ describe('CastReceiver', function() {
}

beforeAll(function(done) {
var supportTest = shaka.media.DrmEngine.probeSupport()
.then(function(result) { support = result; })
.catch(fail);

// The receiver is only meant to run on the Chromecast, so we have the
// ability to use modern APIs there that may not be available on all of the
// browsers our library supports. Because of this, CastReceiver tests will
Expand All @@ -81,7 +99,9 @@ describe('CastReceiver', function() {
shaka.media.ManifestParser.registerParserByMime(
'application/x-test-manifest',
shaka.test.TestScheme.ManifestParser);
shaka.test.TestScheme.createManifests(shaka, '').then(done);
var createManifests = shaka.test.TestScheme.createManifests(shaka, '');

Promise.all([createManifests, supportTest]).then(done);
});

beforeEach(function() {
Expand Down Expand Up @@ -115,6 +135,21 @@ describe('CastReceiver', function() {

toRestore = [];
pendingWaitWrapperCalls = 0;

fakeInitState = {
player: {
configure: {}
},
'playerAfterLoad': {
setTextTrackVisibility: true
},
video: {
loop: true,
playbackRate: 5
},
manifest: 'test:sintel_no_text',
startTime: 0
};
});

afterEach(function(done) {
Expand All @@ -141,23 +176,35 @@ describe('CastReceiver', function() {
}
});

it('sends update messages at every stage of loading', function(done) {
drm_it('sends reasonably-sized update messages', function(done) {
checkChromeOrChromecast();
checkKeySystems();

var fakeInitState = {
player: {
configure: {}
},
'playerAfterLoad': {
setTextTrackVisibility: true
},
video: {
loop: true,
playbackRate: 5
},
manifest: 'test:sintel_no_text',
startTime: 0
// Use an encrypted asset, to make sure DRM info doesn't balloon the size.
fakeInitState.manifest = 'test:sintel-enc';

var onLoadedData = function() {
video.removeEventListener('loadeddata', onLoadedData);
// Wait for an update message.
waitForUpdateMessage().then(function(message) {
// Check that the update message is of a reasonable size.
expect(message.length).toBeLessThan(5000);
}).then(done);
};
video.addEventListener('loadeddata', onLoadedData);
addOnError(done);

// Start the process of loading by sending a fake init message.
fakeConnectedSenders(1);
fakeIncomingMessage({
type: 'init',
initState: fakeInitState,
appData: {}
}, mockShakaMessageBus);
});

it('sends update messages at every stage of loading', function(done) {
checkChromeOrChromecast();

// Add wrappers to various methods along player.load to make sure that,
// at each stage, the cast receiver can form an update message without
Expand All @@ -183,12 +230,7 @@ describe('CastReceiver', function() {
waitForUpdateMessage().then(done);
};
video.addEventListener('loadeddata', onLoadedData);

var onError = function(event) {
fail(event.detail);
done();
};
player.addEventListener('error', onError);
addOnError(done);

// Start the process of loading by sending a fake init message.
fakeConnectedSenders(1);
Expand Down Expand Up @@ -226,6 +268,14 @@ describe('CastReceiver', function() {
});
}

function addOnError(done) {
var onError = function(event) {
fail(event.detail);
done();
};
player.addEventListener('error', onError);
}

function waitForUpdateMessage() {
messageWaitPromise = new shaka.util.PublicPromise();
return messageWaitPromise;
Expand Down Expand Up @@ -271,7 +321,7 @@ describe('CastReceiver', function() {
var parsed = CastUtils.deserialize(message);
if (parsed.type == 'update' && messageWaitPromise) {
shaka.log.debug('Received update message. Proceeding...');
messageWaitPromise.resolve();
messageWaitPromise.resolve(message);
messageWaitPromise = null;
}
});
Expand Down
1 change: 1 addition & 0 deletions test/cast/cast_utils_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('CastUtils', function() {
'getMediaElement', // Handled specially
'setMaxHardwareResolution',
'destroy', // Should use CastProxy.destroy instead
'getManifest', // Too large to proxy

// Test helper methods (not @export'd)
'createDrmEngine',
Expand Down

0 comments on commit 31b2984

Please sign in to comment.