Summary: | [Windows] Expose database storage and cache locations via preferences | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||
Component: | WebKit Misc. | Assignee: | Brent Fulgham <bfulgham> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aroben, bfulgham, commit-queue, dpranke, glenn, thorton, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Windows 7 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 75708 | ||||||||
Attachments: |
|
Description
Brent Fulgham
2013-05-24 11:04:08 PDT
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> |