We can no longer assume that WebKit is always run within a sandbox. Code running in the browser process should not use the VFS. File system operations should be implementable.
Created attachment 34005 [details] Patch v1
Created attachment 34029 [details] Patch v1
LGTM. Minor: add NOTREACHED()s to SQLiteFileSystemChromium{Mac,Linux}.cpp
Created attachment 34030 [details] Patch v1
Comment on attachment 34030 [details] Patch v1 > Index: WebCore/platform/chromium/ChromiumBridge.h ... > // File --------------------------------------------------------------- > - static bool getFileSize(const String& path, long long& result); > + static bool sandboxEnabled(); I think sandboxEnabled is not specific to file system stuff. How about putting this in a separate section? > Index: WebCore/platform/sql/chromium/SQLiteFileSystemChromiumLinux.cpp ... > // stub for registering Chromium's SQLite VFS for Linux > + ASSERT_NOT_REACHED(); Isn't notImplemented() more correct? > Index: WebCore/platform/sql/chromium/SQLiteFileSystemChromiumMac.cpp ... > // stub for registering Chromium's SQLite VFS for Mac > + ASSERT_NOT_REACHED(); ditto > Index: WebCore/platform/sql/chromium/SQLiteFileSystemChromiumWin.cpp ... > + // FIXME: Make sure there aren't any unintended consequences when VFS code is called in the browser process. > + if (!ChromiumBridge::sandboxEnabled()) { > + ASSERT_NOT_REACHED(); > + return; > + } notImplemented? These issues aren't major, so R=me
(In reply to comment #5) > (From update of attachment 34030 [details]) > > Index: WebCore/platform/chromium/ChromiumBridge.h > ... > > // File --------------------------------------------------------------- > > - static bool getFileSize(const String& path, long long& result); > > + static bool sandboxEnabled(); > > I think sandboxEnabled is not specific to file system stuff. How about putting > this in a separate section? I'll make this change. > > Index: WebCore/platform/sql/chromium/SQLiteFileSystemChromiumLinux.cpp > ... > > // stub for registering Chromium's SQLite VFS for Linux > > + ASSERT_NOT_REACHED(); > > Isn't notImplemented() more correct? Not really. notImplemented is not very noisy. In this case, if we're hitting that code, something is very wrong. Either way, the code should be going away as Dumi completes the implementation.
Sending WebCore/ChangeLog Sending WebCore/platform/chromium/ChromiumBridge.h Sending WebCore/platform/chromium/FileSystemChromium.cpp Sending WebCore/platform/sql/chromium/SQLiteFileSystemChromium.cpp Sending WebCore/platform/sql/chromium/SQLiteFileSystemChromiumLinux.cpp Sending WebCore/platform/sql/chromium/SQLiteFileSystemChromiumMac.cpp Sending WebCore/platform/sql/chromium/SQLiteFileSystemChromiumWin.cpp Transmitting file data ....... Committed revision 46741. http://trac.webkit.org/changeset/46741