Replacing Cursor Delete In DBEngine

After looking into the speeds for removing data from IndexedDB, it was
found that using a cursor was slower than the other methods. This change
takes the remove logic and changes it to remove each key as part of
one transaction.

Closes #756

Change-Id: Iec3916650d8a4fe2b6353b604c070d8a0af7426c

lib/offline/db_engine.js

@@ -150,61 +150,53 @@ shaka.offline.DBEngine.prototype.destroy = function() {
 
 /** @override */
 shaka.offline.DBEngine.prototype.get = function(storeName, key) {
-  return this.createOperation_(storeName, 'readonly', function(store) {
-    return store.get(key);
-  });
+  var request;
+  return this.createTransaction_(storeName, 'readonly', function(store) {
+    request = store.get(key);
+  }).then(function() { return request.result; });
 };
 
 
 /** @override */
 shaka.offline.DBEngine.prototype.forEach = function(storeName, callback) {
-  return this.createOperation_(storeName, 'readonly', function(store) {
-    return store.openCursor();
-  }, function(/** IDBCursor */ cursor) {
-    if (!cursor) return;
-
-    callback(cursor.value);
-    cursor.continue();
+  return this.createTransaction_(storeName, 'readonly', function(store) {
+    var request = store.openCursor();
+    request.onsuccess = function(event) {
+      var cursor = event.target.result;
+      if (cursor) {
+        callback(cursor.value);
+        cursor.continue();
+      }
+    };
   });
 };
 
 
 /** @override */
 shaka.offline.DBEngine.prototype.insert = function(storeName, value) {
-  return this.createOperation_(storeName, 'readwrite', function(store) {
-    return store.put(value);
+  return this.createTransaction_(storeName, 'readwrite', function(store) {
+    store.put(value);
   });
 };
 
 
 /** @override */
 shaka.offline.DBEngine.prototype.remove = function(storeName, key) {
-  return this.createOperation_(storeName, 'readwrite', function(store) {
-    return store.delete(key);
+  return this.createTransaction_(storeName, 'readwrite', function(store) {
+    store.delete(key);
   });
 };
 
 
 /** @override */
-shaka.offline.DBEngine.prototype.removeWhere = function(storeName, callback) {
-  var async = [];
-  return this.createOperation_(storeName, 'readwrite', function(store) {
-    return store.openCursor();
-  }, function(/** IDBCursor */ cursor) {
-    if (!cursor) return;
-
-    if (callback(cursor.value)) {
-      var request = cursor.delete();
-      var p = new shaka.util.PublicPromise();
-      request.onsuccess = p.resolve;
-      request.onerror = shaka.offline.DBEngine.onError_.bind(null, request, p);
-      async.push(p);
+shaka.offline.DBEngine.prototype.removeKeys = function(storeName,
+                                                       keys,
+                                                       opt_onKeyRemoved) {
+  return this.createTransaction_(storeName, 'readwrite', function(store) {
+    for (var i = 0; i < keys.length; i++) {
+      var request = store.delete(keys[i]);
+      request.onsuccess = opt_onKeyRemoved || function(event) { };
     }
-    cursor.continue();
-  }).then(function() {
-    return Promise.all(async);
-  }).then(function() {
-    return async.length;
   });
 };
 
@@ -225,75 +217,85 @@ shaka.offline.DBEngine.prototype.reserveId = function(storeName) {
  * @private
  */
 shaka.offline.DBEngine.prototype.getNextId_ = function(storeName) {
-  var ret = 0;
-  return this.createOperation_(storeName, 'readonly', function(store) {
-    return store.openCursor(null, 'prev');
-  }, function(/** IDBCursor */ cursor) {
-    if (cursor)
-      ret = cursor.key + 1;
-  }).then(function() { return ret; });
+  var id = 0;
+  return this.createTransaction_(storeName, 'readonly', function(store) {
+    var request = store.openCursor(null, 'prev');
+    request.onsuccess = function(event) {
+      var cursor = event.target.result;
+      if (cursor) {
+        id = cursor.key + 1;
+      }
+    };
+  }).then(function() { return id; });
 };
 
 
 /**
- * Creates a new transaction for the given store name and calls the given
- * callback to create a request.  It then wraps the given request in an
- * operation and returns the resulting promise.  The Promise resolves when
- * the transaction is complete, which will be after opt_success is called.
+ * Creates a new transaction for the given store name and calls |action| to
+ * modify the store. The transaction will resolve or reject the promise
+ * returned by this function.
  *
  * @param {string} storeName
  * @param {string} type
- * @param {function(!IDBObjectStore):!IDBRequest} createRequest
- * @param {(function(*))=} opt_success The value of onsuccess for the request.
+ * @param {!function(IDBObjectStore)} action
+ *
  * @return {!Promise}
  * @private
  */
-shaka.offline.DBEngine.prototype.createOperation_ = function(
-    storeName, type, createRequest, opt_success) {
+shaka.offline.DBEngine.prototype.createTransaction_ = function(storeName,
+                                                               type,
+                                                               action) {
+
   goog.asserts.assert(this.db_, 'Must not be destroyed');
   goog.asserts.assert(type == 'readonly' || type == 'readwrite',
                       'Type must be "readonly" or "readwrite"');
 
-  var trans = this.db_.transaction([storeName], type);
-  var request = createRequest(trans.objectStore(storeName));
-
-  var p = new shaka.util.PublicPromise();
-  if (opt_success)
-    request.onsuccess = function(event) { opt_success(event.target.result); };
-  request.onerror = function(event) {
-    shaka.log.info('The request unexpectedly failed',
-                   request.error ? request.error.message : 'unknown error');
+  var op = {
+    transaction: this.db_.transaction([storeName], type),
+    promise: new shaka.util.PublicPromise()
   };
 
-  var op = {transaction: trans, promise: p};
-  this.operations_.push(op);
-
-  // Only remove the transaction once it has completed, which may be after the
-  // request is complete (e.g. it may need to write to disk).
-  var removeOp = (function() {
-    var i = this.operations_.indexOf(op);
-    goog.asserts.assert(i >= 0, 'Operation must be in the list.');
-    this.operations_.splice(i, 1);
+  op.transaction.oncomplete = (function(event) {
+    this.closeOperation_(op);
+    op.promise.resolve();
   }.bind(this));
-  trans.oncomplete = function(event) {
-    removeOp();
-    p.resolve(request.result);
-  };
-  trans.onerror = function(event) {
-    // Otherwise Firefox will raise an error which will cause a karma failure.
-    // This will not stop the onabort callback from firing.
-    event.preventDefault();
-  };
+
   // We will see an onabort call via:
   //   1. request error -> transaction error -> transaction abort
   //   2. transaction commit fail -> transaction abort
   // As any transaction error will result in an abort, it is better to listen
   // for an abort so that we will catch all failed transaction operations.
-  trans.onabort = function(event) {
-    removeOp();
-    shaka.offline.DBEngine.onError_(request, p, event);
-  };
-  return p;
+  op.transaction.onabort = (function(event) {
+    this.closeOperation_(op);
+    shaka.offline.DBEngine.onError_(op.transaction, op.promise, event);
+  }.bind(this));
+
+  // We need to prevent default on the onerror event or else Firefox will
+  // raise an error which will cause a karma failure. This will not stop the
+  // onabort callback from firing.
+  op.transaction.onerror = (function(event) {
+    event.preventDefault();
+  }.bind(this));
+
+  var store = op.transaction.objectStore(storeName);
+  action(store);
+
+  this.operations_.push(op);
+
+  return op.promise;
+};
+
+
+/**
+ * Close an open operation.
+ *
+ * @param {!shaka.offline.DBEngine.Operation} op
+ * @private
+ */
+shaka.offline.DBEngine.prototype.closeOperation_ = function(op) {
+  var i = this.operations_.indexOf(op);
+  goog.asserts.assert(i >= 0, 'Operation must be in the list.');
+  this.operations_.splice(i, 1);
 };
 
 
@@ -357,27 +359,25 @@ shaka.offline.DBEngine.prototype.createConnection_ = function(
 
 
 /**
- * Rejects the given Promise with an unknown error.
+ * Rejects the given Promise using the error fromt the transaction.
  *
- * @param {!IDBRequest} request
+ * @param {!IDBTransaction|!IDBRequest} errorSource
  * @param {!shaka.util.PublicPromise} promise
  * @param {Event} event
  * @private
  */
-shaka.offline.DBEngine.onError_ = function(request, promise, event) {
-  // |request.error| can be null as per 3.3.4 point 2 of the W3 spec. When a
-  // request does not have an error and is passed here it is because it was from
-  // |transaction.onabort| and therefore not an actual error.
-  if (!request.error || request.error.name == 'AbortError') {
+shaka.offline.DBEngine.onError_ = function(errorSource, promise, event) {
+
+  if (errorSource.error) {
     promise.reject(new shaka.util.Error(
         shaka.util.Error.Severity.CRITICAL,
         shaka.util.Error.Category.STORAGE,
-        shaka.util.Error.Code.OPERATION_ABORTED));
+        shaka.util.Error.Code.INDEXED_DB_ERROR, errorSource.error));
   } else {
     promise.reject(new shaka.util.Error(
         shaka.util.Error.Severity.CRITICAL,
         shaka.util.Error.Category.STORAGE,
-        shaka.util.Error.Code.INDEXED_DB_ERROR, request.error));
+        shaka.util.Error.Code.OPERATION_ABORTED));
   }
 
   // Firefox will raise an error which will cause a karma failure.

lib/offline/download_manager.js

@@ -127,11 +127,8 @@ shaka.offline.DownloadManager.prototype.destroy = function() {
   var storage = this.storageEngine_;
   var segments = this.storedSegments_;
   var p = this.promise_ || Promise.resolve();
-  p = p.then(function() {
-    return Promise.all(segments.map(function(id) {
-      return storage.remove('segment', id);
-    }));
-  });
+  p = p.then(function() { return storage.removeKeys('segment', segments); });
+
   // Don't destroy() storageEngine since it is owned by Storage.
 
   this.segments_ = {};

lib/offline/i_storage_engine.js

@@ -101,14 +101,14 @@ shaka.offline.IStorageEngine.prototype.remove;
 
 
 /**
- * Removes all items for which the given predicate returns true.
+ * Removes the elements with the given keys.
  *
  * @param {string} storeName
- * @param {function(T):boolean} callback
- * @return {!Promise.<number>}
- * @template T
+ * @param {!Array<number>} keys
+ * @param {function()=} opt_onKeyRemoved
+ * @return {!Promise}
  */
-shaka.offline.IStorageEngine.prototype.removeWhere;
+shaka.offline.IStorageEngine.prototype.removeKeys;
 
 
 /**

lib/offline/storage.js

@@ -321,14 +321,12 @@ shaka.offline.Storage.prototype.remove = function(content) {
     var deleteCount = 0;
     var segmentCount = segments.length;
     var callback = this.config_.progressCallback;
-    return this.storageEngine_.removeWhere('segment', function(segment) {
-      var i = segments.indexOf(segment.key);
-      if (i >= 0) {
-        callback(content, deleteCount / segmentCount);
-        deleteCount++;
-      }
-      return i >= 0;
-    }.bind(this));
+
+    return this.storageEngine_.removeKeys('segment', segments, function() {
+      deleteCount++;
+      callback(content, deleteCount / segmentCount);
+    });
+
   }.bind(this)).then(function() {
     this.checkDestroyed_();
     this.config_.progressCallback(content, 1);

test/offline/db_engine_unit.js

@@ -113,7 +113,7 @@ describe('DBEngine', function() {
       return db.get('test', 2);
     }).then(function(data) {
       expect(data).toBeFalsy();
-      return db.removeWhere('test', function(s) { return s.i >= 7; });
+      return db.removeKeys('test', [4, 5, 6]);
     }).then(function() {
       return db.get('test', 5);
     }).then(function(data) {
@@ -221,12 +221,11 @@ describe('DBEngine', function() {
     // the transaction will abort. This should cause the promise to be
     // rejected.
     db.insert = function(storeName, value) {
-      return this.createOperation_(storeName, 'readwrite', function(store) {
+      return this.createTransaction_(storeName, 'readwrite', function(store) {
         var request = store.put(value);
         request.onsuccess = function(event) {
           request.transaction.abort();
         };
-        return request;
       });
     };
 

test/test/util/memory_db_engine.js

@@ -105,12 +105,15 @@ shaka.test.MemoryDBEngine.prototype.remove = function(storeName, key) {
 
 
 /** @override */
-shaka.test.MemoryDBEngine.prototype.removeWhere = function(
-    storeName, predicate) {
+shaka.test.MemoryDBEngine.prototype.removeKeys = function(storeName,
+                                                          keys,
+                                                          opt_onKeyRemoved) {
   var store = this.getStore_(storeName);
   for (var key in store) {
-    if (predicate(store[Number(key)]))
-      delete store[Number(key)];
+    key = Number(key);
+    if (keys.indexOf(key) >= 0) {
+      delete store[key];
+    }
   }
   return Promise.resolve();
 };