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

chrome/browser/ui/app_list/app_list_service.cc

 // Copyright 2013 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/ui/app_list/app_list_service.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/process/process_info.h"
@@ skipping 44 matching lines @@
 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) {
   g_original_process_start_time = start_time.ToInternalValue();
   g_app_show_startup_type = startup_type;
 }
 
+void RecordFirstPaintTiming();
+
+void RecordStartupTimings(AppListService* service,
+                          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);
+  service->SetAppListNextPaintCallback(RecordFirstPaintTiming);
+}
+
 void RecordFirstPaintTiming() {
   base::Time start_time(
       base::Time::FromInternalValue(g_original_process_start_time));
   base::TimeDelta elapsed = base::Time::Now() - start_time;
   switch (g_app_show_startup_type) {
     case COLD_START:
       UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintColdStart", elapsed);
       break;
     case WARM_START:
       UMA_HISTOGRAM_LONG_TIMES("Startup.AppListFirstPaintWarmStart", elapsed);
@@ skipping 24 matching lines @@
   registry->RegisterIntegerPref(prefs::kAppLauncherShortcutVersion, 0);
 #endif
 
   // Identifies whether we should show the app launcher promo or not.
   // Note that a field trial also controls the showing, so the promo won't show
   // unless the pref is set AND the field trial is set to a proper group.
   registry->RegisterBooleanPref(prefs::kShowAppLauncherPromo, true);
 }
 
 // static
-void AppListService::RecordShowTimings(const CommandLine& command_line) {
-  base::Time start_time = GetOriginalProcessStartTime(command_line);
-  if (start_time.is_null())
-    return;
+bool AppListService::HandleLaunchCommandLine(
+    const base::CommandLine& command_line,
+    Profile* launch_profile) {
+  InitAll(launch_profile);

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

nit (optional): Upon consideration, I think you should take InitAll out of
HandleLaunchCommand, and call it from StartupBrowserCreatorImpl. I feel as
though initializing the AppListService and handling the command line are two
separate steps which you may wish to do either of without the other (and the
name HandleLaunchCommand doesn't imply it is also going to initialize
AppListService regardless of the launch command).

You should therefore DCHECK that |service| is not NULL after Getting it.

[tapted, 2014/07/07 07:52:34]

Ahhh, that's the thing - ideally, HandleLaunchCommand _shouldn't_ call Init
regardless. `Init` should only happen for each Service once. On Windows and
Linux it currently happens for each `Launch`, and that's partly why all this
misinterpretation and the original failed DCHECK started in the first place. I
think it's a bug, and is possibly making some of the other data screwy.

Encapsulating things in HandleLaunchCommand lets this be cleaned up, but
StartupBrowserCreatorImpl shouldn't care about how that happens.
Done.

+  if (!command_line.HasSwitch(switches::kShowAppList))
+    return false;
 
-  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);
+  // 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);
+  service->ShowForProfile(launch_profile);
+  return true;
 }