Chrome OS: Delay showing sync settings screen.

This Cl delays showing sync settings screen to prevent race with restored tabs.

Bug: 900697
Change-Id: Ia23c867974c5f7b80fedc73d71c29968e354c168
Reviewed-on: https://chromium-review.googlesource.com/c/1325210Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606630}

chrome/browser/chromeos/login/screens/sync_consent_screen.cc

@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/consent_auditor/consent_auditor_factory.h"
@@ -27,6 +28,11 @@ constexpr const char kUserActionContinueWithSyncOnly[] =
 constexpr const char kUserActionContinueWithSyncAndPersonalization[] =
     "continue-with-sync-and-personalization";
 
+// Delay showing chrome sync settings by this amount of time to make them
+// show on top of the restored tabs and windows.
+constexpr base::TimeDelta kSyncConsentSettingsShowDelay =
+    base::TimeDelta::FromSeconds(3);
+
 browser_sync::ProfileSyncService* GetSyncService(Profile* profile) {
   if (ProfileSyncServiceFactory::HasProfileSyncService(profile))
     return ProfileSyncServiceFactory::GetForProfile(profile);
@@ -36,10 +42,21 @@ browser_sync::ProfileSyncService* GetSyncService(Profile* profile) {
 }  // namespace
 
 // static
-void SyncConsentScreen::MaybeLaunchSyncConstentSettings(Profile* profile) {
+void SyncConsentScreen::MaybeLaunchSyncConsentSettings(Profile* profile) {
   if (profile->GetPrefs()->GetBoolean(prefs::kShowSyncSettingsOnSessionStart)) {
-    profile->GetPrefs()->ClearPref(prefs::kShowSyncSettingsOnSessionStart);
-    chrome::ShowSettingsSubPageForProfile(profile, "syncSetup");
+    // TODO (alemate): In a very special case when chrome is exiting at the very
+    // moment we show Settings, it might crash here because profile could be
+    // already destroyed. This needs to be fixed.
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(
+            [](Profile* profile) {
+              profile->GetPrefs()->ClearPref(
+                  prefs::kShowSyncSettingsOnSessionStart);
+              chrome::ShowSettingsSubPageForProfile(profile, "syncSetup");
+            },
+            base::Unretained(profile)),
+        kSyncConsentSettingsShowDelay);
   }
 }
 

chrome/browser/chromeos/login/screens/sync_consent_screen.h

@@ -55,7 +55,7 @@ class SyncConsentScreen : public BaseScreen,
 
   // Launches the sync consent settings dialog if the user requested to review
   // them after completing OOBE.
-  static void MaybeLaunchSyncConstentSettings(Profile* profile);
+  static void MaybeLaunchSyncConsentSettings(Profile* profile);
 
   SyncConsentScreen(BaseScreenDelegate* base_screen_delegate,
                     SyncConsentScreenView* view);

chrome/browser/chromeos/login/session/chrome_session_manager.cc

@@ -195,7 +195,7 @@ void StartUserSession(Profile* user_profile, const std::string& login_user_id) {
 
   UserSessionManager::GetInstance()->CheckEolStatus(user_profile);
   tpm_firmware_update::ShowNotificationIfNeeded(user_profile);
-  SyncConsentScreen::MaybeLaunchSyncConstentSettings(user_profile);
+  SyncConsentScreen::MaybeLaunchSyncConsentSettings(user_profile);
 }
 
 }  // namespace

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -2074,7 +2074,7 @@ void UserSessionManager::DoBrowserLaunchInternal(Profile* profile,
   tpm_firmware_update::ShowNotificationIfNeeded(profile);
 
   if (should_launch_browser_)
-    SyncConsentScreen::MaybeLaunchSyncConstentSettings(profile);
+    SyncConsentScreen::MaybeLaunchSyncConsentSettings(profile);
 }
 
 void UserSessionManager::RespectLocalePreferenceWrapper(