Modify sync unit test to use standard sql::test:: helper.

Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682
Committed: https://crrev.com/4640cc232029f854e84455d2087ca071a3870ee6
Cr-Commit-Position: refs/heads/master@{#426316}

[Scott Hess - ex-Googler, 2016-10-14 22:47:46]

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

[commit-bot: I haz the power, 2016-10-14 22:48:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Scott Hess - ex-Googler, 2016-10-14 22:55:28]

Add on changes for sync_integration_tests.

[Scott Hess - ex-Googler, 2016-10-14 22:55:38]

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

[commit-bot: I haz the power, 2016-10-14 22:55:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Scott Hess - ex-Googler, 2016-10-14 23:07:08]

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

[Scott Hess - ex-Googler, 2016-10-14 23:07:09]

I broke it in pieces so you can see what changed.  Main diff is to do the
corrupting underneath a SQLite transaction, rather than using the
page-cache-clearing call.

If you need further convincing on the set_exclusive_locking() thing, let me know
:-).  Internally, the main thing it does is tell SQLite to never release the
lock, and to assume that the update cookie never changes outside of the current
connection.  That's pretty much the entire reason these tests have this problem,
of course!  Anyhow, the operations set_exclusive_locking() manages to skip are
very well-optimized on our platforms, so it's one of those "If you can, might as
well" knobs.

[commit-bot: I haz the power, 2016-10-14 23:50:19]

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

[commit-bot: I haz the power, 2016-10-14 23:50:19]

Dry run: This issue passed the CQ dry run.

[Gang Wu, 2016-10-19 00:29:41]

Description was changed from

==========
Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682
==========

to

==========
Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682
==========

[Gang Wu, 2016-10-19 00:29:41]

gangwu@chromium.org changed reviewers:
+ maxbogue@chromium.org

[Gang Wu, 2016-10-19 00:30:21]

lgtm

[Gang Wu, 2016-10-19 00:30:23]

lgtm

[maxbogue, 2016-10-19 17:16:03]

lgtm, thanks Scott!

[Scott Hess - ex-Googler, 2016-10-19 18:28:54]

The CQ bit was checked by shess@chromium.org

[commit-bot: I haz the power, 2016-10-19 18:29:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 18:32:52]

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

[commit-bot: I haz the power, 2016-10-19 18:32:53]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Scott Hess - ex-Googler, 2016-10-19 19:43:53]

BUILD.gn merge

[Scott Hess - ex-Googler, 2016-10-19 19:45:18]

The CQ bit was checked by shess@chromium.org

[Scott Hess - ex-Googler, 2016-10-19 19:45:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
maxbogue@chromium.org, gangwu@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2424533002/#ps40001
(title: "BUILD.gn merge")

[commit-bot: I haz the power, 2016-10-19 19:46:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 22:47:49]

Description was changed from

==========
Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682
==========

to

==========
Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682
==========

[commit-bot: I haz the power, 2016-10-19 22:47:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-21 13:12:36]

Description was changed from

==========
Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682
==========

to

==========
Modify sync unit test to use standard sql::test:: helper.

Sync tests for DirectoryBackingStore overwrote/truncated the underlying
database file, which SQLite did not notice because of exclusive locking
and the page cache.  So then the tests accessed a lot of data to defeat
that.  This is brittle in the face of various changes to how
sql::Connection sets up the underlying sqlite3 handle.

Add sql::test::CorruptSizeInHeaderWithLock() to allow corrupting the
database while SQLite connections are outstanding.  Remove
set_exclusive_locking() for DirectoryBackingStore to allow
CorruptSizeInHeaderWithLock() to run.

Removing set_exclusive_locking() should not cause notable performance
degradation.  SQLite keeps a cookie in the header page to track database
changes, which allows it to re-used cache pages.  So there will be
uncontended locking (because nobody else locks the file), and the header
read should be from warm filesystem cache.

BUG=533682

Committed: https://crrev.com/4640cc232029f854e84455d2087ca071a3870ee6
Cr-Commit-Position: refs/heads/master@{#426316}
==========

[commit-bot: I haz the power, 2016-10-21 13:12:37]

Patchset 3 (id:??) landed as
https://crrev.com/4640cc232029f854e84455d2087ca071a3870ee6
Cr-Commit-Position: refs/heads/master@{#426316}