Indexed DB: More exception precedence tests

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

 function indexeddb_test(upgrade_func, body_func, description) {
     async_test(function(t) {
         var dbName = 'db' + self.location.pathname + '-' + description;
         var delete_request = indexedDB.deleteDatabase(dbName);
         delete_request.onerror = t.unreached_func('deleteDatabase should not fail');
         delete_request.onsuccess = t.step_func(function(e) {
             var open_request = indexedDB.open(dbName);
             open_request.onerror = t.unreached_func('open should not fail');
             open_request.onupgradeneeded = t.step_func(function(e) {
                 upgrade_func(t, open_request.result);

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

This is where I added the open_request.transaction argument. It shouldn't hurt
the other tests, and having access to the transaction might be handy.

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

Done.

             });
             open_request.onsuccess = t.step_func(function(e) {
                 body_func(t, open_request.result);
             });
         });
     }, description);
 }
 
 function assert_key_equals(a, b, message) {
     assert_equals(indexedDB.cmp(a, b), 0, message);
 }
 
 // Call with a Test and an array of expected results in order. Returns
 // a function; call the function when a result arrives and when the
 // expected number appear the order will be asserted and test
 // completed.
 function expect(t, expected) {
     var results = [];
     return result => {
         results.push(result);
         if (results.length === expected.length) {
             assert_array_equals(results, expected);
             t.done();
         }
     };
 }
+
+// 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;
+  };
+}