Recreate shortcuts on app update.

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 210 matching lines @@
 
   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 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"));
 }
 
 base::FilePath WebAppShortcutCreator::GetDestinationPath() const {
@@ skipping 122 matching lines @@
   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];
   return bundle_id;
 }
 
-void WebAppShortcutCreator::RevealGeneratedBundleInFinder(
-    const base::FilePath& generated_bundle) const {
+void WebAppShortcutCreator::RevealAppShimInFinder() const {
   [[NSWorkspace sharedWorkspace]
-                    selectFile:base::mac::FilePathToNSString(generated_bundle)
+                    selectFile:base::mac::FilePathToNSString(GetShortcutPath())

[benwells, 2013/06/13 00:09:07]

GetShortcutPath can fail, so this should check it's result.

[jackhou1, 2013/06/13 01:47:42]

Done.

       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())
     return;
 
@@ skipping 18 matching lines @@
   if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableAppShims))
     return;
 
   content::BrowserThread::PostTask(
       content::BrowserThread::FILE, FROM_HERE,
       base::Bind(&LaunchShimOnFileThread, shortcut_info));
 }
 
 namespace internals {
 
 base::FilePath GetAppBundleByExtensionId(std::string extension_id) {

[benwells, 2013/06/13 00:09:07]

This function doesn't appear to be used anymore.

[jackhou1, 2013/06/13 01:47:42]

Removed.

   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*/) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
-  string16 bundle_id = UTF8ToUTF16(base::mac::BaseBundleID());
-  WebAppShortcutCreator shortcut_creator(web_app_path, shortcut_info,
-                            bundle_id);
-  return shortcut_creator.CreateShortcut();
+  WebAppShortcutCreator shortcut_creator(
+      web_app_path, shortcut_info, UTF8ToUTF16(base::mac::BaseBundleID()));
+  bool success = shortcut_creator.CreateShortcut();
+  if (success)
+    shortcut_creator.RevealAppShimInFinder();
+
+  return success;
 }
 
 void DeletePlatformShortcuts(
     const base::FilePath& web_app_path,
-    const ShellIntegration::ShortcutInfo& info) {
+    const ShellIntegration::ShortcutInfo& shortcut_info) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  WebAppShortcutCreator shortcut_creator(
+      web_app_path, shortcut_info, UTF8ToUTF16(base::mac::BaseBundleID()));
+  base::FilePath app_path = shortcut_creator.GetShortcutPath();
+  if (app_path.empty())
+    return;
 
-  base::FilePath bundle_path = GetAppBundleByExtensionId(info.extension_id);

[benwells, 2013/06/13 00:09:07]

What is the effect of replacing GetAppBundleByExtensionId with GetShortcutPath?
The previous function seemed to do some sort of search
(LSFindApplicationForInfo), does that mean it could update shims which had been
moved?

[jackhou1, 2013/06/13 01:47:42]

LSFindApplicationForInfo finds one app bundle that matches the bundle ID. I
couldn't find a similar function that returns all app bundles matching that ID.
Since we are going to have multiple copies of the same bundle, and they have
unique paths, it seems better to find them by path. If the user moves or renames
one, we stop updating it. An alternative would be to give each bundle a unique
ID, e.g. by appending the profile dir, and whether it's the visible or hidden
bundle. 

[benwells, 2013/06/13 02:19:31]

Your alternative sounds pretty good. Any reason not to do this? It would have
the advantage of updates being more robust, and removing shims after uninstall
being more robust as well.

-  file_util::Delete(bundle_path, true);
+  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);
 }
 
 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));

[benwells, 2013/06/13 00:09:07]

Make sure you handle the shim created in the web_app path as well. (i.e. update
after https://codereview.chromium.org/16304005/ lands, or update that patch
after this lands).

[jackhou1, 2013/06/13 01:47:42]

I'll merge the two CLs since they touch the same files.

+  WebAppShortcutCreator shortcut_creator(
+      web_app_path, shortcut_info, UTF8ToUTF16(base::mac::BaseBundleID()));
+  base::FilePath app_path = shortcut_creator.GetShortcutPath();
+  if (app_path.empty())
+    return;
+
+  file_util::Delete(app_path, true);
+  shortcut_creator.CreateShortcut();
 }
 
 }  // namespace internals
 
 }  // namespace web_app