[webkit-reviews] review denied: [Bug 53728] indexeddb: make setVersion fire blocked event if other connections are open : [Attachment 81565] Patch

Mon Feb 7 19:32:38 PST 2011

Jeremy Orlow <jorlow at chromium.org> has denied David Grogan
<dgrogan at chromium.org>'s request for review:
Bug 53728: indexeddb: make setVersion fire blocked event if other connections
are open
https://bugs.webkit.org/show_bug.cgi?id=53728

Attachment 81565: Patch
https://bugs.webkit.org/attachment.cgi?id=81565&action=review

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=81565&action=review

closer

> LayoutTests/storage/indexeddb/set_version_blocked.html:1
> +<html>

You need to create expected results.

> LayoutTests/storage/indexeddb/set_version_blocked.html:20
> +    if ('webkitIndexedDB' in window) {

no {} for one liner

> LayoutTests/storage/indexeddb/set_version_blocked.html:58
> +function setVersion(tries) {

newline

> LayoutTests/storage/indexeddb/set_version_blocked.html:65
> +function unexpectedBlockCallback() {

put in the shared.js file...and call it "Blocked" not "Block"

> Source/WebCore/dom/EventTarget.h:143
> +	   virtual IDBVersionChangeRequest* toIDBVersionChangeRequest();

alpha order

> Source/WebCore/storage/IDBDatabase.cpp:135
> +    if (m_noNewTransactions)

Stuff like this should be tested in the layout test.

> Source/WebCore/storage/IDBDatabaseBackendImpl.h:65
> +    virtual void open();

You'll need to add this to the WebKit:: stuff.

> Source/WebCore/storage/IDBRequest.cpp:190
> +void IDBRequest::scheduleBlockedEvent()

You need to sync your repo.  This has all changed a bunch.

> Source/WebCore/storage/IDBVersionChangeRequest.h:32
> +#include "Timer.h"

Why is this needed?

> Source/WebCore/storage/IDBVersionChangeRequest.h:33
> +#include <wtf/Vector.h>

why is this needed?

> Source/WebCore/storage/IDBVersionChangeRequest.h:37
> +class IDBTransactionBackendInterface;

why is this needed?

> Source/WebCore/storage/IDBVersionChangeRequest.h:41
> +    static PassRefPtr<IDBVersionChangeRequest>
create(ScriptExecutionContext* context, PassRefPtr<IDBAny> source) { return
adoptRef(new IDBVersionChangeRequest(context, source)); }

lately i've been shying away from inlining these.  Maybe move it?

> Source/WebCore/storage/IDBVersionChangeRequest.h:43
> +    virtual void onBlocked();

usually we do a newline between the constructor/destructor stuff and other
stuff

> Source/WebCore/storage/IDBVersionChangeRequest.h:49
> +

no blank line