[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 #83570|review?                     |review-
               Flag|                            |




--- Comment #3 from Jeremy Orlow <jorlow at chromium.org>  2011-02-23 18:22:31 PST ---
(From update of attachment 83570)
View in context: https://bugs.webkit.org/attachment.cgi?id=83570&action=review

overall looks pretty good...minus the connection id thing

> LayoutTests/storage/indexeddb/set_version_queue.html:71
> +    versionChangeNotifications.push("connection " + connectionNum +

no need to wrap..and you could maybe just put it in the closure above?

> Source/WebCore/ChangeLog:12
> +        4) When a connections call setVersion, backend calls

When a connection calls setVersion, the backend calls

> Source/WebCore/storage/IDBDatabase.cpp:177
> +bool IDBDatabase::dispatchEvent(PassRefPtr<Event> event)

Is there any reason the default dispatchEvent won't do?

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:50
> +    int connectionId() { return m_connectionId; }

newline before private..

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:218
> +void IDBDatabaseBackendImpl::open(int connectionId, PassRefPtr<IDBDatabaseCallbacks> callbacks)

You should just have one version of this function.  And it should simply take in the PassRefPtr

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:229
> +void IDBDatabaseBackendImpl::close(int connectionId)

This should take in an IDBDatabaseCallbacks ptr...and assert it's there before you remove it.

> Source/WebCore/storage/IDBDatabaseBackendImpl.h:101
> +    int m_nextId;

no longer necessary

> Source/WebCore/storage/IDBDatabaseBackendImpl.h:106
> +    typedef HashMap<int, RefPtr<IDBDatabaseCallbacks> > DatabaseCallbacksMap;

This can probably just be a HashSet.  Actually use a LinkHashSet so that you can iterate over them deterministically.  Make a comment that this is important.

> Source/WebCore/storage/IDBDatabaseCallbacks.h:34
> +#include "SerializedScriptValue.h"

not needed...you likely will need PlatformString. if you delete tho.

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

extra newline

> Source/WebKit/chromium/public/WebIDBDatabaseCallbacks.h:38
> +

no newlines

> Source/WebKit/chromium/src/IDBDatabaseCallbacksProxy.h:55
> +

no double newline

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