Bug 38555 - Small code refactoring: move the logic to figure out the path to the databases directory to another method
Summary: Small code refactoring: move the logic to figure out the path to the database...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Ada Chan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-04 16:45 PDT by Ada Chan
Modified: 2010-05-04 18:43 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.57 KB, patch)
2010-05-04 16:47 PDT, Ada Chan
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ada Chan 2010-05-04 16:45:10 PDT
Move the logic to figure out the path to the databases directory to another method, so new code that needs that information doesn't need to duplicate that code.
Comment 1 Ada Chan 2010-05-04 16:47:11 PDT
Created attachment 55071 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2010-05-04 16:50:15 PDT
Comment on attachment 55071 [details]
Patch

> Index: WebKit/mac/Storage/WebDatabaseManager.mm
> +static NSString *databasesDirectoryPath();

I don't think static is needed here.  Also, this definition isn't needed unless the method appears after the first use (IIRC).

> @@ -119,6 +121,16 @@ - (BOOL)deleteDatabase:(NSString *)datab
> +NSString *databasesDirectoryPath()

But "static" should definitely be added above.

> +{
> +    NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
> +    NSString *databasesDirectory = [defaults objectForKey:WebDatabaseDirectoryDefaultsKey];
> +    if (!databasesDirectory || ![databasesDirectory isKindOfClass:[NSString class]])
> +        databasesDirectory = @"~/Library/WebKit/Databases";
> +    
> +    return [databasesDirectory stringByStandardizingPath];
> +}

r=me

Dave
Comment 3 Ada Chan 2010-05-04 16:58:38 PDT
Fixed in r58787

http://trac.webkit.org/changeset/58787
Comment 4 WebKit Review Bot 2010-05-04 17:48:36 PDT
http://trac.webkit.org/changeset/58787 might have broken Qt Linux Release
Comment 5 David Kilzer (:ddkilzer) 2010-05-04 18:01:25 PDT
(In reply to comment #4)
> http://trac.webkit.org/changeset/58787 might have broken Qt Linux Release

Lies.
Comment 6 Eric Seidel (no email) 2010-05-04 18:43:13 PDT
"might"! :)