Convert Extension* to extension id in AppLaunchParams.

chrome/browser/ui/extensions/application_launch.cc

 // Copyright (c) 2012 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/extensions/application_launch.h"
 
 #include <string>
 
 #include "apps/launcher.h"
 #include "base/command_line.h"
@@ skipping 100 matching lines @@
   ExtensionService* service_;
   Profile* profile_;
   chrome::HostDesktopType desktop_type_;
   std::string extension_id_;
   base::Closure callback_;
   scoped_ptr<ExtensionEnableFlow> flow_;
 
   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.
 GURL UrlForExtension(const Extension* extension,
                      const GURL& override_url) {
   if (!extension)
     return override_url;
 
   GURL url;
   if (!override_url.is_empty()) {
@@ skipping 35 matching lines @@
     return ui::SHOW_STATE_MAXIMIZED;
   else if (launch_type == extensions::LAUNCH_TYPE_WINDOW)
     return ui::SHOW_STATE_NORMAL;
 #endif
 
   return ui::SHOW_STATE_DEFAULT;
 }
 
 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);
   GURL url = UrlForExtension(extension, url_input);
   Browser::CreateParams browser_params(
       Browser::TYPE_POPUP, profile, params.desktop_type);
 
   browser_params.app_name = extension ?
       web_app::GenerateApplicationNameFromExtensionId(extension->id()) :
       web_app::GenerateApplicationNameFromURL(url);
@@ skipping 30 matching lines @@
 
   browser->window()->Show();
 
   // TODO(jcampan): http://crbug.com/8123 we should not need to set the initial
   //                focus explicitly.
   web_contents->GetView()->SetInitialFocus();
   return web_contents;
 }
 
 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,
                                                false,
                                                launch_params.desktop_type);
   WebContents* contents = NULL;
   if (!browser) {
     // No browser for this profile, need to open a new one.
     browser = new Browser(Browser::CreateParams(Browser::TYPE_TABBED,
                                                 profile,
@@ skipping 71 matching lines @@
       chrome::ToggleFullscreenWithChromeOrFallback(browser);
 #else
       chrome::ToggleFullscreenMode(browser);
 #endif
     }
   }
   return contents;
 }
 
 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)->
       extension_service()->extension_prefs();
   prefs->SetActiveBit(extension->id(), true);
 
   UMA_HISTOGRAM_ENUMERATION(
       "Extensions.AppLaunchContainer", params.container, 100);
 
   if (extension->is_platform_app()) {
@@ skipping 48 matching lines @@
   return tab;
 }
 
 }  // namespace
 
 AppLaunchParams::AppLaunchParams(Profile* profile,
                                  const extensions::Extension* extension,
                                  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);
 
   // Look up the app preference to find out the right launch container. Default
   // is to launch as a regular tab.
   container = extensions::GetLaunchContainer(
       service->extension_prefs(), extension);
 }
 
 AppLaunchParams::AppLaunchParams(Profile* profile,
                                  const extensions::Extension* extension,
                                  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) {
     container = extensions::LAUNCH_CONTAINER_WINDOW;
   } else {
     ExtensionService* service =
         extensions::ExtensionSystem::Get(profile)->extension_service();
     DCHECK(service);
 
     // Look at preference to find the right launch container.  If no preference
     // is set, launch as a regular tab.
     container = extensions::GetLaunchContainer(
         service->extension_prefs(), extension);
     disposition = NEW_FOREGROUND_TAB;
   }
 }
 
 WebContents* OpenApplication(const AppLaunchParams& params) {
   return OpenEnabledApplication(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();
   if (!service->IsExtensionEnabled(extension->id())) {
     (new EnableViaAppListFlow(
         service, profile, params.desktop_type, extension->id(),
         base::Bind(base::IgnoreResult(OpenEnabledApplication), params)))->Run();
     return;
   }
 
@@ skipping 20 matching lines @@
   // up in LoadingStateChanged to schedule a GetApplicationInfo. And when
   // the web app info is available, extensions::TabHelper notifies Browser via
   // OnDidGetApplicationInfo, which calls
   // web_app::UpdateShortcutForTabContents when it sees UPDATE_SHORTCUT as
   // pending web app action.
   extensions::TabHelper::FromWebContents(tab)->set_pending_web_app_action(
       extensions::TabHelper::UPDATE_SHORTCUT);
 
   return tab;
 }