Isolate Download Estimation

Isolate the download estimation code out of DownloadManager so that it
can be tested in isolation and allow the code in DownloadManager to
focus on downloading.

Change-Id: I509adeaf50acb77bfaa9b2128f730de2653eda81

build/types/offline

@@ -1,6 +1,7 @@
 # The offline storage system and manifest parser plugin.
 
 +../../lib/offline/download_manager.js
++../../lib/offline/download_progress_estimator.js
 +../../lib/offline/indexeddb/db_connection.js
 +../../lib/offline/indexeddb/db_operation.js
 +../../lib/offline/indexeddb/eme_session_storage_cell.js

lib/offline/download_manager.js

@@ -19,6 +19,7 @@ goog.provide('shaka.offline.DownloadManager');
 
 goog.require('goog.asserts');
 goog.require('shaka.net.NetworkingEngine');
+goog.require('shaka.offline.DownloadProgressEstimator');
 goog.require('shaka.util.Error');
 goog.require('shaka.util.IDestroyable');
 
@@ -63,29 +64,8 @@ shaka.offline.DownloadManager = class {
      */
     this.onProgress_ = onProgress;
 
-    /**
-     * We track progress using the estimated size (not the actual size) since
-     * the denominator (current / total) will be based on estimates.
-     *
-     * @private {number}
-     */
-    this.downloadedEstimatedBytes_ = 0;
-
-    /**
-     * When we queue a segment, the estimated size is added to this value. This
-     * is used to track progress (downloaded / expected).
-     *
-     * @private {number}
-     */
-    this.expectedEstimatedBytes_ = 0;
-
-    /**
-     * When a segment is downloaded, the actual size of the segment is added to
-     * this value. We use this to know how large the final asset is.
-     *
-     * @private {number}
-     */
-    this.downloadedBytes_ = 0;
+    /** @private {shaka.offline.DownloadProgressEstimator} */
+    this.estimator_ = new shaka.offline.DownloadProgressEstimator();
   }
 
   /** @override */
@@ -114,8 +94,7 @@ shaka.offline.DownloadManager = class {
         !this.destroyed_,
         'Do not call |queue| after |destroy|');
 
-    // Update our estimate.
-    this.expectedEstimatedBytes_ += estimatedByteLength;
+    const id = this.estimator_.open(estimatedByteLength);
 
     const group = this.groups_.get(groupId) || Promise.resolve();
 
@@ -132,15 +111,10 @@ shaka.offline.DownloadManager = class {
       }
 
       // Update all our internal stats.
-      this.downloadedEstimatedBytes_ += estimatedByteLength;
-      this.downloadedBytes_ += response.byteLength;
-
-      const progress =
-          this.expectedEstimatedBytes_ ?
-          this.downloadedEstimatedBytes_ / this.expectedEstimatedBytes_ :
-          0;
-
-      this.onProgress_(progress, this.downloadedBytes_);
+      this.estimator_.close(id, response.byteLength);
+      this.onProgress_(
+          this.estimator_.getEstimatedProgress(),
+          this.estimator_.getTotalDownloaded());
 
       return onDownloaded(response);
     }));
@@ -154,7 +128,7 @@ shaka.offline.DownloadManager = class {
    */
   async waitToFinish() {
     await Promise.all(this.groups_.values());
-    return this.downloadedBytes_;
+    return this.estimator_.getTotalDownloaded();
   }
 
   /**

lib/offline/download_progress_estimator.js

@@ -0,0 +1,137 @@
+/**
+ * @license
+ * Copyright 2016 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+goog.provide('shaka.offline.DownloadProgressEstimator');
+
+
+/**
+ * The download progress estimator class encapsulates all the logic for tracking
+ * how much content has been downloaded and estimating its progress.
+ *
+ * @final
+ */
+shaka.offline.DownloadProgressEstimator = class {
+  constructor() {
+    /**
+     * This is the sum of all estimates passed to |open|. This is used as the
+     * denominator when measuring progress.
+     *
+     * @private {number}
+     */
+    this.estimatedTotal_ = 0;
+
+    /**
+     * This is the sum of all estimates pass to |open| but only after |close|
+     * has been called. This is used as the numerator when measuring progress so
+     * that |estimatedTotal_ == estimatedDownloaded_| after everything is
+     * downloaded.
+     *
+     * @private {number}
+     */
+    this.estimatedDownloaded_ = 0;
+
+    /**
+     * This is the total number of bytes actually downloaded. This will most
+     * likely differ from |estimatedTotal_| after everything is downloaded since
+     * our estimates will be off.
+     *
+     * @private {number}
+     */
+    this.actualDownloaded_ = 0;
+
+    /**
+     * This is a map of all pending downloads. This maps their download id (an
+     * internal id) to the estimate. This will allow us to update
+     * |estimatedDownloaded_| when |close| is called.
+     *
+     * @private {!Map.<number, number>}
+     */
+    this.pending_ = new Map();
+
+    /**
+     * This number is used to provide unique (to estimator) ids for each
+     * download. This allows us to track each download in |pending_|.
+     *
+     * @private {number}
+     */
+    this.nextId_ = 0;
+  }
+
+  /**
+   * Open a new download in the progress estimator. This will create an entry so
+   * that we can track the download progress.
+   *
+   * This will return an id for the download. This id must be passed to |close|
+   * in order for the |close| to be paired with this call to |open|.
+   *
+   * @param {number} estimate
+   * @return {number}
+   */
+  open(estimate) {
+    this.estimatedTotal_ += estimate;
+
+    const id = this.nextId_;
+    this.nextId_++;
+
+    this.pending_.set(id, estimate);
+    return id;
+  }
+
+  /**
+   * Close a download in the estimator. This will signal that we have finished
+   * downloading a segment and we can update the progress estimate.
+   *
+   * @param {number} id
+   * @param {number} actual
+   */
+  close(id, actual) {
+    if (!this.pending_.has(id)) {
+      return;
+    }
+
+    const estimate = this.pending_.get(id);
+    this.pending_.delete(id);
+
+    this.estimatedDownloaded_ += estimate;
+    this.actualDownloaded_ += actual;
+  }
+
+  /**
+   * Get the current estimate for how much progress we've made downloading the
+   * content. Progress will be between 0 and 1.
+   *
+   * Depending on the order of calls to |open| and |close|,
+   * |getEstimatedProgress| will fluctuate and is not guaranteed to always be
+   * increasing.
+   *
+   * @return {number}
+   */
+  getEstimatedProgress() {
+    return this.estimatedTotal_ == 0 ?
+           0 :
+           this.estimatedDownloaded_ / this.estimatedTotal_;
+  }
+
+  /**
+   * Get the total number of bytes that were actually downloaded.
+   *
+   * @return {number}
+   */
+  getTotalDownloaded() {
+    return this.actualDownloaded_;
+  }
+};

test/offline/download_progress_estimator.js

@@ -0,0 +1,101 @@
+/**
+ * @license
+ * Copyright 2016 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+describe('DownloadProgressEstimator', () => {
+  /** @type {!shaka.offline.DownloadProgressEstimator} */
+  let estimator;
+
+  beforeEach(() => {
+    estimator = new shaka.offline.DownloadProgressEstimator();
+  });
+
+  it('closing updates total download', () => {
+    const id = estimator.open(/* estimate= */ 12);
+    estimator.close(id, /* actual= */ 13);
+
+    expect(estimator.getTotalDownloaded()).toBe(13);
+  });
+
+  // If we call |close| with an id that was not returned by |open|, then it
+  // should be ignored.
+  it('closing with invalid id is ignored', () => {
+    expect(estimator.getTotalDownloaded()).toBe(0);
+    estimator.close(72, /* actual= */ 13);
+    expect(estimator.getTotalDownloaded()).toBe(0);
+  });
+
+  it('closing the same id twice has no effect', () => {
+    expect(estimator.getEstimatedProgress()).toBe(0);
+
+    estimator.open(/* estimate= */ 15);
+    const id = estimator.open(/* estimate= */ 5);
+
+    expect(estimator.getEstimatedProgress()).toBe(0);
+
+    estimator.close(id, /* actual= */ 5);
+    expect(estimator.getEstimatedProgress()).toBeCloseTo(0.25);
+    expect(estimator.getTotalDownloaded()).toBe(5);
+
+    // We will close |id| again, but nothing should change. We will even say
+    // the downloaded size is different, but that should not change anything.
+    estimator.close(id, /* actual= */ 200);
+    expect(estimator.getEstimatedProgress()).toBeCloseTo(0.25);
+    expect(estimator.getTotalDownloaded()).toBe(5);
+  });
+
+  // Calling both |open| and |close| should affect our progress. Calling |open|
+  // should cause our progress to grow smaller (we are saying there is more work
+  // to do) and calling |close| should cause our progress to grow larger (we are
+  // saying we completed work).
+  it('opening and closing updates progress', () => {
+    expect(estimator.getEstimatedProgress()).toBe(0);
+
+    const id0 = estimator.open(/* estimate= */ 5);
+    const id1 = estimator.open(/* estimate= */ 15);
+
+    expect(estimator.getEstimatedProgress()).toBe(0);
+
+    estimator.close(id0, /* actual= */ 5);
+    expect(estimator.getEstimatedProgress()).toBeCloseTo(0.25);
+
+    estimator.close(id1, /* actual= */ 15);
+    expect(estimator.getEstimatedProgress()).toBeCloseTo(1.0);
+
+    // Open a new estimate with a size equal to all previous estimates. Since
+    // everything else was closed, this should drop our progress from 100% to
+    // 50%.
+    estimator.open(/* estimate= */ 20);
+    expect(estimator.getEstimatedProgress()).toBeCloseTo(0.5);
+  });
+
+  // When tracking progress, we want to use the estimated size. This means no
+  // matter what value we provide for the actual downloaded size, it should not
+  // affect the progress (the original estimate should be used).
+  it('actual bytes do not affect progres', () => {
+    expect(estimator.getEstimatedProgress()).toBe(0);
+
+    estimator.open(/* estimate= */ 15);
+    const id = estimator.open(/* estimate= */ 5);
+
+    expect(estimator.getEstimatedProgress()).toBe(0);
+
+    // Use an actual value so large that if used for progress, it would break
+    // the progress value bounds.
+    estimator.close(id, /* actual= */ 200);
+    expect(estimator.getEstimatedProgress()).toBeCloseTo(0.25);
+  });
+});