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+

Adam Barth
Reported 2012-02-26 23:39:59 PST
ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE)
Attachments
work in progress (23.51 KB, patch)
2012-02-26 23:40 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
work in progress (27.05 KB, patch)
2012-02-27 00:05 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (35.23 KB, patch)
2012-02-27 00:15 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (36.06 KB, patch)
2012-02-27 14:58 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (32.97 KB, patch)
2012-02-29 17:22 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (39.56 KB, patch)
2012-02-29 17:25 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (39.56 KB, patch)
2012-02-29 17:45 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (40.35 KB, patch)
2012-02-29 17:51 PST, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (41.21 KB, patch)
2012-02-29 18:44 PST, Adam Barth 		abarth: commit-queue+
DetailsFormatted DiffDiff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-02-26 23:40:33 PST
Created attachment 128966 [details] work in progress
Adam Barth
Comment 2 2012-02-26 23:40:57 PST
Currently just a work-in-progress.
Adam Barth
Comment 3 2012-02-27 00:05:56 PST
Created attachment 128973 [details] work in progress
Adam Barth
Comment 4 2012-02-27 00:15:12 PST
Created attachment 128975 [details] Patch
Hajime Morrita
Comment 5 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.
Hajime Morrita
Comment 6 2012-02-27 01:30:48 PST
BTW I'm totally unware that ScriptExceptionContext has such a ifdef-ed crap. Supplementable effectively helps ;-)
Adam Barth
Comment 7 2012-02-27 11:00:53 PST
Great points. I'll update the patch. Thanks!
Adam Barth
Comment 8 2012-02-27 14:58:33 PST
Created attachment 129103 [details] Patch
Eric Seidel (no email)
Comment 9 2012-02-27 15:09:53 PST
Comment on attachment 129103 [details] Patch LGTM.
Adam Barth
Comment 10 2012-02-29 17:22:34 PST
Created attachment 129558 [details] Patch for landing
Adam Barth
Comment 11 2012-02-29 17:25:05 PST
Created attachment 129560 [details] Patch for landing
Adam Barth
Comment 12 2012-02-29 17:45:45 PST
Created attachment 129565 [details] Patch for landing
Adam Barth
Comment 13 2012-02-29 17:51:48 PST
Created attachment 129566 [details] Patch for landing
Adam Barth
Comment 14 2012-02-29 18:44:10 PST
Created attachment 129610 [details] Patch for landing
Adam Barth
Comment 15 2012-02-29 23:46:36 PST
Committed r109319: <http://trac.webkit.org/changeset/109319>
Note You need to log in before you can comment on or make changes to this bug.