Fix potential memory leak in SegmentIndex

SegmentIndex has had a destroy() method for a long time, but it has
never been called.  Now that SegmentIndex has a timer which adds new
references for DASH SegmentTemplate live streams, it is more important
than ever to properly stop this active part of SegmentIndex.

This change replaces async destroy() with synchronous release() and
calls it from Player when the manifest is being disposed of.  This
will ensure that SegmentIndexes don't grow out of control after
content is unloaded.

This would not have affected v2.5, since we didn't have this
timer-driven growth of DASH SegmentTemplate live streams in that
release branch.

Related to #1339 (fixes issues introduced for period flattening)

Change-Id: I419a06a62eaa507d92132e20d4cc2d6e45c83ff2

lib/media/segment_index.js

@@ -8,14 +8,14 @@ goog.provide('shaka.media.SegmentIterator');
 
 goog.require('goog.asserts');
 goog.require('shaka.media.SegmentReference');
-goog.require('shaka.util.IDestroyable');
+goog.require('shaka.util.IReleasable');
 goog.require('shaka.util.Timer');
 
 
 /**
  * SegmentIndex.
  *
- * @implements {shaka.util.IDestroyable}
+ * @implements {shaka.util.IReleasable}
  * @implements {Iterable.<!shaka.media.SegmentReference>}
  * @export
  */
@@ -50,15 +50,13 @@ shaka.media.SegmentIndex = class {
    * @override
    * @export
    */
-  destroy() {
+  release() {
     this.references_ = [];
 
     if (this.timer_) {
       this.timer_.stop();
     }
     this.timer_ = null;
-
-    return Promise.resolve();
   }
 
 

lib/player.js

@@ -1346,6 +1346,24 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     this.assetUri_ = null;
     this.bufferObserver_ = null;
     this.loadingTextStreams_.clear();
+
+    if (this.manifest_) {
+      for (const period of this.manifest_.periods) {
+        for (const variant of period.variants) {
+          for (const stream of [variant.audio, variant.video]) {
+            if (stream && stream.segmentIndex) {
+              stream.segmentIndex.release();
+            }
+          }
+        }
+        for (const stream of period.textStreams) {
+          if (stream.segmentIndex) {
+            stream.segmentIndex.release();
+          }
+        }
+      }
+    }
+
     this.manifest_ = null;
     this.stats_ = new shaka.util.Stats(); // Replace with a clean stats object.
     this.lastTextFactory_ = null;

test/player_unit.js

@@ -159,6 +159,26 @@ describe('Player', () => {
       expect(playhead.release).toHaveBeenCalled();
       expect(mediaSourceEngine.destroy).toHaveBeenCalled();
       expect(streamingEngine.destroy).toHaveBeenCalled();
+
+      const segmentIndexes = [];
+      for (const period of manifest.periods) {
+        for (const variant of period.variants) {
+          if (variant.audio) {
+            segmentIndexes.push(variant.audio.segmentIndex);
+          }
+          if (variant.video) {
+            segmentIndexes.push(variant.video.segmentIndex);
+          }
+        }
+        for (const textStream of period.textStreams) {
+          segmentIndexes.push(textStream.segmentIndex);
+        }
+      }
+      for (const segmentIndex of segmentIndexes) {
+        if (segmentIndex) {
+          expect(segmentIndex.release).toHaveBeenCalled();
+        }
+      }
     });
 
     it('destroys mediaSourceEngine before drmEngine', async () => {

test/test/util/simple_fakes.js

@@ -427,8 +427,7 @@ shaka.test.FakeClosedCaptionParser = class {
 shaka.test.FakeSegmentIndex = class {
   constructor() {
     /** @type {!jasmine.Spy} */
-    this.destroy =
-        jasmine.createSpy('destroy').and.returnValue(Promise.resolve());
+    this.release = jasmine.createSpy('release');
 
     /** @type {!jasmine.Spy} */
     this.find = jasmine.createSpy('find').and.returnValue(null);