Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(339)

Unified Diff: chrome/browser/ui/app_list/app_list_service.cc

Issue 23315002: Add UMA for first app list paint on Windows. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 7 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/app_list/app_list_service.cc
diff --git a/chrome/browser/ui/app_list/app_list_service.cc b/chrome/browser/ui/app_list/app_list_service.cc
index 5a5f9331883747652241c54aef9f6ea7f6732558..2576ab6865061aa52d9647132e7f91e1cbf5ca45 100644
--- a/chrome/browser/ui/app_list/app_list_service.cc
+++ b/chrome/browser/ui/app_list/app_list_service.cc
@@ -4,6 +4,8 @@
#include "chrome/browser/ui/app_list/app_list_service.h"
+#include "base/bind.h"
+#include "base/callback.h"
#include "base/command_line.h"
#include "base/metrics/histogram.h"
#include "base/prefs/pref_registry_simple.h"
@@ -15,13 +17,53 @@
namespace {
-base::TimeDelta GetTimeFromOriginalProcessStart(
- const CommandLine& command_line) {
- std::string start_time_string =
- command_line.GetSwitchValueASCII(switches::kOriginalProcessStartTime);
- int64 remote_start_time;
- base::StringToInt64(start_time_string, &remote_start_time);
- return base::Time::Now() - base::Time::FromInternalValue(remote_start_time);
+enum StartupType {
+ COLD_START,
+ WARM_START,
+ WARM_START_FAST,
+};
+
+base::Time GetOriginalProcessStartTime(const CommandLine& command_line) {
+ if (command_line.HasSwitch(switches::kOriginalProcessStartTime)) {
+ std::string start_time_string =
+ command_line.GetSwitchValueASCII(switches::kOriginalProcessStartTime);
+ int64 remote_start_time;
+ base::StringToInt64(start_time_string, &remote_start_time);
+ return base::Time::FromInternalValue(remote_start_time);
+ }
+
+// base::CurrentProcessInfo::CreationTime() is only defined on some
+// platforms.
+#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX)
+ return base::CurrentProcessInfo::CreationTime();
benwells 2013/08/21 04:54:09 Nit - indenting should be back two chars, this is
koz (OOO until 15th September) 2013/08/21 06:54:32 Done.
+#endif
+ return base::Time();
+}
+
+StartupType GetStartupType(const CommandLine& command_line) {
+ // The presence of kOriginalProcessStartTime implies that another process
+ // has sent us its command line to handle, ie: we are already running.
+ if (command_line.HasSwitch(switches::kOriginalProcessStartTime)) {
+ return command_line.HasSwitch(switches::kFastStart) ?
+ WARM_START_FAST : WARM_START;
+ }
+ return COLD_START;
+}
+
+void RecordFirstPaintTiming(StartupType startup_type,
+ const base::Time& start_time) {
+ base::TimeDelta elapsed = base::Time::Now() - start_time;
+ switch (startup_type) {
+ case WARM_START_FAST:
benwells 2013/08/21 04:54:09 Nit: inside switch {} should be indented 2 more ch
koz (OOO until 15th September) 2013/08/21 06:54:32 Done.
+ UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStartFast", elapsed);
+ break;
+ case WARM_START:
+ UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStart", elapsed);
+ break;
+ case COLD_START:
+ UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintColdStart", elapsed);
+ break;
+ }
}
} // namespace
@@ -38,21 +80,24 @@ void AppListService::RegisterPrefs(PrefRegistrySimple* registry) {
// static
void AppListService::RecordShowTimings(const CommandLine& command_line) {
- // The presence of kOriginalProcessStartTime implies that another process
- // has sent us its command line to handle, ie: we are already running.
- if (command_line.HasSwitch(switches::kOriginalProcessStartTime)) {
- base::TimeDelta elapsed = GetTimeFromOriginalProcessStart(command_line);
- if (command_line.HasSwitch(switches::kFastStart))
- UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListWarmStartFast", elapsed);
- else
- UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListWarmStart", elapsed);
- } else {
- // base::CurrentProcessInfo::CreationTime() is only defined on some
- // platforms.
-#if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX)
- UMA_HISTOGRAM_LONG_TIMES(
- "Startup.ShowAppListColdStart",
- base::Time::Now() - base::CurrentProcessInfo::CreationTime());
-#endif
+ base::Time start_time = GetOriginalProcessStartTime(command_line);
+ if (start_time.is_null())
+ return;
+
+ base::TimeDelta elapsed = base::Time::Now() - start_time;
+ StartupType startup = GetStartupType(command_line);
+ switch (startup) {
+ case COLD_START:
benwells 2013/08/21 04:54:09 Super nit: use same ordering of enum here as in th
koz (OOO until 15th September) 2013/08/21 06:54:32 Done.
+ UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListColdStart", elapsed);
+ break;
+ case WARM_START:
+ UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListWarmStart", elapsed);
+ break;
+ case WARM_START_FAST:
+ UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListWarmStartFast", elapsed);
+ break;
}
+
+ AppListService::Get()->SetAppListNextPaintCallback(
+ base::Bind(RecordFirstPaintTiming, startup, start_time));
}

Powered by Google App Engine
This is Rietveld 408576698