Fix ScreenLocker to not send notifications with empty passwords.

chrome/browser/chromeos/login/screen_locker.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/chromeos/login/screen_locker.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 168 matching lines @@
   if (authentication_start_time_.is_null()) {
     if (!username.empty())
       LOG(WARNING) << "authentication_start_time_ is not set";
   } else {
     base::TimeDelta delta = base::Time::Now() - authentication_start_time_;
     VLOG(1) << "Authentication success time: " << delta.InSecondsF();
     UMA_HISTOGRAM_TIMES("ScreenLocker.AuthenticationSuccessTime", delta);
   }
 
   Profile* profile = ProfileManager::GetDefaultProfile();
-  if (profile) {
-    ProfileSyncService* service(
-        ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile));
-    if (service && !service->HasSyncSetupCompleted()) {
+  if (profile && !password.empty()) {

[Nikita (slow), 2012/06/07 09:43:59]

How could you end up in this state with empty password?
OnLoginSuccess() is called by authenticator when password is validated. Empty
password wouldn't qualify as valid, right?

[Andrew T Wilson (Slow), 2012/06/07 17:22:42]

Here is what Bartosz reported to me:
-=-=-=-
Bartosz: void ScreenLocker::OnLoginSuccess()
this constructs details with the password it was given as an argument
which comes from ParallelAuthenticator::OnLoginSuccess()
this one takes the password from current_state_->password
the current_state_ gets set up in ParallelAuthenticator::AuthenticateToUnlock()
and this is the interesting one
it does current_state_.reset(new AuthAttemptState(...))
the constructor it uses takes a user name and a password hash. so user name and
hash exist, the plain text password is blank
  
this is how (i think) we end up with a blank password
-=-=-=-

It's possible that this is only the case for automated tests? I'm not 100%
familiar with the fail case.

If you're quite certain that we should never get called with an empty password,
I can change this to a DCHECK() instead - let me know what you'd like me to do.

+    // If we have a non-empty password, notify listeners (such as the sync
+    // engine).
+    if (!password.empty()) {

[Nikita (slow), 2012/06/07 09:43:59]

Why do you need another check for empty password after one at line #189?

[Andrew T Wilson (Slow), 2012/06/07 17:22:42]

Bah, I moved the check up above but forgot to remove this one.

       SigninManager* signin = SigninManagerFactory::GetForProfile(profile);
       DCHECK(signin);
       GoogleServiceSigninSuccessDetails details(
           signin->GetAuthenticatedUsername(),
           password);
       content::NotificationService::current()->Notify(
           chrome::NOTIFICATION_GOOGLE_SIGNIN_SUCCESSFUL,
           content::Source<Profile>(profile),
           content::Details<const GoogleServiceSigninSuccessDetails>(&details));
     }
@@ skipping 163 matching lines @@
   bool state = true;
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_SCREEN_LOCK_STATE_CHANGED,
       content::Source<ScreenLocker>(this),
       content::Details<bool>(&state));
   DBusThreadManager::Get()->GetPowerManagerClient()->
       NotifyScreenLockCompleted();
 }
 
 }  // namespace chromeos