[Webkit-unassigned] [Bug 53728] indexeddb: make setVersion fire blocked event if other connections are open

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from David Grogan <dgrogan at chromium.org>  2011-02-11 15:13:49 PST ---
(From update of attachment 81862)
View in context: https://bugs.webkit.org/attachment.cgi?id=81862&action=review

>> LayoutTests/storage/indexeddb/set_version_blocked.html:31
>> +    verifyResult(result);
> 
> In new tests, I'm voiding these because at this point they pretty much just add noise

done

>> LayoutTests/storage/indexeddb/set_version_blocked.html:39
>> +    verifySuccessEvent(event);
> 
> Ditto....fix throughout

done

>> LayoutTests/storage/indexeddb/set_version_blocked.html:42
>> +    if (connections.length < 3) {
> 
> no {}

done

>> LayoutTests/storage/indexeddb/set_version_blocked.html:44

> 
> no {}

done

>> LayoutTests/storage/indexeddb/set_version_blocked.html:63

> 
> Stuff like this should be in shouldBe/testPassed/debug()'s.  The idea is that you should be able to follow along via the on-screen output.

gotcha

>> Source/WebCore/storage/IDBBlockedEvent.cpp:42
>> +    : IDBEvent(eventNames().blockedEvent, source, false/*canBubble*/)
> 
> space before /*?

done

>> Source/WebCore/storage/IDBBlockedEvent.h:45
>> +    virtual String version();
> 
> newline

done

>> Source/WebCore/storage/IDBBlockedEvent.h:48
>> +    IDBBlockedEvent(PassRefPtr<IDBAny> source, const String& version);
> 
> newline

done

>> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:217
>> +        return;
> 
> You need to add this to a queue and start the queue once there's only one connection left.  And put a FIXME to only fire the event if the DB is still open.

done

>> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:260
>>  void IDBDatabaseBackendImpl::close()
> 
> On close, you need to re-try the set-version transaction.

done

>> Source/WebCore/storage/IDBDatabaseBackendImpl.h:65
>> +    virtual void open();
> 
> shouldn't be virtual.  Group up above with the other non-virtual/not-in-idl stuff (like id)

done

>> Source/WebCore/storage/IDBRequest.h:85
>> +    virtual void enqueueEvent(PassRefPtr<Event>);
> 
> don't make stuff virtual unless necessary

done

>> Source/WebCore/storage/IDBRequest.h:86
>> +    virtual IDBAny* source();
> 
> I don't see any reason not to just make this public.  But don't make virtual.

done

>> Source/WebCore/storage/IDBVersionChangeRequest.cpp:40
>> +IDBVersionChangeRequest::IDBVersionChangeRequest(ScriptExecutionContext* context, PassRefPtr<IDBAny> source, const String& version)
> 
> newline

done

>> Source/WebCore/storage/IDBVersionChangeRequest.h:46
>> +    String m_version;
> 
> maybe a newline between these?

done

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