[cros] Member variables cleanup in SigninScreenHandler.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/hash_tables.h"
@@ skipping 237 matching lines @@
     SendState(*it, reason);
   }
 }
 
 // SigninScreenHandler implementation ------------------------------------------
 
 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),
       cookie_remover_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       key_event_listener_(NULL) {
   CrosSettings::Get()->AddSettingsObserver(kAccountsPrefAllowNewUser, this);
   CrosSettings::Get()->AddSettingsObserver(kAccountsPrefAllowGuest, this);
 }
 
@@ skipping 275 matching lines @@
   ShowSigninScreenIfReady();
 }
 
 void SigninScreenHandler::ShowSigninScreenIfReady() {
   LOG(ERROR) << "ShowSigninScreenIfReady: dns=" << dns_cleared_
              << ", cookie=" << cookies_cleared_;
 
   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);
   }
 }
 
 void SigninScreenHandler::LoadAuthExtension(
     bool force, bool silent_load, bool offline) {
   DictionaryValue params;
 
   params.SetBoolean("forceReload", force);
   params.SetBoolean("silentLoad", silent_load);
@@ skipping 146 matching lines @@
   }
 
   delegate_->RemoveUser(email);
 }
 
 void SigninScreenHandler::HandleShowAddUser(const base::ListValue* args) {
   email_.clear();
   // |args| can be null if it's OOBE.
   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();
   } else {
     StartClearingDnsCache();
     StartClearingCookies();
   }
 }
 
 void SigninScreenHandler::HandleToggleEnrollmentScreen(
@@ skipping 79 matching lines @@
   web_ui()->CallJavascriptFunction("login.AccountPickerScreen.loadUsers",
                                    users_list, animated_value);
 }
 
 void SigninScreenHandler::HandleAccountPickerReady(
     const base::ListValue* args) {
   // Fetching of the extension is not started before account picker page is
   // 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(
         chrome::NOTIFICATION_LOCK_WEBUI_READY,
         content::NotificationService::AllSources(),
         content::NotificationService::NoDetails());
   }
 }
 
 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.
     const char code[] = "var gWindowOnLoad = window.onload; "
                         "window.onload=function() {};";
     RenderViewHost* rvh = web_ui()->GetWebContents()->GetRenderViewHost();
     rvh->ExecuteJavascriptInWebFrame(
         ASCIIToUTF16("//iframe[@id='signin-frame']\n//iframe"),
         ASCIIToUTF16(code));
   }
@@ skipping 65 matching lines @@
 
   cookie_remover_ = new BrowsingDataRemover(
       Profile::FromWebUI(web_ui()),
       BrowsingDataRemover::EVERYTHING,
       base::Time());
   cookie_remover_->AddObserver(this);
   cookie_remover_->Remove(BrowsingDataRemover::REMOVE_SITE_DATA);
 }
 
 }  // namespace chromeos