Import SQLite 3.8.7.4.

Import SQLite 3.8.7.4.

Ran through the import script in third_party/sqlite/README.Chromium,
including the SQLite test suite.  There are a few pager errors which
are because of a change required for WebDatabase support (documented
in README).

SQLite changes are at http://www.sqlite.org/changes.html , Chromium
previously used 3.7.6.3.

All patches were applied and the results reviewed to make sure
backported patches were safe to remove, and retained patches were still
covering what was necessary.

Keep fts4 disabled, and also the new fts3 virtual table and unicode61
tokenizer.  Once enabled, these are very hard to disable, and there
doesn't seem to be any pressure to enable them.  Other SQLITE_* flags
were reviewed for applicability, none looked essential.

Fixes to Chromium:
 - In recovery.cc, pk_column now follows the documentation.
 - Short garbage files now see SQLITE_NOTADB rather than
   SQLITE_IOERR_SHORT_READ.
   - Adjust to allow clients to use ScopedErrorIgnore without adding
     dependencies.
 - More-specific SQLITE_CONSTRAINT_* errors aren't necessary.
 - Force recovery test to scan table rather than index.

BUG=340757
TEST=*EVERYTHING* continues to work.
R=michaeln@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/dcf12048055030a2b5858ceca5ce26294a82a6e4

[Scott Hess - ex-Googler, 2015-02-04 22:10:35]

Update patches, README, and sqlite3.c generator.

[Scott Hess - ex-Googler, 2015-02-04 22:17:40]

Apply patches to SQLite.

[Scott Hess - ex-Googler, 2015-02-04 22:24:51]

Chromium changes to support SQLite 3.8.7.4.

[Scott Hess - ex-Googler, 2015-02-04 22:28:38]

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

[Scott Hess - ex-Googler, 2015-02-04 22:28:48]

Sigh, this CL sucks.  In my brain the vendor copy would cause things in src/ to
look like a clear rename/modify, but what actually happened is sometimes it does
rename/modify, sometimes it just compares to the previous version in src/, and
it differs from file to file.

Patch#1 is placing vendor SQLite 3.8.7.4 into src/
Patch#2 is updating patches/, the README, and the amalgamation generator.
Patch#3 applies the updated patches.
Patch#4 is the changes to Chromium itself.

This change will not work with trybots or waterfall. 
ttps://codereview.chromium.org/886513006/ is a hacked CL which breaks things up
so that it's possible to run things through the trybots.  I'm keeping them in
sync in my local git.

If you find things truly horrible to review (just updating the page is
slooooow), I think the Chromium changes could be landed up front just fine.  Now
I'm starting to wonder if I should have looked harder at whether you could do
merges in separate directories, then those could have been done in the vendor
src and swizzled over at the end.

If you want something more bite-sized, we could strip out everything but src/src
and src/ext and other specific files we care about, and land src/test and
src/tools and the like via rubberstamp review.

https://codereview.chromium.org/901033002/diff/20001/third_party/sqlite/patch...
File
third_party/sqlite/patches/0005-Modify-default-VFS-to-support-WebDatabase.patch
(right):

https://codereview.chromium.org/901033002/diff/20001/third_party/sqlite/patch...
third_party/sqlite/patches/0005-Modify-default-VFS-to-support-WebDatabase.patch:36:
+  return 0;
This was the problem preventing WebDatabase from working.  It was one of those
things which worked fine for awhile until some operation led here.  AFAICT this
is the only item of this sort.

https://codereview.chromium.org/901033002/diff/20001/third_party/sqlite/patch...
third_party/sqlite/patches/0005-Modify-default-VFS-to-support-WebDatabase.patch:58:
+  return fillInUnixFile(vfs, fd, file, fileName, ctrlFlags);
I'm pretty nervous about this, but AFAICT there aren't run-on changes necessary.

[Scott Hess - ex-Googler, 2015-02-09 20:13:30]

On 2015/02/04 22:28:48, Scott Hess wrote:
> Sigh, this CL sucks.

Ping!  As before, I'm willing to perturb the review if that will make things
easier.  Like maybe we could pull out the test/ files and perhaps some others to
concentrate on the bits which are directly relevant to Chromium.

[michaeln, 2015-02-09 20:35:51]

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
File third_party/sqlite/src/src/os_unix.c (right):

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
third_party/sqlite/src/src/os_unix.c:1326: ** running in the sandbox.
Does this have any impact on sqlite consumers in the main browser process?

[Scott Hess - ex-Googler, 2015-02-09 21:36:27]

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
File third_party/sqlite/src/src/os_unix.c (right):

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
third_party/sqlite/src/src/os_unix.c:1326: ** running in the sandbox.
On 2015/02/09 20:35:51, michaeln wrote:
> Does this have any impact on sqlite consumers in the main browser process?

The intention is to notice cases where the underlying file has been moved or
replaced, since SQLite makes assumptions about how the main database and journal
files relate (so moving things around could result in replaying a journal
against the wrong database file, for instance).  The current code in Chromium
doesn't have this check, so if this is currently happening we don't know about
it.  Beyond that, profile databases are Chromium-internal, so there are no
guarantees if people are moving those around while Chromium is running.

So I think this function should never have a non-zero return in normal Chromium
operation (browser or renderer), and if it did this change only takes us back to
how things have operated up to now.

Long-term, I don't have any story on how to fix this, short of rewriting the
vfs.  In theory the current vfs could pass some sort of TLS data down to a
custom osStat() implementation, but osStat() is used quite frequently in this
file, so that would make me very nervous.  Such a solution would also add to the
fragility of our current vfs.

It is possible that having this fail is a bug in SQLite.  Reading os_unix.c I
cannot find anything which should actually fail rather than warn due to this.

[michaeln, 2015-02-09 22:53:40]

i think this lgtm

> Long-term, I don't have any story on how to fix this, short of rewriting the
> vfs.  In theory the current vfs could pass some sort of TLS data down to a
> custom osStat() implementation, but osStat() is used quite frequently in this
> file, so that would make me very nervous.  Such a solution would also add to
the
> fragility of our current vfs.

Less long-term, would could make a distinction between running in the sandbox vs
not and only do this when in the sandbox? A decent proxy for being in the
sandbox could be whether a chromium_vfs was ever created in the process.

I wonder how the bug motivating the new checks manifested and if it could occur
with our usage?

[michaeln, 2015-02-09 22:55:35]

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
File third_party/sqlite/src/src/os_unix.c (right):

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
third_party/sqlite/src/src/os_unix.c:3824: *(int*)pArg = fileHasMoved(pFile);
does this one only result in warnings too?

[Scott Hess - ex-Googler, 2015-02-10 21:14:23]

On 2015/02/09 22:53:40, michaeln wrote:
> > Long-term, I don't have any story on how to fix this, short of rewriting the
> > vfs.  In theory the current vfs could pass some sort of TLS data down to a
> > custom osStat() implementation, but osStat() is used quite frequently in
this
> > file, so that would make me very nervous.  Such a solution would also add to
> the
> > fragility of our current vfs.
> 
> Less long-term, would could make a distinction between running in the sandbox
vs
> not and only do this when in the sandbox? A decent proxy for being in the
> sandbox could be whether a chromium_vfs was ever created in the process.

See comment below, I think it might be possible to bake this into the vfs
itself.

> I wonder how the bug motivating the new checks manifested and if it could
occur
> with our usage?

AFAICT it landed about a year ago, but was in development earlier on a branch. 
I find myself not able to follow their repo well enough to track it down to an
original report.

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
File third_party/sqlite/src/src/os_unix.c (right):

https://codereview.chromium.org/901033002/diff/30001/third_party/sqlite/src/s...
third_party/sqlite/src/src/os_unix.c:3824: *(int*)pArg = fileHasMoved(pFile);
On 2015/02/09 22:55:34, michaeln wrote:
> does this one only result in warnings too?

I went digging, and this is the operational one.  pager.c function
databaseIsUnmoved() calls sqlite3OsFileControl() with SQLITE_FCNTL_HAS_MOVED. 
If it has moved, then the database is changed to read-only.

I could "fix" this in SQLiteFileSystemPosix.cpp by further interception in the
vfs structure to replace the sqlite3_io_methods structure with a proxy.  I
_think_ that could be done entirely from above without any additional SQLite
injection.  I'm not entirely negative on this approach, as I think SQLite
specifically supports this notion of writing a proxy wrapper like this.  I can
go prototype it and circle back later.

[commit-bot: I haz the power, 2015-02-10 21:37:46]

Patchset 4 (id:??) landed as
https://crrev.com/dcf12048055030a2b5858ceca5ce26294a82a6e4
Cr-Commit-Position: refs/heads/master@{#315646}

[Scott Hess - ex-Googler, 2015-02-10 21:39:45]

Committed patchset #4 (id:30001) manually as
dcf12048055030a2b5858ceca5ce26294a82a6e4 (presubmit successful).