1598164 - Make transactions inactive during clone

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, enhancement, P3)

Description

[Andrew Sutherland [:asuth] (he/him)]

There's a nice invariant cleanup at https://github.com/w3c/IndexedDB/pull/310 that would be good to implement.

Tests already were upstreamed by Chrome to WPT in https://github.com/web-platform-tests/wpt/commit/fee6738f9523f5e257accfaea8ddfef2e28cfbcd and we have the test in-tree (although failing) at https://searchfox.org/mozilla-central/source/testing/web-platform/tests/IndexedDB/structured-clone-transaction-state.any.js

:sg, this seems up your alley with your recent cleanups.

Comment 1

[Simon Giesecke [:sg] [he/him]]

Implementing this properly requires the cleanup of the IDBTranscation state model to better match the spec planned for Bug 1497007 (explicit commit).

Comment 2

[Simon Giesecke [:sg] [he/him]]

Is calling IDBObjectStore.deleteIndex allowed during structured clone? We have a custom test for that here: dom/indexedDB/test/unit/test_clone_before_key_evaluation.js, which fails after making the wpt work.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

It was previously allowed but a bad idea to allow it. test_clone_before_key_evaluation.js can be removed as with the changes to the spec and WPT we will have sufficient coverage of this area of code.

Er, and to be explicit: deleteIndex is no longer allowed during structured clone per the spec changes. (And I think this is a significant improvement in the spec!)

Comment 4

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Improved test assertion messages and removed code duplication. r=#dom-workers-and-storage

Comment 5

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Fix structured-clone-transaction-state.any.js test case. r=#dom-workers-and-storage

Also removed obsolete dom/indexedDB/test/test_clone_before_key_evaluation.html test case,
which tested for the opposite behaviour.

Depends on D54266

Comment 6

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Use scoped enums in IDBTransaction. r=#dom-workers-and-storage

Depends on D54267

Comment 7

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Align ReadyState with states defined in the spec. r=#dom-workers-and-storage

Depends on D54321

Comment 8

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Make use of std::copy_if/MakeBackInserter in IDBTransaction::AbortInternal. r=#dom-workers-and-storage

Depends on D54322

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18bfaf645c42
Improved test assertion messages and removed code duplication. r=dom-workers-and-storage-reviewers,edenchuang
https://hg.mozilla.org/integration/autoland/rev/fa4b9c36227a
Fix structured-clone-transaction-state.any.js test case. r=dom-workers-and-storage-reviewers,janv
https://hg.mozilla.org/integration/autoland/rev/82141e34c89f
Use scoped enums in IDBTransaction. r=dom-workers-and-storage-reviewers,edenchuang
https://hg.mozilla.org/integration/autoland/rev/812780d706ae
Make use of std::copy_if/MakeBackInserter in IDBTransaction::AbortInternal. r=dom-workers-and-storage-reviewers,ytausky

Comment 10

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Fixed log messages, use serial number of transaction. r=#dom-workers-and-storage

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/18bfaf645c42
https://hg.mozilla.org/mozilla-central/rev/fa4b9c36227a
https://hg.mozilla.org/mozilla-central/rev/82141e34c89f
https://hg.mozilla.org/mozilla-central/rev/812780d706ae

Comment 12

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Fixed log message printf string. r=#dom-workers-and-storage

Comment 13

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Remove uses of already_AddRefed. r=#dom-workers-and-storage

Depends on D54322

Comment 14

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Added FlippedOnce class template to help reducing statefulness of boolean flags. r=#dom-workers-and-storage

Depends on D55079

Comment 15

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Made some boolean flags in IDBTransaction FlippedOnce to reduce statefulness. r=#dom-workers-and-storage

Depends on D55080

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e0a3e5cdb4d
Fixed log message printf string. r=dom-workers-and-storage-reviewers,janv

Comment 17

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1598164 - Implement transaction inactive state according to spec. r=#dom-workers-and-storage

Depends on D55081

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97a03c24e865
Align ReadyState with states defined in the spec. r=dom-workers-and-storage-reviewers,edenchuang
https://hg.mozilla.org/integration/autoland/rev/fb6184becfbd
Remove uses of already_AddRefed. r=dom-workers-and-storage-reviewers,janv

Comment 19

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/7e0a3e5cdb4d
https://hg.mozilla.org/mozilla-central/rev/97a03c24e865
https://hg.mozilla.org/mozilla-central/rev/fb6184becfbd

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c56dd45e9f5
Added FlippedOnce class template to help reducing statefulness of boolean flags. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/2c2c3b33ea1b
Made some boolean flags in IDBTransaction FlippedOnce to reduce statefulness. r=dom-workers-and-storage-reviewers,ytausky

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebf5f725d0b3
Implement transaction inactive state according to spec. r=dom-workers-and-storage-reviewers,ytausky

Comment 22

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/9c56dd45e9f5
https://hg.mozilla.org/mozilla-central/rev/2c2c3b33ea1b

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ebf5f725d0b3

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca182ba97ae6
Fixed log messages, use serial number of transaction. r=dom-workers-and-storage-reviewers,janv

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/ca182ba97ae6