Bug 104748

Summary: webdatabase: Split code between the DatabaseManager and DatabaseServer
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: WebCore Misc.Assignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: adamk, ap, beidson, dglazkov, gyuyoung.kim, peter+ews, philn, rakuco, sam, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 104741, 104934    
Bug Blocks: 103668    
Attachments:
Description Flags
work in progress 1.
webkit-ews: commit-queue-
work in progress again: fixed Qt issue.
eflews.bot: commit-queue-
wip1: attempt at fixing the elf build failure.
none
work in progress 2: DatabaseTracker is now only accessed from DBBackend::Server + moving more stuff to DatabaseManager and the server.
peter+ews: commit-queue-
wip2: + fixed chromium breakage. test ews again.
webkit.review.bot: commit-queue-
wip2: + fixed 2nd chromium breakage. test ews again.
none
wp2: svn up'ed.
none
The final (hopefully) patch.
webkit-ews: commit-queue-
The fixed patch: added needed #include's.
none
The fixed patch minus some development changes in the Xcode project not ready for a commit yet. beidson: review+

Description Mark Lam 2012-12-11 17:34:02 PST
Split the DatabaseManager into a front-end and a back-end DatabaseServer.
Comment 1 Mark Lam 2012-12-12 20:39:58 PST
Created attachment 179186 [details]
work in progress 1.
Comment 2 Early Warning System Bot 2012-12-12 20:45:11 PST
Comment on attachment 179186 [details]
work in progress 1.

Attachment 179186 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15310107
Comment 3 Mark Lam 2012-12-12 20:49:24 PST
Created attachment 179188 [details]
work in progress again: fixed Qt issue.
Comment 4 EFL EWS Bot 2012-12-12 22:12:58 PST
Comment on attachment 179188 [details]
work in progress again: fixed Qt issue.

Attachment 179188 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15322103
Comment 5 Mark Lam 2012-12-12 23:12:40 PST
Created attachment 179207 [details]
wip1: attempt at fixing the elf build failure.
Comment 6 Mark Lam 2012-12-13 23:48:18 PST
Created attachment 179429 [details]
work in progress 2: DatabaseTracker is now only accessed from DBBackend::Server + moving more stuff to DatabaseManager and the server.
Comment 7 Peter Beverloo (cr-android ews) 2012-12-14 00:18:28 PST
Comment on attachment 179429 [details]
work in progress 2: DatabaseTracker is now only accessed from DBBackend::Server + moving more stuff to DatabaseManager and the server.

Attachment 179429 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15311587
Comment 8 WebKit Review Bot 2012-12-14 00:21:00 PST
Comment on attachment 179429 [details]
work in progress 2: DatabaseTracker is now only accessed from DBBackend::Server + moving more stuff to DatabaseManager and the server.

Attachment 179429 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15317579
Comment 9 Mark Lam 2012-12-14 00:25:38 PST
Created attachment 179437 [details]
wip2: + fixed chromium breakage. test ews again.
Comment 10 WebKit Review Bot 2012-12-14 00:36:37 PST
Comment on attachment 179437 [details]
wip2: + fixed chromium breakage. test ews again.

Attachment 179437 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15316634
Comment 11 Mark Lam 2012-12-14 00:45:31 PST
Created attachment 179441 [details]
wip2: + fixed 2nd chromium breakage. test ews again.
Comment 12 Mark Lam 2012-12-14 01:06:45 PST
Created attachment 179444 [details]
wp2: svn up'ed.
Comment 13 Mark Lam 2012-12-17 17:56:39 PST
Created attachment 179842 [details]
The final (hopefully) patch.

This patch performs some initial refactoring to split Database functionality between the front-end DatabaseManager and the backend DBBackend::Server.  With this patch,
1. All DatabaseTracker accesses now go thru the DBBackend::Server.
2. Database::openDatabase() and DatabaseSync::openDatabaseSync() factory methods have been moved into the DatabaseManager.
3. AbstractDatabase::isAvailable() and AbstractDatabase::setIsAvailable() have been moved into the DatabaseManager.
4. Added DatabaseBasicTypes.h to hold common basic / primitive types that the webdatabase module uses.

This refactoring does not change how the code flows / works yet.  It only moves and renames things.
Comment 14 Early Warning System Bot 2012-12-17 18:13:12 PST
Comment on attachment 179842 [details]
The final (hopefully) patch.

Attachment 179842 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15374929
Comment 15 Early Warning System Bot 2012-12-17 18:13:33 PST
Comment on attachment 179842 [details]
The final (hopefully) patch.

Attachment 179842 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15374928
Comment 16 Mark Lam 2012-12-17 18:49:39 PST
Created attachment 179855 [details]
The fixed patch: added needed #include's.
Comment 17 Mark Lam 2012-12-17 19:21:06 PST
Created attachment 179863 [details]
The fixed patch minus some development changes in the Xcode project not ready for a commit yet.
Comment 18 Brady Eidson 2012-12-18 16:05:06 PST
Comment on attachment 179863 [details]
The fixed patch minus some development changes in the Xcode project not ready for a commit yet.

Seeing the pattern that this is largely a refactor that is shuffling code around, I didn't look at certain pieces of it in depth.  Seems fine as long as layouttests pass, etc etc.
Comment 19 Mark Lam 2012-12-18 16:30:01 PST
(In reply to comment #18)
> (From update of attachment 179863 [details])
> Seeing the pattern that this is largely a refactor that is shuffling code around, I didn't look at certain pieces of it in depth.  Seems fine as long as layouttests pass, etc etc.

Yes, this phase of the refactor is largely just shuffling code.

The patch has been landed in r138085: <http://trac.webkit.org/changeset/138085>.
Comment 20 Adam Klein 2012-12-18 16:41:13 PST
(In reply to comment #19)
> (In reply to comment #18)
> > (From update of attachment 179863 [details] [details])
> > Seeing the pattern that this is largely a refactor that is shuffling code around, I didn't look at certain pieces of it in depth.  Seems fine as long as layouttests pass, etc etc.
> 
> Yes, this phase of the refactor is largely just shuffling code.
> 
> The patch has been landed in r138085: <http://trac.webkit.org/changeset/138085>.

r138085 seems very broken. In particular, at least one file is completely doubled (i.e., contains two copies of the same thing), resulting in compile failures:

http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/webdatabase/DBBackendServer.cpp?rev=138085
Comment 21 Mark Lam 2012-12-18 16:44:04 PST
(In reply to comment #20)
> (In reply to comment #19)
> > (In reply to comment #18)
> > > (From update of attachment 179863 [details] [details] [details])
> > > Seeing the pattern that this is largely a refactor that is shuffling code around, I didn't look at certain pieces of it in depth.  Seems fine as long as layouttests pass, etc etc.
> > 
> > Yes, this phase of the refactor is largely just shuffling code.
> > 
> > The patch has been landed in r138085: <http://trac.webkit.org/changeset/138085>.
> 
> r138085 seems very broken. In particular, at least one file is completely doubled (i.e., contains two copies of the same thing), resulting in compile failures:
> 
> http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/webdatabase/DBBackendServer.cpp?rev=138085

My apologies.  It looks like a merge issue in my local files.  I'm fixing it now.
Comment 22 Mark Lam 2012-12-18 16:54:07 PST
(In reply to comment #21)
> My apologies.  It looks like a merge issue in my local files.  I'm fixing it now.

Fixed and landed in r138088: <http://trac.webkit.org/changeset/138088>.