Offer user to send feedback from profile error dialog

components/webdata/common/web_database_backend.cc

Index: components/webdata/common/web_database_backend.cc
diff --git a/components/webdata/common/web_database_backend.cc b/components/webdata/common/web_database_backend.cc
index 7cb343e62f02e5a76ba03a4fff2f999a07a33491..01e51550a47eb63cd65d21e29c3b8a61cb036373 100644
--- a/components/webdata/common/web_database_backend.cc
+++ b/components/webdata/common/web_database_backend.cc
@@ -4,13 +4,16 @@
 
 #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;
@@ -35,7 +38,7 @@ void WebDatabaseBackend::AddTable(std::unique_ptr<WebDatabaseTable> table) {
 void WebDatabaseBackend::InitDatabase() {
   LoadDatabaseIfNecessary();
   if (delegate_) {
-    delegate_->DBLoaded(init_status_);
+    delegate_->DBLoaded(init_status_, db_diagnostics_);
   }
 }
 
@@ -46,14 +49,19 @@ sql::InitStatus WebDatabaseBackend::LoadDatabaseIfNecessary() {
   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_;
   }
@@ -113,6 +121,16 @@ 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_);