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

Issue 622352 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

IndexedDB: "versionchange" can be fired at connection before "success" for deleteDatabase

Project Member Reported by jsb...@chromium.org, Jun 22 2016

Issue description

Found by palakj@ while working on writing tests for  issue 153119 

Scenario:

indexedDB.deleteDatabase('db');

let r1 = indexedDB.open('db', 1);
r1.onupgradeneeded = ...;
r1.onsuccess = e => {
  let c1 = r1.result;
  c1.onversionchange = e => c2.close(); // politely get out of the way  
};

let r2 = indexedDB.open('db', 2);

Note that r1/c1 is trying to do the right thing - if a later version comes along, close and get out of the way.

Once r1's upgrade is processed, r2 can start being processed. r2 will cause "versionchange" to be fired at c1, but this can happen before "success" is fired at r1, so there is no versionchange handler assigned yet.

We need to ensure that the "versionchange" is not fired until any preceding open requests have sent out "success".

This will probably require more logic in IndexedDBDatabase::ProcessPendingCalls() - https://cs.chromium.org/chromium/src/content/browser/indexed_db/indexed_db_database.cc?sq=package:chromium&rcl=1466594228&l=1606 - possibly a full queue as per  issue 479388  but there may be a smaller fix.
 

Comment 1 by jsb...@chromium.org, Jun 22 2016

Components: Blink>Storage>IndexedDB
Hrm, maybe I'm misremembering. Can't repro exactly as written above. But there's this repro:

indexedDB.deleteDatabase('db');

var r1 = indexedDB.open('db', 1);
r1.onupgradeneeded = e => {
  console.log('r1 upgradeneeded');
  var db = r1.result;
  db.onversionchange = e => console.log('r1 versionchange 1');
};
r1.onsuccess = e => {
  console.log('r1 success');
  var db = r1.result;
  db.onversionchange = e => console.log('r1 versionchange 2');
};

var r2 = indexedDB.deleteDatabase('db');
r2.onblocked = e => console.log('r2 blocked');
r2.onsuccess = e => console.log('r2 success');

I don't see either onversionchange or onblocked. Related?

Comment 2 by jsb...@chromium.org, Jun 22 2016

Digging in, the reason is that DeleteDatabase() is processed two ways:

1. if there are open connections, call OnVersionChange() on them and add a PendingDeleteCall to the queue; when that is processed, DeleteDatabaseFinal() runs.
2. otherwise, there are no open connections; call DeleteDatabaseFinal() directly.

An in-process upgrade falls into #2, so the delete runs immediately.

We instead need something equivalent to:

1. if is an in process upgrade, defer until that completes, then try again
2. otherwise, if there are open connections, call OnVersionChange() on them and add a PendingDeleteCall to the queue; when that is processed, DeleteDatabaseFinal() runs.
3. otherwise, there are no open connections; call DeleteDatabaseFinal() directly.


Comment 3 by jsb...@chromium.org, Jun 22 2016

Owner: jsb...@chromium.org
Status: Started (was: Available)
Summary: IndexedDB: "versionchange" can be fired at connection before "success" for deleteDatabase (was: IndexedDB: "versionchange" can be fired at connection before "success" )
Spec issue: https://github.com/w3c/IndexedDB/issues/78

Comment 4 by jsb...@chromium.org, Jul 12 2016

Status: Fixed (was: Started)
Fixed by https://codereview.chromium.org/2084053004/

(I had the wrong issue # in that CL)

Sign in to add a comment