Refactoring: get rid of notificataions in ParallelAuthenticator

BUG=387613
R=nkostylev@chromium.org
TBR=stevenjb@chromium.org

Review URL: https://codereview.chromium.org/409113002

Cr-Commit-Position: refs/heads/master@{#291045}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291045 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 46 insertions, 79 deletions

diff --git a/chrome/browser/chrome_notification_types.h b/chrome/browser/chrome_notification_types.h
index 19adfc6..690ddb4 100644
--- a/chrome/browser/chrome_notification_types.h
+++ b/chrome/browser/chrome_notification_types.h
@@ -492,10 +492,6 @@ enum NotificationType {
   // default profile image or no profile image at all. No details are expected.
   NOTIFICATION_PROFILE_IMAGE_UPDATE_FAILED,
 
-  // Sent when a chromium os user attempts to log in.  The source is
-  // all and the details are AuthenticationNotificationDetails.
-  NOTIFICATION_LOGIN_AUTHENTICATION,
-
   // Sent when a network error message is displayed on the WebUI login screen.
   // First paint event of this fires NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE.
   NOTIFICATION_LOGIN_NETWORK_ERROR_SHOWN,
diff --git a/chrome/browser/chromeos/boot_times_loader.cc b/chrome/browser/chromeos/boot_times_loader.cc
index eb4633e..d77d7e1 100644
--- a/chrome/browser/chromeos/boot_times_loader.cc
+++ b/chrome/browser/chromeos/boot_times_loader.cc
@@ -26,7 +26,6 @@
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/chromeos/login/auth/authentication_notification_details.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_iterator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -428,8 +427,6 @@ void BootTimesLoader::RecordLoginAttempted() {
   AddLoginTimeMarker("LoginStarted", false);
   if (!have_registered_) {
     have_registered_ = true;
-    registrar_.Add(this, chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
-                   content::NotificationService::AllSources());
     registrar_.Add(this, content::NOTIFICATION_LOAD_START,
                    content::NotificationService::AllSources());
     registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
@@ -475,21 +472,20 @@ void BootTimesLoader::AddMarker(std::vector<TimeMarker>* vector,
   }
 }
 
+void BootTimesLoader::RecordAuthenticationSuccess() {
+  AddLoginTimeMarker("Authenticate", true);
+  RecordCurrentStats(kLoginSuccess);
+}
+
+void BootTimesLoader::RecordAuthenticationFailure() {
+  // Do nothing for now.
+}
+
 void BootTimesLoader::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
-    case chrome::NOTIFICATION_LOGIN_AUTHENTICATION: {
-      content::Details<AuthenticationNotificationDetails> auth_details(details);
-      if (auth_details->success()) {
-        AddLoginTimeMarker("Authenticate", true);
-        RecordCurrentStats(kLoginSuccess);
-        registrar_.Remove(this, chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
-                          content::NotificationService::AllSources());
-      }
-      break;
-    }
     case content::NOTIFICATION_LOAD_START: {
       NavigationController* tab =
           content::Source<NavigationController>(source).ptr();
diff --git a/chrome/browser/chromeos/boot_times_loader.h b/chrome/browser/chromeos/boot_times_loader.h
index 4dbb9c2..d9622b6 100644
--- a/chrome/browser/chromeos/boot_times_loader.h
+++ b/chrome/browser/chromeos/boot_times_loader.h
@@ -41,13 +41,10 @@ class BootTimesLoader : public content::NotificationObserver,
   static BootTimesLoader* Get();
 
   // LoginEventRecorder::Delegate override.
-
-  // Add a time marker for login. A timeline will be dumped to
-  // /tmp/login-times-sent after login is done. If |send_to_uma| is true
-  // the time between this marker and the last will be sent to UMA with
-  // the identifier BootTime.|marker_name|.
   virtual void AddLoginTimeMarker(const std::string& marker_name,
                                   bool send_to_uma) OVERRIDE;
+  virtual void RecordAuthenticationSuccess() OVERRIDE;
+  virtual void RecordAuthenticationFailure() OVERRIDE;
 
   // Add a time marker for logout. A timeline will be dumped to
   // /tmp/logout-times-sent after logout is done. If |send_to_uma| is true
diff --git a/chrome/browser/chromeos/login/auth/authentication_notification_details.h b/chrome/browser/chromeos/login/auth/authentication_notification_details.h
deleted file mode 100644
index 1402213..0000000
--- a/chrome/browser/chromeos/login/auth/authentication_notification_details.h
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_AUTH_AUTHENTICATION_NOTIFICATION_DETAILS_H_
-#define CHROME_BROWSER_CHROMEOS_LOGIN_AUTH_AUTHENTICATION_NOTIFICATION_DETAILS_H_
-
-// A class to hold the parameters we get back from an authentication attempt
-// through the login manager
-class AuthenticationNotificationDetails {
- public:
-  explicit AuthenticationNotificationDetails(bool success) : success_(success) {
-  }
-
-  bool success() const { return success_; }
-
- private:
-  bool success_;
-
-  DISALLOW_COPY_AND_ASSIGN(AuthenticationNotificationDetails);
-};
-
-#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_AUTH_AUTHENTICATION_NOTIFICATION_DETAILS_H_
diff --git a/chrome/browser/chromeos/login/auth/parallel_authenticator.cc b/chrome/browser/chromeos/login/auth/parallel_authenticator.cc
index 8ca278d..f73b56c 100644
--- a/chrome/browser/chromeos/login/auth/parallel_authenticator.cc
+++ b/chrome/browser/chromeos/login/auth/parallel_authenticator.cc
@@ -8,9 +8,6 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/chromeos/boot_times_loader.h"
-#include "chrome/browser/chromeos/login/auth/authentication_notification_details.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/common/chrome_switches.h"
@@ -23,10 +20,10 @@
 #include "chromeos/login/auth/user_context.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/login/user_names.h"
+#include "chromeos/login_event_recorder.h"
 #include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_type.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_service.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 using content::BrowserThread;
@@ -76,7 +73,7 @@ void TriggerResolveWithLoginTimeMarker(
     scoped_refptr<ParallelAuthenticator> resolver,
     bool success,
     cryptohome::MountError return_code) {
-  chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(marker_name, false);
+  chromeos::LoginEventRecorder::Get()->AddLoginTimeMarker(marker_name, false);
   TriggerResolve(attempt, resolver, success, return_code);
 }
 
@@ -86,7 +83,7 @@ void Mount(AuthAttemptState* attempt,
            int flags,
            const std::string& system_salt) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(
+  chromeos::LoginEventRecorder::Get()->AddLoginTimeMarker(
       "CryptohomeMount-Start", false);
   // Set state that username_hash is requested here so that test implementation
   // that returns directly would not generate 2 OnLoginSucces() calls.
@@ -164,7 +161,7 @@ void Migrate(AuthAttemptState* attempt,
              const std::string& old_password,
              const std::string& system_salt) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(
+  chromeos::LoginEventRecorder::Get()->AddLoginTimeMarker(
       "CryptohomeMigrate-Start", false);
   cryptohome::AsyncMethodCaller* caller =
       cryptohome::AsyncMethodCaller::GetInstance();
@@ -198,7 +195,7 @@ void Migrate(AuthAttemptState* attempt,
 void Remove(AuthAttemptState* attempt,
             scoped_refptr<ParallelAuthenticator> resolver) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  chromeos::BootTimesLoader::Get()->AddLoginTimeMarker(
+  chromeos::LoginEventRecorder::Get()->AddLoginTimeMarker(
       "CryptohomeRemove-Start", false);
   cryptohome::AsyncMethodCaller::GetInstance()->AsyncRemove(
       attempt->user_context.GetUserID(),
@@ -395,12 +392,7 @@ void ParallelAuthenticator::LoginAsKioskAccount(
 void ParallelAuthenticator::OnRetailModeAuthSuccess() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   VLOG(1) << "Retail mode login success";
-  // Send notification of success
-  AuthenticationNotificationDetails details(true);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
-      content::NotificationService::AllSources(),
-      content::Details<AuthenticationNotificationDetails>(&details));
+  chromeos::LoginEventRecorder::Get()->RecordAuthenticationSuccess();
   if (consumer_)
     consumer_->OnRetailModeAuthSuccess(current_state_->user_context);
 }
@@ -409,11 +401,7 @@ void ParallelAuthenticator::OnAuthSuccess() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   VLOG(1) << "Login success";
   // Send notification of success
-  AuthenticationNotificationDetails details(true);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
-      content::NotificationService::AllSources(),
-      content::Details<AuthenticationNotificationDetails>(&details));
+  chromeos::LoginEventRecorder::Get()->RecordAuthenticationSuccess();
   {
     base::AutoLock for_this_block(success_lock_);
     already_reported_success_ = true;
@@ -424,12 +412,7 @@ void ParallelAuthenticator::OnAuthSuccess() {
 
 void ParallelAuthenticator::OnOffTheRecordAuthSuccess() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // Send notification of success
-  AuthenticationNotificationDetails details(true);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
-      content::NotificationService::AllSources(),
-      content::Details<AuthenticationNotificationDetails>(&details));
+  chromeos::LoginEventRecorder::Get()->RecordAuthenticationSuccess();
   if (consumer_)
     consumer_->OnOffTheRecordAuthSuccess();
 }
@@ -450,13 +433,7 @@ void ParallelAuthenticator::OnAuthFailure(const AuthFailure& error) {
     RemoveEncryptedData();
     return;
   }
-
-  // Send notification of failure
-  AuthenticationNotificationDetails details(false);
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_LOGIN_AUTHENTICATION,
-      content::NotificationService::AllSources(),
-      content::Details<AuthenticationNotificationDetails>(&details));
+  chromeos::LoginEventRecorder::Get()->RecordAuthenticationFailure();
   LOG(WARNING) << "Login failed: " << error.GetErrorString();
   if (consumer_)
     consumer_->OnAuthFailure(error);
diff --git a/chrome/browser/chromeos/login/auth/parallel_authenticator.h b/chrome/browser/chromeos/login/auth/parallel_authenticator.h
index c4c9ed1..5f50bf2e 100644
--- a/chrome/browser/chromeos/login/auth/parallel_authenticator.h
+++ b/chrome/browser/chromeos/login/auth/parallel_authenticator.h
@@ -12,7 +12,6 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
-#include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chromeos/login/auth/auth_attempt_state.h"
 #include "chromeos/login/auth/auth_attempt_state_resolver.h"
 #include "chromeos/login/auth/authenticator.h"
diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi
index baa8233..cf4ec26 100644
--- a/chrome/chrome_browser_chromeos.gypi
+++ b/chrome/chrome_browser_chromeos.gypi
@@ -530,7 +530,6 @@
         'browser/chromeos/login/auth/login_performer.h',
         'browser/chromeos/login/auth/parallel_authenticator.cc',
         'browser/chromeos/login/auth/parallel_authenticator.h',
-        'browser/chromeos/login/authentication_notification_details.h',
         'browser/chromeos/login/chrome_restart_request.cc',
         'browser/chromeos/login/chrome_restart_request.h',
         'browser/chromeos/login/default_pinned_apps_field_trial.cc',
diff --git a/chromeos/login_event_recorder.cc b/chromeos/login_event_recorder.cc
index e2e2e3f..c3d67d6 100644
--- a/chromeos/login_event_recorder.cc
+++ b/chromeos/login_event_recorder.cc
@@ -34,4 +34,14 @@ void LoginEventRecorder::AddLoginTimeMarker(const std::string& marker_name,
     delegate_->AddLoginTimeMarker(marker_name, send_to_uma);
 }
 
+void LoginEventRecorder::RecordAuthenticationSuccess() {
+  if (delegate_)
+    delegate_->RecordAuthenticationSuccess();
+}
+
+void LoginEventRecorder::RecordAuthenticationFailure() {
+  if (delegate_)
+    delegate_->RecordAuthenticationFailure();
+}
+
 }  // namespace chromeos
diff --git a/chromeos/login_event_recorder.h b/chromeos/login_event_recorder.h
index ec695b4..f4d0f8b 100644
--- a/chromeos/login_event_recorder.h
+++ b/chromeos/login_event_recorder.h
@@ -18,8 +18,18 @@ class CHROMEOS_EXPORT LoginEventRecorder {
  public:
   class Delegate {
    public:
+  // Add a time marker for login. A timeline will be dumped to
+  // /tmp/login-times-sent after login is done. If |send_to_uma| is true
+  // the time between this marker and the last will be sent to UMA with
+  // the identifier BootTime.|marker_name|.
     virtual void AddLoginTimeMarker(const std::string& marker_name,
                                     bool send_to_uma) = 0;
+
+  // Record events for successful authentication.
+    virtual void RecordAuthenticationSuccess() = 0;
+
+  // Record events for authentication failure.
+    virtual void RecordAuthenticationFailure() = 0;
   };
   LoginEventRecorder();
   virtual ~LoginEventRecorder();
@@ -34,6 +44,12 @@ class CHROMEOS_EXPORT LoginEventRecorder {
   // the identifier BootTime.|marker_name|.
   void AddLoginTimeMarker(const std::string& marker_name, bool send_to_uma);
 
+  // Record events for successful authentication.
+  void RecordAuthenticationSuccess();
+
+  // Record events for authentication failure.
+  void RecordAuthenticationFailure();
+
  private:
   Delegate* delegate_;