Fix WebSQL error on sandboxed OSX and Linux.

Fix WebSQL error on sandboxed OSX and Linux.

Both restrict the ftruncate() system call.  Proxy it over to the browser
process.

BUG=457905
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193230

[Scott Hess - ex-Googler, 2015-03-18 01:41:56]

browser side of the patch:
   https://codereview.chromium.org/1006423008

[Scott Hess - ex-Googler, 2015-03-18 01:45:45]

shess@chromium.org changed reviewers:
+ michaeln@chromium.org

[Scott Hess - ex-Googler, 2015-03-18 01:45:46]

I think this is getting into the right region for implementing a proxied
xTruncate call for SQLite in WebSQL.

https://codereview.chromium.org/1012353002/diff/1/LayoutTests/platform/linux/...
File LayoutTests/platform/linux/storage/websql/sql-error-codes-expected.txt
(right):

https://codereview.chromium.org/1012353002/diff/1/LayoutTests/platform/linux/...
LayoutTests/platform/linux/storage/websql/sql-error-codes-expected.txt:11: PASS:
expected and got error code VERSION_ERR
Actually, this file is now the same as the general version, so should be
deleted.

https://codereview.chromium.org/1012353002/diff/1/LayoutTests/platform/mac/st...
File LayoutTests/platform/mac/storage/websql/sql-error-codes-expected.txt
(right):

https://codereview.chromium.org/1012353002/diff/1/LayoutTests/platform/mac/st...
LayoutTests/platform/mac/storage/websql/sql-error-codes-expected.txt:11: PASS:
expected and got error code VERSION_ERR
This one too.

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
File Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp (right):

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:150: sqlite3_file*
id, int desiredFlags, int* usedFlags)
Still figuring out why this indentation is throwing presubmit complaints, I'll
circle back and fix it.

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:190: sqlite3_file*
id, int desiredFlags, int* usedFlags)
This indentation is throwing pre-submit complaints, I don't know why because I
think it's identical to the original version.

https://codereview.chromium.org/1012353002/diff/1/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/1012353002/diff/1/public/platform/Platform.h#...
public/platform/Platform.h:194: virtual bool databaseSetFileSize(const
WebString& vfsFileName, long long size) { return false; }
This file needs to be changed before the browser change, then the browser
change, then the rest of this CL, I think.

[michaeln, 2015-03-19 00:14:05]

thank you for doing this!

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
File Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp (right):

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:150: sqlite3_file*
id, int desiredFlags, int* usedFlags)
On 2015/03/18 01:45:46, Scott Hess wrote:
> Still figuring out why this indentation is throwing presubmit complaints, I'll
> circle back and fix it.

probably it predates the tool and the checks

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:231: // Methods
above are valid for version 1.
Looks like the default posix io methods provides the v2 methods for a sharedmem
based locking strategy (but there are quite a few different set of iomethods in
there??)

  int (*xShmMap)(sqlite3_file*, int iPg, int pgsz, int, void volatile**);
  int (*xShmLock)(sqlite3_file*, int offset, int n, int flags);
  void (*xShmBarrier)(sqlite3_file*);
  int (*xShmUnmap)(sqlite3_file*, int deleteFlag);
  /* Methods above are valid for version 2 */

By not having those methods here, are we forcing a different strategy to be
used.

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:321:
unix_vfs->pAppData,
this func ptr is supposed to be a FINDER function

  ** Most finders simply return a pointer to a fixed sqlite3_io_methods
  ** object.  But the "autolockIoFinder" available on MacOSX does a little
  ** more than that; it looks at the filesystem type that hosts the 
  ** database file and tries to choose an locking method appropriate for
  ** that filesystem time.

should we have a func ptr here that returns chromiumIoMethods?

https://codereview.chromium.org/1012353002/diff/1/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/1012353002/diff/1/public/platform/Platform.h#...
public/platform/Platform.h:194: virtual bool databaseSetFileSize(const
WebString& vfsFileName, long long size) { return false; }
On 2015/03/18 01:45:46, Scott Hess wrote:
> This file needs to be changed before the browser change, then the browser
> change, then the rest of this CL, I think.

i think you can do this in 2 commits first chromium, then the blink. just don't
use override keyword in chromium.

[Scott Hess - ex-Googler, 2015-03-25 20:19:55]

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
File Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp (right):

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:150: sqlite3_file*
id, int desiredFlags, int* usedFlags)
On 2015/03/19 00:14:04, michaeln wrote:
> On 2015/03/18 01:45:46, Scott Hess wrote:
> > Still figuring out why this indentation is throwing presubmit complaints,
I'll
> > circle back and fix it.
> 
> probably it predates the tool and the checks

Looks like I can just not wrap it...

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:231: // Methods
above are valid for version 1.
On 2015/03/19 00:14:04, michaeln wrote:
> Looks like the default posix io methods provides the v2 methods for a
sharedmem
> based locking strategy (but there are quite a few different set of iomethods
in
> there??)
> 
>   int (*xShmMap)(sqlite3_file*, int iPg, int pgsz, int, void volatile**);
>   int (*xShmLock)(sqlite3_file*, int offset, int n, int flags);
>   void (*xShmBarrier)(sqlite3_file*);
>   int (*xShmUnmap)(sqlite3_file*, int deleteFlag);
>   /* Methods above are valid for version 2 */
> 
> By not having those methods here, are we forcing a different strategy to be
> used.

My understanding is that the version-2 bits are for write-ahead-logging to
implement a shared-memory log index.  This would need someone to run "PRAGMA
journal_mode = WAL", which is forbidden in the renderer, so I think that's safe.

The version-3 bits are for a memory-mapped access pattern.  I _think_ the
unixFile things which key off of that can only happen if xFetch() is called, so
since we don't implement that, we should be fine.

I posed a new CL to land before this which makes all of the existing sqlite3_vfs
methods proxied.  I think that with that change, the different levels can work
right in isolation without having to worry about whether we're crossing the
streams incorrectly.

https://codereview.chromium.org/1012353002/diff/1/Source/modules/webdatabase/...
Source/modules/webdatabase/sqlite/SQLiteFileSystemPosix.cpp:321:
unix_vfs->pAppData,
On 2015/03/19 00:14:04, michaeln wrote:
> this func ptr is supposed to be a FINDER function
> 
>   ** Most finders simply return a pointer to a fixed sqlite3_io_methods
>   ** object.  But the "autolockIoFinder" available on MacOSX does a little
>   ** more than that; it looks at the filesystem type that hosts the 
>   ** database file and tries to choose an locking method appropriate for
>   ** that filesystem time.
> 
> should we have a func ptr here that returns chromiumIoMethods?

pAppData is just a place you can tuck random information into the structure. 
As-is chromium_vfs is pretty incorrect because those directly-inherited methods
could potentially have implicit expectations about the form of this structure. 
Only unixOpen() actually uses it, so we've been lucky, but better to just proxy
everything.

As a bonus, proxying with the wrapped vfs in pAppData will remove the need for
szUnixFile global.

https://codereview.chromium.org/1012353002/diff/1/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/1012353002/diff/1/public/platform/Platform.h#...
public/platform/Platform.h:194: virtual bool databaseSetFileSize(const
WebString& vfsFileName, long long size) { return false; }
On 2015/03/19 00:14:04, michaeln wrote:
> On 2015/03/18 01:45:46, Scott Hess wrote:
> > This file needs to be changed before the browser change, then the browser
> > change, then the rest of this CL, I think.
> 
> i think you can do this in 2 commits first chromium, then the blink. just
don't
> use override keyword in chromium.

I may not understand things, then - without this change, the Chromium-side
change won't compile without an error.

How about I just post this now while waiting for the other CL to clear, since
this is isolated from that.

[Scott Hess - ex-Googler, 2015-04-06 20:12:58]

OK - this version is good to go!

AFIACT at this point, SQLite intentionally considers wrapped proxies to be fine
for these objects.  It internally uses this technique for error injection in
some tests.  The vfs implementations must all be able to handle v1-only calls
just fine for binary-compatibility reasons, and SQLite takes that pretty
seriously so I'm comfortable that it will work fine for us.

[michaeln, 2015-04-06 22:37:50]

lgtm, much tidier!

[Scott Hess - ex-Googler, 2015-04-06 23:17:52]

The CQ bit was checked by shess@chromium.org

[Scott Hess - ex-Googler, 2015-04-06 23:17:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org
Link to the patchset: https://codereview.chromium.org/1012353002/#ps80001
(title: "test-authorizer-expected.txt also fixed!")

[commit-bot: I haz the power, 2015-04-06 23:17:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1012353002/80001

[Scott Hess - ex-Googler, 2015-04-06 23:19:30]

On 2015/04/06 22:37:50, michaeln wrote:
> lgtm, much tidier!

Ha, the red layout tests were because someone's expectations were fixed.  AFAICT
that's the only one.

[commit-bot: I haz the power, 2015-04-07 00:48:42]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193230