Use ExtensionEnableFlow for shim initiated launches. (Mac)

apps/app_shim/extension_app_shim_handler_mac.cc

Index: apps/app_shim/extension_app_shim_handler_mac.cc
diff --git a/apps/app_shim/extension_app_shim_handler_mac.cc b/apps/app_shim/extension_app_shim_handler_mac.cc
index 3c08f2d4c8b2d6ddb76e8f598060dc5b072124cf..bc8384a73881b4c1b2e4f93abd831d21474cd184 100644
--- a/apps/app_shim/extension_app_shim_handler_mac.cc
+++ b/apps/app_shim/extension_app_shim_handler_mac.cc
@@ -20,6 +20,8 @@
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/extensions/extension_enable_flow.h"
+#include "chrome/browser/ui/extensions/extension_enable_flow_delegate.h"
 #include "chrome/browser/ui/web_applications/web_app_ui.h"
 #include "chrome/browser/ui/webui/ntp/core_app_launcher_handler.h"
 #include "chrome/browser/web_applications/web_app_mac.h"
@@ -73,6 +75,48 @@ bool FocusWindows(const ShellWindowList& windows) {
   return true;
 }
 
+// Attempts to launch a packaged app, prompting the user to enable it if
+// necessary. The prompt is shown in its own window.
+// This class manages its own lifetime.
+class EnableViaPrompt : public ExtensionEnableFlowDelegate {
+ public:
+  EnableViaPrompt(Profile* profile,
+                  const std::string& extension_id,
+                  const base::Callback<void()>& callback)
+      : profile_(profile),
+        extension_id_(extension_id),
+        callback_(callback) {
+  }
+
+  virtual ~EnableViaPrompt() {
+  }
+
+  void Run() {
+    flow_.reset(new ExtensionEnableFlow(profile_, extension_id_, this));
+    flow_->StartForCurrentlyNonexistentWindow(
+        base::Callback<gfx::NativeWindow(void)>());
+  }
+
+ private:
+  // ExtensionEnableFlowDelegate overrides.
+  virtual void ExtensionEnableFlowFinished() OVERRIDE {
+    callback_.Run();
+    delete this;
+  }
+
+  virtual void ExtensionEnableFlowAborted(bool user_initiated) OVERRIDE {
+    callback_.Run();
+    delete this;
+  }
+
+  Profile* profile_;
+  std::string extension_id_;
+  base::Callback<void()> callback_;
+  scoped_ptr<ExtensionEnableFlow> flow_;
+
+  DISALLOW_COPY_AND_ASSIGN(EnableViaPrompt);
+};
+
 }  // namespace
 
 namespace apps {
@@ -126,6 +170,13 @@ ExtensionAppShimHandler::Delegate::GetAppExtension(
   return extension && extension->is_platform_app() ? extension : NULL;
 }
 
+void ExtensionAppShimHandler::Delegate::EnableExtension(
+    Profile* profile,
+    const std::string& extension_id,
+    const base::Callback<void()>& callback) {
+  (new EnableViaPrompt(profile, extension_id, callback))->Run();
+}
+
 void ExtensionAppShimHandler::Delegate::LaunchApp(
     Profile* profile,
     const extensions::Extension* extension,
@@ -286,15 +337,6 @@ void ExtensionAppShimHandler::OnProfileLoaded(
     const std::vector<base::FilePath>& files,
     Profile* profile) {
   const std::string& app_id = host->GetAppId();
-  // TODO(jackhou): Add some UI for this case and remove the LOG.
-  const extensions::Extension* extension =
-      delegate_->GetAppExtension(profile, app_id);
-  if (!extension) {
-    LOG(ERROR) << "Attempted to launch nonexistent app with id '"
-               << app_id << "'.";
-    host->OnAppLaunchComplete(APP_SHIM_LAUNCH_APP_NOT_FOUND);
-    return;
-  }
 
   // The first host to claim this (profile, app_id) becomes the main host.
   // For any others, focus or relaunch the app.
@@ -307,16 +349,44 @@ void ExtensionAppShimHandler::OnProfileLoaded(
     return;
   }
 
+  if (launch_type != APP_SHIM_LAUNCH_NORMAL) {
+    host->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS);
+    return;
+  }
+
   // TODO(jeremya): Handle the case that launching the app fails. Probably we
   // need to watch for 'app successfully launched' or at least 'background page
   // exists/was created' and time out with failure if we don't see that sign of
   // life within a certain window.
-  if (launch_type == APP_SHIM_LAUNCH_NORMAL)
+  const extensions::Extension* extension =
+      delegate_->GetAppExtension(profile, app_id);
+  if (extension) {
     delegate_->LaunchApp(profile, extension, files);
-  else
-    host->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS);
+    return;
+  }
+
+  delegate_->EnableExtension(
+      profile, app_id,
+      base::Bind(&ExtensionAppShimHandler::OnExtensionEnabled,
+                 weak_factory_.GetWeakPtr(),
+                 host, profile, files));

[tapted, 2013/12/12 02:41:21]

profile.. might be OK. But is it safe to reference the Host* in a Closure and
copy it out of this stack? What happens if you have upgraded permissions and
`kill -TERM` the Shim while the dialog is still showing?

[jackhou1, 2013/12/13 03:47:10]

Done.

 }
 
+void ExtensionAppShimHandler::OnExtensionEnabled(
+    Host* host,
+    Profile* profile,
+    const std::vector<base::FilePath>& files) {
+  const extensions::Extension* extension =
+      delegate_->GetAppExtension(profile, host->GetAppId());
+  if (!extension) {
+    host->OnAppLaunchComplete(APP_SHIM_LAUNCH_APP_NOT_FOUND);
+    return;
+  }
+
+  delegate_->LaunchApp(profile, extension, files);
+}
+
+
 void ExtensionAppShimHandler::OnShimClose(Host* host) {
   // This might be called when shutting down. Don't try to look up the profile
   // since profile_manager might not be around.