Depends on D40953

Depends on D40954

Depends on D40955

Depends on D40956

The functionality has been moved to DatabaseOperationBase::BindKeyRangeToStatement
resp. DatabaseOperationBase::GetBindingClauseForKeyRange as part of Bug 994190.

Depends on D40957

Also reduced duplication within the extracted methods.

Depends on D41024

Depends on D41025

Depends on D41026

Depends on D41191

Depends on D41192

Depends on D41225

Depends on D41403

Depends on D41404

Please land revisions D40953, D40957, D41024 and D41191.

Do not land D41403 yet, it requires changes.

The other revisions have not yet completed review.

Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/4f3b348afe01
Fixed wrong method name in log message. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/26a290615bbd
Reduce code duplication for different cursor directions. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/f0f5d12cc2b7
Removed unused methods in IDBKeyRange. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/fa95640baa86
Added missing assignment of Exists result to rv. r=ttung,asuth

In the future, please order the changesets that the ones ready to land are directly after the base commit else the automatic landing system Lando will block them. This can be done with Edit Related Revision on the right side of a phabricator page.

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #43)

In the future, please order the changesets that the ones ready to land are directly after the base commit else the automatic landing system Lando will block them. This can be done with Edit Related Revision on the right side of a phabricator page.

Ok, thanks for the advice. I reordered the commits locally, and resubmitted them with moz-phab, which I hoped did the adjustment of the parent-child-relationships automatically, but apparently it does not modify them if the revisions already exist on Phabricator.

Depends on D41404

Depends on D42852

https://hg.mozilla.org/mozilla-central/rev/4f3b348afe01
https://hg.mozilla.org/mozilla-central/rev/26a290615bbd
https://hg.mozilla.org/mozilla-central/rev/f0f5d12cc2b7
https://hg.mozilla.org/mozilla-central/rev/fa95640baa86

Depends on D42853

Depends on D43038

When implementing this, I found a problem with a web-platform-test here: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/IndexedDB/idbcursor_iterating.htm#54

This deletes the next record after the current cursor position via IDBObjectStore.delete, and expects that the cursor does not iterate to that deleted record. If that record has been preloaded, the cursor however does not take notice of that with the current implementation approach, and I don't see a way right now how to resolve this efficiently. If we need to check in the child for each record if it still exists, then preloading will become ineffective IMO. However, I am not sure if this web-platform-test is correct. Does the specification really imply that the cursor must take notice of this?

Note that this issue doesn't exist if IDBCursor.delete is called. In that case, we can invalidate the cached records, since this is local to the cursor object. (Apart from that probably not being necessary at all, since IDBCursor.delete can only delete the current record, but not a subsequent one.)

(In reply to Simon Giesecke [:sg] [he/him] from comment #50)

When implementing this, I found a problem with a web-platform-test here: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/IndexedDB/idbcursor_iterating.htm#54

This deletes the next record after the current cursor position via IDBObjectStore.delete, and expects that the cursor does not iterate to that deleted record. If that record has been preloaded, the cursor however does not take notice of that with the current implementation approach, and I don't see a way right now how to resolve this efficiently. If we need to check in the child for each record if it still exists, then preloading will become ineffective IMO. However, I am not sure if this web-platform-test is correct. Does the specification really imply that the cursor must take notice of this?

Note that this issue doesn't exist if IDBCursor.delete is called. In that case, we can invalidate the cached records, since this is local to the cursor object. (Apart from that probably not being necessary at all, since IDBCursor.delete can only delete the current record, but not a subsequent one.)

I think I resolved this after discussion with :ytausky, at least for the particular case mentioned above: the delete call is done in the same transaction the cursor was opened from, and in that case, we can actually notify the cursor.

As Yaron pointed out, the spec says "It is possible for the list of records which the cursor is iterating over to change before the full range of the cursor has been iterated." and some more related things in the rest of the paragraph, but this can interpreted in different ways. One interpretation (and Yaron thought this is more likely) is that this states that the cursor indeed should observe such changes, at least in some cases (and the test referenced above materializes this). However, another interpretation would just be that it is a note to the implementor not to try to maintain the position as an index (which is indeed explicitly stated in the next sentence: "It is possible for the list of records which the cursor is iterating over to change before the full range of the cursor has been iterated."), which would be invalidated by deletion operation, and saying nothing about the observability of changes.

NB: I found the discussion leading to this wording, see https://www.w3.org/Bugs/Public/show_bug.cgi?id=10088 and the thread starting with http://lists.w3.org/Archives/Public/public-webapps/2010JulSep/0056.html

Following up on this, I found the statements in https://w3c.github.io/IndexedDB/#transaction-scheduling, which clarify the isolation of multiple transactions (while never using the term "isolation"). I am not sure which way suggested in the box after read-only transactions we take, however I haven't seen any mention of snapshots taken in the code, so I assume we don't do that.

Depends on D43039

Depends on D43249

The key is assumed to be unset for now.

Depends on D43250

Depends on D43251

Depends on D43252

Depends on D43460

Right, our implementation doesn't do snapshots. Per https://github.com/w3c/IndexedDB/issues/253 I think Chrome/Blink did implement snapshotting and they've now removed it (or are still planning to).

The conclusions in comment 51 seem right to me. Such mutations can only occur in a write transaction and a write transaction can only occur in a single global, allowing for synchronous access to all related interfaces exposed to content. Requests are serialized in terms of mutation (http://lists.w3.org/Archives/Public/public-webapps/2010JulSep/0063.html), which makes it feasible and appropriate for us to synchronously invalidate all preloaded results for all cursors belonging to the mutated object store or its indices that haven't already been "consumed" by the act of invoking continue() which posts a task with the consumed result.

Depends on D43461

Depends on D43466

Depends on D43467

Depends on D43470

Depends on D43631

I think by the existing patches I solved this mostly. One case that is not handled nicely on the child side is the use of ContinuePrimaryKey, which does not use the preloaded results, but invalidates all. I might open a separate bug to fix that.

However, one more critical thing that still needs to be solved is the ordering when there are multiple interleaved requests. Currently, two wpts are failing (large-requests-abort.html, request-event-ordering.html), complaining about wrong event ordering (https://treeherder.mozilla.org/#/jobs?repo=try&revision=b948a3fc9243a8ff46c3b96316117296bf188748&selectedJob=264203112). This appears to be trickier than I thought. Without preloading, the sequence was implicitly kept because each child request was processed by the parent, and it processed them in order. With preloading, some child requests are handled by the child only. While they are not handled synchronously, but the response is dispatched and the order is not automatically held. It must be postponed until the right place in the sequence of requests. However, at the moment, a request sequence number is only maintained for logging purposes (IDBRequest::LoggingSerialNumber), and I have the impression that it is not always correct at places where it was not used before.

Any advice how to deal with this is appreciated. In particular:

• Is it the right approach to check serial/sequence numbers to produce the response at the right time?
• Can the IDBRequest::LoggingSerialNumber be used for this purpose? (which should probably be renamed not to refer to Logging anymore in that case)

Depends on D44009

Depends on D44377

This is the commit that introduced the ordering tests to web-platform-tests: https://github.com/web-platform-tests/wpt/commit/a414c1dd8ea33b9884ea87d13bd493629b640b44. It has a rather long description, but the main purpose of this is related to blob serialization, and I don't oversee in detail how this relates to the requirement for request ordering.

Okay, so AIUI (as I understand it) https://w3c.github.io/IndexedDB/#transaction-lifetime in step 2 explicitly does call out that requests must be processed in order, so the test is currently right.

Approach-wise, the simplest approach if of course just to invalidate if there's ever a request issued that isn't a consecutive cursor operation. But that seems likely to induce prefetch misses that would have been prefetch hits if we could delay the response. This might need/want a serial number mechanism anyways? If you're okay with the complexity for this stack, I think it would certainly be desirable to pursue delaying the results appropriately.

I think we can repurpose LoggingSerialNumber, yes. It seems like we issue it right now off of the weird ThreadLocal thing at https://searchfox.org/mozilla-central/rev/e5327b05c822cdac24e233afa37d72c0552dbbaf/dom/indexedDB/ActorsChild.h#59 from a per-thread one-up counter. We also have one-up counters for transaction serial numbers and version change transaction serial numbers. I guess we can leave transaction numbers as-is, but I'm fine with any/all overhauls here as long as we ensure that the IDB_LOG invocations still create a unique tuple in what they log over thread identifiers, pointers logged, and the various transaction/request id's. We can add and log an id on the database if needed.

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #68)

Okay, so AIUI (as I understand it) https://w3c.github.io/IndexedDB/#transaction-lifetime in step 2 explicitly does call out that requests must be processed in order, so the test is currently right.

Right, it is very explicit about that.

Approach-wise, the simplest approach if of course just to invalidate if there's ever a request issued that isn't a consecutive cursor operation.
I am just becoming unsure if there might be a missing invalidation. I already invalidate the cached results in some cases that could affect their validity. However, this might not solve the problem if the invalidation is refined more, and only done conditionally for cursors that might actually be affected, since the change affects their remaining range. Currently, the invalidation does not check that.

But that seems likely to induce prefetch misses that would have been prefetch hits if we could delay the response. This might need/want a serial number mechanism anyways? If you're okay with the complexity for this stack, I think it would certainly be desirable to pursue delaying the results appropriately.

As said above, maybe currently there is only an invalidation missing. In that case, I might only add that now, and postpone the serial number issue to a separate bug that will introduce conditional invalidation as mentioned above. Otherwise, I will do it as part of this bug.

I think we can repurpose LoggingSerialNumber, yes. It seems like we issue it right now off of the weird ThreadLocal thing at https://searchfox.org/mozilla-central/rev/e5327b05c822cdac24e233afa37d72c0552dbbaf/dom/indexedDB/ActorsChild.h#59 from a per-thread one-up counter. We also have one-up counters for transaction serial numbers and version change transaction serial numbers. I guess we can leave transaction numbers as-is, but I'm fine with any/all overhauls here as long as we ensure that the IDB_LOG invocations still create a unique tuple in what they log over thread identifiers, pointers logged, and the various transaction/request id's. We can add and log an id on the database if needed.

Ok, thanks for the judgment!

Depends on D44443

Depends on D44988

Please checkin D41403, D42645, D40954, D40955, D40956

Also D41025, D41026 and D41225

Depends on D45308

Pushed by malexandru@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e72ef41a4a78
Added documentation on Cursor data members. r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/07cafa88a5b1
Added comments on almost-const data members r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e46dad972c8
Reduce the use of plain pointers, and use const for Cursor data members where possible. r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6aa1c5bc7d9
Change definition of OpenCursorParams subtypes to allow reducing duplicated code r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/300dc3034eeb
Reduce code duplication for different open cursor variants. r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9c38ead5e55
Extracted methods Cursor::OpenOp::PrepareKeyConditionClauses and PrepareIndexKeyConditionClause. r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/eff806e9615d
Unified handling of literal strings using NS_LITERAL_(C)STRING and removed some duplications. r=ttung
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5809ccdc7d2
Extract DatabaseConnection::ExecuteCachedStatement function. r=ttung

Backed out 8 changesets for causing failures in DatabaseConnection::CachedStatement function.

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/7c22109981db288df076212551147c9e050565e3

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&tochange=7c22109981db288df076212551147c9e050565e3&fromchange=a5809ccdc7d225f326359350494cd62bac3eed9a

Failure logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=265936275&repo=mozilla-inbound&lineNumber=1478
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=265936398&repo=mozilla-inbound&lineNumber=1474

[task 2019-09-10T15:23:04.350Z] 15:23:04 INFO - Assertion failure: mStatement, at /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:10086
[task 2019-09-10T15:23:04.351Z] 15:23:04 INFO - Assertion failure: mStatement, at /builds/worker/workspace/build/src/dom/indexedDB/ActorsParent.cpp:10086

[task 2019-09-10T15:22:31.605Z] 15:22:31 INFO - TEST-START | /animation-worklet/animation-worklet-inside-iframe.https.html
[task 2019-09-10T15:22:31.607Z] 15:22:31 INFO - Closing window 14
[task 2019-09-10T15:22:32.528Z] 15:22:32 INFO - IOError on command, setting status to CRASH
[task 2019-09-10T15:22:33.891Z] 15:22:33 INFO - mozcrash Copy/paste: /builds/worker/workspace/build/linux64-minidump_stackwalk /tmp/tmpjq9cwD/475fa87d-291d-37e4-f4f6-1f3078823238.dmp /builds/worker/workspace/build/symbols
[task 2019-09-10T15:22:39.497Z] 15:22:39 INFO - mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/475fa87d-291d-37e4-f4f6-1f3078823238.dmp
[task 2019-09-10T15:22:39.497Z] 15:22:39 INFO - mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/475fa87d-291d-37e4-f4f6-1f3078823238.extra
[task 2019-09-10T15:22:39.616Z] 15:22:39 INFO - PROCESS-CRASH | /animation-worklet/animation-worklet-inside-iframe.https.html | application crashed [@ mozilla::dom::indexedDB::(anonymous namespace)::DatabaseConnection::CachedStatement::operator mozIStorageStatement*() const]
[task 2019-09-10T15:22:39.616Z] 15:22:39 INFO - Crash dump filename: /tmp/tmpjq9cwD/475fa87d-291d-37e4-f4f6-1f3078823238.dmp
[task 2019-09-10T15:22:39.616Z] 15:22:39 INFO - Operating system: Android
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - 0.0.0 Linux 3.10.0+ #260 SMP PREEMPT Fri May 19 12:48:14 PDT 2017 x86_64
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - CPU: amd64
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - family 6 model 6 stepping 3
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - 4 CPUs
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO -
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - GPU: UNKNOWN
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO -
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Crash reason: SIGSEGV /SEGV_MAPERR
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Crash address: 0x0
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Process uptime: not available
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO -
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Thread 42 (crashed)
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - 0 libxul.so!mozilla::dom::indexedDB::(anonymous namespace)::DatabaseConnection::CachedStatement::operator mozIStorageStatement*() const [ActorsParent.cpp:a5809ccdc7d225f326359350494cd62bac3eed9a : 10086 + 0x29]
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rax = 0x00007e298609e313 rdx = 0x0000000000000004
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rcx = 0x00007e2988df2ab0 rbx = 0x00007e298897a590
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rsi = 0x00007e2988979dc0 rdi = 0x000000000000001b
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rbp = 0x00007e298897a480 rsp = 0x00007e298897a470
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r8 = 0x0000000000000000 r9 = 0x00007e29a5a2a090
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r10 = 0x0000000000000013 r11 = 0x0000000000000246
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r12 = 0x00007e29763c6eb0 r13 = 0x0000000000000002
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r14 = 0x00007e298897a50c r15 = 0x00007e298897a590
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rip = 0x00007e29820990f8
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Found by: given as instruction pointer in context
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - 1 libxul.so!mozilla::dom::indexedDB::(anonymous namespace)::DatabaseConnection::GetFreelistCount(mozilla::dom::indexedDB::(anonymous namespace)::DatabaseConnection::CachedStatement&, unsigned int*) [ActorsParent.cpp:a5809ccdc7d225f326359350494cd62bac3eed9a : 9925 + 0x8]
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rbx = 0x00007e298897a4a0 rbp = 0x00007e298897a4f0
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rsp = 0x00007e298897a490 r12 = 0x00007e29763c6eb0
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r13 = 0x0000000000000002 r14 = 0x00007e298897a50c
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r15 = 0x00007e298897a590 rip = 0x00007e2982098806
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Found by: call frame info
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - 2 libxul.so!mozilla::dom::indexedDB::(anonymous namespace)::DatabaseConnection::DoIdleProcessing(bool) [ActorsParent.cpp:a5809ccdc7d225f326359350494cd62bac3eed9a : 9744 + 0xb]
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rbx = 0x00007e298897a590 rbp = 0x00007e298897a6a0
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - rsp = 0x00007e298897a500 r12 = 0x00007e2976324f10
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r13 = 0x0000000000000002 r14 = 0x0000000000000001
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - r15 = 0x00007e29763c6eb0 rip = 0x00007e2982097a9d
[task 2019-09-10T15:22:39.617Z] 15:22:39 INFO - Found by: call frame info
[task 2019-09-10T15:22:39.618Z] 15:22:39 INFO - 3 libxul.so!mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::IdleConnectionRunnable::Run() [ActorsParent.cpp:a5809ccdc7d225f326359350494cd62bac3eed9a : 11717 + 0x11]

I was able to reproduce the crash, however only after rebasing. I will investigate which intermediate changes cause this.

I hopefully fixed this, please land D41403, D42645, D40954, D40955, D40956, D41025, D41026 and D41225 again.

Pushed by malexandru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0133605c6169
Added documentation on Cursor data members. r=asuth
https://hg.mozilla.org/integration/autoland/rev/acae76cea927
Added comments on almost-const data members r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/f5aee1ae7282
Reduce the use of plain pointers, and use const for Cursor data members where possible. r=ttung
https://hg.mozilla.org/integration/autoland/rev/6078d5a9c010
Change definition of OpenCursorParams subtypes to allow reducing duplicated code r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/5be5a3bf0499
Reduce code duplication for different open cursor variants. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/4858e4e132de
Extracted methods Cursor::OpenOp::PrepareKeyConditionClauses and PrepareIndexKeyConditionClause. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/1485cea56f41
Unified handling of literal strings using NS_LITERAL_(C)STRING and removed some duplications. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/066bee573945
Extract DatabaseConnection::ExecuteCachedStatement function. r=ytausky,asuth

Backed out 8 changesets for failures in idbcursor-continuePrimaryKey.htm

Backout link: https://hg.mozilla.org/integration/autoland/rev/e78f7880e00a07add5028fc0bb4e860151907dd7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=266090246&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&tochange=e78f7880e00a07add5028fc0bb4e860151907dd7&fromchange=066bee5739450f5f62f64e22da65611a108eeea5&searchStr=wpt18

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266092823&repo=autoland&lineNumber=14042

[task 2019-09-11T11:01:24.811Z] 11:01:24 INFO - PID 5132 | ++DOMWINDOW == 4 (000001FEEB890000) [pid = 10624] [serial = 4] [outer = 000001FEEB812020]
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO -
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - TEST-UNEXPECTED-FAIL | /IndexedDB/idbcursor-continuePrimaryKey.htm | IndexedDB: IDBCursor method continuePrimaryKey() - cursor is null
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - verifyContinueCalls/request.onsuccess<@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:113:15
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - EventHandlerNonNullverifyContinueCalls@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:109:31
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - EventHandlerNonNullverifyContinueCalls@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:103:28
[task 2019-09-11T11:01:25.022Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - EventHandlerNonNullverifyContinueCalls@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:103:28
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - EventHandlerNonNullverifyContinueCalls@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:103:28
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - EventHandlerNonNullverifyContinueCalls@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:103:28
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - EventHandlerNonNullverifyContinueCalls@http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:103:28
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - @http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:132:7
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - indexeddb_test/</open.onsuccess<@http://web-platform.test:8000/IndexedDB/support.js:131:20
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1635:35
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - EventHandlerNonNull*indexeddb_test/<@http://web-platform.test:8000/IndexedDB/support.js:128:26
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1611:25
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - async_test@http://web-platform.test:8000/resources/testharness.js:576:22
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - indexeddb_test@http://web-platform.test:8000/IndexedDB/support.js:105:13
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - @http://web-platform.test:8000/IndexedDB/idbcursor-continuePrimaryKey.htm:13:15
[task 2019-09-11T11:01:25.023Z] 11:01:25 INFO - TEST-OK | /IndexedDB/idbcursor-continuePrimaryKey.htm | took 511ms

Fixed the issue in idbcursor-continuePrimaryKey.htm. Please land D41403, D42645, D40954, D40955, D40956, D41025, D41026 and D41225 again.

Pushed by malexandru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/96ab87dbe78f
Added documentation on Cursor data members. r=asuth
https://hg.mozilla.org/integration/autoland/rev/f25d82f6f518
Added comments on almost-const data members r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/556059030dc6
Reduce the use of plain pointers, and use const for Cursor data members where possible. r=ttung
https://hg.mozilla.org/integration/autoland/rev/0e77abe58616
Change definition of OpenCursorParams subtypes to allow reducing duplicated code r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/23fc7469065d
Reduce code duplication for different open cursor variants. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/fb5e8aa853b0
Extracted methods Cursor::OpenOp::PrepareKeyConditionClauses and PrepareIndexKeyConditionClause. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/f951fbdc5160
Unified handling of literal strings using NS_LITERAL_(C)STRING and removed some duplications. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/d1ce54f71152
Extract DatabaseConnection::ExecuteCachedStatement function. r=ytausky,asuth

https://hg.mozilla.org/mozilla-central/rev/96ab87dbe78f
https://hg.mozilla.org/mozilla-central/rev/f25d82f6f518
https://hg.mozilla.org/mozilla-central/rev/556059030dc6
https://hg.mozilla.org/mozilla-central/rev/0e77abe58616
https://hg.mozilla.org/mozilla-central/rev/23fc7469065d
https://hg.mozilla.org/mozilla-central/rev/fb5e8aa853b0
https://hg.mozilla.org/mozilla-central/rev/f951fbdc5160
https://hg.mozilla.org/mozilla-central/rev/d1ce54f71152

Depends on D45506

Depends on D45674

Please land patches D41404, D42852, D42853, D43038, D43039, D43249, D43250

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65938107d86c
Move the functionality of OpenOp::GetKeyRangeInfo to Cursor::SetOptionalKeyRange r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/dc5a09a61450
Remove code duplication in DatabaseOperationBase::BindKeyRangeToStatement overloads r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/96ced3a647df
Refactored building of key range and direction clauses. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/8cc54010f8a2
Make limit parameter a statement parameter. r=ytausky,asuth
https://hg.mozilla.org/integration/autoland/rev/714d36c43204
Inline Key::Assert function to make effective use of MOZ_ASSERT diagnostics. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/9966f5d71bb2
Apply renamings suggested for Cursor::*Key members and related identifiers. r=ytausky,asuth
https://hg.mozilla.org/integration/autoland/rev/d7a6473d622e
Refactored tests, reduced duplicated code. r=ttung,asuth

https://hg.mozilla.org/mozilla-central/rev/65938107d86c
https://hg.mozilla.org/mozilla-central/rev/dc5a09a61450
https://hg.mozilla.org/mozilla-central/rev/96ced3a647df
https://hg.mozilla.org/mozilla-central/rev/8cc54010f8a2
https://hg.mozilla.org/mozilla-central/rev/714d36c43204
https://hg.mozilla.org/mozilla-central/rev/9966f5d71bb2
https://hg.mozilla.org/mozilla-central/rev/d7a6473d622e

Depends on D45675

Backed out changeset 066bee573945 (bug 1168606)
Backed out changeset 1485cea56f41 (bug 1168606)
Backed out changeset 4858e4e132de (bug 1168606)
Backed out changeset 5be5a3bf0499 (bug 1168606)
Backed out changeset 6078d5a9c010 (bug 1168606)
Backed out changeset f5aee1ae7282 (bug 1168606)
Backed out changeset acae76cea927 (bug 1168606)
Backed out changeset 0133605c6169 (bug 1168606)

James, what does this mean? There are no details for the reason of the backout, the Phabricator revisions were not reopened, and I was not needinfo-ed. Is this referring to the issue already mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1168606#c81? I think I resolved that. I am not sure what to do now.

(In reply to Simon Giesecke [:sg] [he/him] from comment #93)

James, what does this mean? There are no details for the reason of the backout, the Phabricator revisions were not reopened, and I was not needinfo-ed. Is this referring to the issue already mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1168606#c81? I think I resolved that. I am not sure what to do now.

Hi, my apologies. I made some mistakes with Mercurial and ending up submitting the wrong patch, so this is totally in error... please ignore.

Please check-in D46576 and D43251

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/340dfa69aed8
Remove duplication in uses of IDB_LOG_MARK. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/d2772a7482ed
Allow current key to be provided with PBackgroundIDBCursor::Continue. r=ttung,asuth

https://hg.mozilla.org/mozilla-central/rev/340dfa69aed8
https://hg.mozilla.org/mozilla-central/rev/d2772a7482ed

Please check-in D43470, D43631, D44988 and D43460

Pushed by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e493a6b5f125
Avoid copying keys on cursor creation. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/9e7a90dcc429
Use const where easily possible. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/f844afbebc6a
Replaced custom for loops by range-based for or STL algorithms. r=ttung
https://hg.mozilla.org/integration/autoland/rev/63cf0966cb41
Do not use memutils for nsTArray of IndexCursorResponse. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/fc5e1bee1332
Send two records with every ObjectStoreCursorResponse. r=ttung,asuth

Backed out 5 changesets (Bug 1168606) for xpcshell failures in dom/indexedDB/test/unit/test_temporary_storage.js

Backout: https://hg.mozilla.org/integration/autoland/rev/e7e97d7a6a43d7012dc5517b088122bb7b543a65

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fc5e1bee13321946704a915ef4c7c80c0941dcb3&selectedJob=269311913

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269311913&repo=autoland&lineNumber=2639

I fixed the issue with xpcshell tests on Android, and the patches should be ready for landing now.

https://phabricator.services.mozilla.com/D43631 has a conflict:

applying /tmp/tmp3LcxUW dom/indexedDB/ActorsParent.cpp Hunk #26 FAILED at 16475. 1 out of 62 hunks FAILED -- saving rejects to file dom/indexedDB/ActorsParent.cpp.rej abort: patch command failed: exited with status 256

https://hg.mozilla.org/mozilla-central/rev/ea140aa9122543c1964c4641cb566d168e127022#l1.169 conflicted.

I rebased the patches to current mozilla-central.

I ran tests at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=05e33e6d918f35d3b3f84290d5fb17ee9c2beb75

D43470, D43631, D44988 and D43460 should be ready now.

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc11a47c3ea7
Avoid copying keys on cursor creation. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/cddcfdcb52f6
Use const where easily possible. r=ttung,asuth
https://hg.mozilla.org/integration/autoland/rev/acf2d5bca8b1
Replaced custom for loops by range-based for or STL algorithms. r=ttung
https://hg.mozilla.org/integration/autoland/rev/ad5ebdad4096
Do not use memutils for nsTArray of IndexCursorResponse. r=froydnj

https://hg.mozilla.org/mozilla-central/rev/fc11a47c3ea7
https://hg.mozilla.org/mozilla-central/rev/cddcfdcb52f6
https://hg.mozilla.org/mozilla-central/rev/acf2d5bca8b1
https://hg.mozilla.org/mozilla-central/rev/ad5ebdad4096

NI on myself to look deeper into the https://phabricator.services.mozilla.com/D48504 test case.

Depends on D48504

Depends on D46593

I'm seeing a crash on OSX local builds of mozilla-central.
It ends in a EXC_BAD_ACCESS from mozilla::dom::indexedDB::(anonymous namespace)::Cursor::OpenOp::DoDatabaseWork(this=0x000000012ca69500, aConnection=0x000000012ca7bb80)::DatabaseConnection*) at ActorsParent.cpp:26316

https://paste.rs/0Y7

So far it looks like I'm the only person seeing this.

As reported in https://bugzilla.mozilla.org/show_bug.cgi?id=1168606#c110,
some Mac OS X release build crashes with the original code, probably due
to some temporary string being destroyed prematurely. Assigning to a
named variable reportedly solves the issue.

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

A Pernosco trace for the https://phabricator.services.mozilla.com/D48504 test case is now at https://pernos.co/debug/xIn5BlmpbBNwDwZyvOQ3Fg/index.html

Backed out for causing build bustages on ActorsParent.cpp and Assertions.h.

Pushes with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=a2ae085f929f99c56ce147e21950dc0eb982583e&selectedJob=274622737

https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=307bd8d244079e2ee96773b93938f8d556041211&selectedJob=274628664

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=274622737&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=274628664&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/2172f9a7aaf042b10bc1c789f7fea97fc50ba0ef

https://hg.mozilla.org/mozilla-central/rev/2f0447b228c7
https://hg.mozilla.org/mozilla-central/rev/89817a19f3b3
https://hg.mozilla.org/mozilla-central/rev/c009bbdbbfd5
https://hg.mozilla.org/mozilla-central/rev/b525ad6838f0
https://hg.mozilla.org/mozilla-central/rev/cf53f4e57313
https://hg.mozilla.org/mozilla-central/rev/1abd3e7f9df4
https://hg.mozilla.org/mozilla-central/rev/9eeb9fd126bc
https://hg.mozilla.org/mozilla-central/rev/c6d3254dcd8e
https://hg.mozilla.org/mozilla-central/rev/61c29e7a5419
https://hg.mozilla.org/mozilla-central/rev/bd872bd17560
https://hg.mozilla.org/mozilla-central/rev/c33c9703069b

This applies to https://phabricator.services.mozilla.com/D48504

Feel free to obsolete this log and the change patch when addressed.

And to be super clear, we should only enable the (amazing, awesome, altogether massive performance enhancement) preloading feature once that test passes.

Depends on D49274

https://hg.mozilla.org/mozilla-central/rev/43dbad71664b
https://hg.mozilla.org/mozilla-central/rev/a36036d07b9f
https://hg.mozilla.org/mozilla-central/rev/69a0758705a9

https://hg.mozilla.org/mozilla-central/rev/fe3a07722fee
https://hg.mozilla.org/mozilla-central/rev/fc34f807986e
https://hg.mozilla.org/mozilla-central/rev/1ca7e5b24181