Bug 76723

Summary: Switch indexeddb to use supplemental IDL for DOMWindow
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: New BugsAssignee: 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 Flags
Patch
none
Patch none

Mark Pilgrim (Google)
Reported 2012-01-20 11:59:11 PST
Switch indexeddb to use supplemental IDL for DOMWindow
Attachments
Patch (13.33 KB, patch)
2012-01-20 17:27 PST, Mark Pilgrim (Google) 		no flags
DetailsFormatted DiffDiff
Patch (13.25 KB, patch)
2012-01-20 18:54 PST, Mark Pilgrim (Google) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Pilgrim (Google)
Comment 1 2012-01-20 17:27:51 PST
Created attachment 123418 [details] Patch
Adam Barth
Comment 2 2012-01-20 17:32:03 PST
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 3 2012-01-20 17:59:08 PST
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)
Comment 4 2012-01-20 18:04:17 PST
(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
Comment 5 2012-01-20 18:06:57 PST
> 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
Comment 6 2012-01-20 18:10:16 PST
http://www.webkit.org/coding/RefPtr.html might be worth a read if you haven't seen it before.
Mark Pilgrim (Google)
Comment 7 2012-01-20 18:54:36 PST
Created attachment 123426 [details] Patch
Adam Barth
Comment 8 2012-01-20 18:57:40 PST
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 9 2012-01-20 22:07:04 PST
Comment on attachment 123426 [details] Patch Clearing flags on attachment: 123426 Committed r105569: <http://trac.webkit.org/changeset/105569>
WebKit Review Bot
Comment 10 2012-01-20 22:07:09 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.