Bug 76004 – Use the [Supplemental] IDL in SQLDatabase

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

We've been working on WebKit modularization.

By using the [Supplemental] IDL, we can move SQLDatabase related code from WebCore/page/DOMWindow.{h,cpp,idl} to WebCore/storage/DOMWindowSQLDatabase.{h,cpp,idl}.

[Kentaro Hara]

Created attachment 121932 [details]
Patch

[Adam Barth]

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?

[Kentaro Hara]

Created attachment 121938 [details]
patch for commit

[Kentaro Hara]

(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!

[WebKit Review Bot]

Comment on attachment 121938 [details]
patch for commit

Clearing flags on attachment: 121938

Committed r104654: <http://trac.webkit.org/changeset/104654>

[Adam Barth]

I think this has been fixed.