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.
<rdar://problem/13984436>
Created attachment 202862 [details] Patch
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?
Created attachment 202867 [details] Patch
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.
(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.
Committed r150670: <http://trac.webkit.org/changeset/150670>