[Webkit-unassigned] [Bug 55095] IndexedDB: fire versionchange events when calling setVersion

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=55095


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #84351|review?                     |review-
               Flag|                            |




--- Comment #15 from Jeremy Orlow <jorlow at chromium.org>  2011-03-01 19:25:53 PST ---
(From update of attachment 84351)
View in context: https://bugs.webkit.org/attachment.cgi?id=84351&action=review

> Source/WebCore/storage/IDBDatabase.cpp:145
> +    EventQueue* eventQueue = static_cast<Document*>(scriptExecutionContext())->eventQueue();

assert it's a document before static casting (IsDocument())

> Source/WebCore/storage/IDBDatabase.cpp:146
> +    for (size_t i = 0; i < m_enqueuedEvents.size(); ++i) {

Is this really necessary?  What's the worst case without it?

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:170
> +        callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::NOT_ALLOWED_ERR, "Connection was closed before set version transaction was created"));

Did we agree on NOT_ALLOWED_ERR?  ABORT_ERR might make more sense?

Is this possible?  Seems like it wouldn't be and that we should be blocking it in the frontend.  If so, just do an assert.  If not, why isn't it?

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:173
> +    for (DatabaseCallbacksSet::const_iterator it = m_databaseCallbacksSet.begin(); it != m_databaseCallbacksSet.end(); ++it) {

Are we supposed to send this even when nothing's blocked?

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:234
>          RefPtr<PendingSetVersionCall> pendingSetVersionCall = m_pendingSetVersionCalls.takeFirst();

Can you update the code I just landed to use takeFirst (in EventQueue).  That seems better.

> Source/WebCore/storage/IDBRequest.cpp:193
> +

newline seems kinda random...but no big deal

> Source/WebKit/chromium/src/IDBDatabaseProxy.cpp:95
> +void IDBDatabaseProxy::setVersion(const String& version, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBDatabaseCallbacks> dbcallbacks, ExceptionCode& ec)

databaseCallbacks  (don't abbreviate and camelCase)

here and elsewhere

> Source/WebKit/chromium/src/WebIDBDatabaseImpl.cpp:102
> +    // Use the callbacks that ::open gave us to ensure that the backend in

ASSERT they're not null before passing in.  (The only time we wouldn't assert is if we were about to do something that'd _crash_ if it was null.)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.