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 825e90814b525e65435f160cdcebf37a96668628..db4d3cea14e263dcc3dcb9a7484b86377e437468 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -159,6 +159,16 @@ bool HasExistingExtensionShim(const base::FilePath& destination_directory,
   return false;
 }
 
+void DeleteShortcut(base::FilePath app_path) {

[tapted, 2013/06/18 06:54:59]

const reference

[tapted, 2013/06/18 06:54:59]

Maybe a more descriptive name, to distinguish it from the member function. E.g.
DeletePathAndParentIfEmpty

[jackhou1, 2013/06/18 08:32:40]

Done.

[jackhou1, 2013/06/18 08:32:40]

Done.

+  if (app_path.empty())
+    return;
+
+  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 {
@@ -168,7 +178,7 @@ const char kChromeAppDirName[] = "Chrome Apps.localized";
 WebAppShortcutCreator::WebAppShortcutCreator(
     const base::FilePath& user_data_dir,
     const ShellIntegration::ShortcutInfo& shortcut_info,
-    const string16& chrome_bundle_id)
+    const std::string& chrome_bundle_id)
     : user_data_dir_(user_data_dir),
       info_(shortcut_info),
       chrome_bundle_id_(chrome_bundle_id) {
@@ -177,34 +187,14 @@ 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::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(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);
-
+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)) {
@@ -222,17 +212,83 @@ 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";
+  return true;
+}
+
+bool WebAppShortcutCreator::CreateShortcutsIn(
+    const std::vector<base::FilePath>& folders) const {
+  base::ScopedTempDir scoped_temp_dir;
+  if (!scoped_temp_dir.CreateUniqueTempDir())
+    return false;
+
+  base::FilePath app_name = GetShortcutName();
+  base::FilePath staging_path =
+      scoped_temp_dir.path().Append(app_name);
+  if (!BuildShortcut(staging_path))
     return false;
+
+  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;
+    }
+
+    base::mac::RemoveQuarantineAttribute(dst_path.Append(app_name));
   }
 
-  base::mac::RemoveQuarantineAttribute(app_path);
-  RevealGeneratedBundleInFinder(app_path);
+  return true;
+}
 
+bool WebAppShortcutCreator::CreateShortcut() {
+  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;
+  }

[tapted, 2013/06/18 06:54:59]

nit: blank line after for early return

[jackhou1, 2013/06/18 08:32:40]

Done.

+  if (!file_util::CreateDirectory(dst_path)) {
+    LOG(ERROR) << "Creating directory " << dst_path.value() << " failed.";
+    return false;
+  }
+
+  std::vector<base::FilePath> paths;
+  paths.push_back(dst_path);
+  if (!CreateShortcutsIn(paths))
+    return false;
+
+  RevealAppShimInFinder();
   return true;
 }
 
+void WebAppShortcutCreator::DeleteShortcuts() {
+  base::FilePath dst_path = GetDestinationPath();
+  if (!dst_path.empty())
+    DeleteShortcut(dst_path.Append(GetShortcutName()));
+  // In case the user has moved/renamed/copied the app bundle.
+  DeleteShortcut(GetAppBundleById(GetBundleIdentifier()));
+}
+
+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());
+
+  if (app_path.empty())
+    return false;
+
+  file_util::Delete(app_path, true);
+
+  std::vector<base::FilePath> paths;
+  paths.push_back(app_path.DirName());
+  return CreateShortcutsIn(paths);
+}
+
 base::FilePath WebAppShortcutCreator::GetAppLoaderPath() const {
   return base::mac::PathForFrameworkBundleResource(
       base::mac::NSToCFCast(@"app_mode_loader.app"));
@@ -249,7 +305,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,
@@ -280,7 +336,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(user_data_dir_)
             forKey:app_mode::kCrAppModeUserDataDirKey];
@@ -356,25 +412,44 @@ 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];
+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);
+  base::mac::ScopedCFTypeRef<CFURLRef> url(url_ref);

[tapted, 2013/06/18 06:54:59]

move this below checking status != noErr?

[jackhou1, 2013/06/18 08:32:40]

Done.

+
+  if (status != noErr)
+    return base::FilePath();
+
+  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 {
+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];
 }
 
@@ -398,8 +473,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) {
@@ -413,51 +490,33 @@ 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& web_app_path,
     const ShellIntegration::ShortcutInfo& shortcut_info,
-    const ShellIntegration::ShortcutLocations& /*creation_locations*/) {
+    const ShellIntegration::ShortcutLocations& creation_locations) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
-  string16 bundle_id = UTF8ToUTF16(base::mac::BaseBundleID());
-  WebAppShortcutCreator shortcut_creator(web_app_path, shortcut_info,
-                            bundle_id);
+  WebAppShortcutCreator shortcut_creator(
+      web_app_path, shortcut_info, base::mac::BaseBundleID());
   return shortcut_creator.CreateShortcut();
 }
 
 void DeletePlatformShortcuts(
     const base::FilePath& web_app_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(
+      web_app_path, shortcut_info, base::mac::BaseBundleID());
+  shortcut_creator.DeleteShortcuts();
 }
 
 void UpdatePlatformShortcuts(
     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.
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  WebAppShortcutCreator shortcut_creator(
+      web_app_path, shortcut_info, base::mac::BaseBundleID());
+  shortcut_creator.UpdateShortcuts();
 }
 
 }  // namespace internals