Refactor handling of --show-app-list command line.

chrome/browser/ui/app_list/app_list_service.cc

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 3fb43fe204de50c8aa57a98a4220aa4b27c41b93..d3fd28525ed379ab4bb34e0404b1804cb0cbba71 100644
--- a/chrome/browser/ui/app_list/app_list_service.cc
+++ b/chrome/browser/ui/app_list/app_list_service.cc
@@ -21,6 +21,15 @@ enum StartupType {
   WARM_START_FAST,
 };
 
+// For when an app list show request is received via CommandLine. Indicates
+// whether the Profile the app list was previously showing was the SAME, OTHER
+// or NONE with respect to the new Profile to show.
+enum ProfileLoadState {
+  PROFILE_LOADED_SAME,
+  PROFILE_LOADED_OTHER,
+  PROFILE_LOADED_NONE,
+};
+
 base::Time GetOriginalProcessStartTime(const CommandLine& command_line) {
   if (command_line.HasSwitch(switches::kOriginalProcessStartTime)) {
     std::string start_time_string =
@@ -57,9 +66,22 @@ int64 g_original_process_start_time;
 // The type of startup the the current app list show has gone through.
 StartupType g_app_show_startup_type;
 
-void RecordStartupInfo(StartupType startup_type, const base::Time& start_time) {
+// The state of the active app list profile at the most recent launch.
+ProfileLoadState g_profile_load_state;
+
+void RecordStartupInfo(StartupType startup_type,
+                       const base::Time& start_time,
+                       Profile* current_profile,
+                       Profile* launch_profile) {
   g_original_process_start_time = start_time.ToInternalValue();
   g_app_show_startup_type = startup_type;
+
+  if (!current_profile)
+    g_profile_load_state = PROFILE_LOADED_NONE;
+  else if (current_profile == launch_profile)
+    g_profile_load_state = PROFILE_LOADED_SAME;
+  else
+    g_profile_load_state = PROFILE_LOADED_OTHER;
 }
 
 void RecordFirstPaintTiming() {
@@ -68,16 +90,53 @@ void RecordFirstPaintTiming() {
   base::TimeDelta elapsed = base::Time::Now() - start_time;
   switch (g_app_show_startup_type) {
     case COLD_START:
-      UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintColdStart", elapsed);
+      if (g_profile_load_state == PROFILE_LOADED_NONE)

[Matt Giuca, 2014/07/02 07:35:18]

nit: Can this just be a DCHECK (avoiding the NOTREACHED)?

[tapted, 2014/07/04 03:00:02]

Done.

+        UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintColdStart", elapsed);
+      else
+        NOTREACHED();
+      break;
+    case WARM_START:
+      // For warm starts, only record showing the same profile. "NONE" should
+      // only occur in the first 30 seconds after startup. "OTHER" only occurs
+      // for multi-profile cases. In these cases, timings are also affected by
+      // whether or not a profile has been loaded from disk, which makes the
+      // profile load asynchronous and skews results unpredictably.
+      if (g_profile_load_state == PROFILE_LOADED_SAME)
+        UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStart", elapsed);
+      break;
+    case WARM_START_FAST:
+      if (g_profile_load_state == PROFILE_LOADED_SAME) {
+        UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStartFast",
+                                 elapsed);
+      }
+      break;
+  }
+}
+
+void RecordStartupTimings(AppListService* service,

[Matt Giuca, 2014/07/02 07:35:18]

nit: I think it would read nicer if this method was just above or below
RecordStartupInfo.

[tapted, 2014/07/04 03:00:02]

Done - I think it reads nice that way too, but note it required a prototype
declaration for anonymous::RecordFirstPaintTiming();

[Matt Giuca, 2014/07/04 04:37:15]

Hmm that's not so nice. I'd be happy if you put it back where it was (to avoid a
forward declaration). Up to you!

+                          const CommandLine& command_line,

[Matt Giuca, 2014/07/02 07:35:18]

Your CL description mentions "RecordShowTimings"; since you've renamed this
method you should update the description.

[tapted, 2014/07/04 03:00:02]

Done.

+                          Profile* launch_profile) {
+  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:
+      UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListColdStart", elapsed);
       break;
     case WARM_START:
-      UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStart", elapsed);
+      UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListWarmStart", elapsed);
       break;
     case WARM_START_FAST:
-      UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStartFast",
-                               elapsed);
+      UMA_HISTOGRAM_LONG_TIMES("Startup.ShowAppListWarmStartFast", elapsed);
       break;
   }
+
+  Profile* current_profile = service->GetCurrentAppListProfile();
+  RecordStartupInfo(startup, start_time, current_profile, launch_profile);
+  service->SetAppListNextPaintCallback(RecordFirstPaintTiming);
 }
 
 }  // namespace
@@ -106,26 +165,17 @@ void AppListService::RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 // static
-void AppListService::RecordShowTimings(const CommandLine& command_line) {
-  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:
-      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;
-  }
-
-  RecordStartupInfo(startup, start_time);
-  Get(chrome::HOST_DESKTOP_TYPE_NATIVE)->SetAppListNextPaintCallback(
-      RecordFirstPaintTiming);
+bool AppListService::HandleLaunchCommandLine(
+    const base::CommandLine& command_line,
+    Profile* launch_profile) {
+  InitAll(launch_profile);
+  if (!command_line.HasSwitch(switches::kShowAppList))
+    return false;
+
+  // The --show-app-list switch is used for shortcuts on the native desktop.
+  AppListService* service =
+      AppListService::Get(chrome::HOST_DESKTOP_TYPE_NATIVE);
+  RecordStartupTimings(service, command_line, launch_profile);
+  service->ShowForProfile(launch_profile);
+  return true;
 }