Closed Bug 1274161 Opened 8 years ago Closed 8 years ago

Fix failure of idbobjectstore_createIndex6-event_order.htm in web-platform test.

Categories

(Core :: Storage: IndexedDB, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: bevis, Assigned: bevis)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tw-dom] btpp-active)

Attachments

(1 file)

This is a follow-up of issue#2 in bug 1273833 comment 1.
After further investigation, I found that we didn't follow |the steps for aborting a transaction| in [1] well:
1. The |transaction.error| shall be set to ConstraintError from the error cause of createIndex() in idbobjectstore_createIndex6-event_order.htm at step#3 of [1].
2. Then, dispatch error event to all the pending requests with AbortError in step#4 after *aborting* the procedure of the |steps for asynchronously executing a request| in [2].
3. Otherwise, |the steps for aborting a transaction| could be executed again by |Fire an error event| [3] in step#8 of [2] if not aborted.

The issue in gecko is that
1. We keep the ConstraintError in TransactionBase::mResultCode in ActorsParent.cpp for step 3.
2. However, we dispatch the error to the pending request without aborting the steps in [2] which cause gecko to set transaction.error to AbortError unexpectedly in [4].
3. This prevents us to update the ConstraintError to |transaction.error| in [5].

[1] http://w3c.github.io/IndexedDB/#steps-for-aborting-a-transaction
[2] http://w3c.github.io/IndexedDB/#dfn-steps-for-asynchronously-executing-a-request
[3] http://w3c.github.io/IndexedDB/#dfn-fire-an-error-event
[4] https://hg.mozilla.org/mozilla-central/annotate/c4449eab07d39e20ea315603f1b1863eeed7dcfe/dom/indexedDB/ActorsChild.cpp#l752
[5] https://hg.mozilla.org/mozilla-central/annotate/c4449eab07d39e20ea315603f1b1863eeed7dcfe/dom/indexedDB/IDBTransaction.cpp#l804
--
Failures in createIndex6 for reference:
"rq_add1.success",
"rq_add2.success",
"rq_add3.error: AbortError",
"transaction.error: AbortError",
"db.error: AbortError",
"transaction.abort: AbortError",  <-- "ConstraintError" is expected
"db.abort: AbortError",           <-- "ConstraintError" is expected
"open_rq.error: ConstraintError"  <-- "AbortError" is expected(Shall be address by issue#1 in bug 1273833 comment1.)
Depends on: 1273833
As explained in comment 0, we should dispatch error event with AbortError directly to the pending request without finishing the |steps for asynchronously executing a request| to prevent aborting the transaction proactively at this stage with incorrect transaction.error being set if the transaction is already aborted in ActorsParent.cpp.
Attachment #8754342 - Flags: review?(khuey)
Comment on attachment 8754342 [details] [diff] [review]
(v1) Patch: Do not abort the transaction twice if the IDBRequest is failed due to the abortion of its transaction.

pause the review until bug 1273833 comment 5 is addressed.
Attachment #8754342 - Flags: review?(khuey)
Status: NEW → ASSIGNED
Comment on attachment 8754342 [details] [diff] [review]
(v1) Patch: Do not abort the transaction twice if the IDBRequest is failed due to the abortion of its transaction.

Resume the review after bug 1273833 comment 5 is addressed.
Attachment #8754342 - Flags: review?(khuey)
Treeherder result in comment 5 looks fine.
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd26cef5ff88
Do not abort the transaction twice if the IDBRequest is failed due to the abortion of its transaction. r=khuey
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fd26cef5ff88
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: