Indexed DB: More exception precedence tests

third_party/WebKit/LayoutTests/storage/indexeddb/resources/testharness-helpers.js

Index: third_party/WebKit/LayoutTests/storage/indexeddb/resources/testharness-helpers.js
diff --git a/third_party/WebKit/LayoutTests/storage/indexeddb/resources/testharness-helpers.js b/third_party/WebKit/LayoutTests/storage/indexeddb/resources/testharness-helpers.js
index 846481f55bb877737446463029967f3760967bc1..c4de2f0ffff33749cdb8a09657e3bf2d383551e1 100644
--- a/third_party/WebKit/LayoutTests/storage/indexeddb/resources/testharness-helpers.js
+++ b/third_party/WebKit/LayoutTests/storage/indexeddb/resources/testharness-helpers.js
@@ -34,3 +34,20 @@ function expect(t, expected) {
         }
     };
 }
+
+// Pins the transaction associated with the given store open by
+// issuing a chain of get(0) requests. Returns a function to call to
+// stop the requests and let the transaction complete. Useful when you
+// need to ensure a transaction is alive across other asynchronous
+// operations (e.g. setTimeout)
+function pin_transaction(store) {

[pwnall, 2016/10/14 07:57:05]

The docs do state that the point of it is to keep the transaction running. I
think they should also talk about the subtlety that the pinned transaction isn't
necessarily active while it's running. Specifically, I think that transactions
are only active right after they are created and while their event handlers run,
because of step 1 in [1],  step 4 in [2] and step 4 in [3].

[1] https://w3c.github.io/IndexedDB/#transaction-lifetime
[2] https://w3c.github.io/IndexedDB/#fire-success-event
[3] https://w3c.github.io/IndexedDB/#fire-error-event

If I understand the code correctly, it seems that pin_transaction ensures that
implementations can distinguish between alive but inactive transactions and
finished transactions. However, many of the exception conditions checked above
seem to only care about whether an active transaction exists or not, so the
distinction may be moot. In that case, using pin_transaction seems to make the
test harder to reason about.

Specifically, I think that only createObjectStore and deleteObjectStore
distinguish between running and active transactions, because they reference the
database's currently running versionchange transaction. It seems like pinning is
unnecessary in the other tests, and hits a Firefox bug in those two tests.

[jsbell, 2016/10/17 19:42:04]

Agreed, but...
Yeah, so we can probably remove. This is necessary in tests that e.g. want to
specifically probe this state, but it's not necessary for the new tests that
were using this helper.

+  let finish = false;
+  (function spin() {
+    if (finish)
+      return;
+    store.get(0).onsuccess = spin;
+  }());
+  return () => {
+    finish = true;
+  };
+}