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..501e5b8270d00b4aa775f6c66ef428b4727895a4 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,21 @@ 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);
 
+  // Unretained to avoid a ref loop with db_.

[sky, 2016/08/02 13:27:21]

The interesting part to document is why unretained is safe.

[afakhry, 2016/08/02 23:58:24]

Done.

+  db_->set_error_callback(base::Bind(&WebDatabaseBackend::DatabaseErrorCallback,
+                                     base::Unretained(this)));
+  db_diagnostics_.clear();
   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 =

[sky, 2016/08/02 13:27:21]

Same comment about using shared function here.

[afakhry, 2016/08/02 23:58:24]

Done.

+        db_path_.DirName().BaseName().AsUTF8Unsafe() + "/" +
+        db_path_.BaseName().AsUTF8Unsafe();
+    base::StringAppendF(&db_diagnostics_, "Corrupted file: %s\n",
+                        corrupted_file_name.c_str());
     db_.reset(NULL);
     return init_status_;
   }
@@ -113,6 +123,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;

[sky, 2016/08/02 13:27:21]

Why is this static and not a member variable? I would expect to ignore it per
profile, which I believe means per WebDBBackend.

[afakhry, 2016/08/02 23:58:24]

Hmm, you're right. Thanks! Done.

+  if (!should_ignore_errors && sql::IsErrorCatastrophic(error)) {
+    should_ignore_errors = true;
+    db_diagnostics_ = db_->GetDiagnosticInfo(error, stmt);
+  }
+}
+
 void WebDatabaseBackend::Commit() {
   DCHECK(db_);
   DCHECK_EQ(sql::INIT_OK, init_status_);