Summary: | Switch indexeddb to use supplemental IDL for DOMWindow | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Pilgrim (Google) <pilgrim> | ||||||
Component: | New Bugs | Assignee: | Mark Pilgrim (Google) <pilgrim> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, haraken, ojan, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 79327 | ||||||||
Attachments: |
|
Description
Mark Pilgrim (Google)
2012-01-20 11:59:11 PST
Created attachment 123418 [details]
Patch
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. 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. (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. > 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).
http://www.webkit.org/coding/RefPtr.html might be worth a read if you haven't seen it before. Created attachment 123426 [details]
Patch
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. Comment on attachment 123426 [details] Patch Clearing flags on attachment: 123426 Committed r105569: <http://trac.webkit.org/changeset/105569> All reviewed patches have been landed. Closing bug. |