[Webkit-unassigned] [Bug 40054] [indexedDB] It is impossible to create object stores

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from Andrei Popescu <andreip at google.com>  2010-06-02 09:56:17 PST ---
(In reply to comment #7)
> (From update of attachment 57655 [details])
> WebKit/chromium/src/WebIDBDatabaseImpl.h: 
>  +  
> this newline is probably OK...if you want.
> 

I want.

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

Added.

> 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.
>

Indeed, especially now that we pass a raw pointer.

> 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.
>

Done.

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

Done.

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

Removed.

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

Added.

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

Removed.

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

Done.

> 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.
>

Yes, but the IDBDatabase needs to be able to create Request objects of its own, so it need to pass the weak reference to them.


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

Added.

> 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?
>

Unfortunately not. The ! operator applies to the object, not to a pointer to an object. So when you apply it to the RefPtr, it will test its m_ptr pointer against NULL. It will not apply the ! operator to the object pointed by m_ptr.

> 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.

I am not sure I agree. Right now, the weak pointer is a private implementation detail of the IDB module. The entry point to that is the IndexedDatabaseRequest class so I think it is neater to let the DOMWindow only know about the IndexedDatabaseRequest instance. Perhaps we should reconsider this if we are to use the weak pointer idea in other places?

Thanks,
Andrei

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