Convert Extension* to extension id in AppLaunchParams.

chrome/browser/ui/extensions/application_launch.cc

Index: chrome/browser/ui/extensions/application_launch.cc
diff --git a/chrome/browser/ui/extensions/application_launch.cc b/chrome/browser/ui/extensions/application_launch.cc
index 629032384a430f491c60ef3ccab2413b7bcf7641..194af2e0287fad8b89c66cdffa045691cb88f0cd 100644
--- a/chrome/browser/ui/extensions/application_launch.cc
+++ b/chrome/browser/ui/extensions/application_launch.cc
@@ -118,6 +118,16 @@ class EnableViaAppListFlow : public ExtensionEnableFlowDelegate {
   DISALLOW_COPY_AND_ASSIGN(EnableViaAppListFlow);
 };
 
+const Extension* GetExtension(const AppLaunchParams& params) {
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(params.profile)->extension_service();
+  const Extension* extension = service->GetExtensionById(

[benwells, 2013/12/13 06:31:58]

Nit: There is a GetExtensionById variant that takes an include mask. Did you
consider using that instead of this one and then GetTerminatedExtension?

[koz (OOO until 15th September), 2013/12/16 00:21:45]

I didn't - that would be clearer. Done.

+      params.extension_id, true);
+  if (!extension)
+    extension = service->GetTerminatedExtension(params.extension_id);
+  return extension;
+}
+
 // Get the launch URL for a given extension, with optional override/fallback.
 // |override_url|, if non-empty, will be preferred over the extension's
 // launch url.
@@ -173,7 +183,7 @@ ui::WindowShowState DetermineWindowShowState(
 
 WebContents* OpenApplicationWindow(const AppLaunchParams& params) {
   Profile* const profile = params.profile;
-  const extensions::Extension* const extension = params.extension;
+  const Extension* const extension = GetExtension(params);
   const GURL url_input = params.override_url;
 
   DCHECK(!url_input.is_empty() || extension);
@@ -224,8 +234,9 @@ WebContents* OpenApplicationWindow(const AppLaunchParams& params) {
 }
 
 WebContents* OpenApplicationTab(const AppLaunchParams& launch_params) {
+  const Extension* extension = GetExtension(launch_params);
+  CHECK(extension);
   Profile* const profile = launch_params.profile;
-  const extensions::Extension* extension = launch_params.extension;
   WindowOpenDisposition disposition = launch_params.disposition;
 
   Browser* browser = chrome::FindTabbedBrowser(profile,
@@ -317,8 +328,10 @@ WebContents* OpenApplicationTab(const AppLaunchParams& launch_params) {
 }
 
 WebContents* OpenEnabledApplication(const AppLaunchParams& params) {
+  const Extension* extension = GetExtension(params);
+  if (!extension)
+    return NULL;
   Profile* profile = params.profile;
-  const extensions::Extension* extension = params.extension;
 
   WebContents* tab = NULL;
   ExtensionPrefs* prefs = extensions::ExtensionSystem::Get(profile)->
@@ -387,25 +400,25 @@ AppLaunchParams::AppLaunchParams(Profile* profile,
                                  extensions::LaunchContainer container,
                                  WindowOpenDisposition disposition)
     : profile(profile),
-      extension(extension),
+      extension_id(extension ? extension->id() : ""),

[tapted, 2013/12/13 05:16:47]

nit: "" -> std::string()  [it's much faster, since "" often needs a malloc()]

[koz (OOO until 15th September), 2013/12/16 00:21:45]

Done.

       container(container),
       disposition(disposition),
       desktop_type(chrome::GetActiveDesktop()),
       override_url(),
       override_bounds(),
-      command_line(NULL) {}
+      command_line(CommandLine::NO_PROGRAM) {}
 
 AppLaunchParams::AppLaunchParams(Profile* profile,
                                  const extensions::Extension* extension,
                                  WindowOpenDisposition disposition)
     : profile(profile),
-      extension(extension),
+      extension_id(extension ? extension->id() : ""),

[tapted, 2013/12/13 05:16:47]

I think this overload, and the next, actually can't be called with a NULL
extension: it looks like there are no checks leading up to GetAppLaunchInfo(..)
in app_launch_info.cc where it would dereference NULL.

So, I'd maybe leave these, so we die closer to the caller.

[koz (OOO until 15th September), 2013/12/16 00:21:45]

I think it's simpler if we just have extension_id.empty() be equivalent to
passing a NULL Extension* in the old code. Besides, we didn't use to have a
CHECK(extension) in the old constructors so I'd rather not introduce one now.

       container(extensions::LAUNCH_CONTAINER_NONE),
       disposition(disposition),
       desktop_type(chrome::GetActiveDesktop()),
       override_url(),
       override_bounds(),
-      command_line(NULL) {
+      command_line(CommandLine::NO_PROGRAM) {
   ExtensionService* service =
       extensions::ExtensionSystem::Get(profile)->extension_service();
   DCHECK(service);
@@ -421,13 +434,13 @@ AppLaunchParams::AppLaunchParams(Profile* profile,
                                  int event_flags,
                                  chrome::HostDesktopType desktop_type)
     : profile(profile),
-      extension(extension),
+      extension_id(extension ? extension->id() : ""),
       container(extensions::LAUNCH_CONTAINER_NONE),
       disposition(ui::DispositionFromEventFlags(event_flags)),
       desktop_type(desktop_type),
       override_url(),
       override_bounds(),
-      command_line(NULL) {
+      command_line(CommandLine::NO_PROGRAM) {
   if (disposition == NEW_FOREGROUND_TAB || disposition == NEW_BACKGROUND_TAB) {
     container = extensions::LAUNCH_CONTAINER_TAB;
   } else if (disposition == NEW_WINDOW) {
@@ -450,8 +463,10 @@ WebContents* OpenApplication(const AppLaunchParams& params) {
 }
 
 void OpenApplicationWithReenablePrompt(const AppLaunchParams& params) {
+  const Extension* extension = GetExtension(params);
+  if (!extension)
+    return;
   Profile* profile = params.profile;
-  const extensions::Extension* extension = params.extension;
 
   ExtensionService* service =
       extensions::ExtensionSystem::Get(profile)->extension_service();