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

Side by Side 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:

« 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')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

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

	3 <meta charset=utf-8>

3 <link rel="help" href="https://w3c.github.io/IndexedDB/#dom-idbdatabase-createob jectstore">	4 <link rel="help" href="https://w3c.github.io/IndexedDB/#dom-idbdatabase-createob jectstore">

4 <script src="../../resources/testharness.js"></script>	5 <script src="../../resources/testharness.js"></script>

5 <script src="../../resources/testharnessreport.js"></script>	6 <script src="../../resources/testharnessreport.js"></script>

6 <script src="resources/testharness-helpers.js"></script>	7 <script src="resources/testharness-helpers.js"></script>

7

8 <script>	8 <script>

9	9

10 indexeddb_test(	10 indexeddb_test(

11 function(t, db) {	11 (t, db) => {

12 db.createObjectStore('s');	12 db.createObjectStore('s');

13 assert_throws('SyntaxError', function() {	13 },

14 db.createObjectStore('s', {keyPath: 'not a valid key path'});	14 (t, db) => {

15 }, '"Invalid keyath" check (SyntaxError) should precede ' +	15 const tx = db.transaction('s');

16 '"duplicate store name" check (ConstraintError)');	16 const store = tx.objectStore('s');

17 t.done();	17 const finish = pin_transaction(store);

18 },	18

19 function(t, db) {},	19 setTimeout(t.step_func(() => {
	pwnall 2016/10/14 07:57:04 This alternative seems easier to reason about: in This alternative seems easier to reason about: indexeddb_test( (t, db, txn) => { db.createObjectStore('s'); txn.abort(); setTimeout(t.step_func(() => { assert_throws( 'InvalidStateError', () => { db.createObjectStore('s2'); }, '"running an upgrade transaction" check (InvalidStateError) ' + 'should precede "not active" check (TransactionInactiveError)'); t.done(); }), 0); }, (t, db) => {}, 'IDBDatabase.createObjectStore exception order: ' + 'InvalidStateError vs. TransactionInactiveError' ); It passes in Chrome Canary and FF Nightly. Commenting out the setTimeout triggers the TransactionInactiveError. WDYT? jsbell 2016/10/17 19:42:03 Done, w/ slight tweaks. Done, w/ slight tweaks.
20 'IDBDatabase.createObjectStore exception order: ConstraintError vs. SyntaxEr ror'	20 assert_throws(

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

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

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

	24

	25 finish();

	26 t.done();

	27 }), 0);

	28 },

	29 'IDBDatabase.createObjectStore exception order: ' +

	30 'InvalidStateError vs. TransactionInactiveError'

	31 );

	32

	33 indexeddb_test(

	34 (t, db) => {

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

	36 const finish = pin_transaction(store);

	37

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

	39 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. Awesome investigation. Done, w/ slight tweaks.
	40 'TransactionInactiveError',

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

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

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

	44 finish();

	45 t.done();

	46 }), 0);

	47 },

	48 (t, db) => {},

	49 'IDBDatabase.createObjectStore exception order: ' +

	50 'TransactionInactiveError vs. SyntaxError'

	51 );

	52

	53 indexeddb_test(

	54 (t, db) => {

	55 db.createObjectStore('s');

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

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

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

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

	60 t.done();

	61 },

	62 (t, db) => {},

	63 'IDBDatabase.createObjectStore exception order: ' +

	64 'SyntaxError vs. ConstraintError'

	65 );

	66

	67 indexeddb_test(

	68 (t, db) => {

	69 db.createObjectStore('s');

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

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

	72 keyPath: ''});

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

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

	75 t.done();

	76 },

	77 (t, db) => {},

	78 'IDBDatabase.createObjectStore exception order: ' +

	79 'ConstraintError vs. InvalidAccessError'

21 );	80 );

22	81

23 </script>	82 </script>

OLD	NEW