[cros] Member variables cleanup in SigninScreenHandler.

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

Index: chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
diff --git a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
index 32920b77286091e5d272b4d407fa9767942ba9db..34a41dfab7bcb9049b134bb836c7bf49f287a769 100644
--- a/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
+++ b/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
@@ -255,8 +255,8 @@ SigninScreenHandler::SigninScreenHandler()
     : delegate_(NULL),
       show_on_init_(false),
       oobe_ui_(false),
-      is_first_webui_ready_(false),
-      is_first_attempt_(true),
+      focus_is_stolen_(false),
+      gaia_is_preloaded_(false),
       dns_cleared_(false),
       dns_clear_task_running_(false),
       cookies_cleared_(false),
@@ -552,12 +552,12 @@ void SigninScreenHandler::ShowSigninScreenIfReady() {
   if (!dns_cleared_ || !cookies_cleared_)
     return;
 
-  LoadAuthExtension(!is_first_attempt_, false, false);
+  LoadAuthExtension(!gaia_is_preloaded_, false, false);
   ShowScreen(kGaiaSigninScreen, NULL);
 
-  if (is_first_attempt_) {
-    is_first_attempt_ = false;
-    if (is_first_webui_ready_)
+  if (gaia_is_preloaded_) {
+    gaia_is_preloaded_ = false;

[Daniel Erat, 2012/03/06 15:23:47]

could you add a comment about this?  it seems a bit strange to me that the
preloaded variable goes from true to false here.

[altimofeev, 2012/03/06 17:39:22]

Done.

+    if (focus_is_stolen_)
       HandleLoginWebuiReady(NULL);
   }
 }
@@ -725,9 +725,9 @@ void SigninScreenHandler::HandleShowAddUser(const base::ListValue* args) {
   if (args)
     args->GetString(0, &email_);
   LOG(ERROR) << "HandleShowAddUser: email=" << email_ << ", first_attempt="

[Nikita (slow), 2012/03/06 14:36:02]

nit: update comment

[altimofeev, 2012/03/06 17:39:22]

Done.

-             << is_first_attempt_;
+             << gaia_is_preloaded_;
 
-  if (is_first_attempt_ && email_.empty()) {
+  if (gaia_is_preloaded_ && email_.empty()) {
     dns_cleared_ = true;
     cookies_cleared_ = true;
     ShowSigninScreenIfReady();
@@ -827,10 +827,12 @@ void SigninScreenHandler::HandleAccountPickerReady(
   // loaded because it can affect the loading speed.
   // Do not load the extension for the screen locker, see crosbug.com/25018.
   if (!ScreenLocker::default_screen_locker() &&
-      is_first_attempt_ &&
+      !gaia_is_preloaded_ &&
       !cookie_remover_ &&
-      !dns_clear_task_running_)
+      !dns_clear_task_running_) {
+    gaia_is_preloaded_ = true;

[Nikita (slow), 2012/03/06 14:36:02]

GAIA is not preloaded at this point. So this variable means that GAIA will be
preloaded or already is preloaded.

[altimofeev, 2012/03/06 17:39:22]

Done.

     LoadAuthExtension(true, true, false);

[Nikita (slow), 2012/03/06 14:36:02]

Should be LoadAuthExtension(gaia_is_preloaded_, true, false);
This way it is more clear why are you setting gaia_is_preloaded_ in this block.

[altimofeev, 2012/03/06 17:39:22]

it couldn't be substituted everywhere, so from my point of view it will only
complicate the situation

+  }
 
   if (ScreenLocker::default_screen_locker()) {
     content::NotificationService::current()->Notify(
@@ -841,29 +843,28 @@ void SigninScreenHandler::HandleAccountPickerReady(
 }
 
 void SigninScreenHandler::HandleLoginWebuiReady(const base::ListValue* args) {
-  LOG(ERROR) << "HandleLoginWebuiReady: first_webui_ready="
-             << is_first_webui_ready_
-             << ", first_attempt=" << is_first_attempt_;
+  DLOG(INFO) << "HandleLoginWebuiReady: focus_is_stolen=" << focus_is_stolen_
+             << ", gaia_is_preloaded=" << gaia_is_preloaded_;
 
-  if (is_first_webui_ready_) {
+  if (focus_is_stolen_) {
     // Set focus to the Gaia page.
     // TODO(altimofeev): temporary solution, until focus parameters are
     // implemented on the Gaia side.
     // Do this only once. Any subsequent call would relod GAIA frame.
-    is_first_webui_ready_ = false;
+    focus_is_stolen_ = false;
     const char code[] = "gWindowOnLoad();";
     RenderViewHost* rvh = web_ui()->GetWebContents()->GetRenderViewHost();
     rvh->ExecuteJavascriptInWebFrame(
         ASCIIToUTF16("//iframe[@id='signin-frame']\n//iframe"),
         ASCIIToUTF16(code));
   }
-  if (!is_first_attempt_) {
+  if (!gaia_is_preloaded_) {

[Daniel Erat, 2012/03/06 15:23:47]

is_first_attempt_ was initialized to true, but gaia_is_preloaded_ is initialized
to false.  does this still work because it's now getting set to true in
HandleAccountPickerReady()?

[altimofeev, 2012/03/06 17:39:22]

Yes, it is correct now. We should only grab the focus when silent gaia page
loading has been done.

common flow:

- login screen loading started
- AccountPickerReady
- silent load of Gaia page
- WebuiReady fired by silently loaded Gaia page
- user clicks on 'Add new user'
- Gaia page is shown (silently loaded one)
- user clicks 'Cancel', account picker is shown
- user clicks on 'Add new user'
- Cookies, etc. are cleared, Gaia page is reloaded

     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_LOGIN_WEBUI_READY,
         content::NotificationService::AllSources(),
         content::NotificationService::NoDetails());
   } else {
-    is_first_webui_ready_ = true;
+    focus_is_stolen_ = true;
     // Prevent focus stealing by the Gaia page.
     // TODO(altimofeev): temporary solution, until focus parameters are
     // implemented on the Gaia side.