Bug 76723 – Switch indexeddb to use supplemental IDL for DOMWindow

• Component: New Bugs
• Status: RESOLVED FIXED

[Mark Pilgrim (Google)]

Switch indexeddb to use supplemental IDL for DOMWindow

[Mark Pilgrim (Google)]

Created attachment 123418 [details]
Patch

[Adam Barth]

Comment on attachment 123418 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123418&action=review

> Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.cpp:62
> +       window->setIDBFactory(IDBFactory::create(page->group().idbFactory()));

I think this indent is off by one.

> Source/WebCore/page/DOMWindow.cpp:753
> +PassRefPtr<IDBFactory> DOMWindow::getIDBFactory()

Rather than returning a PassRefPtr<IDBFactory> here, we should just return a raw pointer.  We use PassRefPtr when we're passing ownership of an object.

[Adam Barth]

Comment on attachment 123418 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123418&action=review

> Source/WebCore/page/DOMWindow.h:416
> +        PassRefPtr<IDBFactory> getIDBFactory();

Another nit: this function should be called idbFactory().  WebKit doesn't use the "get" prefix for getters.

[Mark Pilgrim (Google)]

(In reply to comment #2)
> > Source/WebCore/page/DOMWindow.cpp:753
> > +PassRefPtr<IDBFactory> DOMWindow::getIDBFactory()
> 
> Rather than returning a PassRefPtr<IDBFactory> here, we should just return a raw pointer.  We use PassRefPtr when we're passing ownership of an object.

But IDBFactory::create(page->group().idbFactory()) is a PassRefPtr.

[Adam Barth]

> But IDBFactory::create(page->group().idbFactory()) is a PassRefPtr.

Sure, but that gets given to the setter, which should take a PassRefPtr (since it grabs a reference).

[Adam Barth]

http://www.webkit.org/coding/RefPtr.html might be worth a read if you haven't seen it before.

[Mark Pilgrim (Google)]

Created attachment 123426 [details]
Patch

[Adam Barth]

Comment on attachment 123426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123426&action=review

> Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.cpp:52
> +    Document* document = window->document();
> +    if (!document)
> +        return 0;

We can probably remove this check, but that's something for a later patch.

[WebKit Review Bot]

Comment on attachment 123426 [details]
Patch

Clearing flags on attachment: 123426

Committed r105569: <http://trac.webkit.org/changeset/105569>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.