Enable SQLite's secure-delete at runtime so that

Enable SQLite's secure-delete at runtime so that
we don't need to rely on system SQLite being compiled
with specific flags.

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

[Paweł Hajdan Jr., 2011-08-09 18:42:11]

By the way, there seems to be a lot of duplication in
chrome/common/sqlite_utils. Would it be OK try transition existing sqlite_utils
users to sql/ ?

[Scott Hess - ex-Googler, 2011-08-09 20:33:37]

http://codereview.chromium.org/7552022/diff/1/sql/connection.cc
File sql/connection.cc (right):

http://codereview.chromium.org/7552022/diff/1/sql/connection.cc#newcode380
sql/connection.cc:380: NOTREACHED() << "Could not enable secure_delete: " <<
GetErrorMessage();
NOTREACHED() means you'll never get to the Close() and false return.

Also, is it possible/likely that Chromium will be linked with versions of SQLite
which do not support this pragma?

[Paweł Hajdan Jr., 2011-08-09 23:05:00]

http://codereview.chromium.org/7552022/diff/1/sql/connection.cc
File sql/connection.cc (right):

http://codereview.chromium.org/7552022/diff/1/sql/connection.cc#newcode380
sql/connection.cc:380: NOTREACHED() << "Could not enable secure_delete: " <<
GetErrorMessage();
On 2011/08/09 20:33:37, shess wrote:
> NOTREACHED() means you'll never get to the Close() and false return.

I used NOTREACHED() because code above uses it. However, NOTREACHED() will not
fire in Release mode, and in this case I believe it's really important to ensure
we succeed with running this pragma.

> Also, is it possible/likely that Chromium will be linked with versions of
SQLite
> which do not support this pragma?

Possible, but not likely. If some distro breaks it it's their fault. And distros
do their testing too. It's only important to make it fail loudly, i.e. fail the
connection instead of just logging somewhere where possibly nobody is looking.

[Scott Hess - ex-Googler, 2011-08-09 23:13:43]

lgtm

http://codereview.chromium.org/7552022/diff/1/sql/connection.cc
File sql/connection.cc (right):

http://codereview.chromium.org/7552022/diff/1/sql/connection.cc#newcode380
sql/connection.cc:380: NOTREACHED() << "Could not enable secure_delete: " <<
GetErrorMessage();
On 2011/08/09 23:05:00, Paweł Hajdan Jr. wrote:
> On 2011/08/09 20:33:37, shess wrote:
> > NOTREACHED() means you'll never get to the Close() and false return.
> 
> I used NOTREACHED() because code above uses it. However, NOTREACHED() will not
> fire in Release mode, and in this case I believe it's really important to
ensure
> we succeed with running this pragma.

Doh!  You're right, for some reason I was thinking it did a CHECK() not a
DCHECK().

> > Also, is it possible/likely that Chromium will be linked with versions of
SQLite
> > which do not support this pragma?
> 
> Possible, but not likely. If some distro breaks it it's their fault. And
distros
> do their testing too. It's only important to make it fail loudly, i.e. fail
the
> connection instead of just logging somewhere where possibly nobody is looking.

OK.  As long as the case was realized.