MacPWAs: Fix multiple UpdateShortcuts calls happening at once

Our metrics showed high error rates for shortcut creation on
Mac.  While using the file handling feature (FileHandlingAPI),
I found that multiple CopyDirectory() calls could sometimes
happen at once which could trigger errors.

When the CopyDirectory() calls overlapped, one thread would be
deleting files while another thread creates them, and this
would lead to one CopyDirectory() call failing with an error.

This change adds a lock that UpdateShortcuts() acquires before
proceeding to ensure that its later CreateShortcutsAt() call
is protected against other threads.

Bug: 1090548
Change-Id: I6cb439975e3dd51608420a3f2e14de351b88d0b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250590
Auto-Submit: Chase Phillips <cmp@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779521}

chrome/browser/web_applications/components/web_app_shortcut_mac.mm

@@ -924,12 +924,28 @@ bool WebAppShortcutCreator::BuildShortcut(
   return result;
 }
 
+// Returns a reference to the static UpdateShortcuts lock.
+// See https://crbug.com/1090548 for more info.
+base::Lock& GetUpdateShortcutsLock() {
+  static base::NoDestructor<base::Lock> lock;
+  return *lock;
+}
+
 void WebAppShortcutCreator::CreateShortcutsAt(
     const std::vector<base::FilePath>& dst_app_paths,
     std::vector<base::FilePath>* updated_paths) const {
   DCHECK(updated_paths && updated_paths->empty());
   DCHECK(!dst_app_paths.empty());
 
+  // CreateShortcutsAt() modifies the app shim on disk, first by deleting
+  // the destination app shim (if it exists), then by copying a new app shim
+  // from the source app to the destination.  To ensure that process works,
+  // we must guarantee that no more than one CreateShortcutsAt() call will
+  // ever run at a time.  We have an UpdateShortcuts lock for this purpose,
+  // so check that lock has been acquired on this thread before proceeding.
+  // See https://crbug.com/1090548 for more info.
+  GetUpdateShortcutsLock().AssertAcquired();
+
   base::ScopedTempDir scoped_temp_dir;
   if (!scoped_temp_dir.CreateUniqueTempDir()) {
     RecordCreateShortcut(CreateShortcutResult::kFailToCreateTempDir);
@@ -1013,6 +1029,13 @@ bool WebAppShortcutCreator::UpdateShortcuts(
     }
   }
 
+  // Acquire the UpdateShortcuts lock.  This ensures only a single
+  // UpdateShortcuts call at a time will run at once past here.  Not
+  // protecting against that can result in multiple CreateShortcutsAt()
+  // calls deleting and creating the app shim folder at once.
+  // See https://crbug.com/1090548 for more info.
+  base::AutoLock auto_lock(GetUpdateShortcutsLock());
+
   // Get the list of paths to (re)create by bundle id (wherever it was moved
   // or copied by the user).
   std::vector<base::FilePath> app_paths = GetAppBundlesById();