Reduce state in dom/indexedDB classes
Categories
(Core :: Storage: IndexedDB, task, P3)
Tracking
()
People
(Reporter: sg, Assigned: sg)
References
(Regressed 1 open bug)
Details
Attachments
(31 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Assignee | ||
Comment 1•1 year ago
|
||
Depends on D55081
Assignee | ||
Comment 2•1 year ago
|
||
Depends on D55674
Assignee | ||
Comment 3•1 year ago
|
||
Depends on D55675
Assignee | ||
Comment 4•1 year ago
|
||
Depends on D55676
Assignee | ||
Comment 5•1 year ago
|
||
Depends on D55677
Assignee | ||
Comment 6•1 year ago
|
||
Depends on D55678
Assignee | ||
Comment 7•1 year ago
|
||
Depends on D55679
Assignee | ||
Comment 8•1 year ago
|
||
Depends on D55680
Assignee | ||
Comment 9•1 year ago
|
||
Depends on D55681
Comment 10•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d6c086ca5780 Make use of FlippedOnce in VersionChangeTransaction. r=dom-workers-and-storage-reviewers,ytausky
Assignee | ||
Updated•1 year ago
|
Comment 11•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
bugherder |
Comment 13•1 year ago
|
||
bugherder |
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
Assignee | ||
Comment 14•1 year ago
|
||
Assignee | ||
Comment 15•1 year ago
|
||
Depends on D56005
Assignee | ||
Comment 16•1 year ago
|
||
Depends on D56006
Assignee | ||
Comment 17•1 year ago
|
||
Depends on D56007
Assignee | ||
Comment 18•1 year ago
|
||
Depends on D56008
Assignee | ||
Comment 19•1 year ago
|
||
Depends on D56009
Assignee | ||
Comment 20•1 year ago
|
||
Depends on D56010
Assignee | ||
Comment 21•1 year ago
|
||
Depends on D56011
Assignee | ||
Comment 22•1 year ago
|
||
Depends on D56012
Assignee | ||
Comment 23•1 year ago
|
||
Depends on D56013
Assignee | ||
Comment 24•1 year ago
|
||
Depends on D56014
Assignee | ||
Comment 25•1 year ago
|
||
Depends on D56015
Comment 26•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c73a68b20dde Make use of FlippedOnce in TransactionBase. r=dom-workers-and-storage-reviewers,ytausky
Comment 27•1 year ago
|
||
bugherder |
Comment 28•1 year ago
|
||
Comment 29•1 year ago
|
||
bugherder |
Comment 30•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
bugherder |
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
Assignee | ||
Comment 33•1 year ago
|
||
Assignee | ||
Comment 34•1 year ago
|
||
Depends on D57986
Assignee | ||
Comment 35•1 year ago
|
||
Depends on D57987
Assignee | ||
Comment 36•1 year ago
|
||
Depends on D57988
Assignee | ||
Comment 37•1 year ago
|
||
Depends on D57989
Assignee | ||
Comment 38•1 year ago
|
||
Depends on D57990
Assignee | ||
Comment 39•1 year ago
|
||
Depends on D57991
Assignee | ||
Comment 40•1 year ago
|
||
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•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Comment 43•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
bugherder |
Comment 45•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
bugherder |
Assignee | ||
Comment 47•1 year ago
|
||
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
Assignee | ||
Comment 48•1 year ago
|
||
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•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
Backed out for bustages on StaticAnalysisFunctions.h
backout: https://hg.mozilla.org/integration/autoland/rev/da9250ac90ba0ebed64a9141e73e0b8cec52daed
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 - ^
Assignee | ||
Comment 51•1 year ago
•
|
||
(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•1 year ago
|
||
Pushed by sgiesecke@mozilla.com: 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•1 year ago
|
||
bugherder |
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 54•1 year ago
|
||
I consider this bug done by now. State has been significantly reduced. Future similar work should be done under a new bug.
Description
•