Offer user to send feedback from profile error dialog

components/history/core/browser/history_backend.cc

 // Copyright (c) 2012 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/history/core/browser/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
 #include <memory>
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_enumerator.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/favicon_base/select_favicon_frames.h"
 #include "components/history/core/browser/download_constants.h"
 #include "components/history/core/browser/download_row.h"
 #include "components/history/core/browser/history_backend_client.h"
 #include "components/history/core/browser/history_backend_observer.h"
 #include "components/history/core/browser/history_constants.h"
 #include "components/history/core/browser/history_database.h"
@@ skipping 26 matching lines @@
       DownloadDatabase (stores a list of downloads)
       VisitDatabase (stores a list of visits for the URLs)
       VisitSegmentDatabase (stores groups of URLs for the most visited view).
 
     ExpireHistoryBackend (manages deleting things older than 3 months)
 */
 
 namespace history {
 
 namespace {
+
 void RunUnlessCanceled(
     const base::Closure& closure,
     const base::CancelableTaskTracker::IsCanceledCallback& is_canceled) {
   if (!is_canceled.Run())
     closure.Run();
 }
+
 }  // namespace
 
 // How long we'll wait to do a commit, so that things are batched together.
 const int kCommitIntervalSeconds = 10;
 
 // The amount of time before we re-fetch the favicon.
 const int kFaviconRefetchDays = 7;
 
 // The maximum number of items we'll allow in the redirect list before
 // deleting some.
@@ skipping 558 matching lines @@
 
   // History database.
   db_.reset(new HistoryDatabase(
       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_diagnostics_.clear();
   sql::InitStatus status = db_->Init(history_name);
   switch (status) {
     case sql::INIT_OK:
       break;
-    case sql::INIT_TOO_NEW:
-      delegate_->NotifyProfileError(status);
-      db_.reset();
-      return;
     case sql::INIT_FAILURE: {
       // A null db_ will cause all calls on this object to notice this error
       // and to not continue. If the error callback scheduled killing the
       // database, the task it posted has not executed yet. Try killing the
       // database now before we close it.
       bool kill_db = scheduled_kill_db_;
       if (kill_db)
         KillHistoryDatabase();
       UMA_HISTOGRAM_BOOLEAN("History.AttemptedToFixProfileError", kill_db);
-      delegate_->NotifyProfileError(status);
+    }  // Falls through.
+    case sql::INIT_TOO_NEW: {
+      const std::string corrupted_file_name =

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

You have similar code to this in chrome. Please extract into a common function
which provides documentation as to why it uses '/' and is not localized. I'm
thinking of something like:
std::string GenerateSqlErrorDiagnostics(error_code, file_path);

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

Yes, absolutely! Thanks.

However, The sql diagnostics should be collected on the DB thread from the error
callback when a catastrophic error occurs. Not always we have a profile error
due to a sql db error. For example, an error in preferences. So to make this
common code flexible enough, I suggest:

void AppendCorruptFileNameToDiagnostics(&diagnostics_, file_path);

Now the million dollars question: Where on earth should I put this function? :(
I tried to put it in components, so it's shareable by componenets and chrome.
But then it felt like an overkill to create a component that has only two files.
So I moved it to sql/error_delegate_util.h.

Done. Please let me know if there are better alternatives.

[sky, 2016/08/03 15:19:30]

How about sql/?
Also, style guide suggests out params be last.

[afakhry, 2016/08/03 17:51:23]

Yes, it's in sql/error_delegate_util.h.
Done for the params order.

+          history_name.DirName().BaseName().AsUTF8Unsafe() + "/" +
+          history_name.BaseName().AsUTF8Unsafe();
+      base::StringAppendF(&db_diagnostics_, "Corrupted file: %s\n",
+                          corrupted_file_name.c_str());
+      delegate_->NotifyProfileError(status, db_diagnostics_);
       db_.reset();
       return;
     }
     default:
       NOTREACHED();
   }
 
   // Fill the in-memory database and send it back to the history service on the
   // main thread.
   {
@@ skipping 1720 matching lines @@
     // that we can delete all associated icons in the case of deleting an
     // unvisited bookmarked URL.
     if (visits.empty())
       expirer_.DeleteURL(*i);  // There are no more visits; nuke the URL.
   }
 }
 
 void HistoryBackend::DatabaseErrorCallback(int error, sql::Statement* stmt) {
   if (!scheduled_kill_db_ && sql::IsErrorCatastrophic(error)) {
     scheduled_kill_db_ = true;
+
+    db_diagnostics_ = db_->GetDiagnosticInfo(error, stmt);
+
     // 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
     // RazeAndClose().  Then it can be cleared immediately.
     task_runner_->PostTask(
         FROM_HERE, base::Bind(&HistoryBackend::KillHistoryDatabase, this));
   }
 }
 
 void HistoryBackend::KillHistoryDatabase() {
@@ skipping 200 matching lines @@
   // transaction is currently open.
   db_->CommitTransaction();
   db_->Vacuum();
   db_->BeginTransaction();
   db_->GetStartDate(&first_recorded_time_);
 
   return true;
 }
 
 }  // namespace history