Exception in compiled mode playing certain content #2528

joeyparrish · 2020-04-23T22:06:47Z

Have you read the FAQ and checked for duplicate open issues?
Yes

What version of Shaka Player are you using?
master

Can you reproduce the issue with our latest release version?
No

Can you reproduce the issue with the latest code from master?
Yes

Are you using the demo app or your own custom app?
Demo

What browser and OS are you using?
ChromeOS, but also reproducible on every system in our lab

What are the manifest and license server URIs?
https://nightly-dot-shaka-player-demo.appspot.com/demo/#audiolang=de;textlang=de;uilang=de;asset=https://content.uplynk.com/847859273a4b4a81959d8fea181672a4.mpd?pr.version=2&pr.playenabler=B621D91F-EDCC-4035-8D4B-DC71760D43E9&pr.securitylevel=150;panel=ALL_CONTENT;panelData=source:UPLYNK;build=compiled

What did you do?
Load "Multi DRM - 8 Byte IV" from VDMS in the nightly demo.

What did you expect to happen?
It should play in both compiled and uncompiled mode.

What actually happened?
The compiled version throws "Uncaught (in promise) TypeError: Cannot read property 'done' of undefined".

This is triggered inside the compiled version of periods.js, but the code itself is fine:

for (const unusedStreams of unusedStreamsPerPeriod) {
  // ...
}

And so is the data structure of unusedStreamsPerPeriod, which in this case is an Array containing three empty Sets.

The issue is in the compiler-supplied polyfill for the iterator. For some reason, the polyfilled iterator returns undefined. When the generated code then checks ".done" on that, it throws an exception.

This must be a bug in the compiler or its polyfills. We could perhaps work around it with a for loop, but I think the only long-term fix will be a compiler upgrade.

The text was updated successfully, but these errors were encountered:

This was caught by a compiler upgrade. Issue #2528 Change-Id: Ica262703aa30dce2e59c139c88f3c1e74d5d9e2f

When we changed the constructors for the V2 cells with the introduction of V5 format, these call sites didn't get updated. This doesn't seem to be a user-facing bug, and did not affect any releases. This was caught by a compiler upgrade. Issue #2528 Change-Id: I77228d17631ef44a6b85a81b88054790461617bc

We had made some mistakes in these prefixed names. The corrections were made after consulting both MDN and Apple's docs. This was caught by a compiler upgrade. Issue #2528 Change-Id: I80994569b04a14b26b3e2fbb2673992ca7e68f50

DrmEngine is adding an extra property to MediaKeySystemConfiguration objects, knowing that EME will ignore it. The latest Closure Compiler won't allow this. In the future, we should refactor DrmEngine and consider tracking this in some other way. For now, this changes the dot to square brackets. Issue #2528 Change-Id: I544c93dfa2534e9d62ac5ea47e20a6dc3cb79e3a

These polyfills don't have the exact same type as the thing they are polyfilling. Instead of being constructors for VTTCue (which doesn't exist if we are using the polyfill), they are functions that return TextTrackCue (the base class of VTTCue and the closest thing we have on some platforms). Because the types don't match, the latest Closure Compiler complains. To get around this, we use square brackets. Issue #2528 Change-Id: Ie1e3f13dc51f2145f7d3c9d510b64d9a8d7b1f64

When CastProxy refers to a proxy object, the type is necessarily merely "Object". This isn't specific enough for the compiler to check access to this.videoProxy_.paused, so we convert this to use square brackets. Issue #2528 Change-Id: Ic51fa9fdd20b8ac494fea1e844a945fba319f3c8

The compiler is very picky about the use of the "disabled" property on HTMLElement, since it is only defined on certain subclasses of that. This adds a method to create a button with the correct type to satisfy the compiler. Issue #2528 Change-Id: I31cacd62a35acc87b245ab362dbab55d791cf34d

The latest Closure Compiler is more picky about Event types, and complained about the use of "key" on Event. This updates the Event types so that the compiler has the correct type info. This also stops using "keyCode", which is deprecated, and only uses the current "key" property, which is even supported on IE, and should not create any compatibility issues. Issue #2528 Change-Id: Ic565772b1cc9597e358df015a73c40ac245edd6a

This adds an annotation to suppress a bogus compiler error in StreamingEngine. In this method, we set updateTimer to null, then await some async operation, then check to see if it's still null. The latest Closure Compiler thinks this is a useless check, but we know better. The updateTimer could have been created while the async operation was in progress. Issue #2528 Change-Id: I9a28b7dac1d7cb8bcfd836847507454030af52c6

This adds or augments type info for TextTrack, Cue, and related types. This was caught by a compiler upgrade. Issue #2528 Change-Id: I233e46eeaed8d6503702e181c3380d674ddbb499

These classes were over-idented, making formatting more challenging. I wanted to fix this before making compiler-upgrade-related changes in these files. Issue #2528 Change-Id: Ic44448760161a3e724d8c92f41f07c1a0babfa72

The latest Closure Compiler initially complained about "initData" not being a known property of "Event". Correcting the type to MediaEncryptedEvent exposed a potential bug, which is that we were assigning Uint8Array in some cases instead of the correct ArrayBuffer. This fixes both the type of the event and converts the Uint8Arrays to ArrayBuffers. This also works around a complaint about "code" on "Error". We use "Error" objects as look-alikes for DOMException because there is no exposed constructor for DOMExceptions. To satisfy the compiler, we use square brackets now to set the "code" field on these DOMException look-alikes. Finally, this removes the "method" field on certain Errors in the WebKit EME polyfill. These must have been leftover from some debugging, and are not used at all. Issue #2528 Change-Id: I32c4617b14a30c412d5bc532ec17a46fdc1fea1a

In many places, the implicit type info was insufficient. For example, document.createElement returns Element, but the actual return is always a subclass of Element. In many cases, we need the compiler to know that a specific subclass is in use, so that it can correctly check our use of subclass-specific properties. Another common pattern is confusion between Node and Element (which is a subclass of Node). Almost all of the changes in the demo and UI are Element-related. In some places, we referred to HTMLMediaElement, used in the Player API, instead of the more specific HTMLVideoElement in use in our demo. Since the demo uses video-specific properties, we must use the more specific type. Another case is the use of document.createEvent, which returns Event according to the compiler, but in reality always returns a subclass, like CustomEvent. In one case in NetworkingEngine, correcting the type of an AbortableOperation led to the discovery that we had been incorrectly accessing a private method of that type. In goog.Uri, there were several instances of "*" for a type, which the newer compiler won't accept. These have all been corrected. Finally, in some places, we had the wrong nullability on a type. These were all caught by a compiler upgrade. Issue #2528 Change-Id: I7f2d070e3da32fe9ff5f444315649f3cbdb5a4a5

The demo listens for shaka.Player.ErrorEvent, but it doesn't have type information for that type. This is because the type is defined only in the docs, and not in the externs. There is no actual class defined for the event types. The extern generator should be improved to handle this, but in the meantime, we will use square brackets. This was caught by a compiler upgrade. Issue #2528 Change-Id: I55fc5fcc7f391f3155c1edc712064c5091fa363e

The Storage API just changed to return an AbortableOperation instead of a Promise. The demo did not adopt this new API at the same time. The runtime backward compatibility provided by the Storage API was working, so the demo was not broken. But we should be using the API as defined. This was caught by a compiler upgrade. Issue #2528 Change-Id: I2d427ffb0400739c5d2de17a06c5fd79de3d39b2

The error display in the demo app contains a link to the documentation for a given error code. We were previously abusing that object to hold the error severity, as well. This should be stored separately in its own member variable. This was caught by a compiler upgrade. Issue #2528 Change-Id: Ie7fede5fba72e2d28d201dc3e7437eafa87dc38a

The receiver app is attached to the window so it can be debugged. This should use square brackets, since "receiver" is not defined on "window". Issue #2528 Change-Id: I8e725bb8292cdcb88ffccd3bc65d6d54181affa6

joeyparrish · 2020-04-29T19:06:51Z

We've fixed enough of the compiler's complaints now that I can confirm: the compiler upgrade does definitely fix the issue.

Also, I just watched the clip linked above (the Blender Institute's "Glass Half") all the way through for the first time. That animated short is ADORABLE.

This was caught by a compiler upgrade. Issue #2528 Change-Id: Iae8c495e58347b5c4184bfcec15ff9a48abe6214

The invokeSpy utility is meant to work around an issue where the compiler doesn't understand that jasmine.Spy is a callable function. That workaround needed to be updated for the newest Closure Compiler release. That said, it's not clear why we should need this utility at all. We should be able to inform the compiler that this type is a callable function. This change also adds notes about possible future cleanup and relevant issues filed against the compiler. Issue #2528 Change-Id: I0e9a0e9821bd9a81fcf1349aaba6039ca59dbeeb

The built-in externs for DOMStringList in Closure Compiler do not seem to list it as Iterable. This overrides them to fix it. This was caught by a compiler upgrade. Issue #2528 Change-Id: I5a404f2502fce8fc3e956f68f6a51728d68f0654

In "Fix missing or bad type info" (Change-Id I7f2d070e3da32fe9ff5f444315649f3cbdb5a4a5), I added a bad assertion that failed at runtime. The placement was wrong and the type I was asserting was too strict. This fixes both mistakes. Issue #2528 Change-Id: I297f8c846bf13775a4a59fd43349c96cfcaf3729

In "Remove extraneous exports" (Change-Id Iaf142397f31bd927bf942499a79da595f77361d5), I removed an export that was actually required in the newer compiler. This replaces the missing export. Issue #2528 Change-Id: I3e9ec7085d813365dab917b4712571585d2286c8

We recently updated the Storage API to return AbortableOperation instead of Promise. These tests did not get updated at that time. The runtime backward compatibility kept the tests from failing. This was caught by a compiler upgrade. Issue #2528 Change-Id: I05f75a5e4443b111c63d7969950777db78133626

We were passing an extra parameter to this.dump(), but the older compiler version didn't understand the use of "this" in static methods, so it didn't catch the mistake. This removes the extra parameter. This was caught by a compiler upgrade. Issue #2528 Change-Id: I15fcd79dff270bde2dbb35f5da02a9c9a4173407

These metadata types were never defined, and the old compiler would allow this. The newer compiler is more strict, so this adds typedefs for the metadata we use to generate manifests and streams for tests. This also fixes a bug where we didn't correctly set the mimeType field of the resulting streams. This was caught by a compiler upgrade. Issue #2528 Change-Id: Id16606b5c118793ee17dcf28942bf04d0ee22ba9

The tests for shaka.ui.TextDisplayer created some Cue objects to verify their rendering. These used string literals where they should have been enums, and the newer compiler didn't like that. It also complained that we had assigned a number where there should have been a string. This fixes both issues. This was caught by a compiler upgrade. Issue #2528 Change-Id: Ie3f1e79ca1ed2745f55b270e13411966940cdf62

The new Closure Compiler complains that a number|string union from a map in MpdUtils is used in a calculation. In practice, we know that this specific value is always a number, so we add an assertion to satisfy the newer compiler's type checks. Issue #2528 Change-Id: Id12de47d2dd4a12f9cc35879bee8da5ee25cdd70

Various issues with the nullability of string types led to various fixes, including: - adding an assertion or runtime check that something is not null - moving an existing null check to before a calculation - converting a test expectation into an assertion that the compiler understands (which will still fail the test if the assertion fails) These issues were caught by a compiler upgrade. Issue #2528 Change-Id: I11da091c9e7974c8bea84b3b584cbd29d1e320e2

The built-in externs for IndexedDB in Closure Compiler are missing some type info. This overrides them to add what's missing. This also tweaks type information in a few places in lib/offline/ to match. This was caught by a compiler upgrade. Issue #2528 Change-Id: I97096656f53b426067219e2d4e3aa16f32b87188

Various issues with the nullability of number types led to various fixes, including: - defaulting a nullable number to 0 to avoid propagating a null value through calculations - adding an assertion or runtime check that something is not null - moving an existing null check to before the calculation - returning early on null during an iteration - changing a nullable number to non-nullable - defaulting to NaN instead of null These issues were caught by a compiler upgrade. Issue #2528 Change-Id: I86d516c74a42ee3624c33d7513d2d4c76d3ea589

The usage of SegmentIndex in tests was correct, but the latest Closure Compiler is more strict about nullability. We need to add assertions and refactor to avoid nullable numbers where a non-nullable number is required. We also need to add assertions for the nullability of Stream.segmentIndex after createSegmentIndex() is called. This was caught by a compiler upgrade. Issue #2528 Change-Id: I5615ffa27b878f86739d507f993c2b66ae8eb61a

In many places in the tests, we used "Object" or "*" or just no type at all for various fakes. These were all flagged by the new Closure Compiler version we are adopting. In some other places, we mixed up similar types or had the wrong nullability on a type. In still others, types were missing fields. These issues were caught by a compiler upgrade. Issue #2528 Change-Id: I324e0b28f7e30a4102aa26ec2c9901fa9732211b

The old compiler would let us export a static method on a class without exporting the whole class and its constructor. The new compiler silently ignores `@export` for any methods of a non-exported class. This means that for the latest Closure Compiler, we must export any class with exported static methods. In some cases, these are non-utility classes with constructors we'd rather not export, but the constructor is implicitly exported by exporting the class itself. This was initially caught by the integration tests. The error wasn't especially helpful, so I added a try/catch to loadShaka that makes the error more apparent: TypeError: Cannot read property 'registerParserByMime' of undefined To make sure we do not make this mistake again, I've added a check to the extern generator, which was already able to detect these types of classes. I don't know a compiler-based way to do it, since the compiler silently ignores the export annotations in these cases. Issue #2528 Change-Id: I797c75a8098b0bb3cf837588569f878253dec2cf

We used to alias static utility methods by assigning the method itself to a local variable. This is not allowed by the new Closure Compiler release that we are adopting, and for good reason. The old compiler did not understand the use of "this" in static methods, but the new one does. And it turns out that when we start using "this" in static methods (see #2532), aliasing the method itself can break everything. When you refer to "this" in a static method, it refers to the class. This is really useful in utility classes that have private static methods they use to perform common tasks. However, just as it ruins the value of "this" to alias an instance method, the same is true of an ES6 class's static method. The fix is to always alias the class instead of the method. The new compiler will simply not let us get away with the old way any more, so regressions after this are unlikely. Issue #2528 (compiler upgrade) Issue #2532 (static "this") Change-Id: Id800d466e639c7cbcf4aa6fbb05114c772a2229f

We had made some mistakes in these prefixed names. The corrections were made after consulting both MDN and Apple's docs. This was caught by a compiler upgrade. Issue #2528 Backported to v2.5.x Change-Id: I80994569b04a14b26b3e2fbb2673992ca7e68f50

The latest Closure Compiler is more picky about Event types, and complained about the use of "key" on Event. This updates the Event types so that the compiler has the correct type info. This also stops using "keyCode", which is deprecated, and only uses the current "key" property, which is even supported on IE, and should not create any compatibility issues. Issue #2528 Backported to v2.5.x Change-Id: Ic565772b1cc9597e358df015a73c40ac245edd6a

The latest Closure Compiler initially complained about "initData" not being a known property of "Event". Correcting the type to MediaEncryptedEvent exposed a potential bug, which is that we were assigning Uint8Array in some cases instead of the correct ArrayBuffer. This fixes both the type of the event and converts the Uint8Arrays to ArrayBuffers. This also works around a complaint about "code" on "Error". We use "Error" objects as look-alikes for DOMException because there is no exposed constructor for DOMExceptions. To satisfy the compiler, we use square brackets now to set the "code" field on these DOMException look-alikes. Finally, this removes the "method" field on certain Errors in the WebKit EME polyfill. These must have been leftover from some debugging, and are not used at all. Issue #2528 Backported to v2.5.x Change-Id: I32c4617b14a30c412d5bc532ec17a46fdc1fea1a

The StreamingEngine tests use various fakes that did not have strong type info before. The new Closure Compiler won't allow this, and for good reason. Cleaning up the type info exposed a few ways that we had left out important things or failed to remove methods that are no longer on the types we're faking. We also had some fakes for StreamingEngine tests that were redundant with strongly-typed fakes that already existed in shaka.test. This deduplicates them. The fakes for StreamingEngine are now just instances of the other fakes with specific behavior attached. Issue #2528 Change-Id: I0da67bea462e855fcfcb1b391fe83027ffa70702

joeyparrish added the type: bug Something isn't working correctly label Apr 23, 2020

shaka-bot added this to the v2.6 milestone Apr 23, 2020

joeyparrish self-assigned this Apr 24, 2020

joeyparrish mentioned this issue Apr 27, 2020

Use "this" in static methods #2532

Closed

shaka-bot pushed a commit that referenced this issue Apr 28, 2020

Fix implicit string-to-number conversions

8867db6

This was caught by a compiler upgrade. Issue #2528 Change-Id: Ica262703aa30dce2e59c139c88f3c1e74d5d9e2f

shaka-bot pushed a commit that referenced this issue Apr 28, 2020

Fix type info for TextTrack and Cue types

01970ef

This adds or augments type info for TextTrack, Cue, and related types. This was caught by a compiler upgrade. Issue #2528 Change-Id: I233e46eeaed8d6503702e181c3380d674ddbb499

joeyparrish mentioned this issue Apr 29, 2020

Add a presentationDelay setting for HLS live #2373

Closed

shaka-bot pushed a commit that referenced this issue Apr 29, 2020

Fix implicit string-to-number conversions in tests

e70a516

This was caught by a compiler upgrade. Issue #2528 Change-Id: Iae8c495e58347b5c4184bfcec15ff9a48abe6214

shaka-bot closed this as completed in 1076b78 May 1, 2020

shaka-bot added the archived label Jun 30, 2020

shaka-project locked and limited conversation to collaborators Jun 30, 2020

shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exception in compiled mode playing certain content #2528

Exception in compiled mode playing certain content #2528

joeyparrish commented Apr 23, 2020

joeyparrish commented Apr 29, 2020

Exception in compiled mode playing certain content #2528

Exception in compiled mode playing certain content #2528

Comments

joeyparrish commented Apr 23, 2020

joeyparrish commented Apr 29, 2020