[webkit-reviews] review granted: [Bug 27954] [Chromium] Make it possible for FileSystem routines to work when outside the sandbox. : [Attachment 34030] Patch v1

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has granted Jeremy Orlow
<jorlow at chromium.org>'s request for review:
Bug 27954: [Chromium] Make it possible for FileSystem routines to work when
outside the sandbox.
https://bugs.webkit.org/show_bug.cgi?id=27954

Attachment 34030: Patch v1
https://bugs.webkit.org/attachment.cgi?id=34030&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> 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