desktop-pwas: Don't DCHECK launcher update result

This change removes a DCHECK from the result of updating Progressive
Web App (PWA) launchers, to prevent debug builds of Chrome from
crashing when an update fails.

PWA launcher update could fail for any of the following reasons:
* Latest launcher version path does not exist
* Failed to create a temporary directory
* Failed to rename any launcher path to "_old"
* Failed to create a hardlink/copy of the latest launcher

Because some of the above failures are file operations that could
reasonably fail, a DCHECK on their success is misleading as it implies
"this should never happen".

A more appropriate way to handle update failure is to stop the update
(without crashing) and emit a UMA metric indicating the failure
reason. This change adds TODOs for that as a next step.

Bug: 1067241
Change-Id: If2f2145fc62619618917c80f26234930d3a26d0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134584Reviewed-by: Olivier Yiptong <oyiptong@chromium.org>
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Cr-Commit-Position: refs/heads/master@{#756831}

chrome/browser/web_applications/chrome_pwa_launcher/launcher_update.cc

@@ -43,16 +43,20 @@ void DeleteHardLinkOrCopyCallback(const base::FilePath& launcher_path,
 // to or copy of |latest_version_path| at |launcher_path|. Makes a best-effort
 // attempt to delete |old_path|. Aside from the best-effort deletion, all
 // changes are rolled back if any step fails.
-bool ReplaceLauncherWithLatestVersion(const base::FilePath& launcher_path,
+void ReplaceLauncherWithLatestVersion(const base::FilePath& launcher_path,
                                       const base::FilePath& latest_version_path,
                                       const base::FilePath& old_path) {
-  if (!base::PathExists(latest_version_path))
-    return false;
+  if (!base::PathExists(latest_version_path)) {
+    // TODO(jessemckenna): emit failure metric to UMA.
+    return;
+  }
 
   // Create a temporary backup directory for use while moving in-use files.
   base::ScopedTempDir temp_dir;
-  if (!temp_dir.CreateUniqueTempDirUnderPath(launcher_path.DirName()))
-    return false;
+  if (!temp_dir.CreateUniqueTempDirUnderPath(launcher_path.DirName())) {
+    // TODO(jessemckenna): emit failure metric to UMA.
+    return;
+  }
 
   // Move |launcher_path| to |old_path|.
   std::unique_ptr<WorkItemList> change_list(WorkItem::CreateWorkItemList());
@@ -76,9 +80,8 @@ bool ReplaceLauncherWithLatestVersion(const base::FilePath& launcher_path,
 
   if (!change_list->Do()) {
     change_list->Rollback();
-    return false;
+    // TODO(jessemckenna): emit failure metric to UMA.
   }
-  return true;
 }
 
 // Deletes |old_path| and any variations on it (e.g., |old_path| (1), |old_path|
@@ -123,9 +126,7 @@ void UpdatePwaLaunchers(std::vector<base::FilePath> launcher_paths) {
 
     // Make a hardlink or copy of |latest_version_path|, and replace the current
     // launcher with it.
-    const bool did_update =
-        ReplaceLauncherWithLatestVersion(path, latest_version_path, old_path);
-    DCHECK(did_update);
+    ReplaceLauncherWithLatestVersion(path, latest_version_path, old_path);
   }
 }