Send early ShowProfileErrorDialog() to ChromeBrowserMainParts::DeferringTaskRunner

The fix to crbug.com/826701 resulted in crbug.com/827931 highlighting
that calling ShowProfileErrorDialog that early (before ResourceBundle
is initialized) is just not possible.

As discussed @ https://chromium-review.googlesource.com/c/chromium/src/+/985110/3/chrome/browser/prefs/chrome_pref_service_factory.cc#332
it is preferable to defer this message to BrowserThread::UI for when
it is brought up.

Turns out ChromeBrowserMainParts::DeferringTaskRunner already puts an
early ThreadTaskRunnerHandle in place for this very purpose :)

R=sky@chromium.org

Bug: 827931
Change-Id: Icf409103e51cf9957ac60b3ffa4b8310a5b36dbb
Reviewed-on: https://chromium-review.googlesource.com/996215Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548509}

chrome/browser/chrome_browser_main.cc

@@ -715,7 +715,7 @@ const char kMissingLocaleDataMessage[] =
 #if !defined(OS_ANDROID)
 // A TaskRunner that defers tasks until the real task runner is up and running.
 // This is used during early initialization, before the real task runner has
-// been created. DeferredTaskRunner has the following states.
+// been created. DeferringTaskRunner has the following states.
 //
 // . kInstalled: the initial state. Tasks are added to |deferred_runner_|. In
 //   this state this is installed as the active ThreadTaskRunnerHandle.

chrome/browser/prefs/chrome_pref_service_factory.cc

@@ -17,6 +17,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
@@ -301,6 +302,12 @@ GetTrackingConfiguration() {
 // Shows notifications which correspond to PersistentPrefStore's reading errors.
 void HandleReadError(const base::FilePath& pref_filename,
                      PersistentPrefStore::PrefReadError error) {
+  // The error callback is always invoked back on the main thread (which is
+  // BrowserThread::UI unless called during early initialization before the main
+  // thread is promoted to BrowserThread::UI).
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
+         !BrowserThread::IsThreadInitialized(BrowserThread::UI));
+
   // Sample the histogram also for the successful case in order to get a
   // baseline on the success rate in addition to the error distribution.
   UMA_HISTOGRAM_ENUMERATION("PrefService.ReadError", error,
@@ -319,18 +326,14 @@ void HandleReadError(const base::FilePath& pref_filename,
     }
 
     if (message_id) {
-      auto show_profile_error_dialog = base::BindOnce(
-          &ShowProfileErrorDialog, ProfileErrorType::PREFERENCES, message_id,
-          sql::GetCorruptFileDiagnosticsInfo(pref_filename));
-      if (BrowserThread::IsThreadInitialized(BrowserThread::UI)) {
-        BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                                std::move(show_profile_error_dialog));
-      } else {
-        // In early startup (e.g. on error loading Local State before most
-        // browser pieces are up), the only option is to show the dialog
-        // synchronously.
-        std::move(show_profile_error_dialog).Run();
-      }
+      // Note: ThreadTaskRunnerHandle() is usually BrowserThread::UI but during
+      // early startup it can be ChromeBrowserMainParts::DeferringTaskRunner
+      // which will forward to BrowserThread::UI when it's initialized.
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE,
+          base::BindOnce(&ShowProfileErrorDialog, ProfileErrorType::PREFERENCES,
+                         message_id,
+                         sql::GetCorruptFileDiagnosticsInfo(pref_filename)));
     }
 #else
     // On ChromeOS error screen with message about broken local state
@@ -397,7 +400,8 @@ void PrepareFactory(sync_preferences::PrefServiceSyncableFactory* factory,
   factory->set_command_line_prefs(
       base::MakeRefCounted<ChromeCommandLinePrefStore>(
           base::CommandLine::ForCurrentProcess()));
-  factory->set_read_error_callback(base::Bind(&HandleReadError, pref_filename));
+  factory->set_read_error_callback(
+      base::BindRepeating(&HandleReadError, pref_filename));
   factory->set_user_prefs(std::move(user_pref_store));
   factory->SetPrefModelAssociatorClient(
       ChromePrefModelAssociatorClient::GetInstance());

components/prefs/pref_service.h

@@ -358,7 +358,8 @@ class COMPONENTS_PREFS_EXPORT PrefService {
   // Pref Stores and profile that we passed to the PrefValueStore.
   const scoped_refptr<PersistentPrefStore> user_pref_store_;
 
-  // Callback to call when a read error occurs.
+  // Callback to call when a read error occurs. Always invoked on the sequence
+  // this PrefService was created own.
   const base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
       read_error_callback_;
 

components/prefs/pref_service_factory.h

@@ -52,8 +52,9 @@ class COMPONENTS_PREFS_EXPORT PrefServiceFactory {
     recommended_prefs_.swap(prefs);
   }
 
-  // Sets up error callback for the PrefService.  A do-nothing default
-  // is provided if this is not called.
+  // Sets up error callback for the PrefService.  A do-nothing default is
+  // provided if this is not called. This callback is always invoked (async or
+  // not) on the sequence on which Create is invoked.
   void set_read_error_callback(
       base::RepeatingCallback<void(PersistentPrefStore::PrefReadError)>
           read_error_callback) {