Closed Bug 559682 Opened 11 years ago Closed 11 years ago

Ensure we create a valid transaction in batches before trying to commit it.

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a4

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch v1.0 (obsolete) — Splinter Review
Alternatively to this simple approach we could try to use the mozStorageTransaction helper and save it in a local mCurrentBatchTransaction, but looks like useless overhead.

I'm unsure this will solve any of our oranges though (but with Bug 559678 could help clarify something), even if the old code was not error checking beginTransaction() we don't fail on the associated commit() in endBatch but on others.
I'm trying to figure out if in some situation a batch could commit a transaction in the middle of another method, but looks like impossible.
Attachment #439376 - Flags: review?(sdwilsh)
Comment on attachment 439376 [details] [diff] [review]
patch v1.0

It won't be additional overhead; we already store mBatchHasTransaction, but we can replace this with the member variable to hold the transaction logic.  We also then have less code size, so it should be  win overall (plus less complexity).
Attachment #439376 - Flags: review?(sdwilsh) → review-
Attached patch patch v1.1Splinter Review
when you're right, you're right.
Attachment #439376 - Attachment is obsolete: true
Attachment #439396 - Flags: review?(sdwilsh)
note to self: pointers style should go toward type.
Comment on attachment 439396 [details] [diff] [review]
patch v1.1

r=sdwilsh
Attachment #439396 - Flags: review?(sdwilsh) → review+
http://hg.mozilla.org/mozilla-central/rev/3e1263cff09e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a4
Blocks: 558218
Blocks: 557525
Blocks: 557430
Blocks: 555014
Blocks: 537347
You need to log in before you can comment on or make changes to this bug.