Skip to content
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

The done flag is never unset between upgradeneeded and success #161

Closed
inexorabletash opened this issue Mar 3, 2017 · 4 comments
Closed
Milestone

Comments

@inexorabletash
Copy link
Member

There's a note in the request definition:

Requests are not typically re-used, but there are exceptions. When a cursor is iterated, the success of the iteration is reported on the same request object used to open the cursor. And when an upgrade transaction is necessary, the same open request is used for both the upgradeneeded event and final result of the open operation itself. In both cases, the request’s done flag will be unset then set again, and the result may change.

For cursor iteration there are explicit "Unset the done flag on request." steps (e.g. in continue()). But there's nothing in the steps for upgrade transactions.

I'd expect a final step there that unsets the done flag. Ostensibly this would be visible as a change in the request's readyState between the "complete" event fired at the transaction and the "success" event fired at the request.

However... I'm unable to test this in either Chrome or Firefox. In Chrome, at least, the code that queues the "complete" and "success" doesn't really leave room for anything to happen in between, at least not in a non-racy way.

So... the note is inconsistent with the spec, but changing the spec without a test seems weird.

@inexorabletash inexorabletash added this to the V2 milestone Mar 3, 2017
@brettz9
Copy link
Contributor

brettz9 commented Apr 27, 2017

Also, if there is an error (abort errors are the specified ones) during or after the "steps to run an upgrade transaction", should open not reset the result to undefined as well? Or can an error co-exist with a result?

Also, as far as the explanatory text you cited mentioning the result possibly changing... If in IDBFactory.open, the "steps to open a database" are run in parallel with the queuing of a task to set the result or error, I would think this task to set a result for open could and probably would technically occur before the task to set a result for the upgrade (within the "steps to run an upgrade transaction"). While the open steps could clarify that the steps should wait until there is a result, the task would still have already been added to the queue before the upgrade task had been added to it if added in parallel.

@brettz9
Copy link
Contributor

brettz9 commented May 3, 2017

Also, since the open request is not using the steps to asynchronously execute a request, the done flag is also not explicitly set anywhere upon a success (though the general definition for requests in general does state "If a request completes successfully, the done flag is set").

@inexorabletash
Copy link
Member Author

Thanks as always @brettz9 !

I tacked this in #202

I'd expect a final step there that unsets the done flag.

I added this in the case where the result is cleared out for abort. I still don't think it's observable, but it made sense to do as part of the related changes...

if there is an error (abort errors are the specified ones) during or after the "steps to run an upgrade transaction", should open not reset the result to undefined as well?

Correct. This appears at the end of "Aborting a transaction" where request's transaction is cleared. The timing is specific here - this is visible after the event fires.

...the "steps to open a database" are run in parallel with the queuing of a task...

That's not what "in parallel" means. It's defined at https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel and means the referenced steps are run parallel to the surrounding algorithm, but are themselves run sequentially. It's known that the terminology is confusing, see whatwg/html#2643 (and in #199 I at least turn "in parallel" into a link)

the done flag is also not explicitly set anywhere upon a success

Ugh, yeah. So that's taken care of in the PR as part of the open/delete steps.

Can you review the PR and comment there?

@brettz9
Copy link
Contributor

brettz9 commented May 12, 2017

Thanks as always to you for the thorough consideration and your ongoing amendments and pain-staking work on such minutia here and in the tests!

And thanks for the reference to the definition of "in parallel" for HTML algorithms.

I've added some comments on the referenced PR so can continue there.

triple-underscore added a commit to triple-underscore/triple-underscore.github.io that referenced this issue May 18, 2017
…e flag appropriately

Set request result/done flag appropriately in open/delete (Resolves
w3c/IndexedDB#161 )
w3c/IndexedDB@e213bcf
92c97770

Define db's associated upgrade tx to align with impls/tests
w3c/IndexedDB@ee40bb3
116a0e44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants