WebApps: Abort manifest update for invalid manifest files

This CL tests that we don't continue manifest updating when the site has
an invalid manifest.

Bug: 926083
Change-Id: I902b828e50f6ec3b32cf254cda9d55a0b1f2729f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831659Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701074}

chrome/browser/web_applications/components/manifest_update_manager_browsertest.cc

@@ -350,6 +350,26 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
             ManifestUpdateResult::kAppUpToDate);
 }
 
+IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
+                       CheckIgnoresInvalidManifest) {
+  const char* manifest_template = R"(
+    {
+      "name": "Test app name",
+      "start_url": ".",
+      "scope": "/",
+      "display": "standalone",
+      "icons": $1,
+      $2
+    }
+  )";
+  OverrideManifest(manifest_template, {kInstallableIconList, ""});
+  AppId app_id = InstallWebApp();
+  OverrideManifest(manifest_template, {kInstallableIconList,
+                                       "invalid manifest syntax !@#$%^*&()"});
+  EXPECT_EQ(GetResultAfterPageLoad(GetAppURL(), &app_id),
+            ManifestUpdateResult::kAppDataInvalid);
+}
+
 IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
                        CheckFindsThemeColorChange) {
   const char* manifest_template = R"(

chrome/browser/web_applications/components/manifest_update_task.cc

@@ -78,7 +78,13 @@ void ManifestUpdateTask::WebContentsDestroyed() {
 void ManifestUpdateTask::OnDidGetInstallableData(const InstallableData& data) {
   DCHECK_EQ(stage_, Stage::kPendingInstallableData);
 
-  if (!IsUpdateNeededForInstallableData(data)) {
+  if (!data.errors.empty()) {
+    DestroySelf(ManifestUpdateResult::kAppDataInvalid);
+    return;
+  }
+
+  DCHECK(data.manifest);
+  if (!IsUpdateNeededForManifest(*data.manifest)) {
     DestroySelf(ManifestUpdateResult::kAppUpToDate);
     return;
   }
@@ -90,17 +96,12 @@ void ManifestUpdateTask::OnDidGetInstallableData(const InstallableData& data) {
                           AsWeakPtr(), *data.manifest));
 }
 
-bool ManifestUpdateTask::IsUpdateNeededForInstallableData(
-    const InstallableData& data) {
-  if (!data.errors.empty())
-    return false;
-
-  DCHECK(data.manifest);
-
-  if (app_id_ != GenerateAppIdFromURL(data.manifest->start_url))
+bool ManifestUpdateTask::IsUpdateNeededForManifest(
+    const blink::Manifest& manifest) const {
+  if (app_id_ != GenerateAppIdFromURL(manifest.start_url))
     return false;
 
-  if (data.manifest->theme_color != registrar_.GetAppThemeColor(app_id_))
+  if (manifest.theme_color != registrar_.GetAppThemeColor(app_id_))
     return true;
 
   // TODO(crbug.com/926083): Check more manifest fields.

chrome/browser/web_applications/components/manifest_update_task.h

@@ -26,6 +26,7 @@ enum ManifestUpdateResult {
   kWebContentsDestroyed,
   kAppUninstalled,
   kAppUpToDate,
+  kAppDataInvalid,
   kAppUpdateFailed,
   kAppUpdated,
 };
@@ -67,7 +68,7 @@ class ManifestUpdateTask final
   };
 
   void OnDidGetInstallableData(const InstallableData& data);
-  bool IsUpdateNeededForInstallableData(const InstallableData& data);
+  bool IsUpdateNeededForManifest(const blink::Manifest& manifest) const;
   void OnAllAppWindowsClosed(blink::Manifest manifest);
   void OnInstallationComplete(const AppId& app_id, InstallResultCode code);
   void DestroySelf(ManifestUpdateResult result);