Index: components/history/core/browser/history_backend.cc |
diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc |
index 42732d85f1fe31ec625d0eedd10747b901a47a50..31871ee97e43db6d2ec0aa107d256f94d98eeaa5 100644 |
--- a/components/history/core/browser/history_backend.cc |
+++ b/components/history/core/browser/history_backend.cc |
@@ -68,12 +68,18 @@ using base::TimeTicks; |
namespace history { |
namespace { |
+ |
void RunUnlessCanceled( |
const base::Closure& closure, |
const base::CancelableTaskTracker::IsCanceledCallback& is_canceled) { |
if (!is_canceled.Run()) |
closure.Run(); |
} |
+ |
+void NullErrorHandler(int error, sql::Statement* stmt) { |
+ // Does nothing. Prevents DCHECKING on SQLITE errors on Debug builds. |
+} |
Scott Hess - ex-Googler
2016/07/10 05:16:52
This makes me suspect you have something going on
afakhry
2016/07/11 16:47:45
When I try to prevent reentrant error callbacks in
Scott Hess - ex-Googler
2016/07/13 01:18:46
I have written a number of tests which test induce
afakhry
2016/07/13 19:57:58
I think this is not needed now. What I used to do
|
+ |
} // namespace |
// How long we'll wait to do a commit, so that things are batched together. |
@@ -206,6 +212,8 @@ HistoryBackend::HistoryBackend( |
scoped_refptr<base::SequencedTaskRunner> task_runner) |
: delegate_(delegate), |
scheduled_kill_db_(false), |
+ error_callback_(base::Bind(&HistoryBackend::DatabaseErrorCallback, |
+ base::Unretained(this))), |
expirer_(this, backend_client.get(), task_runner), |
recent_redirects_(kMaxRedirectCount), |
backend_destroy_message_loop_(nullptr), |
@@ -648,16 +656,15 @@ void HistoryBackend::InitImpl( |
history_database_params.download_interrupt_reason_none, |
history_database_params.download_interrupt_reason_crash)); |
- // Unretained to avoid a ref loop with db_. |
- db_->set_error_callback(base::Bind(&HistoryBackend::DatabaseErrorCallback, |
- base::Unretained(this))); |
+ db_->set_error_callback(error_callback_); |
sql::InitStatus status = db_->Init(history_name); |
switch (status) { |
case sql::INIT_OK: |
break; |
case sql::INIT_TOO_NEW: |
- delegate_->NotifyProfileError(status); |
+ db_diagnostics_["Corrupted file"] = history_name.AsUTF8Unsafe(); |
+ delegate_->NotifyProfileError(status, db_diagnostics_); |
db_.reset(); |
return; |
case sql::INIT_FAILURE: { |
@@ -669,7 +676,8 @@ void HistoryBackend::InitImpl( |
if (kill_db) |
KillHistoryDatabase(); |
UMA_HISTOGRAM_BOOLEAN("History.AttemptedToFixProfileError", kill_db); |
- delegate_->NotifyProfileError(status); |
+ db_diagnostics_["Corrupted file"] = history_name.AsUTF8Unsafe(); |
+ delegate_->NotifyProfileError(status, db_diagnostics_); |
db_.reset(); |
return; |
} |
@@ -2410,6 +2418,12 @@ void HistoryBackend::URLsNoLongerBookmarked(const std::set<GURL>& urls) { |
void HistoryBackend::DatabaseErrorCallback(int error, sql::Statement* stmt) { |
if (!scheduled_kill_db_ && sql::IsErrorCatastrophic(error)) { |
scheduled_kill_db_ = true; |
+ |
+ // Gather diagnostic info, but prevent reentrant error callbacks. |
+ db_->set_error_callback(base::Bind(&NullErrorHandler)); |
+ db_diagnostics_ = db_->GetDiagnosticMap(); |
michaeln
2016/07/12 20:05:32
This function is similar to Connection::ReportDiag
Scott Hess - ex-Googler
2016/07/13 01:18:46
This is reasonable - note the comment in that code
afakhry
2016/07/13 19:57:58
Done.
|
+ db_->set_error_callback(error_callback_); |
+ |
// Don't just do the close/delete here, as we are being called by |db| and |
// that seems dangerous. |
// TODO(shess): Consider changing KillHistoryDatabase() to use |