Delete and recreate the sync database if it is corrupt.

Delete and recreate the sync database if it is corrupt.

BUG=40020
TEST=DirectoryBackingStoreTest.Corruption

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=45712

[albertb, 2010-04-21 22:37:13]

Lei/Tim,

I added some tests to Lei's patch (http://codereview.chromium.org/1711006). I
don't really like the EXPECT_DEATH in debug, but there doesn't seem to be any
elegant way to check whether a sqlite database is valid using SQLStatement
(because it is full of DCHECKs).

The statement that fails and triggers a crash in debug mode is "PRAGMA
synchronous=1". I could prepare it by hand (instead of using SQLStatement) and
check the error code, but I'm not sure it's worth it.

Albert

[Lei Zhang, 2010-04-21 22:45:23]

LGTM

[albertb, 2010-04-22 19:42:27]

Lei/Tim,

It looks like the patch was flaky on XP because the DBS still held an handle to
the sqlite db when we tried to delete the file. I ran the unit tests on my own
Windows box in Release mode and all tests pass.

Thanks!
Albert

[albertb, 2010-04-26 17:38:53]

Ping?

[tim (not reviewing), 2010-04-26 18:06:58]

http://codereview.chromium.org/1733008/diff/11001/12001
File chrome/browser/sync/syncable/directory_backing_store.cc (right):

http://codereview.chromium.org/1733008/diff/11001/12001#newcode188
chrome/browser/sync/syncable/directory_backing_store.cc:188:
sqlite3_close(*handle);
can we use sqlite_utils::scoped_sqlite_db_ptr in sqlite_utils.h?

[albertb, 2010-04-26 21:30:29]

http://codereview.chromium.org/1733008/diff/11001/12001
File chrome/browser/sync/syncable/directory_backing_store.cc (right):

http://codereview.chromium.org/1733008/diff/11001/12001#newcode188
chrome/browser/sync/syncable/directory_backing_store.cc:188:
sqlite3_close(*handle);
On 2010/04/26 18:06:58, timsteele wrote:
> can we use sqlite_utils::scoped_sqlite_db_ptr in sqlite_utils.h?

Done.

[tim (not reviewing), 2010-04-27 14:25:03]

LGTM w/ one question

http://codereview.chromium.org/1733008/diff/17001/18001
File chrome/browser/sync/syncable/directory_backing_store.cc (right):

http://codereview.chromium.org/1733008/diff/17001/18001#newcode211
chrome/browser/sync/syncable/directory_backing_store.cc:211:
scoped_handle.release();
this is necessary?

[albertb, 2010-04-27 15:05:41]

http://codereview.chromium.org/1733008/diff/17001/18001
File chrome/browser/sync/syncable/directory_backing_store.cc (right):

http://codereview.chromium.org/1733008/diff/17001/18001#newcode211
chrome/browser/sync/syncable/directory_backing_store.cc:211:
scoped_handle.release();
On 2010/04/27 14:25:03, timsteele wrote:
> this is necessary?

Yes. If the scoped ptr holds a valid db handle, it calls sqlite3_close when it
goes out of scope.