Bug 135440 – IDB versionchange event not constructed with proper eventType

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebCore Misc.

[Vicki Pfau]

When constructing a versionchange event in IDBDatabase::onVersionChange, the event is not constructed with a proper eventType, giving it a null event type.

<rdar://problem/17394855>

[Vicki Pfau]

Created attachment 235785 [details]
Patch

[Pratik Solanki]

Comment on attachment 235785 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBDatabase.cpp:311
> +    enqueueEvent(IDBVersionChangeEvent::create(oldVersion, newVersion, newVersionNullness, eventNames().versionchangeEvent));

We should just make IDBVersionChangeEvent::create() pass the correct event name instead of having callers pass it. See for example EventFactory::create() which has

#if ENABLE(INDEXED_DATABASE)
    if (type == "IDBVersionChangeEvent")
        return IDBVersionChangeEvent::create();
#endif

That would also create an event with no name.

[Build Bot]

Comment on attachment 235785 [details]
Patch

Attachment 235785 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5219620243898368

New failing tests:
media/track/add-and-remove-track.html

[Build Bot]

Created attachment 235802 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Darin Adler]

Comment on attachment 235785 [details]
Patch

Setting review- so we can do what Pratik suggested.

[Vicki Pfau]

The reason I hadn't done that yet it because some of the invocations of the constructor pass different event names. Which itself is weird and probably bad. I can have it have a sane default argument though, instead of just being null.

[Pratik Solanki]

I think we should just remove the default arg and have it be one that callers are forced to pass. Make it the first argument of the create method and the constructor. Something like the following.

-    static PassRefPtr<IDBVersionChangeEvent> create(unsigned long long oldVersion = 0, unsigned long long newVersion = 0, IndexedDB::VersionNullness newVersionNullness = IndexedDB::VersionNullness::Null, const AtomicString& eventType = AtomicString())
+    static PassRefPtr<IDBVersionChangeEvent> create(const AtomicString& eventType, unsigned long long oldVersion = 0, unsigned long long newVersion = 0, IndexedDB::VersionNullness newVersionNullness = IndexedDB::VersionNullness::Null)

[Vicki Pfau]

A patch identical to the r-'d patch here (minus tests) was landed with bug #136583.

*** This bug has been marked as a duplicate of bug 136583 ***