Bug 41404

Summary:

DB clean up: Make all DatabaseTasks internal Database classes

Product:

WebKit

Reporter:

Dumitru Daniliuc <dumi>

Component:

New Bugs

Assignee:

Dumitru Daniliuc <dumi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, eric, michaeln, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

41494

Bug Blocks:

Attachments:

Description	Flags
patch	dumi: commit-queue-
patch	none
patch	fishd: review+, dumi: commit-queue-

Dumitru Daniliuc

Reported 2010-06-30 04:51:03 PDT

All DatabaseTasks are used only by the async DBs, so we should "scope" them somehow to the Database class. If we make them internal Database classes, then we can also make some methods that are currently public (like performOpenAndVerify() and close()) private.

Attachments
patch (25.96 KB, patch) 2010-06-30 16:01 PDT, Dumitru Daniliuc	dumi: commit-queue-	Details Formatted Diff Diff
patch (25.99 KB, patch) 2010-06-30 20:04 PDT, Dumitru Daniliuc	no flags	Details Formatted Diff Diff
patch (25.25 KB, patch) 2010-07-02 00:40 PDT, Dumitru Daniliuc	fishd: review+ dumi: commit-queue-	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Dumitru Daniliuc

Comment 1 2010-06-30 16:01:49 PDT

Created attachment 60160 [details] patch

Michael Nordman

Comment 2 2010-06-30 18:35:21 PDT

Gotta love patches that delete more code than they add ;) Really glad to see this cleanup. Like we talked about offline, i'm not sure having the db thread call the virtual closeImmediately() method is an improvement over the simpler more direct close() method.

Dumitru Daniliuc

Comment 3 2010-06-30 20:04:32 PDT

Created attachment 60187 [details] patch (In reply to comment #2) > Like we talked about offline, i'm not sure having the db thread call the virtual closeImmediately() method is an improvement over the simpler more direct close() method. done.

Michael Nordman

Comment 4 2010-07-01 12:54:37 PDT

This LGreatTM but what's going on with the trybots?

Darin Fisher (:fishd, Google)

Comment 5 2010-07-01 15:57:36 PDT

Comment on attachment 60187 [details] patch rs=me, deferring to michael's review.

Dumitru Daniliuc

Comment 6 2010-07-01 16:34:08 PDT

Landed: r62321.

WebKit Review Bot

Comment 7 2010-07-01 16:45:49 PDT

http://trac.webkit.org/changeset/62321 might have broken Chromium Linux Release

Dumitru Daniliuc

Comment 8 2010-07-01 17:24:49 PDT

Reopening, patch broke the Mac build, so I had to revert it.

Dumitru Daniliuc

Comment 9 2010-07-02 00:40:49 PDT

Created attachment 60344 [details] patch Same patch, with minor syntax changes. The Windows, Mac, Qt and Chromium/Win ports have built successfully with this patch applied.

Eric Seidel (no email)

Comment 10 2010-07-02 03:17:30 PDT

Comment on attachment 60187 [details] patch Cleared Darin Fisher's review+ from obsolete attachment 60187 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Darin Fisher (:fishd, Google)

Comment 11 2010-07-02 14:32:48 PDT

Comment on attachment 60344 [details] patch R=me, but you don't need any of the Database:: prefixes inside the class definitions.

Dumitru Daniliuc

Comment 12 2010-07-02 14:42:33 PDT

(In reply to comment #11) > (From update of attachment 60344 [details]) > R=me, but you don't need any of the Database:: prefixes inside the class definitions. Removed.

Dumitru Daniliuc

Comment 13 2010-07-02 15:51:14 PDT

re-landed: r62411.

Note You need to log in before you can comment on or make changes to this bug.