Skip to content

Specify IDBOpenDBRequest processing as a queue? #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
inexorabletash opened this issue Jun 24, 2016 · 9 comments
Closed

Specify IDBOpenDBRequest processing as a queue? #79

inexorabletash opened this issue Jun 24, 2016 · 9 comments
Labels

Comments

@inexorabletash
Copy link
Member

Consider:

indexedDB.open(name).onsuccess = e => {console.log('open1'); e.target.result.close(); };
indexedDB.open(name).onsuccess = e=>{ console.log('open2'); e.target.result.close(); };
indexedDB.deleteDatabase(n).onsuccess = e => { console.log('delete'); }
  • Firefox logs: open1, open2, delete.
  • Chrome logs: open1, delete, open2

IIRC we discussed this before. Chrome is following the letter of the spec, that in "steps for deleting a database" the "delete pending flag" is set immediately (no waiting) and in "steps for opening a database" the steps wait on the "delete pending flag". Firefox instead processes IDBOpenDBRequests (opens, deletes) on a named database as a FIFO queue.

This is an observable incompatibility, so we should probably standardize the behavior one way or the other.

@beidson
Copy link

beidson commented Jun 25, 2016

Chrome is following the letter of the spec, that in "steps for deleting a database" the "delete pending flag" is set immediately (no waiting)

In v1 of the spec the language for deleteDatabase is "The method then queues up an operation to run the steps for deleting a database"
In v2 of the spec the language for deleteDatabase is "Queue a task to run these substeps:...Let result be the result of running the steps for deleting a database"

In both cases the "steps for deleting a database" are queued and run asynchronously. In neither case is the "delete pending flag" set immediately.

So the example code actually does the following:

indexedDB.open - "The method then queues up an operation to run the steps for opening a database."
indexedDB.open - "The method then queues up an operation to run the steps for opening a database."
indexedDB.deleteDatabase - "The method then queues up an operation to run the steps for deleting a database."

There's no mention of setting the "delete pending flag" anytime before the steps for deleting a database are executed.

Trunk WebKit and Firefox treat these as a queue as mentioned above, and I think they're right.

@inexorabletash
Copy link
Member Author

Ah, yes. Are the queues in your implementations specific to the database - (origin, name) pair?

@beidson
Copy link

beidson commented Jun 26, 2016

For WebKit, the queues are specific to the unique database.

I don't know about Firefox.

@inexorabletash
Copy link
Member Author

Cool. I'll work on improving the spec text and get some web-platform-tests together.

@inexorabletash
Copy link
Member Author

Note to self: the V2 spec should not use the "queue a task" phrase here since that's specific to event loops. These queues span execution contexts.

@nox
Copy link
Contributor

nox commented Jul 13, 2016

I'm pretty sure it should still say that it should "queue a task" somewhere, otherwise how does the actual opening of the connection interact with the event queue from the caller?

inexorabletash added a commit that referenced this issue Jul 15, 2016
Also:
* Remove 'delete pending' flag which is not longer needed.
* Simplify some section names.
* Move the versionchange/blocked logic from upgrade steps to
  open steps, for symmetry with delete steps.
* Rename 'Authorization' section to 'Security Concerns'
@inexorabletash
Copy link
Member Author

Should be resolved by d1147cb - take a peek?

@nox
Copy link
Contributor

nox commented Jul 15, 2016

AFAICT this is not good enough, because that means an IDBOpenDBRequest's result can be set before event handlers were even set on it, given now the setting etc occur in parallel.

@inexorabletash
Copy link
Member Author

Thanks @nox - take another look? I'd also forgotten to update the deleteDatabase steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants