Bug 116729 – [Windows] Expose database storage and cache locations via preferences

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

[Brent Fulgham]

The Windows port of WebKit stores all local data (e.g., URL cache, Icon Database, DOM Storage) in the OS-defined locations CSIDL_LOCAL_APPDATA or CSIDL_APPDATA (depending on roaming context).  There is no way for clients of WebKit to indicate that these items should be stored elsewhere, which could lead to concurrent WebKit clients interacting with each-other's data.

We should implement the same mechanism used by the Mac port, which uses an application default/preference setting to specify where this should go.

Use the same key labels as the mac port for consistency:

NSString *WebDatabaseDirectoryDefaultsKey = @"WebDatabaseDirectory";
NSString *WebKitLocalCacheDefaultsKey = @"WebKitLocalCache";
NSString *WebStorageDirectoryDefaultsKey = @"WebKitLocalStorageDatabasePathPreferenceKey";

This bug needs to:

1. Modify the WebStorageManager to use the WebStorageDirectoryDefaultsKey key.
2. Modify the WebDatabaseManager to use the WebDatabaseDirectoryDefaultsKey key.
3. Modify the URL cache code to use the WebKitLocalCache key.

[Radar WebKit Bug Importer]

<rdar://problem/13984436>

[Brent Fulgham]

Created attachment 202862 [details]
Patch

[Tim Horton]

Comment on attachment 202862 [details]
Patch

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

> Source/WebKit/win/WebDatabaseManager.cpp:413
> +    CFStringRef directoryPref = static_cast<CFStringRef>(CFPreferencesCopyAppValue(WebDatabaseDirectoryDefaultsKey, kCFPreferencesCurrentApplication));

You should check that the returned CFPropertyListRef is of the right type before casting it!

> Source/WebKit/win/WebView.cpp:485
> +        if (!cfurlCacheDirectory || CFStringGetTypeID() != CFGetTypeID(cfurlCacheDirectory.get()))

ditto.

> Source/WebKit/win/WebView.cpp:2608
> +    if (cacheDirectory && CFStringGetTypeID() == CFGetTypeID(cacheDirectory))

etc.

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:204
> +        if (path[path.length() - 1] != '\\')

What if the path is ""?

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:1300
> +    CFPreferencesSetAppValue(WebDatabaseDirectoryDefaultsKey, WebCore::pathByAppendingComponent(path, "Databases").createCFString().get(), kCFPreferencesCurrentApplication);

Are these created strings going to leak? If not, why not?

[Brent Fulgham]

Created attachment 202867 [details]
Patch

[Tim Horton]

Comment on attachment 202867 [details]
Patch

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

Seems reasonable.

> Tools/DumpRenderTree/win/DumpRenderTree.cpp:209
> +    DWORD size = ::GetEnvironmentVariable(dumpRenderTreeTemp, 0, 0);
> +    Vector<TCHAR> buffer(size);
> +    if (::GetEnvironmentVariable(dumpRenderTreeTemp, buffer.data(), buffer.size())) {
> +        wstring path = buffer.data();
> +        if (!path.empty() && (path[path.length() - 1] != L'\\'))
> +            path.append(L"\\");
> +        return WTF::String (path.data(), path.length());
> +    }
> +
> +    return WebCore::localUserSpecificStorageDirectory();

Do you really need all these explicit namespaces? This looks unusual.

[Brent Fulgham]

(In reply to comment #5)
> (From update of attachment 202867 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202867&action=review
> 
> Seems reasonable.
> 
> > Tools/DumpRenderTree/win/DumpRenderTree.cpp:209
> > +    DWORD size = ::GetEnvironmentVariable(dumpRenderTreeTemp, 0, 0);
> > +    Vector<TCHAR> buffer(size);
> > +    if (::GetEnvironmentVariable(dumpRenderTreeTemp, buffer.data(), buffer.size())) {
> > +        wstring path = buffer.data();
> > +        if (!path.empty() && (path[path.length() - 1] != L'\\'))
> > +            path.append(L"\\");
> > +        return WTF::String (path.data(), path.length());
> > +    }
> > +
> > +    return WebCore::localUserSpecificStorageDirectory();
> 
> Do you really need all these explicit namespaces? This looks unusual.

aroben told me (years ago) that they were using the global namespace decoration on all Windows API calls, so I got in the habit of using it.

I think the WTF is explicitly used in DumpRenderTree for some reason.  We could add a "using namespace WTF", but I was trying to minimize the changes.

[Brent Fulgham]

Committed r150670: <http://trac.webkit.org/changeset/150670>