Bug 94972 – IndexedDB: Remove IDBDatabase.setVersion API

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

[Joshua Bell]

The IDBDatabase.setVersion() API is from an older version of the spec, and maintained for compatibility until sites using the Chromium implementation of IndexedDB have had a chance to update. The new IDBFactory.open(name, version) / "upgradeneeded" event model is the new way to version databases.

Once a grace period has passed, we should remove it.

[David Grogan]

Created attachment 175078 [details]
Patch

[David Grogan]

Created attachment 175080 [details]
Patch

[David Grogan]

This patch leaves some setVersion artifacts in IDBDatabaseBackendImpl, that class is pretty fragile. There are also some remnants in the webkit api that can't be taken out until chromium code is updated.

[WebKit Review Bot]

Comment on attachment 175080 [details]
Patch

Attachment 175080 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14906449

New failing tests:
storage/indexeddb/pending-version-change-on-exit.html
http/tests/inspector/indexeddb/database-data.html
http/tests/inspector/indexeddb/database-structure.html
http/tests/inspector/indexeddb/resources-panel.html

[Joshua Bell]

Comment on attachment 175080 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:314
> +    ASSERT(m_pendingSetVersionCalls.isEmpty());

Why keep this?

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h:110
> +    // FIXME: Remove these.

And these? I don't see anything remaining that creates them.

[David Grogan]

Created attachment 175312 [details]
Patch

[David Grogan]

(In reply to comment #5)
> (From update of attachment 175080 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=175080&action=review
> 
> > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:314
> > +    ASSERT(m_pendingSetVersionCalls.isEmpty());
> 
> Why keep this?
> 
> > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h:110
> > +    // FIXME: Remove these.
> 
> And these? I don't see anything remaining that creates them.

Removed both.

[Joshua Bell]

Comment on attachment 175312 [details]
Patch

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

> LayoutTests/ChangeLog:14
> +        IndexedDB: remove setVersion from inspector tests

Looks like this patch was based on another (leading to two changelog entries and the extra diff below). Rebase please?

> LayoutTests/storage/indexeddb/removed-expected.txt:9
> +PASS 'setVersion' in IDBDatabase.prototype is false

FYI this will need rebasing as nearby lines have changed.

[David Grogan]

Created attachment 176296 [details]
Patch

[Joshua Bell]

Comment on attachment 176296 [details]
Patch

lgtm, w00t! (looks like it will need another rebase on top of r135856 though)

[David Grogan]

Created attachment 176306 [details]
Patch

[David Grogan]

Tony, could you review this?

[WebKit Review Bot]

Comment on attachment 176306 [details]
Patch

Clearing flags on attachment: 176306

Committed r135904: <http://trac.webkit.org/changeset/135904>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.