Bug 75751 – IndexedDB: Make WebIDBDatabase::close() idempotent

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

[Joshua Bell]

IndexedDB: Make WebIDBDatabase::close() idempotent

[Joshua Bell]

Created attachment 121508 [details]
Patch

[Joshua Bell]

The simplest/cleanest fix for http://code.google.com/p/chromium/issues/detail?id=80111 requires that chromium's implementation of WebIDBDatabase::close() be idempotent. This is one way to achieve that, which keeps the assertion behavior when called directly (i.e. non-Chromium builds)

That said, I'm not positive that this is the best approach.

[David Grogan]

Comment on attachment 121508 [details]
Patch

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

LGTM

> Source/WebKit/chromium/src/WebIDBDatabaseImpl.cpp:106
>      ASSERT(m_databaseCallbacks);

You can delete this line now.

[Joshua Bell]

Created attachment 121527 [details]
Patch

[Joshua Bell]

tony@ - r? cq?

[Tony Chang]

Comment on attachment 121527 [details]
Patch

Nit: Should we have a layout test that calls close() twice?  The assert should be hit in a debug build without this change.

[Joshua Bell]

(In reply to comment #6)
> (From update of attachment 121527 [details])
> Nit: Should we have a layout test that calls close() twice?  The assert should be hit in a debug build without this change.

The IDBDatabase object has an internal m_closePending flag which makes a script's call to db.close() exit early if called twice. If that wasn't present, then in the Chromium port the call would eventually make it into WebIDBDatabase::close() and we'd hit this assert. 

All that said, we didn't appear to have a layout test that called close() twice, so I've added that to database-basics, but it passes both with and without this patch.

[Joshua Bell]

Created attachment 121700 [details]
Patch

[WebKit Review Bot]

Comment on attachment 121700 [details]
Patch

Clearing flags on attachment: 121700

Committed r104481: <http://trac.webkit.org/changeset/104481>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.