[sqlite] sql::Recovery working under USE_SYSTEM_SQLITE.

[sqlite] sql::Recovery working under USE_SYSTEM_SQLITE.

Removes the USE_SYSTEM_SQLITE conditional sections in sql::Recover
implementation and tests.  third_party/sqlite includes sqlite_recover
library for the USE_SYSTEM_SQLITE case, with recover.c compiled against
the system version of SQLite.

Additionally remove interiorCursorEOF() from recover.c, as it was
unused, rather than adding a compile flag to allow it.

BUG=584407
Committed: https://crrev.com/7776a37d651f0afdf61f150fba50884f2ad3ac5d
Cr-Commit-Position: refs/heads/master@{#380076}

[Scott Hess - ex-Googler, 2016-03-01 23:49:00]

The CQ bit was checked by shess@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-01 23:49:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1757653002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1757653002/1

[commit-bot: I haz the power, 2016-03-02 00:41:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-02 00:41:57]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Scott Hess - ex-Googler, 2016-03-02 01:00:32]

https://codereview.chromium.org/1757653002/diff/1/sql/recovery_unittest.cc
File sql/recovery_unittest.cc (right):

https://codereview.chromium.org/1757653002/diff/1/sql/recovery_unittest.cc#ne...
sql/recovery_unittest.cc:714:
ASSERT_TRUE(PathService::Get(sql::test::DIR_TEST_DATA, &golden_path));
This is failing on iOS.  I'm guessing something about my incantations in
sql/test/sql_test_suite.h is wrong?

This test _could_ be disabled for iOS, because what it's testing is
cross-platform.

[Scott Hess - ex-Googler, 2016-03-02 01:24:44]

Annotate for reviewer.

https://codereview.chromium.org/1757653002/diff/1/third_party/sqlite/BUILD.gn
File third_party/sqlite/BUILD.gn (right):

https://codereview.chromium.org/1757653002/diff/1/third_party/sqlite/BUILD.gn...
third_party/sqlite/BUILD.gn:228: ]
Since this is a custom-to-us module, we should be able to safely include it for
_any_ use_system_sqlite case.  sqlite_regexp is more debatable (though I suspect
it would also work fine for any use_system_sqlite case, too).

https://codereview.chromium.org/1757653002/diff/1/third_party/sqlite/BUILD.gn...
third_party/sqlite/BUILD.gn:243: }
This module could be pulled outside of the use_system_sqlite case entirely, and
the sql/BUILD.gn could depend on sqlite:sqlite_recover for either case.  It
should work either way, since the module uses no globals.  The main reason I
went with this split is simply being conservative.

Of course, another solution would be to pull the entire module into sql/
somewhere.  That would make the in-SQLite testing harder, though it could be
rewritten as a unit test.  I'd like to hold off on that kind of move until I try
upstreaming things.  If SQLite accepts it, then lots of problems are solved.

https://codereview.chromium.org/1757653002/diff/1/third_party/sqlite/src/src/...
File third_party/sqlite/src/src/recover.c (left):

https://codereview.chromium.org/1757653002/diff/1/third_party/sqlite/src/src/...
third_party/sqlite/src/src/recover.c:705: *                     parent cursors
can return any more data.
This wasn't being used.  Since Chromium SQLite is compiled with
-Wno-unused-function, I never noticed.

Given the note below, I believe that this function doesn't have a future in any
case.

[Scott Hess - ex-Googler, 2016-03-02 18:44:59]

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

[Scott Hess - ex-Googler, 2016-03-02 18:44:59]

Thanks for volunteering!

[Scott Hess - ex-Googler, 2016-03-07 21:50:14]

https://codereview.chromium.org/1757653002/diff/1/sql/recovery_unittest.cc
File sql/recovery_unittest.cc (right):

https://codereview.chromium.org/1757653002/diff/1/sql/recovery_unittest.cc#ne...
sql/recovery_unittest.cc:714:
ASSERT_TRUE(PathService::Get(sql::test::DIR_TEST_DATA, &golden_path));
On 2016/03/02 01:00:32, Scott Hess wrote:
> This is failing on iOS.  I'm guessing something about my incantations in
> sql/test/sql_test_suite.h is wrong?
> 
> This test _could_ be disabled for iOS, because what it's testing is
> cross-platform.

OK, I think this needs to be disabled.  DIR_SOURCE_ROOT comes through as
something like:
/private/tmp/tmp<X>/Library/Developer/CoreSimulator/Devices/<GUID>/data/Containers/Bundle/Application/<GUID>/sql_unittests.app/
which is entirely different from what I see on OSX (src/ dir).  So I'm guessing
this kind of thing only works if someone does additional work to include part of
the root as a resource or something, so I'm going to comment this one out for
iOS.

[Scott Hess - ex-Googler, 2016-03-07 23:09:59]

copy_test_data_ios.gypi for test data.

[Scott Hess - ex-Googler, 2016-03-07 23:11:15]

https://codereview.chromium.org/1757653002/diff/1/sql/recovery_unittest.cc
File sql/recovery_unittest.cc (right):

https://codereview.chromium.org/1757653002/diff/1/sql/recovery_unittest.cc#ne...
sql/recovery_unittest.cc:714:
ASSERT_TRUE(PathService::Get(sql::test::DIR_TEST_DATA, &golden_path));
On 2016/03/07 21:50:13, Scott Hess wrote:
> On 2016/03/02 01:00:32, Scott Hess wrote:
> > This is failing on iOS.  I'm guessing something about my incantations in
> > sql/test/sql_test_suite.h is wrong?
> > 
> > This test _could_ be disabled for iOS, because what it's testing is
> > cross-platform.
> 
> OK, I think this needs to be disabled.  DIR_SOURCE_ROOT comes through as
> something like:
>
/private/tmp/tmp<X>/Library/Developer/CoreSimulator/Devices/<GUID>/data/Containers/Bundle/Application/<GUID>/sql_unittests.app/
> which is entirely different from what I see on OSX (src/ dir).  So I'm
guessing
> this kind of thing only works if someone does additional work to include part
of
> the root as a resource or something, so I'm going to comment this one out for
> iOS.

OK, figured it out by poking around with base_unittests a bit.  Last patch fixes
it right, without disabling the test.

[Scott Hess - ex-Googler, 2016-03-07 23:11:30]

The CQ bit was checked by shess@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-07 23:11:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1757653002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1757653002/20001

[commit-bot: I haz the power, 2016-03-08 00:00:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-08 00:00:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sdefresne, 2016-03-08 15:50:28]

lgtm

[Scott Hess - ex-Googler, 2016-03-08 23:23:18]

The CQ bit was checked by shess@chromium.org

[commit-bot: I haz the power, 2016-03-08 23:23:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1757653002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1757653002/20001

[commit-bot: I haz the power, 2016-03-09 00:54:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-09 00:54:55]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Scott Hess - ex-Googler, 2016-03-09 04:49:46]

The CQ bit was checked by shess@chromium.org

[commit-bot: I haz the power, 2016-03-09 04:50:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1757653002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1757653002/20001

[commit-bot: I haz the power, 2016-03-09 05:30:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-09 05:31:53]

Description was changed from

==========
[sqlite] sql::Recovery working under USE_SYSTEM_SQLITE.

Removes the USE_SYSTEM_SQLITE conditional sections in sql::Recover
implementation and tests.  third_party/sqlite includes sqlite_recover
library for the USE_SYSTEM_SQLITE case, with recover.c compiled against
the system version of SQLite.

Additionally remove interiorCursorEOF() from recover.c, as it was
unused, rather than adding a compile flag to allow it.

BUG=584407
==========

to

==========
[sqlite] sql::Recovery working under USE_SYSTEM_SQLITE.

Removes the USE_SYSTEM_SQLITE conditional sections in sql::Recover
implementation and tests.  third_party/sqlite includes sqlite_recover
library for the USE_SYSTEM_SQLITE case, with recover.c compiled against
the system version of SQLite.

Additionally remove interiorCursorEOF() from recover.c, as it was
unused, rather than adding a compile flag to allow it.

BUG=584407

Committed: https://crrev.com/7776a37d651f0afdf61f150fba50884f2ad3ac5d
Cr-Commit-Position: refs/heads/master@{#380076}
==========

[commit-bot: I haz the power, 2016-03-09 05:31:54]

Patchset 2 (id:??) landed as
https://crrev.com/7776a37d651f0afdf61f150fba50884f2ad3ac5d
Cr-Commit-Position: refs/heads/master@{#380076}

[Scott Hess - ex-Googler, 2016-03-09 06:20:38]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1778823002/ by shess@chromium.org.

The reason for reverting is:
https://build.chromium.org/p/chromium/builders/Linux/builds/72746/steps/compi...

FAILED: /b/build/slave/Linux/build/src/build/goma/client/gomacc
../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF
obj/third_party/sqlite/src/src/sqlite_recover.recover_varint.o.d
-DSQLITE_ENABLE_FTS3 -DSQLITE_DISABLE_FTS3_UNICODE
-DSQLITE_DISABLE_FTS4_DEFERRED -DSQLITE_ENABLE_ICU
-DSQLITE_ENABLE_MEMORY_MANAGEMENT -DSQLITE_SECURE_DELETE
-DSQLITE_SEPARATE_CACHE_POOLS -DTHREADSAFE -DSQLITE_MAX_WORKER_THREADS=0
-DSQLITE_MAX_MMAP_SIZE=268435456 -DSQLITE_MMAP_READ_ONLY=1
-DSQLITE_DEFAULT_PCACHE_INITSZ=0 -DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2
-D_FILE_OFFSET_BITS=64 -DCHROMIUM_BUILD -DCR_CLANG_REVISION=261368-1
-DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1
-DUSE_DEFAULT_RENDER_THEME=1 -DUSE_LIBJPEG_TURBO=1 -DUSE_X11=1
-DUSE_CLIPBOARD_AURAX11=1 -DENABLE_WEBRTC=1 -DENABLE_MEDIA_ROUTER=1
-DENABLE_PEPPER_CDMS -DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS
-DENABLE_TOPCHROME_MD=1 -DUSE_UDEV -DFIELDTRIAL_TESTING_ENABLED
-DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1 -DENABLE_PDF=1 -DENABLE_PLUGINS=1
-DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1
-DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1
-DENABLE_SPELLCHECK=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1
-DENABLE_SETTINGS_APP=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_MDNS=1
-DENABLE_SERVICE_DISCOVERY=1 -DV8_USE_EXTERNAL_STARTUP_DATA -DFULL_SAFE_BROWSING
-DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DUSE_LIBPCI=1 -DUSE_OPENSSL=1
-DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DNDEBUG -DNVALGRIND
-DDYNAMIC_ANNOTATIONS_ENABLED=0 -Igen -fstack-protector
--param=ssp-buffer-size=4 -Werror -pthread -fno-strict-aliasing -Wall
-Wno-unused-parameter -Wno-missing-field-initializers -fvisibility=hidden -pipe
-fPIC -Xclang -load -Xclang
/b/build/slave/Linux/build/src/third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so
-Xclang -add-plugin -Xclang find-bad-constructs -Xclang
-plugin-arg-find-bad-constructs -Xclang check-templates -Xclang
-plugin-arg-find-bad-constructs -Xclang follow-macro-expansion
-momit-leaf-frame-pointer -mstack-alignment=16 -mstackrealign
-fcolor-diagnostics
-B/b/build/slave/Linux/build/src/third_party/binutils/Linux_x64/Release/bin
-Wheader-hygiene -Wno-char-subscripts -Wno-unneeded-internal-declaration
-Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing
-Wno-deprecated-register -Wno-inconsistent-missing-override
-Wno-shift-negative-value -Wno-unused-variable -msse2 -mfpmath=sse -mmmx -m32
--sysroot=/b/build/slave/Linux/build/src/build/linux/debian_wheezy_i386-sysroot
-O2 -fno-ident -fdata-sections -ffunction-sections -funwind-tables   -c
../../third_party/sqlite/src/src/recover_varint.c -o
obj/third_party/sqlite/src/src/sqlite_recover.recover_varint.o
../../third_party/sqlite/src/src/recover_varint.c:18:10: fatal error:
'sqlite3.h' file not found
#include "sqlite3.h"
.