Bug 107475 – webdatabase: Split openDatabase() between front and back end work

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mark Lam]

1. Need to refactor the openDatabase() process to isolate the front-end script activity from back-end database activity.

Also, ...
2. Rename AbstractDatabase to DatabaseBackend.
    - Currently, this is just a rename operation.  In a subsequent task, we'll change Database and DatabaseSync to delegate to DatabaseBackend instead of inheriting from it.
3. Rename DBBackend::Server to DatabaseServer.
    - This simplifies things.  I realized that splitting out a DBBackend namespace to encapsulate all backend code isn't worth the effort yet.
4. Create a DatabaseBackendContext that the database backend uses.
    - Currently, it is only a wrapper which inherits from DatabaseContext.  Later on, we'll need to split the 2 apart.
5. Fix up add/removeOpenDatabase() so that they are called only when a database has been opened and closed respectively.
    - Previously, client code needs to call removeOpenDatabase() directly, and the Database constructor automatically calls addOpenDatabase() even when the database has not been opened yet.
    - Currently, addOpenDatabase() is only called if the database is successfully opened, and removeOpenDatabase() is only called when the database is closed.
    - In addition, we ensure that the database destructor will always close the database if it is still opened.

[Mark Lam]

Created attachment 185358 [details]
work in progress 1.

Uploading in case anyone wants to take a preliminary look at this.  This patch builds should build but still need to self-review (after the latest svn up which changed some logic) and re-test.  In it's current form, it shows how all the pieces come together for this refactor, but the patch as a whole is big.  I will break them out into smaller pieces in separate bugs.

[Mark Lam]

Created attachment 186093 [details]
work in progress 2

[Mark Lam]

Created attachment 186740 [details]
work in progress 3: time to test on the EWS bots.

[WebKit Review Bot]

Attachment 186740 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/Modules/webdatabase/AbstractDatabaseServer.h', u'Source/WebCore/Modules/webdatabase/DOMWindowWebDatabase.cpp', u'Source/WebCore/Modules/webdatabase/Database.cpp', u'Source/WebCore/Modules/webdatabase/Database.h', u'Source/WebCore/Modules/webdatabase/DatabaseBackend.h', u'Source/WebCore/Modules/webdatabase/DatabaseBackendAsync.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseBackendAsync.h', u'Source/WebCore/Modules/webdatabase/DatabaseBackendSync.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseBackendSync.h', u'Source/WebCore/Modules/webdatabase/DatabaseError.h', u'Source/WebCore/Modules/webdatabase/DatabaseManager.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseManager.h', u'Source/WebCore/Modules/webdatabase/DatabaseServer.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseServer.h', u'Source/WebCore/Modules/webdatabase/DatabaseSync.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseSync.h', u'Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseTracker.h', u'Source/WebCore/Modules/webdatabase/WorkerContextWebDatabase.cpp', u'Source/WebCore/Modules/webdatabase/chromium/DatabaseTrackerChromium.cpp']" exit_code: 1
Source/WebCore/Modules/webdatabase/AbstractDatabaseServer.h:55:  Missing space before {  [whitespace/braces] [5]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 186740 [details]
work in progress 3: time to test on the EWS bots.

Attachment 186740 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16373911

[Early Warning System Bot]

Comment on attachment 186740 [details]
work in progress 3: time to test on the EWS bots.

Attachment 186740 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16374975

[kov's GTK+ EWS bot]

Comment on attachment 186740 [details]
work in progress 3: time to test on the EWS bots.

Attachment 186740 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16389044

[Early Warning System Bot]

Comment on attachment 186740 [details]
work in progress 3: time to test on the EWS bots.

Attachment 186740 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16376970

[Mark Lam]

Comment on attachment 186740 [details]
work in progress 3: time to test on the EWS bots.

Time to stop the EWS bleeding.  =(

[Mark Lam]

Created attachment 186759 [details]
work in progress 4: let's check the ews bots again.

[WebKit Review Bot]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Attachment 186759 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16398005

[Early Warning System Bot]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Attachment 186759 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16394066

[WebKit Review Bot]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Attachment 186759 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16393077

[EFL EWS Bot]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Attachment 186759 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16394068

[Early Warning System Bot]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Attachment 186759 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16391122

[Peter Beverloo (cr-android ews)]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Attachment 186759 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/16388154

[Mark Lam]

Comment on attachment 186759 [details]
work in progress 4: let's check the ews bots again.

Arggg... GCC!!!  Oh well.  Will upload another patch shortly with some unreachable code to placate GCC because it doesn't have enough smarts to infer that the end of the function DatabaseManager::exceptionCodeForDatabaseError() is unreachable.

[Mark Lam]

Created attachment 186761 [details]
work in progress 5: let's check those ews bots again. =(

[WebKit Review Bot]

Comment on attachment 186761 [details]
work in progress 5: let's check those ews bots again. =(

Attachment 186761 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16388161

[Mark Lam]

Created attachment 186767 [details]
the patch.

[Mark Lam]

Created attachment 186791 [details]
the patch with comments fixed up.

[Anders Carlsson]

Comment on attachment 186791 [details]
the patch with comments fixed up.

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

> Source/WebCore/Modules/webdatabase/DatabaseManager.cpp:243
> +    if (!backend) {
> +        ASSERT(error != DatabaseError::None);
> +
> +        switch (error) {
> +        case DatabaseError::DatabaseIsBeingDeleted:
> +        case DatabaseError::DatabaseSizeOverflowed:
> +        case DatabaseError::GenericSecurityError:
> +            goto handleError;
> +
> +        case DatabaseError::InvalidDatabaseState:
> +            goto handleInvalidDatabaseState;

Please restructure the code so you can avoid these gotos.

[Mark Lam]

Created attachment 186911 [details]
No more gotos.

[Mark Lam]

Thanks for the review.  Landed in r142030: <http://trac.webkit.org/changeset/142030>.