third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html - Issue 2415203002: Indexed DB: More exception precedence tests

Unified Diff: third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html

Issue 2415203002: Indexed DB: More exception precedence tests (Closed)

Patch Set: Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« third_party/WebKit/LayoutTests/storage/indexeddb/idbcursor-update-exception-order.html ('K') | « third_party/WebKit/LayoutTests/storage/indexeddb/idbcursor-update-exception-order.html ('k') | third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-deleteObjectStore-exception-order.html » ('j') | third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-deleteObjectStore-exception-order.html » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html

diff --git a/third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html b/third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html

index 2c9236640a5f1cce7263a2fde3f2eb4395f0abc4..7af30b00ed4b6d15efe75ae349784c080acd4d79 100644

--- a/third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html

+++ b/third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html

@@ -1,23 +1,82 @@

<!DOCTYPE html>

<title>IndexedDB: IDBDatabase createObjectStore() Exception Ordering</title>

+<meta charset=utf-8>

indexeddb_test(

- function(t, db) {

- db.createObjectStore('s');

- assert_throws('SyntaxError', function() {

- db.createObjectStore('s', {keyPath: 'not a valid key path'});

- }, '"Invalid keyath" check (SyntaxError) should precede ' +

- '"duplicate store name" check (ConstraintError)');

- t.done();

- },

- function(t, db) {},

- 'IDBDatabase.createObjectStore exception order: ConstraintError vs. SyntaxError'

+ (t, db) => {

+ db.createObjectStore('s');

+ },

+ (t, db) => {

+ const tx = db.transaction('s');

+ const store = tx.objectStore('s');

+ const finish = pin_transaction(store);

+ setTimeout(t.step_func(() => {

pwnall 2016/10/14 07:57:04 This alternative seems easier to reason about: in

jsbell 2016/10/17 19:42:03 Done, w/ slight tweaks.

+ assert_throws(

+ 'InvalidStateError', () => { db.createObjectStore('s2'); },

+ '"running an upgrade transaction" check (InvalidStateError) ' +

+ 'should precede "not active" check (TransactionInactiveError)');

+ finish();

+ t.done();

+ }), 0);

+ },

+ 'IDBDatabase.createObjectStore exception order: ' +

+ 'InvalidStateError vs. TransactionInactiveError'

+);

+indexeddb_test(

+ (t, db) => {

+ const store = db.createObjectStore('s');

+ const finish = pin_transaction(store);

+ setTimeout(t.step_func(() => {

+ assert_throws(

pwnall 2016/10/14 07:57:04 Firefox Nightly 52.0a1 (2016-10-13) fails this che

Firefox Nightly 52.0a1 (2016-10-13) fails this check. I think Firefox conflates active transactions with running transactions. It seems like they have a TLS variable indicating the currently running transaction for a thread [1]. Their "get the database's currently running transaction" (needed for Step 2 in createObjectStore's spec) seems to be implemented as "get the thread's current transaction and check that it belongs to the database" [2]. The current transaction TLS variable is managed by an AutoSetCurrentTransaction [3] class, which in turn is used by a ResultHelper [4] class. Last, I think ResultHelper is only used when a request result is created and an event is dispatched. This assertion is a bit hard to check, because it requires going through all the code snippets that create ResultHelpers [5], but I'm reasonably sure that it's true, based on some spot-checking. On a brighter note, I was able to get the desired behavior for this test with the following changes: testharness-helpers.js:10: upgrade_func(t, open_request.result, open_request.transaction); idbdatabase-createObjectStore-exception-order.html: (comments left to emphasize diff) indexeddb_test( (t, db, txn) => { const store = db.createObjectStore('s'); //const finish = pin_transaction(store); txn.abort(); //setTimeout(t.step_func(() => { assert_throws( 'TransactionInactiveError', () => { db.createObjectStore('s2', {keyPath: '-invalid-'}); }, '"not active" check (TransactionInactiveError) should precede ' + '"valid key path" check (SyntaxError)'); //finish(); t.done(); //}), 0); }, (t, db) => {}, 'IDBDatabase.createObjectStore exception order: ' + 'TransactionInactiveError vs. SyntaxError' ); My approach side-steps Firefox's bug by testing for the currently running transaction synchronously inside the onupgradeneeded handler. FWIW, I tested my idea in both Chrome Canary and FF Nightly. The code above passes. Removing the txn.abort() call yields a SyntaxError. In order to get Firefox to pass a test that uses transaction pinning, I think that that the easiest way forward may be to collapse the InvalidStateError and TransactionInactiveError cases to a single exception type. [1] https://dxr.mozilla.org/mozilla-central/source/dom/indexedDB/IDBTransaction.c... [2] https://dxr.mozilla.org/mozilla-central/source/dom/indexedDB/IDBDatabase.cpp#423 [3] https://dxr.mozilla.org/mozilla-central/search?q=%2Bfunction-ref%3A%22mozilla... [4] https://dxr.mozilla.org/mozilla-central/search?q=%2Bcallers%3A%22mozilla%3A%3... [5] https://dxr.mozilla.org/mozilla-central/search?q=%2Btype-ref%3A%22mozilla%3A%...

jsbell 2016/10/17 19:42:03 Awesome investigation. Done, w/ slight tweaks.

+ 'TransactionInactiveError',

+ () => { db.createObjectStore('s2', {keyPath: '-invalid-'}); },

+ '"not active" check (TransactionInactiveError) should precede ' +

+ '"valid key path" check (SyntaxError)');

+ finish();

+ t.done();

+ }), 0);

+ },

+ (t, db) => {},

+ 'IDBDatabase.createObjectStore exception order: ' +

+ 'TransactionInactiveError vs. SyntaxError'

+);

+indexeddb_test(

+ (t, db) => {

+ db.createObjectStore('s');

+ assert_throws('SyntaxError', () => {

+ db.createObjectStore('s', {keyPath: 'not a valid key path'});

+ }, '"Invalid keyath" check (SyntaxError) should precede ' +

+ '"duplicate store name" check (ConstraintError)');

+ t.done();

+ },

+ (t, db) => {},

+ 'IDBDatabase.createObjectStore exception order: ' +

+ 'SyntaxError vs. ConstraintError'

+);

+indexeddb_test(

+ (t, db) => {

+ db.createObjectStore('s');

+ assert_throws('ConstraintError', () => {

+ db.createObjectStore('s', {autoIncrement: true,

+ keyPath: ''});

+ }, '"already exists" check (ConstraintError) should precede ' +

+ '"autoIncrement vs. keyPath" check (InvalidAccessError)');

+ t.done();

+ },

+ (t, db) => {},

+ 'IDBDatabase.createObjectStore exception order: ' +

+ 'ConstraintError vs. InvalidAccessError'

);

</script>