Bug 94402 – IndexedDB: Fire error at request when abort is called in upgradeneeded

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[David Grogan]

IndexedDB: Fire error at request when abort is called in upgradeneeded

[David Grogan]

Created attachment 159250 [details]
Patch

[David Grogan]

Created attachment 159251 [details]
Patch

[David Grogan]

Josh, could you look at this?

The first of many addenda to the integer version stuff.

[David Grogan]

Created attachment 159253 [details]
Patch

[Peter Beverloo (cr-android ews)]

Comment on attachment 159253 [details]
Patch

Attachment 159253 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13535059

[Joshua Bell]

Comment on attachment 159253 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159253&action=review

lgtm

> Source/WebCore/ChangeLog:8
> +        Tests - updated expected.txt

Give test path/name here too.

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:325
>      RefPtr<IDBTransactionBackendImpl> transaction = prpTransaction;

Add an ASSERT(m_state == Finished) here?

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:340
> +    RefPtr<IDBTransactionBackendImpl> transaction = prpTransaction;

And here?

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:500
> +    ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersionCalls.size(), "m_pendingSecondHalfOpenWithVersionCalls.size = %lu", m_pendingSecondHalfOpenWithVersionCalls.size());

Based on EWS output, Android port's compiler requires a size_t->long long cast here, apparently.

[David Grogan]

Comment on attachment 159253 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=159253&action=review

>> Source/WebCore/ChangeLog:8
>> +        Tests - updated expected.txt
> 
> Give test path/name here too.

done

>> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:325
>>      RefPtr<IDBTransactionBackendImpl> transaction = prpTransaction;
> 
> Add an ASSERT(m_state == Finished) here?

Are you suggesting adding an accessor for IDBTransactionBackendImpl::m_state? I would guess you didn't realize that'd be necessary and that it's not worth it. But if you did anticipate that and think it's a good idea, I'll add it.

>> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:500
>> +    ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersionCalls.size(), "m_pendingSecondHalfOpenWithVersionCalls.size = %lu", m_pendingSecondHalfOpenWithVersionCalls.size());
> 
> Based on EWS output, Android port's compiler requires a size_t->long long cast here, apparently.

We'll see if zu works.

[Joshua Bell]

> > Add an ASSERT(m_state == Finished) here?
> 
> Are you suggesting adding an accessor for IDBTransactionBackendImpl::m_state? I would guess you didn't realize that'd be necessary and that it's not worth it. But if you did anticipate that and think it's a good idea, I'll add it.

Oops - sorry, my brain wasn't back from vacation here. I was thinking we were still in IDBTransactionBackendImpl.

No, keep state internal, no ASSERT.

[David Grogan]

Created attachment 159770 [details]
Patch

[David Grogan]

%zu worked on the android try job, it'll probably pass on cr-android.
http://build.chromium.org/p/tryserver.chromium/builders/android/builds/30549

[David Grogan]

Tony, could you review this when you get a chance?

[Tony Chang]

Comment on attachment 159770 [details]
Patch

OK

[WebKit Review Bot]

Comment on attachment 159770 [details]
Patch

Clearing flags on attachment: 159770

Committed r126239: <http://trac.webkit.org/changeset/126239>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.