Preload test failure on Safari

[joeyparrish]

Test failure in Safari:

3) goes from drm engine to src equals
     Player Load Graph routing
     TypeError: null is not an object (evaluating 'preloadManager.stopQueuingLatePhaseQueuedOperations') in lib/player.js (line 59043)
_callee14$@lib/player.js:1566:23 <- lib/player.js:59043:31
tryCatch@node_modules/@babel/polyfill/dist/polyfill.js:6473:44
invoke@node_modules/@babel/polyfill/dist/polyfill.js:6702:30
asyncGeneratorStep@test/ui/ui_integration.js:17:[103](https://github.com/shaka-project/shaka-player/actions/runs/7805709988/job/21290543139?pr=6223#step:9:104)
_next@lib/player.js:16:13 <- lib/player.js:56616:212

[theodab]

This exposed an unrelated bug inside the error handler in the load method, which was fixed by #6238.
After fixing that, this is now reporting the true error, as a Shaka Error MEDIA.VIDEO_ERROR (4,,).

[joeyparrish]

The same src= content plays fine in other tests in the same suite. Testing with --browsers Safari --uncompiled --no-build --no-babel --filter '(ignores media source path|goes from drm engine to src equals)' --enable-logging debug --random --seed 1:

With seed 1, which happens to run the failing test first:

  Player Load Graph
    routing
      ✗ goes from drm engine to src equals [Safari 17.3.1 (Mac OS 10.15.7)]
	Shaka Error MEDIA.VIDEO_ERROR (4,,)

    without media source
      ✓ loading ignores media source path [Safari 17.3.1 (Mac OS 10.15.7)]

With seed 2, which happens to run the failing test second:

  Player Load Graph
    without media source
      ✓ loading ignores media source path [Safari 17.3.1 (Mac OS 10.15.7)]
    routing
      ✗ goes from drm engine to src equals [Safari 17.3.1 (Mac OS 10.15.7)]
	Shaka Error MEDIA.VIDEO_ERROR (4,,)

So the order doesn't matter and the failure is consistent.

[joeyparrish]

Adding the following logs to the error handler in lib/player.js:

diff --git a/lib/player.js b/lib/player.js
index 4899b8117..feeba5849 100644
--- a/lib/player.js
+++ b/lib/player.js
@@ -6616,6 +6616,10 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     // Extra error information from Chrome:
     const message = this.video_.error.message;
 
+    const readyState = this.video_.readyState;
+    const src = this.video_.src;
+    console.log('Video error:', {code, extended, message, readyState, src});
+
     return new shaka.util.Error(
         shaka.util.Error.Severity.CRITICAL,
         shaka.util.Error.Category.MEDIA,

Produces:

{code: 4, extended: undefined, message: '', readyState: 0, src: 'test:sintel'}

So we can see that this code in the test setup isn't having the effect we expected:

      const actions = new Map()
          // ...
          .set('src-equals', async () => {
            await player.attach(video, /* initMediaSource= */ false);
            await player.load(SMALL_MP4_CONTENT_URI, 0, 'video/mp4');
          });

[joeyparrish]

It appears that unload() isn't being called at the proper time in Player, if at all. The old load() URI is kept alive, possibly in this.assetUri_, and recycled erroneously into video.src during src= setup.

[joeyparrish]

Something is very wrong. I'm not sure if this is broken because of preload or the pre-work to remove the load graph.

The load() call for the first content is still in-progress when the call for the second content is initiated. The first content's load() sets the asset URI after the second content's load() does so. The second call should not progress until the first call is complete or canceled and an internal unload() operation has both started and completed for the first content.

[joeyparrish]

Setting a small delay in the test function goTo() prevents this bug from triggering. That means we are merely lucky that the test only fails on Safari, and that the interruption/deferment mechanisms in Player are broken in general.

[joeyparrish]

This test when run against v4.7.10 passes:
https://github.com/shaka-project/shaka-player/actions/runs/7995799678/job/21837063214

Safari 17.3.1 (Mac OS 10.15.7): Executed 1 of 2617 (skipped 2616) SUCCESS (1.017 secs / 0.137 secs)
TOTAL: 1 SUCCESS

Compared to the failure in main today:
https://github.com/shaka-project/shaka-player/actions/runs/7995992166/job/21837682100

  Player Load Graph
    routing
      ✗ goes from drm engine to src equals [Safari 17.3.1 (Mac OS 10.15.7)]
	Shaka Error MEDIA.VIDEO_ERROR (4,,)
Safari 17.3.1 (Mac OS 10.15.7): Executed 1 of 2626 (1 FAILED) (skipped 2625) (0.913 secs / 0.026 secs)
TOTAL: 1 FAILED, 0 SUCCESS

These runs were done with browsers set to Safari and filter set to goes from drm engine to src equals (one exact test).

So this issue does not affect any existing release, and is likely caused by preload (which is only in main), rather than the removal of the load graph (which is in v4.6.x and v4.7.x).

[joeyparrish]

An async operation on preloadManager in Player.load() was not using the mutex and interruption detection mechanism. I've audited load() and this was the only operation that was missing the use of the mutex. PR forthcoming.