From 37bf7fe7d87cb0407a76d2e8760e6ca5841d78cc Mon Sep 17 00:00:00 2001
From: Joey Parrish <joeyparrish@google.com>
Date: Tue, 28 Nov 2017 10:03:28 -0800
Subject: [PATCH] Fix race condition between metadata and timeupdate

On Edge & IE, the timing of "loadedmetadata" and "timeupdate" events
is different, and exposed a race condition in the fix for #1105.
This solves the race by canceling any pending "early seek" handling
once "loadedmetadata" fires.

A fix for the fix of issue #1105

Change-Id: I5587a72e12c6b28beb0b3ea36f2665a0f1e39f08
---
 lib/media/playhead.js       |  9 ++++++++-
 test/media/playhead_unit.js | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/lib/media/playhead.js b/lib/media/playhead.js
index 7909bd8692..110bdbd9c0 100644
--- a/lib/media/playhead.js
+++ b/lib/media/playhead.js
@@ -345,9 +345,16 @@ shaka.media.Playhead.prototype.onLoadedMetadata_ = function() {
   var targetTime = this.getStartTime_();
 
   // We are out of the phase in which "early seek" may occur, so stop listening
-  // for the 'timeupdate' event.  See onEarlySeek_() for details.
+  // for the 'timeupdate' event.
   this.eventManager_.unlisten(this.video_, 'timeupdate');
 
+  // Cancel any pending early seek timer.  Cancelation is important because it
+  // resolves a race between the first "timeupdate" and the "loadedmetadata"
+  // event.  On some browsers, "timeupdate" fires right before "loadedmetadata",
+  // which would cause earlySeekTimer_ to fire inappropriately and trigger a
+  // failed assertion.
+  this.earlySeekTimer_.cancel();
+
   if (Math.abs(this.video_.currentTime - targetTime) < 0.001) {
     this.eventManager_.listen(
         this.video_, 'seeking', this.onSeeking_.bind(this));
diff --git a/test/media/playhead_unit.js b/test/media/playhead_unit.js
index a9fcd6f713..afe0fd384c 100644
--- a/test/media/playhead_unit.js
+++ b/test/media/playhead_unit.js
@@ -293,6 +293,32 @@ describe('Playhead', function() {
       expect(playhead.getTime()).toBe(30);
     });
 
+    it('does not treat timeupdate as seek close to metadata load', function() {
+      playhead = new shaka.media.Playhead(
+          video,
+          manifest,
+          config,
+          5 /* startTime */,
+          Util.spyFunc(onSeek),
+          Util.spyFunc(onEvent));
+
+      expect(video.addEventListener).toHaveBeenCalledWith(
+          'loadedmetadata', jasmine.any(Function), false);
+
+      expect(playhead.getTime()).toBe(5);
+      expect(video.currentTime).toBe(0);
+
+      // Realism: Edge fires "timeupdate" right before "loadedmetadata".
+      // This was causing a failed assertion in onEarlySeek_();
+      video.currentTime = 5.001;
+      video.on['timeupdate']();
+      video.readyState = HTMLMediaElement.HAVE_METADATA;
+      video.on['loadedmetadata']();
+
+      // Delay to let Playhead batch up changes to currentTime and observe.
+      jasmine.clock().tick(1000);
+    });
+
     // This is important for recovering from drift.
     // See: https://github.com/google/shaka-player/issues/1105
     // TODO: Re-evaluate after https://github.com/google/shaka-player/issues/999