Clean-up Downloading Groups

Went through the download code and tried to make it easier to read
and understand.

Issue #1248

Change-Id: I74316d3337248f0d16f2fb6cc1734e1e57739966

lib/offline/download_manager.js

@@ -67,8 +67,8 @@ shaka.offline.DownloadManager = function(
   /** @private {?shakaExtern.ManifestDB} */
   this.manifest_ = null;
 
-  /** @private {Promise} */
-  this.promise_ = null;
+  /** @private {!Promise} */
+  this.promise_ = Promise.resolve();
 
   /** @private {number} */
   this.downloadExpected_ = 0;
@@ -78,6 +78,9 @@ shaka.offline.DownloadManager = function(
 
   /** @private {!Array.<function(number, number)>} */
   this.progressListeners_ = [];
+
+  /** @private {boolean} */
+  this.destroyed_ = false;
 };
 
 
@@ -116,23 +119,26 @@ shaka.offline.DownloadManager.Segment;
 shaka.offline.DownloadManager.prototype.destroy = function() {
   let storage = this.storageEngine_;
   let segments = this.storedSegmentIds_;
-  let p = this.promise_ || Promise.resolve();
 
   // Don't try to remove segments if there are none.  That may trigger an error
   // in storage if the DB connection was never created.
   if (segments.length) {
-    p = p.then(function() { return storage.removeSegments(segments, null); });
+    this.promise_ = this.promise_.then(() => {
+      return storage.removeSegments(segments, null);
+    });
   }
 
   // Don't destroy() storageEngine since it is owned by Storage.
+  let p = this.promise_;
 
   this.groups_ = {};
   this.storedSegmentIds_ = [];
   this.storageEngine_ = null;
   this.netEngine_ = null;
   this.retryParams_ = null;
   this.manifest_ = null;
-  this.promise_ = null;
+  this.promise_ = Promise.resolve();
+
   return p;
 };
 
@@ -166,7 +172,7 @@ shaka.offline.DownloadManager.prototype.addSegment = function(
  * manifest object.
  *
  * @param {shakaExtern.ManifestDB} manifest
- * @return {!Promise<number>}
+ * @return {!Promise.<number>}
  */
 shaka.offline.DownloadManager.prototype.downloadAndStore = function(manifest) {
   // Clear any old progress.
@@ -182,86 +188,113 @@ shaka.offline.DownloadManager.prototype.downloadAndStore = function(manifest) {
     segments.forEach((segment) => this.markAsPending_(segment));
   });
 
-  // Create separate download chains for different content types.  This will
-  // allow audio and video to be downloaded in parallel.
-  let async = groups.map((segments) => {
-    let i = 0;
-    let downloadNext = (() => {
-      if (!this.manifest_) {
-        return Promise.reject(new shaka.util.Error(
-            shaka.util.Error.Severity.CRITICAL,
-            shaka.util.Error.Category.STORAGE,
-            shaka.util.Error.Code.OPERATION_ABORTED));
-      }
-      if (i >= segments.length) return Promise.resolve();
-      let segment = segments[i++];
-      return this.downloadSegment_(segment).then(downloadNext);
-    });
-    return downloadNext();
-  });
-
-  this.promise_ = Promise.all(async).then(() => {
+  /** @type {!Promise.<number>} */
+  let p = Promise.resolve().then(() => {
+    this.checkDestroyed_();
+    return Promise.all(groups.map((group) => this.downloadGroup_(group)));
+  }).then(() => {
+    this.checkDestroyed_();
     return this.storageEngine_.addManifest(manifest);
   }).then((id) => {
+    this.checkDestroyed_();
     this.storedSegmentIds_ = [];
     return id;
   });
-  return this.promise_;
+
+  // Amend our new promise chain to our internal promise so that when we destroy
+  // the download manger we will wait for all the downloads to stop.
+  this.promise_ = this.promise_.then(() => p);
+
+  return p;
+};
+
+
+/**
+ * @param {!Array.<shaka.offline.DownloadManager.Segment>} group
+ * @return {!Promise}
+ * @private
+ */
+shaka.offline.DownloadManager.prototype.downloadGroup_ = function(group) {
+  let p = Promise.resolve();
+
+  group.forEach((segment) => {
+    p = p.then(() => {
+      this.checkDestroyed_();
+      return this.downloadSegment_(segment);
+    });
+  });
+
+  return p;
 };
 
 
 /**
- * Downloads the given segment and calls the callback.
- *
  * @param {shaka.offline.DownloadManager.Segment} segment
  * @return {!Promise}
  * @private
  */
 shaka.offline.DownloadManager.prototype.downloadSegment_ = function(segment) {
-  goog.asserts.assert(this.retryParams_,
-                      'DownloadManager must not be destroyed');
-  const type = shaka.net.NetworkingEngine.RequestType.SEGMENT;
-  let request =
-      shaka.net.NetworkingEngine.makeRequest(segment.uris, this.retryParams_);
+  return Promise.resolve().then(() => {
+    this.checkDestroyed_();
+
+    let type = shaka.net.NetworkingEngine.RequestType.SEGMENT;
+    let request = this.createRequest_(segment);
+    return this.netEngine_.request(type, request).promise;
+  }).then((response) => {
+    this.checkDestroyed_();
+
+    // Update progress for the download (won't include the "store" part).
+    this.manifest_.size += response.data.byteLength;
+    this.markAsDone_(segment);
+    this.updateProgress_();
+
+    return this.storageEngine_.addSegment({
+      data: response.data
+    });
+  }).then((id) => {
+    this.checkDestroyed_();
+
+    this.storedSegmentIds_.push(id);
+    segment.onStore(id);
+  });
+};
+
+
+/**
+ * @param {!shaka.offline.DownloadManager.Segment} segment
+ * @return {shakaExtern.Request}
+ * @private
+ */
+shaka.offline.DownloadManager.prototype.createRequest_ = function(segment) {
+  goog.asserts.assert(
+      this.retryParams_,
+      'DownloadManager must not be destroyed');
+
+  let request = shaka.net.NetworkingEngine.makeRequest(
+      segment.uris,
+      this.retryParams_);
+
   if (segment.startByte != 0 || segment.endByte != null) {
     let end = segment.endByte == null ? '' : segment.endByte;
     request.headers['Range'] = 'bytes=' + segment.startByte + '-' + end;
   }
 
-  let byteCount;
-  return this.netEngine_.request(type, request)
-      .then(function(response) {
-        if (!this.manifest_) {
-          return Promise.reject(new shaka.util.Error(
-              shaka.util.Error.Severity.CRITICAL,
-              shaka.util.Error.Category.STORAGE,
-              shaka.util.Error.Code.OPERATION_ABORTED));
-        }
-        byteCount = response.data.byteLength;
-
-        /** @type {shakaExtern.SegmentDataDB} */
-        let segmentDb = {
-          data: response.data
-        };
-
-        return this.storageEngine_.addSegment(segmentDb);
-      }.bind(this))
-      .then(function(id) {
-        if (!this.manifest_) {
-          return Promise.reject(new shaka.util.Error(
-              shaka.util.Error.Severity.CRITICAL,
-              shaka.util.Error.Category.STORAGE,
-              shaka.util.Error.Code.OPERATION_ABORTED));
-        }
-
-        this.manifest_.size += byteCount;
-
-        this.markAsDone_(segment);
-        this.storedSegmentIds_.push(id);
-        segment.onStore(id);
-
-        this.updateProgress_();
-      }.bind(this));
+  return request;
+};
+
+
+/**
+ * Check if the download manager has been destroyed. If so, throw an error to
+ * kill the promise chain.
+ * @private
+ */
+shaka.offline.DownloadManager.prototype.checkDestroyed_ = function() {
+  if (this.destroyed_) {
+    throw new shaka.util.Error(
+        shaka.util.Error.Severity.CRITICAL,
+        shaka.util.Error.Category.STORAGE,
+        shaka.util.Error.Code.OPERATION_ABORTED);
+  }
 };