Bug 107754 – IndexedDB: Remove IDBUpgradeNeededEvent, merge with IDBVersionChangeEvent

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

[Joshua Bell]

IndexedDB: Remove IDBUpgradeNeededEvent, merge with IDBVersionChangeEvent

[Joshua Bell]

Created attachment 184522 [details]
Patch

[Joshua Bell]

alecflett@ - can you take a look?

[Alec Flett]

Comment on attachment 184522 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:69
> +    RefPtr<IDBAny> oldVersionAny = oldVersion ? IDBAny::create(oldVersion) : IDBAny::createNull();

Isn't there an Invalid Version constant somewhere, so this can be (oldVersion != InvalidVersion) ? IDBAny::create(oldVersion) : IDBAny::createNull() ?

> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:70
> +    RefPtr<IDBAny> newVersionAny = m_version ? IDBAny::create(m_version) : IDBAny::createNull();

ditto

> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:109
> +    enqueueEvent(IDBVersionChangeEvent::create(IDBAny::create(oldVersion), IDBAny::create(m_version), eventNames().upgradeneededEvent));

this doesn't need the same oldVersion ? ... : null; checks from above?

[Joshua Bell]

Comment on attachment 184522 [details]
Patch

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

>> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:69
>> +    RefPtr<IDBAny> oldVersionAny = oldVersion ? IDBAny::create(oldVersion) : IDBAny::createNull();
> 
> Isn't there an Invalid Version constant somewhere, so this can be (oldVersion != InvalidVersion) ? IDBAny::create(oldVersion) : IDBAny::createNull() ?

Yeah, I'll switch these to checking against IDBDatabaseMetadata::DefaultIntVersion (which is 0).

>> Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.cpp:109
>> +    enqueueEvent(IDBVersionChangeEvent::create(IDBAny::create(oldVersion), IDBAny::create(m_version), eventNames().upgradeneededEvent));
> 
> this doesn't need the same oldVersion ? ... : null; checks from above?

I didn't change the behavior of upgradeneeded at all, just the interface. So whatever integers were used before will be passed straight through. 

http://crbug.com/153121 captures the issue that when we fire "upgradeneeded" (regardless of the event interface type), the oldVersion is a number. Fixing that requires back-end to front-end plumbing, but is independent of this patch.

[Joshua Bell]

Created attachment 184552 [details]
Patch

[Joshua Bell]

Created attachment 184553 [details]
Patch

[Joshua Bell]

tony@ - can you review?

[WebKit Review Bot]

Comment on attachment 184553 [details]
Patch

Clearing flags on attachment: 184553

Committed r140741: <http://trac.webkit.org/changeset/140741>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 108044

[Joshua Bell]

Relanded as http://trac.webkit.org/changeset/141013