RESOLVED FIXED 79633
ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE)
https://bugs.webkit.org/show_bug.cgi?id=79633
Summary ScriptExecutionContext has too many ifdef ENABLE(SQL_DATABASE)
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
work in progress (27.05 KB, patch)
2012-02-27 00:05 PST, Adam Barth
no flags
Patch (35.23 KB, patch)
2012-02-27 00:15 PST, Adam Barth
no flags
Patch (36.06 KB, patch)
2012-02-27 14:58 PST, Adam Barth
no flags
Patch for landing (32.97 KB, patch)
2012-02-29 17:22 PST, Adam Barth
no flags
Patch for landing (39.56 KB, patch)
2012-02-29 17:25 PST, Adam Barth
no flags
Patch for landing (39.56 KB, patch)
2012-02-29 17:45 PST, Adam Barth
no flags
Patch for landing (40.35 KB, patch)
2012-02-29 17:51 PST, Adam Barth
no flags
Patch for landing (41.21 KB, patch)
2012-02-29 18:44 PST, Adam Barth
abarth: commit-queue+
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
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
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
Note You need to log in before you can comment on or make changes to this bug.