1600906 - Reduce state in dom/indexedDB classes

Status: RESOLVED FIXED

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

Comment 1

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

Attachment: Bug 1600906 - Make use of FlippedOnce in VersionChangeTransaction. r=#dom-workers-and-storage

Depends on D55081

Comment 2

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

Attachment: Bug 1600906 - Make use of FlippedOnce in Database. r=#dom-workers-and-storage

Depends on D55674

Comment 3

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

Attachment: Bug 1600906 - Make use of FlippedOnce in FullIndexMetadata. r=#dom-workers-and-storage

Depends on D55675

Comment 4

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

Attachment: Bug 1600906 - Make use of FlippedOnce in FullObjectStoreMetadata. r=#dom-workers-and-storage

Depends on D55676

Comment 5

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

Attachment: Bug 1600906 - Make use of FlippedOnce in ConnectionPool. r=#dom-workers-and-storage

Depends on D55677

Comment 6

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

Attachment: Bug 1600906 - Make use of FlippedOnce in QuotaClient. r=#dom-workers-and-storage

Depends on D55678

Comment 7

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

Attachment: Bug 1600906 - Make use of FlippedOnce in DatabaseInfo. r=#dom-workers-and-storage

Depends on D55679

Comment 8

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

Attachment: Bug 1600906 - Make use of FlippedOnce in ThreadRunnable. r=#dom-workers-and-storage

Depends on D55680

Comment 9

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

Attachment: Bug 1600906 - Make use of FlippedOnce in TransactionBase. r=#dom-workers-and-storage

Depends on D55681

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6c086ca5780
Make use of FlippedOnce in VersionChangeTransaction. r=dom-workers-and-storage-reviewers,ytausky

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69dba2abeaf4
Make use of FlippedOnce in Database. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/fcea7ad7ad7b
Make use of FlippedOnce in FullIndexMetadata. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/09e4368e2355
Make use of FlippedOnce in FullObjectStoreMetadata. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/7c4e2c13f1c3
Make use of FlippedOnce in ConnectionPool. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/dc96d3709a42
Make use of FlippedOnce in QuotaClient. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/4e0758ff6af7
Make use of FlippedOnce in DatabaseInfo. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/4fe02c654acc
Make use of FlippedOnce in ThreadRunnable. r=dom-workers-and-storage-reviewers,ytausky

Comment 12

[Daniel Varga [:dvarga]]

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

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/69dba2abeaf4
https://hg.mozilla.org/mozilla-central/rev/fcea7ad7ad7b
https://hg.mozilla.org/mozilla-central/rev/09e4368e2355
https://hg.mozilla.org/mozilla-central/rev/7c4e2c13f1c3
https://hg.mozilla.org/mozilla-central/rev/dc96d3709a42
https://hg.mozilla.org/mozilla-central/rev/4e0758ff6af7
https://hg.mozilla.org/mozilla-central/rev/4fe02c654acc

Comment 14

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

Attachment: Bug 1600906 - Extended InitializedOnce to allow restricting the valid values. r=#dom-workers-and-storage

Comment 15

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

Attachment: Bug 1600906 - Reduce statefulness of TransactionBase using const and InitializedOnce. r=#dom-workers-and-storage

Depends on D56005

Comment 16

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

Attachment: Bug 1600906 - Made FlippedOnce member functions constexpr. r=#dom-workers-and-storage

Depends on D56006

Comment 17

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

Attachment: Bug 1600906 - Reduce statefulness of DatabaseConnection using InitializedOnce. r=#dom-workers-and-storage

Depends on D56007

Comment 18

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

Attachment: Bug 1600906 - Added const in UpdateRefcount. r=#dom-workers-and-storage

Depends on D56008

Comment 19

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

Attachment: Bug 1600906 - Added const in IdleConnectionRunnable. r=#dom-workers-and-storage

Depends on D56009

Comment 20

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

Attachment: Bug 1600906 - Use move semantics with ThreadInfo. r=#dom-workers-and-storage

Depends on D56010

Comment 21

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

Attachment: Bug 1600906 - Use move semantics with IdleDatabaseInfo. r=#dom-workers-and-storage

Depends on D56011

Comment 22

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

Attachment: Bug 1600906 - Reduce statefulness of TransactionInfo using const and FlippedOnce. r=#dom-workers-and-storage

Depends on D56012

Comment 23

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

Attachment: Bug 1600906 - Encapsulate DatabaseOperationBase::mResultCode. r=#dom-workers-and-storage

Depends on D56013

Comment 24

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

Attachment: Bug 1600906 - Reduce statefulness of TransactionDatabaseOperationBase using InitializedOnce. r=#dom-workers-and-storage

Depends on D56014

Comment 25

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

Attachment: Bug 1600906 - Disable remote settings blocklist when running indexedDB tests, as this heavily uses IndexedDB. r=#dom-workers-and-storage

Depends on D56015

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c73a68b20dde
Make use of FlippedOnce in TransactionBase. r=dom-workers-and-storage-reviewers,ytausky

Comment 27

[Narcis Beleuzu [:NarcisB]]

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

Comment 28

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/integration/autoland/rev/ac4d8e02e94357beb32eed83b4a5ab639c56cbb6
https://hg.mozilla.org/integration/autoland/rev/d26634229052dc5e3f0261c2e4bf7ffc14715d52
https://hg.mozilla.org/integration/autoland/rev/8726e5de920473058ffc6df94633a0263e31decd

Comment 29

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ac4d8e02e943
https://hg.mozilla.org/mozilla-central/rev/8726e5de9204
https://hg.mozilla.org/mozilla-central/rev/d26634229052

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69f0ebc63a61
Extended InitializedOnce to allow restricting the valid values. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/0989faf6b5f1
Reduce statefulness of TransactionBase using const and InitializedOnce. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/7a3e7318e28e
Made FlippedOnce member functions constexpr. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/26d96c797b0f
Reduce statefulness of DatabaseConnection using InitializedOnce. r=dom-workers-and-storage-reviewers,ytausky

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b4c307ed8a4
Use move semantics with ThreadInfo. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/bcee5e565d96
Use move semantics with IdleDatabaseInfo. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/cd458b52a7ea
Encapsulate DatabaseOperationBase::mResultCode. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/e722062cbfc2
Reduce statefulness of TransactionDatabaseOperationBase using InitializedOnce. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/1ed684598bd0
Disable remote settings blocklist when running indexedDB tests, as this heavily uses IndexedDB. r=dom-workers-and-storage-reviewers,ytausky

Comment 32

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/69f0ebc63a61
https://hg.mozilla.org/mozilla-central/rev/0989faf6b5f1
https://hg.mozilla.org/mozilla-central/rev/7a3e7318e28e
https://hg.mozilla.org/mozilla-central/rev/26d96c797b0f
https://hg.mozilla.org/mozilla-central/rev/9b4c307ed8a4
https://hg.mozilla.org/mozilla-central/rev/bcee5e565d96
https://hg.mozilla.org/mozilla-central/rev/cd458b52a7ea
https://hg.mozilla.org/mozilla-central/rev/e722062cbfc2
https://hg.mozilla.org/mozilla-central/rev/1ed684598bd0

Comment 33

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

Attachment: Bug 1600906 - Avoid unnecessary copy. r=#dom-workers-and-storage

Comment 34

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

Attachment: Bug 1600906 - Merge identical switch cases. r=#dom-workers-and-storage

Depends on D57986

Comment 35

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

Attachment: Bug 1600906 - Encapsulate ThreadInfo; use std::move instead of swap, use const where then possible. r=#dom-workers-and-storage

Depends on D57987

Comment 36

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

Attachment: Bug 1600906 - Use std::move instead of swap, use const where then possible. r=#dom-workers-and-storage

Depends on D57988

Comment 37

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

Attachment: Bug 1600906 - Use InitializedOnce in DirectoryInfo to allow moving instances without additional state. r=#dom-workers-and-storage

Depends on D57989

Comment 38

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

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

Depends on D57990

Comment 39

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

Attachment: Bug 1600906 - Avoid copying key buffers. r=#dom-workers-and-storage

Depends on D57991

Comment 40

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

Attachment: Bug 1600906 - Convert IDBCursor and BackgroundCursorChild to templates to increase type safety and reduce state. r=#dom-workers-and-storage

This also simplifies delegating calls that are dependent on the cursor type.

Also reduce dependency on IDBCursor.h by moving enums and type traits to IDBCursorType.h

Depends on D57992

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/498d5e653e97
Avoid unnecessary copy. r=dom-workers-and-storage-reviewers,ttung
https://hg.mozilla.org/integration/autoland/rev/5b13eeec7485
Merge identical switch cases. r=dom-workers-and-storage-reviewers,ttung

Comment 42

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/498d5e653e97
https://hg.mozilla.org/mozilla-central/rev/5b13eeec7485

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8b95dac9290
Encapsulate ThreadInfo; use std::move instead of swap, use const where then possible. r=dom-workers-and-storage-reviewers,ytausky

Comment 44

[Raul Gurzau (:RaulG)]

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

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f7410826381
Use std::move instead of swap, use const where then possible. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/801f97f67366
Use InitializedOnce in DirectoryInfo to allow moving instances without additional state. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/94ca5d86b3b0
Use scoped enums in IDBCursor. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/4d7b95550368
Avoid copying key buffers. r=dom-workers-and-storage-reviewers,ytausky

Comment 46

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/8f7410826381
https://hg.mozilla.org/mozilla-central/rev/801f97f67366
https://hg.mozilla.org/mozilla-central/rev/94ca5d86b3b0
https://hg.mozilla.org/mozilla-central/rev/4d7b95550368

Comment 47

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

Attachment: Bug 1600906 - Transform Cursor into a template depending on the cursor type. r=#dom-workers-and-storage

This change reduces the number of data members and state in individual classes
and ensures that for each cursor type only the data members actually used
exist at compile time rather than requiring various assertions and case
distinctions at run-time. The mapping from the type known at run-time to the
compile-time types is done at a single code location in
TransactionBase::AllocCursor.

The ObjectStore vs. Index and Key vs. Value cursor type aspects are now properly
separated in different classes.

This patch also renames the misnamed CursorTypeTraits in ActorsParent.cpp to
PopulateResponseHelper, which is necessary as part of this patch to resolve
the ambiguity with the CursorTypeTraits defined in IDBCursorType.h

It also eliminates the CursorPosition type, which is redundant with the
CursorData template defined in IDBCursorType.h, which is now reused on both
the parent and child sides to avoid unnecessary heterogeneity.

Depends on D57993

Comment 48

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

Attachment: Bug 1600906 - Harmonize types for index and object store ids. r=#dom-workers-and-storage

Formerly, a mixture of uint64_t, int64_t and even int32_t was used. Now,
int64_t is used via an IndexOrObjectStoreId type alias. int64_t is the
type used in PBackgroundIDBSharedTypes.ipdlh

Depends on D59478

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a114b9b8e6ea
Convert IDBCursor and BackgroundCursorChild to templates to increase type safety and reduce state. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/d61a65e2e4ac
Transform Cursor into a template depending on the cursor type. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/aae2faf1907c
Harmonize types for index and object store ids. r=dom-workers-and-storage-reviewers,ytausky

Comment 50

[Natalia Csoregi [:nataliaCs]]

Backed out for bustages on StaticAnalysisFunctions.h

backout: https://hg.mozilla.org/integration/autoland/rev/da9250ac90ba0ebed64a9141e73e0b8cec52daed

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-fuzzing%2Fdebug%2C%28bf%29&revision=aae2faf1907c41bad94d8b71b3fcf1f0e1bf9a55

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284407081&repo=autoland&lineNumber=73798

[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/nsError.h:15,
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - from /builds/worker/workspace/build/src/obj-firefox/dist/include/nscore.h:203,
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - from /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.h:10,
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - from /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:7:
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp: In member function 'bool mozilla::dom::indexedDB::{anonymous}::Cursor<CursorType>::VerifyRequestParams(const mozilla::dom::indexedDB::CursorRequestParams&, mozilla::dom::indexedDB::{anonymous}::CursorPosition<CursorType>&) const':
[task 2020-01-10T14:49:44.990Z] 14:49:44 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/StaticAnalysisFunctions.h:65:47: error: no match for 'operator!' (operand type is 'mozilla::dom::indexedDB::{anonymous}::CursorBase')
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - # define MOZ_CHECK_ASSERT_ASSIGNMENT(expr) (!!(expr))
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - ^~~~~~~
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/Likely.h:17:48: note: in definition of macro 'MOZ_UNLIKELY'
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - # define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
[task 2020-01-10T14:49:44.990Z] 14:49:44 INFO - ^

Comment 51

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

(In reply to Natalia Csoregi [:nataliaCs] from comment #50)

Backed out for bustages on StaticAnalysisFunctions.h

... on the Linux x64 debug build-linux64-fuzzing/debug and Linux x64 debug hazard-linux64-haz/debug (H) builds only. Regular builds seem to be fine. I am reaching out to the fuzzing team to better understand this.

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e65d7e9dea10
Convert IDBCursor and BackgroundCursorChild to templates to increase type safety and reduce state. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/970b5ea41af0
Transform Cursor into a template depending on the cursor type. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/2aa2a5d78d34
Harmonize types for index and object store ids. r=dom-workers-and-storage-reviewers,ytausky

Comment 53

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e65d7e9dea10
https://hg.mozilla.org/mozilla-central/rev/970b5ea41af0
https://hg.mozilla.org/mozilla-central/rev/2aa2a5d78d34

Comment 54

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

I consider this bug done by now. State has been significantly reduced. Future similar work should be done under a new bug.