Remove bare extension pointer from platform app launcher code.

The bare pointer could cause crashes if the extension is uninstalled (thus being deleted) while the launch logic is progressing on the FILE thread; storing an extension ID prevents this.

BUG=372270

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

Cr-Commit-Position: refs/heads/master@{#304993}

1 files changed, 42 insertions, 23 deletions

diff --git a/apps/launcher.cc b/apps/launcher.cc
index 25c3a5f..cda74ac 100644
--- a/apps/launcher.cc
+++ b/apps/launcher.cc
@@ -27,6 +27,7 @@
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/granted_file_entry.h"
 #include "extensions/browser/lazy_background_task_queue.h"
@@ -97,20 +98,25 @@ class PlatformAppPathLauncher
                           const Extension* extension,
                           const std::vector<base::FilePath>& file_paths)
       : profile_(profile),
-        extension_(extension),
+        extension_id(extension->id()),
         file_paths_(file_paths),
         collector_(profile) {}
 
   PlatformAppPathLauncher(Profile* profile,
                           const Extension* extension,
                           const base::FilePath& file_path)
-      : profile_(profile), extension_(extension), collector_(profile) {
+      : profile_(profile), extension_id(extension->id()), collector_(profile) {
     if (!file_path.empty())
       file_paths_.push_back(file_path);
   }
 
   void Launch() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+    const Extension* extension = GetExtension();
+    if (!extension)
+      return;
+
     if (file_paths_.empty()) {
       LaunchWithNoLaunchData();
       return;
@@ -120,7 +126,7 @@ class PlatformAppPathLauncher
       DCHECK(file_paths_[i].IsAbsolute());
     }
 
-    if (HasFileSystemWritePermission(extension_)) {
+    if (HasFileSystemWritePermission(extension)) {
       PrepareFilesForWritableApp(
           file_paths_,
           profile_,
@@ -186,13 +192,22 @@ class PlatformAppPathLauncher
   void LaunchWithNoLaunchData() {
     // This method is required as an entry point on the UI thread.
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+    const Extension* extension = GetExtension();
+    if (!extension)
+      return;
+
     AppRuntimeEventRouter::DispatchOnLaunchedEvent(
-        profile_, extension_, extensions::SOURCE_FILE_HANDLER);
+        profile_, extension, extensions::SOURCE_FILE_HANDLER);
   }
 
   void OnMimeTypesCollected(scoped_ptr<std::vector<std::string> > mime_types) {
     DCHECK(file_paths_.size() == mime_types->size());
 
+    const Extension* extension = GetExtension();
+    if (!extension)
+      return;
+
     // If fetching a mime type failed, then use a fallback one.
     for (size_t i = 0; i < mime_types->size(); ++i) {
       const std::string mime_type =
@@ -203,7 +218,7 @@ class PlatformAppPathLauncher
     // Find file handler from the platform app for the file being opened.
     const extensions::FileHandlerInfo* handler = NULL;
     if (!handler_id_.empty()) {
-      handler = FileHandlerForId(*extension_, handler_id_);
+      handler = FileHandlerForId(*extension, handler_id_);
       if (handler) {
         for (size_t i = 0; i < file_paths_.size(); ++i) {
           if (!FileHandlerCanHandleFile(
@@ -224,7 +239,7 @@ class PlatformAppPathLauncher
       }
       const std::vector<const extensions::FileHandlerInfo*>& handlers =
           extensions::app_file_handler_util::FindFileHandlersForFiles(
-              *extension_, path_and_file_type_set);
+              *extension, path_and_file_type_set);
       if (!handlers.empty())
         handler = handlers[0];
     }
@@ -247,10 +262,9 @@ class PlatformAppPathLauncher
     // call back to us.
     extensions::LazyBackgroundTaskQueue* const queue =
         ExtensionSystem::Get(profile_)->lazy_background_task_queue();
-    if (queue->ShouldEnqueueTask(profile_, extension_)) {
+    if (queue->ShouldEnqueueTask(profile_, extension)) {
       queue->AddPendingTask(
-          profile_,
-          extension_->id(),
+          profile_, extension_id,
           base::Bind(&PlatformAppPathLauncher::GrantAccessToFilesAndLaunch,
                      this));
       return;
@@ -259,39 +273,44 @@ class PlatformAppPathLauncher
     extensions::ProcessManager* const process_manager =
         extensions::ProcessManager::Get(profile_);
     ExtensionHost* const host =
-        process_manager->GetBackgroundHostForExtension(extension_->id());
+        process_manager->GetBackgroundHostForExtension(extension_id);
     DCHECK(host);
     GrantAccessToFilesAndLaunch(host);
   }
 
   void GrantAccessToFilesAndLaunch(ExtensionHost* host) {
+    const Extension* extension = GetExtension();
+    if (!extension)
+      return;
+
     // If there was an error loading the app page, |host| will be NULL.
     if (!host) {
-      LOG(ERROR) << "Could not load app page for " << extension_->id();
+      LOG(ERROR) << "Could not load app page for " << extension_id;
       return;
     }
 
     std::vector<GrantedFileEntry> file_entries;
     for (size_t i = 0; i < file_paths_.size(); ++i) {
-      file_entries.push_back(
-          CreateFileEntry(profile_,
-                          extension_,
-                          host->render_process_host()->GetID(),
-                          file_paths_[i],
-                          false));
+      file_entries.push_back(CreateFileEntry(
+          profile_, extension, host->render_process_host()->GetID(),
+          file_paths_[i], false));
     }
 
     AppRuntimeEventRouter::DispatchOnLaunchedEventWithFileEntries(
-        profile_, extension_, handler_id_, mime_types_, file_entries);
+        profile_, extension, handler_id_, mime_types_, file_entries);
+  }
+
+  const Extension* GetExtension() const {
+    return extensions::ExtensionRegistry::Get(profile_)->GetExtensionById(
+        extension_id, extensions::ExtensionRegistry::EVERYTHING);
   }
 
   // The profile the app should be run in.
   Profile* profile_;
-  // The extension providing the app.
-  // TODO(benwells): Hold onto the extension ID instead of a pointer as it
-  // is possible the extension will be unloaded while we're doing our thing.
-  // See http://crbug.com/372270 for details.
-  const Extension* extension_;
+  // The id of the extension providing the app. A pointer to the extension is
+  // not kept as the extension may be unloaded and deleted during the course of
+  // the launch.
+  const std::string extension_id;
   // The path to be passed through to the app.
   std::vector<base::FilePath> file_paths_;
   std::vector<std::string> mime_types_;