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

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

Attachment: patch v1.0

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.

Comment 1

[Shawn Wilsher :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).

Comment 2

[Marco Bonardo [:mak]]

Attachment: patch v1.1

when you're right, you're right.

Comment 3

[Marco Bonardo [:mak]]

note to self: pointers style should go toward type.

Comment 4

[Shawn Wilsher :sdwilsh]

Comment on attachment 439396 [details] [diff] [review]
patch v1.1

r=sdwilsh

Comment 5

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/3e1263cff09e