Create a copy of each app's shim bundle in the app's app_data_path.

chrome/browser/web_applications/web_app_mac.mm

Index: chrome/browser/web_applications/web_app_mac.mm
diff --git a/chrome/browser/web_applications/web_app_mac.mm b/chrome/browser/web_applications/web_app_mac.mm
index 825e90814b525e65435f160cdcebf37a96668628..ec571cd3588772304ab0d06dc4b3412a0d33d6d8 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -195,6 +195,11 @@ bool WebAppShortcutCreator::CreateShortcut() {
     LOG(ERROR) << "Couldn't find an Applications directory to copy app to.";
     return false;
   }
+  if (!file_util::CreateDirectory(user_data_dir_)) {
+    LOG(ERROR) << "Creating user_data_dir " << user_data_dir_.value()

[tapted, 2013/06/11 06:15:12]

I think this block can be omitted -- user_data_dir_ should always exist.

[jackhou1, 2013/06/12 06:54:46]

The user_data_dir_ here belongs to the app. I.e. <user_data_dir>/Web
Applications/_crx_<extension_id>/. I don't think anything creates it on Mac at
the moment.

[tapted, 2013/06/12 08:26:39]

Eep. Let's fix the name. And the header comment -- it starts off OK, and then
says "Note, the user data directory is the parent of the profile directory.".
Which is actually what means 'intrinsically', so the name shouldn't be used for
other things.

The variable is also in the wrong place -- it should come after all the
functions.

Maybe web_app_path_ -- this is what the argument is in the
{Create,Delete,Update}PlatformShortcuts functions.

Don't forget to update how it's referred to in the CL description, too.

[jackhou1, 2013/06/12 09:05:18]

Done.

+               << " failed.";
+    return false;
+  }
   if (!file_util::CreateDirectory(dst_path)) {
     LOG(ERROR) << "Creating directory " << dst_path.value() << " failed.";
     return false;
@@ -209,7 +214,7 @@ bool WebAppShortcutCreator::CreateShortcut() {
   // a Finder bug where the app's icon doesn't properly update.
   if (!file_util::CopyDirectory(GetAppLoaderPath(), staging_path, true)) {
     LOG(ERROR) << "Copying app to staging path: " << staging_path.value()
-               << " failed";
+               << " failed.";
     return false;
   }
 
@@ -222,12 +227,22 @@ bool WebAppShortcutCreator::CreateShortcut() {
   if (!UpdateIcon(staging_path))
     return false;
 
-  if (!file_util::CopyDirectory(staging_path, dst_path, true)) {
-    LOG(ERROR) << "Copying app to dst path: " << dst_path.value() << " failed";
+  // Put one copy in the app's user_data_dir so we can still run it if the user
+  // deletes the one in the applications folder.
+  if (!file_util::CopyDirectory(staging_path, user_data_dir_, true)) {
+    LOG(ERROR) << "Copying app to user_data_dir: " << user_data_dir_.value()

[tapted, 2013/06/11 06:15:12]

Here, I think `NOTREACHED(); return false;` -- web_app_win.cc behaves like this
in its `CreateShortcutsInPaths`.

[jackhou1, 2013/06/12 06:54:46]

Done.

+               << " failed.";
     return false;
   }
+  base::mac::RemoveQuarantineAttribute(user_data_dir_.Append(app_name));
 
+  if (!file_util::CopyDirectory(staging_path, dst_path, true)) {
+    LOG(ERROR) << "Copying app to applications directory: " << dst_path.value()

[tapted, 2013/06/11 06:15:12]

Logging probably isn't desirable here either -- nobody will see it. `return
false` seems to be the technique from web_app_win.cc.

I think if we start getting bug reports, or add some UMA to check the return
value of CreateShortcutsOnFileThread (rather than base::IngoreResult), and see
problems, then we can propagate up an error string to a dialog the way extension
install does.

[jackhou1, 2013/06/12 06:54:46]

Done.

+               << " failed.";
+    return false;
+  }
   base::mac::RemoveQuarantineAttribute(app_path);
+
   RevealGeneratedBundleInFinder(app_path);
 
   return true;
@@ -385,6 +400,14 @@ void LaunchShimOnFileThread(
   if (shim_path.empty())

[tapted, 2013/06/11 06:15:12]

can this still be true for cases we can recover from (and still care about)?
E.g. /Applications and ~/Applications not writeable, but the user data dir is? I
think we can just merge this into the if statement below.

[jackhou1, 2013/06/12 06:54:46]

Done.

     return;
 
+  // If it's not there, the user may have deleted it, use the one in the app's
+  // user_data_dir.

[tapted, 2013/06/11 06:15:12]

nit: maybe more correct to say "the web app's data dir". And, this might be
clearer inside the `if` block which essentially says the first and last parts of
the comment for you ;). E.g.

if (..) {
  // User may have deleted the copy in the Applications folder.
  base::.. 
}

[jackhou1, 2013/06/12 06:54:46]

Done.

+  if (!file_util::PathExists(shim_path)) {
+    base::FilePath shortcut_data_dir = GetWebAppDataDirectory(
+        shortcut_info.profile_path, shortcut_info.extension_id, GURL());
+    shim_path = shortcut_data_dir.Append(shim_path.BaseName());
+  }
+
   CommandLine command_line(CommandLine::NO_PROGRAM);
   command_line.AppendSwitch(app_mode::kNoLaunchApp);
   base::mac::OpenApplicationWithPath(shim_path, command_line, NULL);