618140 - IndexedDB: Don't fire success event callbacks once a transaction has been aborted

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Once a transaction has been aborted we should suppress any pending success event callbacks.

Comment 1

[Johnny Stenback (:jst)]

This is part of the last set of IndexedDB bugs we've deemed necessary in order to ship 2.0, blocking beta9.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 497326 [details] [diff] [review]
Patch, v1

We should fire an error event with ABORT_ERR rather than nothing.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v2

Ok, this converts everything to ABORT_ERR events instead.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 497435 [details] [diff] [review]
Patch, v2

>@@ -370,16 +378,22 @@ AsyncConnectionHelper::OnSuccess(nsIDOME
...
>+#ifdef DEBUG
>+  if (mTransaction && !mTransaction->TransactionIsOpen()) {
>+    NS_ASSERTION(mTransaction->IsAborted(), "How else can this be closed?!");
>+  }
>+#endif

NS_ASSERTION(!mTransaction ||
             mTransaction->TransactionIsOpen() ||
             mTransaction->IsAborted(), "How else can this be closed?!")

Also, shouldn't TransactionIsOpen simply be named to IsOpen?

>+#ifdef DEBUG
>+    if (mTransaction && !mTransaction->TransactionIsOpen()) {
>+      NS_ASSERTION(mTransaction->IsAborted(), "How else can this be closed?!");
>+  }
>+#endif

Same same.

>@@ -931,59 +944,60 @@ CommitHelper::Run()
...
>+    mTransaction->mFiredCompleteOrAbort = true;

#ifdef DEBUG?

r=me with that.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

http://hg.mozilla.org/mozilla-central/rev/a1f2e2bb9a7a

Comment 7

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)