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

chrome/browser/web_applications/web_app_mac.mm

 // 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.
 
 #import "chrome/browser/web_applications/web_app_mac.h"
 
 #import <Carbon/Carbon.h>
 #import <Cocoa/Cocoa.h>
 
 #include "base/command_line.h"
@@ skipping 177 matching lines @@
 }
 
 bool WebAppShortcutCreator::CreateShortcut() {
   base::FilePath app_path = GetShortcutPath();
   base::FilePath app_name = app_path.BaseName();
   base::FilePath dst_path = app_path.DirName();
   if (app_path.empty() || !file_util::DirectoryExists(dst_path.DirName())) {
     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;
   }
 
   base::ScopedTempDir scoped_temp_dir;
   if (!scoped_temp_dir.CreateUniqueTempDir())
     return false;
   base::FilePath staging_path = scoped_temp_dir.path().Append(app_name);
 
   // Update the app's plist and icon in a temp directory. This works around
   // 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;
   }
 
   if (!UpdatePlist(staging_path))
     return false;
 
   if (!UpdateDisplayName(staging_path))
     return false;
 
   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;
 }
 
 base::FilePath WebAppShortcutCreator::GetAppLoaderPath() const {
   return base::mac::PathForFrameworkBundleResource(
       base::mac::NSToCFCast(@"app_mode_loader.app"));
 }
 
@@ skipping 134 matching lines @@
     const base::FilePath& generated_bundle) const {
   [[NSWorkspace sharedWorkspace]
                     selectFile:base::mac::FilePathToNSString(generated_bundle)
       inFileViewerRootedAtPath:nil];
 }
 
 void LaunchShimOnFileThread(
       const ShellIntegration::ShortcutInfo& shortcut_info) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
   base::FilePath shim_path = web_app::GetAppInstallPath(shortcut_info);
   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);
 }
 
 }  // namespace
 
 namespace web_app {
 
 base::FilePath GetAppInstallPath(
@@ skipping 58 matching lines @@
     const base::FilePath& web_app_path,
     const string16& old_app_title,
     const ShellIntegration::ShortcutInfo& shortcut_info) {
   // TODO(benwells): Implement this when shortcuts / weblings are enabled on
   // mac.
 }
 
 }  // namespace internals
 
 }  // namespace web_app