[sync] Prevent flash of error icon while loading syncSetup page

chrome/browser/ui/webui/settings/people_handler.cc

Index: chrome/browser/ui/webui/settings/people_handler.cc
diff --git a/chrome/browser/ui/webui/settings/people_handler.cc b/chrome/browser/ui/webui/settings/people_handler.cc
index b0a1c045b204ff1a7a11dd4e6dcfe7c810b26146..7b0ed966bbe3d49a17adc25b2fb20f44ba6fb86e 100644
--- a/chrome/browser/ui/webui/settings/people_handler.cc
+++ b/chrome/browser/ui/webui/settings/people_handler.cc
@@ -29,12 +29,14 @@
 #include "chrome/browser/sync/sync_ui_util.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/user_manager.h"
 #include "chrome/browser/ui/webui/profile_helper.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
@@ -196,6 +198,7 @@ PeopleHandler::~PeopleHandler() {
 }
 
 void PeopleHandler::RegisterMessages() {
+  MaybeAcquireSyncBlocker();
   web_ui()->RegisterMessageCallback(
       "SyncSetupDidClosePage",
       base::Bind(&PeopleHandler::OnDidClosePage, base::Unretained(this)));
@@ -625,7 +628,7 @@ void PeopleHandler::OpenSyncSetup() {
   GetLoginUIService()->SetLoginUI(this);
 
   ProfileSyncService* service = GetSyncService();
-  if (service)
+  if (service && !sync_blocker_)
     sync_blocker_ = service->GetSetupInProgressHandle();
 
   // There are several different UI flows that can bring the user here:
@@ -686,6 +689,25 @@ void PeopleHandler::OpenSyncSetup() {
   PushSyncPrefs();
 }
 
+// If we're directly loading the sync setup page, we acquire a
+// SetupInProgressHandle early in order to prevent a lapse in
+// ProfileSyncService's "SetupInProgress" status. This lapse previously occured
+// between when the sync confirmation dialog was closed and when the sync setup
+// page hadn't yet fired the SyncSetupShowSetupUI event.
+void PeopleHandler::MaybeAcquireSyncBlocker() {

[tommycli, 2017/07/17 16:44:22]

I've been trained to not use MaybeFoo() generally, since then the reader
naturally asks "Maybe? Why maybe? under what conditions?"

In this case, maybe you can rename to: InitailizeSyncBlocker -- if the caller
truly does not need to know the details within...

[Patrick Noland, 2017/07/17 20:39:11]

Done.

+  if (!web_ui())
+    return;
+  WebContents* web_contents = web_ui()->GetWebContents();
+  if (web_contents) {
+    ProfileSyncService* service = GetSyncService();
+    const GURL current_url = web_contents->GetVisibleURL();
+    if (service &&
+        current_url == chrome::GetSettingsUrl(chrome::kSyncSetupSubPage)) {
+      sync_blocker_ = service->GetSetupInProgressHandle();
+    }
+  }
+}
+
 void PeopleHandler::FocusUI() {
   WebContents* web_contents = web_ui()->GetWebContents();
   web_contents->GetDelegate()->ActivateContents(web_contents);