New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 657968 link

Starred by 1 user
Status: Fixed
Owner:
pwnall@chromium.org
Closed: Jan 2017
Cc:
jsb...@chromium.org
█ cos...@gmail.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Flaky tests: storage/indexeddb/idbdatabase-{create,delete}ObjectStore-exception-order.html

Project Member Reported by jsb...@chromium.org, Oct 20 2016 
FAIL IDBDatabase.createObjectStore exception order: InvalidStateError vs. TransactionInactiveError assert_throws: "running an upgrade transaction" check (InvalidStateError) should precede "not active" check (TransactionInactiveError) function "() => { db.createObjectStore('s2'); }" threw object "TransactionInactiveError: Failed to execute 'createObjectStore' on 'IDBDatabase': The transaction has finished." that is not a DOMException InvalidStateError: property "code" is equal to 0, expected 11

It looks like this is a race because we gate the "running an upgrade transaction" on whether or not IDBDatabase::m_versionChangeTransaction is set or not. We don't clear it synchronously from IDBTransaction::abort(); we wait until the asynchronous IDBTransaction::onAbort() comes through from the back end acknowledging the abort.

We could look on clearing it synchronously in IDBTransaction::abort(), or change the test be more than if (!m_versionChangeTransaction) (i.e. check the TX's state too)

Comment 1 by jsb...@chromium.org, Oct 20 2016 

Hrm...

if (!m_versionChangeTransaction || m_versionChangeTransaction->isFinishing())

... would certainly be easy. When abort() is called we flip the tx to 'Finishing'. 

But that state is used in two cases: when the front end decides to abort *or* commit.

* In the case where the front-end decides to abort we can can call the transaction "not running" (so InvalidStateError).

* In the case where the transaction is committing it's still *running* I guess (the state machine is not rigorously defined in the spec), so we'd want TransactionInactiveError maybe? Which means adding more state to IDBTransaction?
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e8c7e5674f3c7899652c8073d2f52c8bb693c47

commit 1e8c7e5674f3c7899652c8073d2f52c8bb693c47
Author: jsbell <jsbell@chromium.org>
Date: Fri Oct 28 19:05:26 2016

Mark a couple IndexedDB tests as flaky

BUG= 657968 
TBR=phoglund@chromium.org,pwnall@chromium.org
NOTRY=true

Review-Url: https://codereview.chromium.org/2461763002
Cr-Commit-Position: refs/heads/master@{#428450}

[modify] https://crrev.com/1e8c7e5674f3c7899652c8073d2f52c8bb693c47/third_party/WebKit/LayoutTests/TestExpectations

Comment 3 by pwnall@chromium.org, Dec 1 2016

Owner: pwnall@chromium.org
Status: Started (was: Available)

Comment 4 by pwnall@chromium.org, Dec 1 2016 

I used the following commands to get the flaky behavior, after removing the relevant TestExpectations lines.

third_party/WebKit/Tools/Scripts/run-webkit-tests storage/indexeddb/idbdatabase-createObjectStore-exception-order.html -t Default --repeat-each=200
third_party/WebKit/Tools/Scripts/run-webkit-tests storage/indexeddb/idbdatabase-deleteObjectStore-exception-order.html -t Default --repeat-each=200

Testing note for future me: I got failures (~40% for createObjectStore, ~1% for deleteObjectStore) on Linux, but no failures on Mac.

Comment 5 by pwnall@chromium.org, Dec 9 2016 

Step 10 in the "running an upgrade transaction" algorithm [2] says "When transaction is finished, immediately set request’s transaction to null. This must be done in the same task as the task firing the complete or abort event, but after the event has been fired."

I interpret this to mean that before the abort element is fired, the transaction is still running. My interpretation is reflected in http://crrev.com/2538343002

[2] https://w3c.github.io/IndexedDB/#upgrade-transaction-steps

Comment 6 by pwnall@chromium.org, Dec 9 2016 

I started working on a test that checks all the lifecycle stages. We might not be following (my interpretation of) the standard either.

http://crrev.com/2556373003
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6c814b48c0ec98a495202b17ce55a0aef8364d4

commit c6c814b48c0ec98a495202b17ce55a0aef8364d4
Author: pwnall <pwnall@chromium.org>
Date: Tue Dec 13 01:10:32 2016

De-flake IndexedDB layout tests.

Per the IndexedDB specification, a versionchange transaction is running
until the open request's transaction member is set to null, which
should happen (synchronously) after the onabort() event handler is
called. Therefore, tests can only assume that a transaction stopped
running after the onabort event handler has occurred.

BUG= 657968 

Review-Url: https://codereview.chromium.org/2538343002
Cr-Commit-Position: refs/heads/master@{#437974}

[modify] https://crrev.com/c6c814b48c0ec98a495202b17ce55a0aef8364d4/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/c6c814b48c0ec98a495202b17ce55a0aef8364d4/third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-createObjectStore-exception-order.html
[modify] https://crrev.com/c6c814b48c0ec98a495202b17ce55a0aef8364d4/third_party/WebKit/LayoutTests/storage/indexeddb/idbdatabase-deleteObjectStore-exception-order.html

Comment 9 by pwnall@chromium.org, Jan 5 2017

Status: Fixed (was: Started)
►

Sign in to add a comment