Recreate shortcuts on app update.

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 224c798603cc20e2a178afb918ada8d01b04a7cf..59328ad9ec2a28b0ddff94d3632d6d754c102ba9 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -166,7 +166,16 @@ 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())
+
+  if (shim_path.empty() || !file_util::PathExists(shim_path)) {
+    // The user may have deleted the copy in the Applications folder, use the
+    // one in the web app's app_data_path.
+    base::FilePath app_data_path = web_app::GetWebAppDataDirectory(
+        shortcut_info.profile_path, shortcut_info.extension_id, GURL());
+    shim_path = app_data_path.Append(shim_path.BaseName());
+  }
+
+  if (!file_util::PathExists(shim_path))
     return;
 
   CommandLine command_line(CommandLine::NO_PROGRAM);
@@ -219,6 +228,14 @@ void UpdateAppShortcutsSubdirLocalizedName(
                  atomically:YES];
 }
 
+void DeletePathAndParentIfEmpty(const base::FilePath& app_path) {
+  DCHECK(!app_path.empty());
+  file_util::Delete(app_path, true);
+  base::FilePath apps_folder = app_path.DirName();
+  if (file_util::IsDirectoryEmpty(apps_folder))
+    file_util::Delete(apps_folder, false);
+}
+
 }  // namespace
 
 namespace web_app {
@@ -227,7 +244,7 @@ namespace web_app {
 WebAppShortcutCreator::WebAppShortcutCreator(
     const base::FilePath& app_data_path,
     const ShellIntegration::ShortcutInfo& shortcut_info,
-    const string16& chrome_bundle_id)
+    const std::string& chrome_bundle_id)
     : app_data_path_(app_data_path),
       info_(shortcut_info),
       chrome_bundle_id_(chrome_bundle_id) {
@@ -236,72 +253,137 @@ WebAppShortcutCreator::WebAppShortcutCreator(
 WebAppShortcutCreator::~WebAppShortcutCreator() {
 }
 
-base::FilePath WebAppShortcutCreator::GetShortcutPath() const {
-  base::FilePath dst_path = GetDestinationPath();
-  if (dst_path.empty())
-    return dst_path;
-
+base::FilePath WebAppShortcutCreator::GetShortcutName() const {
   base::FilePath app_name = internals::GetSanitizedFileName(UTF8ToUTF16(
       info_.profile_path.BaseName().value() + " " + info_.extension_id));
-  return dst_path.Append(app_name.ReplaceExtension("app"));
+  return app_name.ReplaceExtension("app");
+}
+
+bool WebAppShortcutCreator::BuildShortcut(
+    const base::FilePath& staging_path) const {
+  // 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.";
+    return false;
+  }
+
+  if (!UpdatePlist(staging_path))
+    return false;
+
+  if (!UpdateDisplayName(staging_path))
+    return false;
+
+  if (!UpdateIcon(staging_path))
+    return false;
+
+  return true;
 }
 
-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())) {
+size_t WebAppShortcutCreator::CreateShortcutsIn(
+    const std::vector<base::FilePath>& folders) const {
+  size_t succeeded = 0;
+
+  base::ScopedTempDir scoped_temp_dir;
+  if (!scoped_temp_dir.CreateUniqueTempDir())
+    return false;

[tapted, 2013/06/19 08:29:40]

false -> 0

[jackhou1, 2013/06/19 12:49:04]

Done.

+
+  base::FilePath app_name = GetShortcutName();
+  base::FilePath staging_path =
+      scoped_temp_dir.path().Append(app_name);
+  if (!BuildShortcut(staging_path))
+    return false;

[tapted, 2013/06/19 08:29:40]

false -> 0

[jackhou1, 2013/06/19 12:49:04]

Done.

+
+  for (std::vector<base::FilePath>::const_iterator it = folders.begin();
+       it != folders.end(); ++it) {
+    const base::FilePath& dst_path = *it;
+    if (!file_util::CopyDirectory(staging_path, dst_path, true)) {
+      LOG(ERROR) << "Copying app to dst path: " << dst_path.value()
+                 << " failed";
+      return false;

[tapted, 2013/06/19 08:29:40]

this should be `return succeeded`

[jackhou1, 2013/06/19 12:49:04]

Done.

+    }
+
+    base::mac::RemoveQuarantineAttribute(dst_path.Append(app_name));
+    ++succeeded;
+  }
+
+  return succeeded;
+}
+
+bool WebAppShortcutCreator::CreateShortcuts() {
+  base::FilePath dst_path = GetDestinationPath();
+  if (dst_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(app_data_path_)) {
     LOG(ERROR) << "Creating app_data_path " << app_data_path_.value()
                << " failed.";
     return false;
   }
+
   if (!file_util::CreateDirectory(dst_path)) {
     LOG(ERROR) << "Creating directory " << dst_path.value() << " failed.";
     return false;
   }
+
   UpdateAppShortcutsSubdirLocalizedName(dst_path);
 
-  base::ScopedTempDir scoped_temp_dir;
-  if (!scoped_temp_dir.CreateUniqueTempDir())
+  std::vector<base::FilePath> paths;
+  paths.push_back(app_data_path_);
+  paths.push_back(dst_path);
+  size_t succeeded = CreateShortcutsIn(paths);

[tapted, 2013/06/19 08:29:40]

nit: maybe num_succeeded or success_count, to make it less booly

[jackhou1, 2013/06/19 12:49:04]

Done.

+  if (!succeeded)

[tapted, 2013/06/19 08:29:40]

nit: I think it's typically preferred to vs 0 for numbers (i.e. succeeded == 0)

[jackhou1, 2013/06/19 12:49:04]

Done.

     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.";
-    return false;
-  }
+  UpdateInternalBundleIdentifier();
 
-  if (!UpdatePlist(staging_path))
+  if (succeeded != paths.size())
     return false;
 
-  if (!UpdateDisplayName(staging_path))
-    return false;
+  RevealAppShimInFinder();
+  return true;
+}
 
-  if (!UpdateIcon(staging_path))
-    return false;
+void WebAppShortcutCreator::DeleteShortcuts() {
+  base::FilePath dst_path = GetDestinationPath();
+  if (!dst_path.empty())
+    DeletePathAndParentIfEmpty(dst_path.Append(GetShortcutName()));
 
-  // Put one copy in the app's app_data_path so we can still run it if the user
-  // deletes the one in the applications folder.
-  if (!file_util::CopyDirectory(staging_path, app_data_path_, true)) {
-    NOTREACHED();
-    return false;
+  // In case the user has moved/renamed/copied the app bundle.
+  base::FilePath bundle_path = GetAppBundleById(GetBundleIdentifier());
+  if (!bundle_path.empty())
+    file_util::Delete(bundle_path, true);
+
+  // Delete the internal one.
+  DeletePathAndParentIfEmpty(app_data_path_.Append(GetShortcutName()));
+}
+
+bool WebAppShortcutCreator::UpdateShortcuts() {
+  base::FilePath dst_path = GetDestinationPath();
+  base::FilePath app_path = dst_path.Append(GetShortcutName());
+
+  // If the path does not exist, check if a matching bundle can be found
+  // elsewhere.
+  if (dst_path.empty() || !file_util::PathExists(app_path))
+    app_path = GetAppBundleById(GetBundleIdentifier());
+
+  std::vector<base::FilePath> paths;
+  paths.push_back(app_data_path_);

[tapted, 2013/06/19 08:29:40]

Do we need to delete this folder, before updating it, like we do for app_path?

[jackhou1, 2013/06/19 12:49:04]

I think whatever reasons for deleting the other one apply to this too. I.e.
making sure we delete any files that should no longer be in the bundle.

+
+  if (!app_path.empty()) {
+    file_util::Delete(app_path, true);
+    paths.push_back(app_path.DirName());
   }
-  base::mac::RemoveQuarantineAttribute(app_data_path_.Append(app_name));
 
-  if (!file_util::CopyDirectory(staging_path, dst_path, true))
+  size_t succeeded = CreateShortcutsIn(paths);

[tapted, 2013/06/19 08:29:40]

nit: less booly

[jackhou1, 2013/06/19 12:49:04]

Done.

+  if (!succeeded)

[tapted, 2013/06/19 08:29:40]

nit: less booly

[jackhou1, 2013/06/19 12:49:04]

Done.

     return false;
 
-  base::mac::RemoveQuarantineAttribute(app_path);
-  RevealGeneratedBundleInFinder(app_path);
-
-  return true;
+  UpdateInternalBundleIdentifier();
+  return succeeded == paths.size() && !app_path.empty();
 }
 
 base::FilePath WebAppShortcutCreator::GetAppLoaderPath() const {
@@ -320,7 +402,7 @@ bool WebAppShortcutCreator::UpdatePlist(const base::FilePath& app_path) const {
   NSString* extension_id = base::SysUTF8ToNSString(info_.extension_id);
   NSString* extension_title = base::SysUTF16ToNSString(info_.title);
   NSString* extension_url = base::SysUTF8ToNSString(info_.url.spec());
-  NSString* chrome_bundle_id = base::SysUTF16ToNSString(chrome_bundle_id_);
+  NSString* chrome_bundle_id = base::SysUTF8ToNSString(chrome_bundle_id_);
   NSDictionary* replacement_dict =
       [NSDictionary dictionaryWithObjectsAndKeys:
           extension_id, app_mode::kShortcutIdPlaceholder,
@@ -351,7 +433,7 @@ bool WebAppShortcutCreator::UpdatePlist(const base::FilePath& app_path) const {
   }
 
   // 2. Fill in other values.
-  [plist setObject:GetBundleIdentifier(plist)
+  [plist setObject:base::SysUTF8ToNSString(GetBundleIdentifier())
             forKey:base::mac::CFToNSCast(kCFBundleIdentifierKey)];
   [plist setObject:base::mac::FilePathToNSString(app_data_path_)
             forKey:app_mode::kCrAppModeUserDataDirKey];
@@ -366,7 +448,8 @@ bool WebAppShortcutCreator::UpdatePlist(const base::FilePath& app_path) const {
   [plist setObject:base::mac::FilePathToNSString(app_name)
             forKey:base::mac::CFToNSCast(kCFBundleNameKey)];
 
-  return [plist writeToFile:plist_path atomically:YES];
+  return [plist writeToFile:plist_path
+                 atomically:YES];
 }
 
 bool WebAppShortcutCreator::UpdateDisplayName(
@@ -427,25 +510,60 @@ bool WebAppShortcutCreator::UpdateIcon(const base::FilePath& app_path) const {
   return icon_family.WriteDataToFile(resources_path.Append("app.icns"));
 }
 
-NSString* WebAppShortcutCreator::GetBundleIdentifier(NSDictionary* plist) const
-{
-  NSString* bundle_id_template =
-    base::mac::ObjCCast<NSString>(
-        [plist objectForKey:base::mac::CFToNSCast(kCFBundleIdentifierKey)]);
-  NSString* extension_id = base::SysUTF8ToNSString(info_.extension_id);
-  NSString* placeholder =
-      [NSString stringWithFormat:@"@%@@", app_mode::kShortcutIdPlaceholder];
-  NSString* bundle_id =
-      [bundle_id_template
-          stringByReplacingOccurrencesOfString:placeholder
-                                    withString:extension_id];
+bool WebAppShortcutCreator::UpdateInternalBundleIdentifier() const {
+  NSString* plist_path = base::mac::FilePathToNSString(
+      app_data_path_.Append(GetShortcutName())
+          .Append("Contents").Append("Info.plist"));
+  NSMutableDictionary* plist =
+      [NSMutableDictionary dictionaryWithContentsOfFile:plist_path];
+
+  [plist setObject:base::SysUTF8ToNSString(GetInternalBundleIdentifier())
+            forKey:base::mac::CFToNSCast(kCFBundleIdentifierKey)];
+  return [plist writeToFile:plist_path
+                 atomically:YES];
+}
+
+base::FilePath WebAppShortcutCreator::GetAppBundleById(
+    const std::string& bundle_id) const {
+  base::mac::ScopedCFTypeRef<CFStringRef> bundle_id_cf(
+      base::SysUTF8ToCFStringRef(bundle_id));
+  CFURLRef url_ref = NULL;
+  OSStatus status = LSFindApplicationForInfo(
+      kLSUnknownCreator, bundle_id_cf.get(), NULL, NULL, &url_ref);
+  if (status != noErr)
+    return base::FilePath();
+
+  base::mac::ScopedCFTypeRef<CFURLRef> url(url_ref);
+  NSString* path_string = [base::mac::CFToNSCast(url.get()) path];
+  return base::FilePath([path_string fileSystemRepresentation]);
+}
+
+std::string WebAppShortcutCreator::GetBundleIdentifier() const {
+  // Replace spaces in the profile path with hyphen.
+  std::string normalized_profile_path;
+  ReplaceChars(info_.profile_path.BaseName().value(),
+               " ", "-", &normalized_profile_path);
+
+  // This matches APP_MODE_APP_BUNDLE_ID in chrome/chrome.gyp.
+  std::string bundle_id =
+      chrome_bundle_id_ + std::string(".app.") +
+      normalized_profile_path + "-" + info_.extension_id;
+
   return bundle_id;
 }
 
-void WebAppShortcutCreator::RevealGeneratedBundleInFinder(
-    const base::FilePath& generated_bundle) const {
+std::string WebAppShortcutCreator::GetInternalBundleIdentifier() const {
+  return GetBundleIdentifier() + "-internal";
+}
+
+void WebAppShortcutCreator::RevealAppShimInFinder() const {
+  base::FilePath dst_path = GetDestinationPath();
+  if (dst_path.empty())
+    return;
+
+  base::FilePath app_path = dst_path.Append(GetShortcutName());
   [[NSWorkspace sharedWorkspace]
-                    selectFile:base::mac::FilePathToNSString(generated_bundle)
+                    selectFile:base::mac::FilePathToNSString(app_path)
       inFileViewerRootedAtPath:nil];
 }
 
@@ -453,8 +571,10 @@ base::FilePath GetAppInstallPath(
     const ShellIntegration::ShortcutInfo& shortcut_info) {
   WebAppShortcutCreator shortcut_creator(base::FilePath(),
                                          shortcut_info,
-                                         string16());
-  return shortcut_creator.GetShortcutPath();
+                                         std::string());
+  base::FilePath dst_path = shortcut_creator.GetDestinationPath();
+  return dst_path.empty() ?
+      base::FilePath() : dst_path.Append(shortcut_creator.GetShortcutName());
 }
 
 void MaybeLaunchShortcut(const ShellIntegration::ShortcutInfo& shortcut_info) {
@@ -468,51 +588,32 @@ void MaybeLaunchShortcut(const ShellIntegration::ShortcutInfo& shortcut_info) {
 
 namespace internals {
 
-base::FilePath GetAppBundleByExtensionId(std::string extension_id) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
-  // This matches APP_MODE_APP_BUNDLE_ID in chrome/chrome.gyp.
-  std::string bundle_id =
-      base::mac::BaseBundleID() + std::string(".app.") + extension_id;
-  base::mac::ScopedCFTypeRef<CFStringRef> bundle_id_cf(
-      base::SysUTF8ToCFStringRef(bundle_id));
-  CFURLRef url_ref = NULL;
-  OSStatus status = LSFindApplicationForInfo(
-      kLSUnknownCreator, bundle_id_cf.get(), NULL, NULL, &url_ref);
-  base::mac::ScopedCFTypeRef<CFURLRef> url(url_ref);
-
-  if (status != noErr)
-    return base::FilePath();
-
-  NSString* path_string = [base::mac::CFToNSCast(url.get()) path];
-  return base::FilePath([path_string fileSystemRepresentation]);
-}
-
 bool CreatePlatformShortcuts(
     const base::FilePath& app_data_path,
     const ShellIntegration::ShortcutInfo& shortcut_info,
     const ShellIntegration::ShortcutLocations& creation_locations) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
-  string16 bundle_id = UTF8ToUTF16(base::mac::BaseBundleID());
-  WebAppShortcutCreator shortcut_creator(app_data_path, shortcut_info,
-                            bundle_id);
-  return shortcut_creator.CreateShortcut();
+  WebAppShortcutCreator shortcut_creator(
+      app_data_path, shortcut_info, base::mac::BaseBundleID());
+  return shortcut_creator.CreateShortcuts();
 }
 
 void DeletePlatformShortcuts(
     const base::FilePath& app_data_path,
-    const ShellIntegration::ShortcutInfo& info) {
+    const ShellIntegration::ShortcutInfo& shortcut_info) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
-
-  base::FilePath bundle_path = GetAppBundleByExtensionId(info.extension_id);
-  file_util::Delete(bundle_path, true);
+  WebAppShortcutCreator shortcut_creator(
+      app_data_path, shortcut_info, base::mac::BaseBundleID());
+  shortcut_creator.DeleteShortcuts();
 }
 
 void UpdatePlatformShortcuts(
     const base::FilePath& app_data_path,
     const string16& old_app_title,
     const ShellIntegration::ShortcutInfo& shortcut_info) {
-  // TODO(benwells): Implement this when shortcuts / weblings are enabled on
-  // mac.
+  WebAppShortcutCreator shortcut_creator(
+      app_data_path, shortcut_info, base::mac::BaseBundleID());
+  shortcut_creator.UpdateShortcuts();
 }
 
 }  // namespace internals