From b8d0f4fb5183a74718cc437ab3f0462949ee8ab1 Mon Sep 17 00:00:00 2001 From: "alemate@chromium.org" Date: Thu, 15 May 2014 00:42:35 +0000 Subject: [PATCH] Fix BootTime.Login , BootTime.Logout UMA histograms. Add new BootTime.LoginNewUser , ShutdownTime.Restart. BootTime.Login / BootTime.Logout were not accurately measured. This CL fixes it. BUG=344934 TEST=manual Review URL: https://codereview.chromium.org/289483002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270543 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/chromeos/boot_times_loader.cc | 53 +++++++++++++++------- chrome/browser/chromeos/boot_times_loader.h | 12 ++++- .../chromeos/login/chrome_restart_request.cc | 2 + .../chromeos/login/existing_user_controller.cc | 4 +- chrome/browser/chromeos/login/login_utils.cc | 2 + chrome/browser/lifetime/application_lifetime.cc | 2 + 6 files changed, 55 insertions(+), 20 deletions(-) diff --git a/chrome/browser/chromeos/boot_times_loader.cc b/chrome/browser/chromeos/boot_times_loader.cc index 6d3c820fdd1f..6994c8510d3d 100644 --- a/chrome/browser/chromeos/boot_times_loader.cc +++ b/chrome/browser/chromeos/boot_times_loader.cc @@ -24,6 +24,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/authentication_notification_details.h" +#include "chrome/browser/chromeos/login/user_manager.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_iterator.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -96,9 +97,11 @@ static const base::FilePath::CharType kChromeFirstRender[] = // Names of login UMA values. static const char kUmaLogin[] = "BootTime.Login"; +static const char kUmaLoginNewUser[] = "BootTime.LoginNewUser"; static const char kUmaLoginPrefix[] = "BootTime."; static const char kUmaLogout[] = "ShutdownTime.Logout"; static const char kUmaLogoutPrefix[] = "ShutdownTime."; +static const char kUmaRestart[] = "ShutdownTime.Restart"; // Name of file collecting login times. static const base::FilePath::CharType kLoginTimes[] = FPL("login-times"); @@ -111,7 +114,9 @@ static base::LazyInstance g_boot_times_loader = BootTimesLoader::BootTimesLoader() : backend_(new Backend()), - have_registered_(false) { + have_registered_(false), + login_done_(false), + restart_requested_(false) { login_time_markers_.reserve(30); logout_time_markers_.reserve(30); } @@ -212,24 +217,37 @@ void BootTimesLoader::WriteTimes( base::WriteFile(log_path.Append(base_name), output.data(), output.size()); } -void BootTimesLoader::LoginDone() { +void BootTimesLoader::LoginDone(bool is_user_new) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (login_done_) + return; + + login_done_ = true; AddLoginTimeMarker("LoginDone", false); RecordCurrentStats(kChromeFirstRender); - registrar_.Remove(this, content::NOTIFICATION_LOAD_START, - content::NotificationService::AllSources()); - registrar_.Remove(this, content::NOTIFICATION_LOAD_STOP, - content::NotificationService::AllSources()); - registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, - content::NotificationService::AllSources()); - registrar_.Remove( - this, - content::NOTIFICATION_RENDER_WIDGET_HOST_DID_UPDATE_BACKING_STORE, - content::NotificationService::AllSources()); + if (have_registered_) { + registrar_.Remove(this, + content::NOTIFICATION_LOAD_START, + content::NotificationService::AllSources()); + registrar_.Remove(this, + content::NOTIFICATION_LOAD_STOP, + content::NotificationService::AllSources()); + registrar_.Remove(this, + content::NOTIFICATION_WEB_CONTENTS_DESTROYED, + content::NotificationService::AllSources()); + registrar_.Remove( + this, + content::NOTIFICATION_RENDER_WIDGET_HOST_DID_UPDATE_BACKING_STORE, + content::NotificationService::AllSources()); + } // Don't swamp the FILE thread right away. BrowserThread::PostDelayedTask( - BrowserThread::FILE, FROM_HERE, - base::Bind(&WriteTimes, kLoginTimes, kUmaLogin, kUmaLoginPrefix, + BrowserThread::FILE, + FROM_HERE, + base::Bind(&WriteTimes, + kLoginTimes, + (is_user_new ? kUmaLoginNewUser : kUmaLogin), + kUmaLoginPrefix, login_time_markers_), base::TimeDelta::FromMilliseconds(kLoginTimeWriteDelayMs)); } @@ -242,7 +260,7 @@ void BootTimesLoader::WriteLogoutTimes() { !BrowserThread::IsMessageLoopValid(BrowserThread::UI)); WriteTimes(kLogoutTimes, - kUmaLogout, + (restart_requested_ ? kUmaRestart : kUmaLogout), kUmaLogoutPrefix, logout_time_markers_); } @@ -277,6 +295,9 @@ void BootTimesLoader::RecordChromeMainStats() { void BootTimesLoader::RecordLoginAttempted() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (login_done_) + return; + login_time_markers_.clear(); AddLoginTimeMarker("LoginStarted", false); if (!have_registered_) { @@ -367,7 +388,7 @@ void BootTimesLoader::Observe( if (render_widget_hosts_loading_.find(rwh) != render_widget_hosts_loading_.end()) { AddLoginTimeMarker("TabPaint: " + GetTabUrl(rwh), false); - LoginDone(); + LoginDone(UserManager::Get()->IsCurrentUserNew()); } break; } diff --git a/chrome/browser/chromeos/boot_times_loader.h b/chrome/browser/chromeos/boot_times_loader.h index 32f88b2f6d18..854dccdabee2 100644 --- a/chrome/browser/chromeos/boot_times_loader.h +++ b/chrome/browser/chromeos/boot_times_loader.h @@ -71,11 +71,17 @@ class BootTimesLoader : public content::NotificationObserver { const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; + // Records "LoginDone" event. + void LoginDone(bool is_user_new); + // Writes the logout times to a /tmp/logout-times-sent. Unlike login // times, we manually call this function for logout times, as we cannot // rely on notification service to tell when the logout is done. void WriteLogoutTimes(); + // Mark that WriteLogoutTimes should handle restart. + void set_restart_requested() { restart_requested_ = true; } + private: // BootTimesLoader calls into the Backend on the file thread to load // the boot times. @@ -128,8 +134,6 @@ class BootTimesLoader : public content::NotificationObserver { std::vector login_times); static void AddMarker(std::vector* vector, TimeMarker marker); - void LoginDone(); - // Used to hold the stats at main(). Stats chrome_main_stats_; scoped_refptr backend_; @@ -143,6 +147,10 @@ class BootTimesLoader : public content::NotificationObserver { std::vector logout_time_markers_; std::set render_widget_hosts_loading_; + bool login_done_; + + bool restart_requested_; + DISALLOW_COPY_AND_ASSIGN(BootTimesLoader); }; diff --git a/chrome/browser/chromeos/login/chrome_restart_request.cc b/chrome/browser/chromeos/login/chrome_restart_request.cc index 56520c4fde6b..a477c17c0f2b 100644 --- a/chrome/browser/chromeos/login/chrome_restart_request.cc +++ b/chrome/browser/chromeos/login/chrome_restart_request.cc @@ -21,6 +21,7 @@ #include "base/values.h" #include "cc/base/switches.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/chromeos/boot_times_loader.h" #include "chrome/browser/chromeos/login/user_manager.h" #include "chrome/browser/lifetime/application_lifetime.h" #include "chrome/common/chrome_constants.h" @@ -365,6 +366,7 @@ std::string GetOffTheRecordCommandLine( void RestartChrome(const std::string& command_line) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + BootTimesLoader::Get()->set_restart_requested(); static bool restart_requested = false; if (restart_requested) { diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc index e6da1cabff3c..cb6764e559e3 100644 --- a/chrome/browser/chromeos/login/existing_user_controller.cc +++ b/chrome/browser/chromeos/login/existing_user_controller.cc @@ -387,8 +387,6 @@ void ExistingUserController::Login(const UserContext& user_context) { // Disable clicking on other windows. login_display_->SetUIEnabled(false); - BootTimesLoader::Get()->RecordLoginAttempted(); - if (last_login_attempt_username_ != user_context.username) { last_login_attempt_username_ = user_context.username; num_login_attempts_ = 0; @@ -406,6 +404,8 @@ void ExistingUserController::PerformLogin( UserManager::Get()->GetUserFlow(last_login_attempt_username_)-> set_host(host_); + BootTimesLoader::Get()->RecordLoginAttempted(); + // Disable UI while loading user profile. login_display_->SetUIEnabled(false); diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc index 7eba33a80e9c..e0ca833b6381 100644 --- a/chrome/browser/chromeos/login/login_utils.cc +++ b/chrome/browser/chromeos/login/login_utils.cc @@ -357,6 +357,8 @@ void LoginUtilsImpl::DoBrowserLaunchOnLocaleLoadedImpl( if (login_host) login_host->Finalize(); UserManager::Get()->SessionStarted(); + chromeos::BootTimesLoader::Get()->LoginDone( + chromeos::UserManager::Get()->IsCurrentUserNew()); } void LoginUtilsImpl::DoBrowserLaunch(Profile* profile, diff --git a/chrome/browser/lifetime/application_lifetime.cc b/chrome/browser/lifetime/application_lifetime.cc index ca2b639c2160..74b6ec368e44 100644 --- a/chrome/browser/lifetime/application_lifetime.cc +++ b/chrome/browser/lifetime/application_lifetime.cc @@ -196,6 +196,8 @@ void AttemptRestart() { pref_service->SetBoolean(prefs::kWasRestarted, true); #if defined(OS_CHROMEOS) + chromeos::BootTimesLoader::Get()->set_restart_requested(); + DCHECK(!g_send_stop_request_to_session_manager); // Make sure we don't send stop request to the session manager. g_send_stop_request_to_session_manager = false; -- 2.11.4.GIT