[IndexedDB] Fixed DisjointRangeLockManager usage in IndexedDB

IndexedDBFactoryImpl isn't scoped per origin and so can be used by
multiple origins. The original code assumed that it was scoped per
origin, so when in actuality it was used by multiple origins, when a
single origin requested a lock this could interrupt other origins in
certain cases.

https://docs.google.com/document/d/1jX6OOQJ0aLrWAeqYLtARXPFawUlJPbHNIKAK-0vmFHU/edit#heading=h.6cqn78xajqmp

R=cmp@chromium.org

Bug: 934790
Change-Id: I10f3e7c1b43ad7310d8824648575d940568b534d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1487369
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Chase Phillips <cmp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638260}

content/browser/indexed_db/indexed_db_active_blob_registry_unittest.cc

@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/macros.h"
+#include "base/test/scoped_task_environment.h"
 #include "base/test/test_simple_task_runner.h"
 #include "content/browser/indexed_db/indexed_db_active_blob_registry.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
@@ -115,6 +116,7 @@ class IndexedDBActiveBlobRegistryTest : public testing::Test {
   IndexedDBActiveBlobRegistry* registry() const { return registry_.get(); }
 
  private:
+  base::test::ScopedTaskEnvironment task_environment_;
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   scoped_refptr<RegistryTestMockFactory> factory_;
   scoped_refptr<MockIDBBackingStore> backing_store_;

content/browser/indexed_db/indexed_db_backing_store.h

@@ -30,6 +30,7 @@
 #include "content/browser/indexed_db/indexed_db_pre_close_task_queue.h"
 #include "content/browser/indexed_db/leveldb/leveldb_iterator.h"
 #include "content/browser/indexed_db/leveldb/leveldb_transaction.h"
+#include "content/browser/indexed_db/scopes/disjoint_range_lock_manager.h"
 #include "content/common/content_export.h"
 #include "storage/browser/blob/blob_data_handle.h"
 #include "third_party/blink/public/common/indexeddb/indexeddb_key.h"
@@ -572,6 +573,8 @@ class CONTENT_EXPORT IndexedDBBackingStore
     return weak_factory_.GetWeakPtr();
   }
 
+  DisjointRangeLockManager* lock_manager() { return &lock_manager_; }
+
  protected:
   friend class base::RefCounted<IndexedDBBackingStore>;
   virtual ~IndexedDBBackingStore();
@@ -669,6 +672,9 @@ class CONTENT_EXPORT IndexedDBBackingStore
   // complete. While > 0, temporary journal entries may exist so out-of-band
   // journal cleaning must be deferred.
   size_t committing_transaction_count_;
+
+  DisjointRangeLockManager lock_manager_ = {kIndexedDBLockLevelCount};
+
 #if DCHECK_IS_ON()
   bool initialized_ = false;
 #endif

content/browser/indexed_db/indexed_db_factory_impl.cc

@@ -126,7 +126,6 @@ IndexedDBFactoryImpl::IndexedDBFactoryImpl(
     base::Clock* clock)
     : context_(context),
       leveldb_factory_(leveldb_factory),
-      lock_manager_(kIndexedDBLockLevelCount),
       clock_(clock),
       earliest_sweep_(GenerateNextGlobalSweepTime(clock_->Now())) {}
 
@@ -550,10 +549,10 @@ void IndexedDBFactoryImpl::DeleteDatabase(
   }
 
   scoped_refptr<IndexedDBDatabase> database;
-  std::tie(database, s) =
-      IndexedDBDatabase::Create(name, backing_store.get(), this,
-                                std::make_unique<IndexedDBMetadataCoding>(),
-                                unique_identifier, &lock_manager_);
+  std::tie(database, s) = IndexedDBDatabase::Create(
+      name, backing_store.get(), this,
+      std::make_unique<IndexedDBMetadataCoding>(), unique_identifier,
+      backing_store->lock_manager());
   if (!database.get()) {
     IndexedDBDatabaseError error(
         blink::kWebIDBDatabaseExceptionUnknownError,
@@ -788,10 +787,10 @@ void IndexedDBFactoryImpl::Open(
   }
 
   scoped_refptr<IndexedDBDatabase> database;
-  std::tie(database, s) =
-      IndexedDBDatabase::Create(name, backing_store.get(), this,
-                                std::make_unique<IndexedDBMetadataCoding>(),
-                                unique_identifier, &lock_manager_);
+  std::tie(database, s) = IndexedDBDatabase::Create(
+      name, backing_store.get(), this,
+      std::make_unique<IndexedDBMetadataCoding>(), unique_identifier,
+      backing_store->lock_manager());
   if (!database.get()) {
     DLOG(ERROR) << "Unable to create the database";
     IndexedDBDatabaseError error(blink::kWebIDBDatabaseExceptionUnknownError,

content/browser/indexed_db/indexed_db_factory_impl.h

@@ -19,7 +19,6 @@
 #include "base/time/time.h"
 #include "content/browser/indexed_db/indexed_db_factory.h"
 #include "content/browser/indexed_db/leveldb/leveldb_env.h"
-#include "content/browser/indexed_db/scopes/disjoint_range_lock_manager.h"
 
 namespace base {
 struct Feature;
@@ -194,7 +193,6 @@ class CONTENT_EXPORT IndexedDBFactoryImpl : public IndexedDBFactory {
       backing_stores_with_active_blobs_;
   std::set<url::Origin> backends_opened_since_startup_;
 
-  DisjointRangeLockManager lock_manager_;
   base::Clock* clock_;
   base::Time earliest_sweep_;
 

third_party/blink/web_tests/external/wpt/IndexedDB/idbfactory-origin-isolation.html

+<!DOCTYPE html>
+<title>Databases on different origins use separate locking</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/get-host-info.sub.js"></script>
+<script src="support.js"></script>
+<script src="support-promises.js"></script>
+<script>
+
+var host_info = get_host_info();
+
+promise_test(async testCase => {
+  await deleteAllDatabases(testCase);
+
+  // Create an iframe to open and hold a database on a different origin.
+  var iframe = document.createElement('iframe');
+  var newLocation = window.location.href.replace('www', 'www2');
+
+  const keepalive_watcher = new EventWatcher(testCase, window, 'message');
+  iframe.src = host_info.HTTP_REMOTE_ORIGIN +
+      '/IndexedDB/resources/idbfactory-origin-isolation-iframe.html';
+  document.body.appendChild(iframe);
+
+  // Wait until the iframe starts its transaction.
+  var event = await keepalive_watcher.wait_for('message');
+  assert_equals("keep_alive_started", event.data);
+
+  // Create our own database with the same name, and perform a simple get.
+  const db = await createNamedDatabase(
+      testCase, 'db-isolation-test', database => {
+        database.createObjectStore('s');
+      });
+  const tx = db.transaction('s');
+  var request = tx.objectStore('s').get(0);
+  request.onsuccess = testCase.step_func_done();
+  request.onerror = testCase.unreached_func("There should be no errors.");
+}, "Test to make sure that origins have separate locking schemes");
+
+</script>
+
+<div id="log"></div>

third_party/blink/web_tests/external/wpt/IndexedDB/resources/idbfactory-origin-isolation-iframe.html

+<!DOCTYPE html>
+<title>This iframe keeps a transaction on a database alive indefinitely to test</title>
+<script>
+
+// Keeps the passed transaction alive indefinitely (by making requests
+// against the named store). Returns a function that asserts that the
+// transaction has not already completed and then ends the request loop so that
+// the transaction may autocommit and complete.
+function keep_alive(tx, store_name) {
+  let completed = false;
+  tx.addEventListener('complete', () => { completed = true; });
+
+  let keepSpinning = true;
+
+  function spin() {
+    if (!keepSpinning)
+      return;
+    tx.objectStore(store_name).get(0).onsuccess = spin;
+  }
+  spin();
+
+  return () => {
+    assert_false(completed, 'Transaction completed while kept alive');
+    keepSpinning = false;
+  };
+}
+
+async function run() {
+  const dbs_to_delete = await indexedDB.databases();
+  for (const db_info of dbs_to_delete) {
+    let request = indexedDB.deleteDatabase(db_info.name);
+    await new Promise((resolve, reject) => {
+      request.onsuccess = resolve;
+      request.onerror = reject;
+    });
+  }
+
+  var openRequest = indexedDB.open('db-isolation-test');
+  openRequest.onupgradeneeded = () => {
+    openRequest.result.createObjectStore('s');
+  };
+  openRequest.onsuccess = () => {
+    var tx = openRequest.result.transaction('s');
+    keep_alive(tx, 's');
+    window.parent.postMessage("keep_alive_started", "*");
+  };
+}
+
+run();
+</script>