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);
 
+  // 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_;
   }
 
   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;

[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_);
   db_->CommitTransaction();
   db_->BeginTransaction();
 }