Offer user to send feedback from profile error dialog

components/webdata/common/web_database_backend.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/webdata/common/web_database_backend.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/strings/stringprintf.h"
 #include "components/webdata/common/web_data_request_manager.h"
 #include "components/webdata/common/web_database.h"
 #include "components/webdata/common/web_database_table.h"
+#include "sql/error_delegate_util.h"
 
 using base::Bind;
 using base::FilePath;
 
 WebDatabaseBackend::WebDatabaseBackend(
     const FilePath& path,
     Delegate* delegate,
     const scoped_refptr<base::SingleThreadTaskRunner>& db_thread)
     : base::RefCountedDeleteOnMessageLoop<WebDatabaseBackend>(db_thread),
       db_path_(path),
       request_manager_(new WebDataRequestManager()),
       init_status_(sql::INIT_FAILURE),
       init_complete_(false),
       delegate_(delegate) {
 }
 
 void WebDatabaseBackend::AddTable(std::unique_ptr<WebDatabaseTable> table) {
   DCHECK(!db_.get());
   tables_.push_back(table.release());
 }
 
 void WebDatabaseBackend::InitDatabase() {
   LoadDatabaseIfNecessary();
   if (delegate_) {
-    delegate_->DBLoaded(init_status_);
+    delegate_->DBLoaded(init_status_, db_diagnostics_);
   }
 }
 
 sql::InitStatus WebDatabaseBackend::LoadDatabaseIfNecessary() {
   if (init_complete_ || db_path_.empty()) {
     return init_status_;
   }
   init_complete_ = true;
   db_.reset(new WebDatabase());
 
-  for (ScopedVector<WebDatabaseTable>::iterator it = tables_.begin();
-       it != tables_.end(); ++it) {
-    db_->AddTable(*it);
-  }
+  for (const auto& table : tables_)
+    db_->AddTable(table);
 
+  db_->set_error_callback(
+      base::Bind(&WebDatabaseBackend::DatabaseErrorCallback, this));
   init_status_ = db_->Init(db_path_);
   if (init_status_ != sql::INIT_OK) {
     LOG(ERROR) << "Cannot initialize the web database: " << init_status_;
+    const std::string corrupted_file_name =
+        db_path_.DirName().BaseName().AsUTF8Unsafe() + "/" +
+        db_path_.BaseName().AsUTF8Unsafe();
+    base::StringAppendF(&db_diagnostics_, "Corrupted file: %s\n",

[michaeln, 2016/07/15 23:30:57]

Ah, i guess the filename being appended here instead of in the error handler to
cover the case where Init fails w/o the error handler having been called?

[afakhry, 2016/07/18 17:43:51]

Yes, that's one of the reasons, plus we the error handler might be called
multiple times with various errors, some of them may not be serious. So we wait
until we know for sure that the Init failed.

+                        corrupted_file_name.c_str());
     db_.reset(NULL);
     return init_status_;
   }
 
   db_->BeginTransaction();
   return init_status_;
 }
 
 void WebDatabaseBackend::ShutdownDatabase() {
   if (db_ && init_status_ == sql::INIT_OK)
@@ skipping 39 matching lines @@
   if (db_ && init_status_ == sql::INIT_OK) {
     return task.Run(db_.get());
   }
   return nullptr;
 }
 
 WebDatabaseBackend::~WebDatabaseBackend() {
   ShutdownDatabase();
 }
 
+void WebDatabaseBackend::DatabaseErrorCallback(int error,
+                                               sql::Statement* stmt) {
+  // We ignore any further error callbacks on the first catastrophic error.
+  static bool should_ignore_errors = false;
+  if (!should_ignore_errors && sql::IsErrorCatastrophic(error)) {
+    should_ignore_errors = true;
+    db_diagnostics_ = db_->GetDiagnosticInfo(error, stmt);

[afakhry, 2016/07/13 19:57:59]

Question to (shess | michaeln): Should we RazeAndClose() here in the event of a
catastrophic error? 

I noticed that any corruption in the Web databases always remain there, and the
profile error page is shown on every login!
Unlike the history DB in which we Raze it so it gets recreated.

[michaeln, 2016/07/15 23:30:57]

maybe? if so lets leave that for a seperate cl

what does chrome do when this it get the profile is busted error? if it deletes
the whole directory, there would be no need to do anything other than report the
error

[afakhry, 2016/07/18 17:43:51]

Right now for the Web databases, we do nothing (unlike what's being done for the
history DB). If a Web database file gets corrupted, it'll stay corrupted forever
and the user would get the profile error page every time on log in. This is
annoying. I think we should do something similar to that we're doing in the
history database:
https://cs.chromium.org/chromium/src/components/history/core/browser/history_....

[michaeln, 2016/07/18 20:05:49]

agreed that the system should dig out of the error condition and continue, but
i'd suggest a separate cl for that work

[afakhry, 2016/07/18 20:21:28]

Sure, thanks! I'll do it as a follow-up CL.

+  }
+}
+
 void WebDatabaseBackend::Commit() {
   DCHECK(db_);
   DCHECK_EQ(sql::INIT_OK, init_status_);
   db_->CommitTransaction();
   db_->BeginTransaction();
 }