[Signin Error Dialog] (2/3) Added handlers and UI constructors

chrome/browser/ui/webui/signin/sync_confirmation_handler.cc

Index: chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
diff --git a/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc b/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
index b538951d47f26e5dc6aeec41d7fdde8ebaf10077..fc808b18db9cb3565c881a9488721cddcf3937b5 100644
--- a/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
+++ b/chrome/browser/ui/webui/signin/sync_confirmation_handler.cc
@@ -10,10 +10,10 @@
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
-#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/signin_view_controller_delegate.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
+#include "chrome/browser/ui/webui/signin/signin_utils.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
@@ -64,13 +64,15 @@ void SyncConfirmationHandler::HandleGoToSettings(const base::ListValue* args) {
 void SyncConfirmationHandler::HandleUndo(const base::ListValue* args) {
   did_user_explicitly_interact = true;
   content::RecordAction(base::UserMetricsAction("Signin_Undo_Signin"));
-  Browser* browser = GetDesktopBrowser();
-  LoginUIServiceFactory::GetForProfile(browser->profile())->
-      SyncConfirmationUIClosed(LoginUIService::ABORT_SIGNIN);
-  SigninManagerFactory::GetForProfile(Profile::FromWebUI(web_ui()))->SignOut(
-      signin_metrics::ABORT_SIGNIN,
-      signin_metrics::SignoutDelete::IGNORE_METRIC);
-  browser->CloseModalSigninWindow();
+  Browser* browser = signin::GetDesktopBrowser(web_ui());
+  if (browser) {
+    LoginUIServiceFactory::GetForProfile(browser->profile())->
+        SyncConfirmationUIClosed(LoginUIService::ABORT_SIGNIN);
+    SigninManagerFactory::GetForProfile(Profile::FromWebUI(web_ui()))->SignOut(
+        signin_metrics::ABORT_SIGNIN,
+        signin_metrics::SignoutDelete::IGNORE_METRIC);
+    browser->CloseModalSigninWindow();
+  }
 }
 
 void SyncConfirmationHandler::SetUserImageURL(const std::string& picture_url) {
@@ -92,51 +94,41 @@ void SyncConfirmationHandler::OnAccountUpdated(const AccountInfo& info) {
   SetUserImageURL(info.picture_url);
 }
 
-Browser* SyncConfirmationHandler::GetDesktopBrowser() {
-  Browser* browser = chrome::FindBrowserWithWebContents(
-      web_ui()->GetWebContents());
-  if (!browser)
-    browser = chrome::FindLastActiveWithProfile(Profile::FromWebUI(web_ui()));
-  DCHECK(browser);
-  return browser;
-}
-
 void SyncConfirmationHandler::CloseModalSigninWindow(
     LoginUIService::SyncConfirmationUIClosedResult result) {
-  Browser* browser = GetDesktopBrowser();
-  LoginUIServiceFactory::GetForProfile(browser->profile())->
-      SyncConfirmationUIClosed(result);
-  browser->CloseModalSigninWindow();
+  Browser* browser = signin::GetDesktopBrowser(web_ui());
+  if (browser) {
+    LoginUIServiceFactory::GetForProfile(browser->profile())->
+        SyncConfirmationUIClosed(result);
+    browser->CloseModalSigninWindow();
+  }
 }
 
 void SyncConfirmationHandler::HandleInitializedWithSize(
     const base::ListValue* args) {
-  Browser* browser = GetDesktopBrowser();
-  Profile* profile = browser->profile();
-  std::vector<AccountInfo> accounts =
-      AccountTrackerServiceFactory::GetForProfile(profile)->GetAccounts();
-
-  if (accounts.empty())
-    return;
-
-  AccountInfo primary_account_info = accounts[0];
-
-  if (!primary_account_info.IsValid())
-    AccountTrackerServiceFactory::GetForProfile(profile)->AddObserver(this);
-  else
-    SetUserImageURL(primary_account_info.picture_url);
-
-  double height;
-  bool success = args->GetDouble(0, &height);
-  DCHECK(success);
-
-  browser->signin_view_controller()->SetModalSigninHeight(
-      static_cast<int>(height));
-
-  // After the dialog is shown, some platforms might have an element focused.
-  // To be consistent, clear the focused element on all platforms.
-  // TODO(anthonyvd): Figure out why this is needed on Mac and not other
-  // platforms and if there's a way to start unfocused while avoiding this
-  // workaround.
-  web_ui()->CallJavascriptFunctionUnsafe("sync.confirmation.clearFocus");
+  Browser* browser = signin::GetDesktopBrowser(web_ui());
+  if (browser) {

[achuithb, 2016/09/13 17:40:53]

I think early return is preferable here at least (and possibly in some of the
other cases too?)

[anthonyvd, 2016/09/13 18:55:59]

You're right about this one, so done. I left the other ones as-is since the
small blocks made the early return ever-so-slightly less readable.

+    Profile* profile = browser->profile();
+    std::vector<AccountInfo> accounts =
+        AccountTrackerServiceFactory::GetForProfile(profile)->GetAccounts();
+
+    if (accounts.empty())
+      return;
+
+    AccountInfo primary_account_info = accounts[0];
+
+    if (!primary_account_info.IsValid())
+      AccountTrackerServiceFactory::GetForProfile(profile)->AddObserver(this);
+    else
+      SetUserImageURL(primary_account_info.picture_url);
+
+    signin::SetInitializedModalHeight(web_ui(), args);
+
+    // After the dialog is shown, some platforms might have an element focused.
+    // To be consistent, clear the focused element on all platforms.
+    // TODO(anthonyvd): Figure out why this is needed on Mac and not other
+    // platforms and if there's a way to start unfocused while avoiding this
+    // workaround.
+    web_ui()->CallJavascriptFunctionUnsafe("sync.confirmation.clearFocus");
+  }
 }