Summary: | Use the [Supplemental] IDL in SQLDatabase | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> | ||||||
Component: | WebCore JavaScript | Assignee: | Kentaro Hara <haraken> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, ojan, rakuco, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 72138 | ||||||||
Attachments: |
|
Description
Kentaro Hara
2012-01-10 16:06:02 PST
Created attachment 121932 [details]
Patch
Comment on attachment 121932 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121932&action=review > Source/WebCore/storage/DOMWindowSQLDatabase.cpp:46 > +PassRefPtr<Database> DOMWindowSQLDatabase::openDatabase(DOMWindow* imp, const String& name, const String& version, const String& displayName, unsigned long estimatedSize, PassRefPtr<DatabaseCallback> creationCallback, ExceptionCode& ec) Maybe |imp| should be |window| ? We like to use more specific names outside of the bindings. > Source/WebCore/storage/DOMWindowSQLDatabase.cpp:52 > + if (imp->frame() && AbstractDatabase::isAvailable() && imp->frame()->document()->securityOrigin()->canAccessDatabase()) imp->frame() will always be true when imp->isCurrentlyDisplayedInFrame() is true. imp->frame()->document() can just be imp->document() (there's no reason to indirect through the frame) Feel free to leave the code as-is since you're just moving it. Maybe we should just add FIXME comments about these nits? > Source/WebCore/storage/DOMWindowSQLDatabase.h:51 > +}; Can you make the constructor and destructor private? Created attachment 121938 [details]
patch for commit
(In reply to comment #2) > (From update of attachment 121932 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=121932&action=review > > > Source/WebCore/storage/DOMWindowSQLDatabase.cpp:46 > > +PassRefPtr<Database> DOMWindowSQLDatabase::openDatabase(DOMWindow* imp, const String& name, const String& version, const String& displayName, unsigned long estimatedSize, PassRefPtr<DatabaseCallback> creationCallback, ExceptionCode& ec) > > Maybe |imp| should be |window| ? We like to use more specific names outside of the bindings. Done. > > Source/WebCore/storage/DOMWindowSQLDatabase.cpp:52 > > + if (imp->frame() && AbstractDatabase::isAvailable() && imp->frame()->document()->securityOrigin()->canAccessDatabase()) > > imp->frame() will always be true when imp->isCurrentlyDisplayedInFrame() is true. > > imp->frame()->document() can just be imp->document() (there's no reason to indirect through the frame) > > Feel free to leave the code as-is since you're just moving it. Maybe we should just add FIXME comments about these nits? OK, then I'll leave it as-is in this patch and write a follow-up patch. > > Source/WebCore/storage/DOMWindowSQLDatabase.h:51 > > +}; > > Can you make the constructor and destructor private? Done. Committed. Thanks! Comment on attachment 121938 [details] patch for commit Clearing flags on attachment: 121938 Committed r104654: <http://trac.webkit.org/changeset/104654> I think this has been fixed. |