[webkit-reviews] review denied: [Bug 40054] [indexedDB] It is impossible to create object stores : [Attachment 57655] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has denied Andrei Popescu
<andreip at google.com>'s request for review:
Bug 40054: [indexedDB] It is impossible to create object stores
https://bugs.webkit.org/show_bug.cgi?id=40054

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

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
WebKit/chromium/src/WebIDBDatabaseImpl.h: 
 +  
this newline is probably OK...if you want.

WebKit/chromium/src/WebIDBDatabaseImpl.h:48
 +	virtual void createObjectStore(const WebString& name, const WebString&
keyPath, bool autoIncrement, WebIDBCallbacks* callbacks, WebFrame*);
Newline before private:

WebKit/chromium/src/WebIDBCallbacksImpl.cpp:69
 +	m_callbacks->onSuccess(IDBDatabaseProxy::create(webKitInstance,
weakFramePtr.release()));
I don't really see a reason not to combine these lines.

WebKit/chromium/src/IndexedDatabaseProxy.h:45
 +	virtual void open(const String& name, const String& description,
PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>,
PassRefPtr<IDBWeakFramePtr>);
This could be a *IDBWeakFramePtr I believe.

WebKit/chromium/src/IndexedDatabaseProxy.cpp:62
 +	Frame* frame = (*framePtr).get();
framePtr->

WebKit/chromium/src/IDBObjectStoreProxy.h:13
 +   * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
Remove

WebKit/chromium/src/IDBObjectStoreProxy.h:57
 +  private:
newline before?

WebKit/chromium/src/IDBObjectStoreProxy.cpp:13
 +   * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
remove

WebKit/chromium/src/IDBDatabaseProxy.h:44
 +	static PassRefPtr<IDBDatabase>
create(PassOwnPtr<WebKit::WebIDBDatabase>, PassRefPtr<IDBWeakFramePtr>);
Just pass pointers.

WebCore/storage/IDBDatabaseImpl.h:60
 +	RefPtr<IDBWeakFramePtr> m_weakFramePtr;
I don't think it makes sense for this to be here.  The frame pointer is really
an issue that should be entirely constrained to the request objects.

WebCore/storage/IDBDatabaseRequest.h:56
 +	PassRefPtr<IDBRequest> createObjectStore(const String& name, const
String& keyPath = "", bool autoIncrement = false);
newline after

WebCore/storage/IDBWeakFramePtr.h:49
 +	bool operator!() const {return !m_frame; }
Wait...doesn't this mean you can just do |if (!weakFramePtr)| since the ref ptr
overloads that operator and this overloads it as well?

WebCore/storage/IndexedDatabaseRequest.h:57
 +	void disconnectFrame() { m_framePtr->disconnectFrame(); }
I think you should get rid of disconnectFrame here, have DOMWindow create the
IDBWeakFramePtr on demand, and call disconnectFrame direclty on it when
necessary.