Bug 79633

Summary: ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE)
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, haraken, japhet, morrita, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79327, 79635, 79636    
Attachments:
Description Flags
work in progress
none
work in progress
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing abarth: commit-queue+

Description Adam Barth 2012-02-26 23:39:59 PST 
ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE)
Comment 1 Adam Barth 2012-02-26 23:40:33 PST 
Created attachment 128966 [details]
work in progress
Comment 2 Adam Barth 2012-02-26 23:40:57 PST 
Currently just a work-in-progress.
Comment 3 Adam Barth 2012-02-27 00:05:56 PST 
Created attachment 128973 [details]
work in progress
Comment 4 Adam Barth 2012-02-27 00:15:12 PST 
Created attachment 128975 [details]
Patch
Comment 5 Hajime Morrita 2012-02-27 01:28:49 PST 
Comment on attachment 128975 [details]
Patch

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

> Source/WebCore/history/PageCache.cpp:124
> +        if (DatabaseContext::from(frame->document())->hasOpenDatabases()) {

It looks this from() call makes the lazy instantiation making less sense.
How about to hide hide this behind a static method?

> Source/WebCore/history/PageCache.cpp:267
> +        && !DatabaseContext::from(document)->hasOpenDatabases()

Ditto.

> Source/WebCore/loader/FrameLoader.cpp:423
> +        DatabaseContext::from(doc)->stopDatabases(0);

Ditto on from().

> Source/WebCore/storage/DatabaseContext.cpp:78
> +    ASSERT(isContextThread());

Windows bot is right. We don't have isContextThread() on DatabaseContext.
Comment 6 Hajime Morrita 2012-02-27 01:30:48 PST 
BTW I'm totally unware that ScriptExceptionContext has such a ifdef-ed crap.
Supplementable effectively helps ;-)
Comment 7 Adam Barth 2012-02-27 11:00:53 PST 
Great points.  I'll update the patch.  Thanks!
Comment 8 Adam Barth 2012-02-27 14:58:33 PST 
Created attachment 129103 [details]
Patch
Comment 9 Eric Seidel (no email) 2012-02-27 15:09:53 PST 
Comment on attachment 129103 [details]
Patch

LGTM.
Comment 10 Adam Barth 2012-02-29 17:22:34 PST 
Created attachment 129558 [details]
Patch for landing
Comment 11 Adam Barth 2012-02-29 17:25:05 PST 
Created attachment 129560 [details]
Patch for landing
Comment 12 Adam Barth 2012-02-29 17:45:45 PST 
Created attachment 129565 [details]
Patch for landing
Comment 13 Adam Barth 2012-02-29 17:51:48 PST 
Created attachment 129566 [details]
Patch for landing
Comment 14 Adam Barth 2012-02-29 18:44:10 PST 
Created attachment 129610 [details]
Patch for landing
Comment 15 Adam Barth 2012-02-29 23:46:36 PST 
Committed r109319: <http://trac.webkit.org/changeset/109319>