Convert Extension* to extension id in AppLaunchParams.

Having raw Extension pointers is problematic because an extension may be
reloaded during the lifetime of AppLaunchParams, which will invalidate the
pointer.

This also converts CommandLine* to CommandLine and lets the empty command
line represent "no command line".

BUG=323220

Review URL: https://codereview.chromium.org/114263005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241143 0039d316-1c4b-4281-b951-d872f2087c98

apps/app_load_service.cc

@@ -99,7 +99,7 @@ void AppLoadService::Observe(int type,
           break;
         case LAUNCH_WITH_COMMAND_LINE:
           LaunchPlatformAppWithCommandLine(
-              profile_, extension, &it->second.command_line,
+              profile_, extension, it->second.command_line,
               it->second.current_dir);
           break;
         default:

apps/launcher.cc

@@ -83,13 +83,13 @@ bool MakePathAbsolute(const base::FilePath& current_directory,
   return true;
 }
 
-bool GetAbsolutePathFromCommandLine(const CommandLine* command_line,
+bool GetAbsolutePathFromCommandLine(const CommandLine& command_line,
                                     const base::FilePath& current_directory,
                                     base::FilePath* path) {
-  if (!command_line || !command_line->GetArgs().size())
+  if (!command_line.GetArgs().size())
     return false;
 
-  base::FilePath relative_path(command_line->GetArgs()[0]);
+  base::FilePath relative_path(command_line.GetArgs()[0]);
   base::FilePath absolute_path(relative_path);
   if (!MakePathAbsolute(current_directory, &absolute_path)) {
     LOG(WARNING) << "Cannot make absolute path from " << relative_path.value();
@@ -312,7 +312,7 @@ class PlatformAppPathLauncher
 
 void LaunchPlatformAppWithCommandLine(Profile* profile,
                                       const Extension* extension,
-                                      const CommandLine* command_line,
+                                      const CommandLine& command_line,
                                       const base::FilePath& current_directory) {
   if (!AppsClient::Get()->CheckAppLaunch(profile, extension))
     return;
@@ -356,7 +356,10 @@ void LaunchPlatformAppWithPath(Profile* profile,
 }
 
 void LaunchPlatformApp(Profile* profile, const Extension* extension) {
-  LaunchPlatformAppWithCommandLine(profile, extension, NULL, base::FilePath());
+  LaunchPlatformAppWithCommandLine(profile,
+                                   extension,
+                                   CommandLine(CommandLine::NO_PROGRAM),
+                                   base::FilePath());
 }
 
 void LaunchPlatformAppWithFileHandler(Profile* profile,

apps/launcher.h

@@ -23,11 +23,11 @@ namespace apps {
 
 // Launches the platform app |extension|. Creates appropriate launch data for
 // the |command_line| fields present. |extension| and |profile| must not be
-// NULL. A NULL |command_line| means there is no launch data. If non-empty,
+// NULL. An empty |command_line| means there is no launch data. If non-empty,
 // |current_directory| is used to expand any relative paths on the command line.
 void LaunchPlatformAppWithCommandLine(Profile* profile,
                                       const extensions::Extension* extension,
-                                      const CommandLine* command_line,
+                                      const CommandLine& command_line,
                                       const base::FilePath& current_directory);
 
 // Launches the platform app |extension| by issuing an onLaunched event

chrome/browser/apps/app_browsertest.cc

@@ -524,7 +524,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, LaunchWithRelativeFile) {
   // Run the test
   AppLaunchParams params(
       browser()->profile(), extension, LAUNCH_CONTAINER_NONE, NEW_WINDOW);
-  params.command_line = CommandLine::ForCurrentProcess();
+  params.command_line = *CommandLine::ForCurrentProcess();
   params.current_directory = test_data_dir_;
   OpenApplication(params);
 

chrome/browser/extensions/api/socket/socket_apitest.cc

@@ -87,7 +87,7 @@ class SocketPpapiTest : public SocketApiTest {
                            extension,
                            extensions::LAUNCH_CONTAINER_NONE,
                            NEW_WINDOW);
-    params.command_line = CommandLine::ForCurrentProcess();
+    params.command_line = *CommandLine::ForCurrentProcess();
     OpenApplication(params);
   }
 

chrome/browser/extensions/extension_apitest.cc

@@ -376,7 +376,7 @@ bool ExtensionApiTest::RunExtensionTestImpl(const std::string& extension_name,
                            extension,
                            extensions::LAUNCH_CONTAINER_NONE,
                            NEW_WINDOW);
-    params.command_line = CommandLine::ForCurrentProcess();
+    params.command_line = *CommandLine::ForCurrentProcess();
     OpenApplication(params);
   }
 

chrome/browser/ui/extensions/application_launch.cc

@@ -118,6 +118,20 @@ class EnableViaAppListFlow : public ExtensionEnableFlowDelegate {
   DISALLOW_COPY_AND_ASSIGN(EnableViaAppListFlow);
 };
 
+const Extension* GetExtension(const AppLaunchParams& params) {
+  if (params.extension_id.empty())
+    return NULL;
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(params.profile)->extension_service();
+  const Extension* extension = service->GetExtensionById(
+      params.extension_id,
+      ExtensionService::INCLUDE_ENABLED | ExtensionService::INCLUDE_DISABLED |
+          ExtensionService::INCLUDE_TERMINATED);
+  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 +187,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 +238,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 +332,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 +404,25 @@ AppLaunchParams::AppLaunchParams(Profile* profile,
                                  extensions::LaunchContainer container,
                                  WindowOpenDisposition disposition)
     : profile(profile),
-      extension(extension),
+      extension_id(extension ? extension->id() : std::string()),
       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() : std::string()),
       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 +438,13 @@ AppLaunchParams::AppLaunchParams(Profile* profile,
                                  int event_flags,
                                  chrome::HostDesktopType desktop_type)
     : profile(profile),
-      extension(extension),
+      extension_id(extension ? extension->id() : std::string()),
       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) {
@@ -445,13 +462,18 @@ AppLaunchParams::AppLaunchParams(Profile* profile,
   }
 }
 
+AppLaunchParams::~AppLaunchParams() {
+}
+
 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();

chrome/browser/ui/extensions/application_launch.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_EXTENSIONS_APPLICATION_LAUNCH_H_
 #define CHROME_BROWSER_UI_EXTENSIONS_APPLICATION_LAUNCH_H_
 
+#include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -45,11 +46,13 @@ struct AppLaunchParams {
                   int event_flags,
                   chrome::HostDesktopType desktop_type);
 
+  ~AppLaunchParams();
+
   // The profile to load the application from.
   Profile* profile;
 
   // The extension to load.
-  const extensions::Extension* extension;
+  std::string extension_id;
 
   // The container type to launch the application in.
   extensions::LaunchContainer container;
@@ -67,9 +70,9 @@ struct AppLaunchParams {
   // position and dimensions.
   gfx::Rect override_bounds;
 
-  // If non-NULL, information from the command line may be passed on to the
+  // If non-empty, information from the command line may be passed on to the
   // application.
-  const CommandLine* command_line;
+  CommandLine command_line;
 
   // If non-empty, the current directory from which any relative paths on the
   // command line should be expanded from.

chrome/browser/ui/startup/startup_browser_creator_impl.cc

@@ -371,7 +371,7 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
       RecordCmdLineAppHistogram(extensions::Manifest::TYPE_PLATFORM_APP);
       AppLaunchParams params(profile, extension,
                              extensions::LAUNCH_CONTAINER_NONE, NEW_WINDOW);
-      params.command_line = &command_line_;
+      params.command_line = command_line_;
       params.current_directory = cur_dir_;
       OpenApplicationWithReenablePrompt(params);
       return true;
@@ -515,7 +515,7 @@ bool StartupBrowserCreatorImpl::OpenApplicationWindow(
     RecordCmdLineAppHistogram(extension->GetType());
 
     AppLaunchParams params(profile, extension, launch_container, NEW_WINDOW);
-    params.command_line = &command_line_;
+    params.command_line = command_line_;
     params.current_directory = cur_dir_;
     WebContents* tab_in_app_window = OpenApplication(params);
 

chrome/browser/ui/views/app_list/win/app_list_controller_delegate_win.cc

@@ -51,7 +51,7 @@ void AppListControllerDelegateWin::FillLaunchParams(AppLaunchParams* params) {
   params->desktop_type = chrome::HOST_DESKTOP_TYPE_NATIVE;
   apps::ShellWindow* any_existing_window =
       apps::ShellWindowRegistry::Get(params->profile)->
-          GetCurrentShellWindowForApp(params->extension->id());
+          GetCurrentShellWindowForApp(params->extension_id);
   if (any_existing_window &&
       chrome::GetHostDesktopTypeForNativeWindow(
           any_existing_window->GetNativeWindow())