From 35d688b7c5984f28b1cfe4816a93b586fabf6cd6 Mon Sep 17 00:00:00 2001
From: Lia Hiscock <[email protected]>
Date: Tue, 03 Mar 2026 14:39:46 -0800
Subject: [PATCH] [WebInstall] Fix triggered_from_element_ sticky state

WebInstallServiceImpl::InstallFromElement() set the member
triggered_from_element_ to true but never reset it. Because the service
instance is per-document, a subsequent navigator.install() call on the
same page reused the stale flag, skipping the permissions-policy gate
and the WEB_APP_INSTALLATION permission prompt.

Replace the mutable member with a local parameter by introducing a
private InstallInternal(options, callback, triggered_from_element)
helper. Install() forwards with false; InstallFromElement() forwards
with true. This ensures the flag is scoped to a single call and cannot
leak across Mojo method invocations.

Add a regression test that performs an <install>-element install
followed by a navigator.install() call on the same document with the
WEB_APP_INSTALLATION permission blocked, and verifies that the JS API
call is correctly denied.

Bug: 487938764, 487568011, 333795265
Change-Id: Ibe56b00bcb1018a25bfadce92dcdc98e4f9c4360
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7615447
Reviewed-by: Lu Huang <[email protected]>
Reviewed-by: Marijn Kruisselbrink <[email protected]>
Commit-Queue: Lia Hiscock <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1593535}
---

diff --git a/chrome/browser/web_applications/install_element_browsertest.cc b/chrome/browser/web_applications/install_element_browsertest.cc
index 46a85c1..23435c21f 100644
--- a/chrome/browser/web_applications/install_element_browsertest.cc
+++ b/chrome/browser/web_applications/install_element_browsertest.cc
@@ -665,4 +665,80 @@
   WaitForDismissEvent(kInstallElementId);
 }
 
+///////////////////////////////////////////////////////////////////////////////
+// Regression tests for interactions between <install> element and JS API.
+///////////////////////////////////////////////////////////////////////////////
+
+// Test fixture that enables both the <install> element and the
+// navigator.install() JS API, so we can test interactions between them on the
+// same document (same WebInstallServiceImpl instance).
+class InstallElementAndApiInteractionBrowserTest
+    : public InstallElementBrowserTest {
+ public:
+  InstallElementAndApiInteractionBrowserTest() {
+    additional_features_.InitWithFeatures(
+        {blink::features::kWebAppInstallation}, {});
+  }
+
+ private:
+  base::test::ScopedFeatureList additional_features_;
+};
+
+// Regression test for crbug.com/487568011: triggered_from_element was set by
+// InstallFromElement() but never reset, causing subsequent Install() calls via
+// navigator.install() on the same document to bypass the permissions-policy and
+// permission prompt checks.
+IN_PROC_BROWSER_TEST_F(InstallElementAndApiInteractionBrowserTest,
+                       InstallApiRespectsPermissionsAfterElementInstall) {
+  // Navigate to a page with <install> elements.
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(
+      browser(), https_server()->GetURL(kInstallElementPageStartUrl)));
+
+  auto auto_accept = SetAutoAcceptPWAInstallConfirmationForTesting();
+
+  // Set the <install> element's installurl to a another test page.
+  const GURL element_install_url =
+      https_server()->GetURL(kCustomIdPageInstallUrl);
+  ASSERT_TRUE(SetButtonInstallUrl(element_install_url));
+
+  // Step 1: Click the <install> element. This calls InstallFromElement() on the
+  // browser-side WebInstallServiceImpl instance for this document, which passes
+  // triggered_from_element = true.
+  {
+    ui_test_utils::BrowserCreatedObserver browser_created_observer;
+    ASSERT_TRUE(ClickElementWithId(kInstallElementId));
+    Browser* web_app_browser = browser_created_observer.Wait();
+    WaitForPromptActionEvent(kInstallElementId);
+    ASSERT_TRUE(AppBrowserController::IsWebApp(web_app_browser));
+  }
+
+  // Step 2: From the SAME page (same WebInstallServiceImpl instance), call
+  // navigator.install(url, manifest_id) via JS. This uses the Install() Mojo
+  // method, NOT InstallFromElement(). The permission check should NOT be
+  // bypassed by the sticky triggered_from_element_ flag from step 1.
+  const GURL api_install_url =
+      https_server()->GetURL(kNoCustomIdPageInstallUrl);
+  const GURL api_manifest_id = https_server()->GetURL(kNoCustomIdPageId);
+
+  // Block the WEB_APP_INSTALLATION permission so that if the logic to prompt
+  // for permission is correctly reached, it will be denied. With the sticky
+  // state bug, the permission check would've been skipped entirely and the
+  // install would've succeeded.
+  BlockWebInstallPermission(api_install_url);
+
+  auto result = content::EvalJs(
+      web_contents(), "navigator.install('" + api_install_url.spec() + "', '" +
+                          api_manifest_id.spec() +
+                          "')"
+                          ".then(result => 'success')"
+                          ".catch(error => error.name)");
+
+  // The Install API call should fail because the WEB_APP_INSTALLATION
+  // permission was blocked. With the sticky state bug,
+  // triggered_from_element_ would still be true from step 1, causing
+  // Install() to skip all permission checks and auto-grant, resulting in
+  // 'success' instead.
+  EXPECT_EQ("AbortError", result.ExtractString());
+}
+
 }  // namespace web_app
diff --git a/chrome/browser/web_applications/web_install_service_impl.cc b/chrome/browser/web_applications/web_install_service_impl.cc
index 3ef74d6c..faadccb 100644
--- a/chrome/browser/web_applications/web_install_service_impl.cc
+++ b/chrome/browser/web_applications/web_install_service_impl.cc
@@ -190,6 +190,21 @@
 
 void WebInstallServiceImpl::Install(blink::mojom::InstallOptionsPtr options,
                                     InstallCallback callback) {
+  InstallInternal(std::move(options), std::move(callback),
+                  /*triggered_from_element=*/false);
+}
+
+void WebInstallServiceImpl::InstallFromElement(
+    blink::mojom::InstallOptionsPtr options,
+    InstallCallback callback) {
+  InstallInternal(std::move(options), std::move(callback),
+                  /*triggered_from_element=*/true);
+}
+
+void WebInstallServiceImpl::InstallInternal(
+    blink::mojom::InstallOptionsPtr options,
+    InstallCallback callback,
+    bool triggered_from_element) {
   // Create source ids for UKM logging.
   ukm::SourceId requesting_page_source_id =
       options ? render_frame_host().GetPageUkmSourceId()
@@ -251,18 +266,18 @@
         }
         std::move(callback).Run(install_result, manifest_id_result);
       },
-      std::move(callback), triggered_from_element_, requesting_page_source_id,
+      std::move(callback), triggered_from_element, requesting_page_source_id,
       installed_app_source_id);
 
   web_app::WebInstallServiceType install_type =
       options ? web_app::WebInstallServiceType::kBackgroundDocument
               : web_app::WebInstallServiceType::kCurrentDocument;
   base::UmaHistogramEnumeration(
-      triggered_from_element_ ? kInstallElementTypeUma : kInstallApiTypeUma,
+      triggered_from_element ? kInstallElementTypeUma : kInstallApiTypeUma,
       install_type);
   base::UmaHistogramEnumeration(
       base::StrCat({"WebApp.WebInstallService.",
-                    triggered_from_element_ ? "Element" : "Api",
+                    triggered_from_element ? "Element" : "Api",
                     ".InstallType"}),
       install_type);
 
@@ -314,10 +329,10 @@
 
   // Skip requesting permission in two cases:
   // 1. The install URL matches the current document URL (user is installing
-  //    the page they're on, even if using background install syntax).
-  // 2. Install triggered from the <install> element (permission is handled
-  //    by the element itself). In both cases, the install dialog is shown.
-  if (triggered_from_element_ || install_target == last_committed_url_) {
+  //    the page they're currently on, just using background install syntax).
+  // 2. Install triggered from the <install> element.
+  // In both cases, the install dialog is always shown.
+  if (triggered_from_element || install_target == last_committed_url_) {
     OnPermissionDecided(
         std::move(callback_with_metrics),
         std::vector<content::PermissionResult>({content::PermissionResult(
@@ -341,13 +356,6 @@
       weak_ptr_factory_.GetWeakPtr(), std::move(callback_with_metrics)));
 }
 
-void WebInstallServiceImpl::InstallFromElement(
-    blink::mojom::InstallOptionsPtr options,
-    InstallCallback callback) {
-  triggered_from_element_ = true;
-  Install(std::move(options), std::move(callback));
-}
-
 void WebInstallServiceImpl::OnInstallNotSupportedDialogClosed(
     InstallCallbackWithMetrics callback_with_metrics) {
   std::move(callback_with_metrics)
diff --git a/chrome/browser/web_applications/web_install_service_impl.h b/chrome/browser/web_applications/web_install_service_impl.h
index 6164059..b8325df 100644
--- a/chrome/browser/web_applications/web_install_service_impl.h
+++ b/chrome/browser/web_applications/web_install_service_impl.h
@@ -98,6 +98,13 @@
                           InstallCallback callback) override;
 

diff --git a/chrome/browser/web_applications/install_element_browsertest.cc b/chrome/browser/web_applications/install_element_browsertest.cc
index 46a85c1..23435c21f 100644
--- a/chrome/browser/web_applications/install_element_browsertest.cc
+++ b/chrome/browser/web_applications/install_element_browsertest.cc
@@ -665,4 +665,80 @@
   WaitForDismissEvent(kInstallElementId);
 }
 
+///////////////////////////////////////////////////////////////////////////////
+// Regression tests for interactions between <install> element and JS API.
+///////////////////////////////////////////////////////////////////////////////
+
+// Test fixture that enables both the <install> element and the
+// navigator.install() JS API, so we can test interactions between them on the
+// same document (same WebInstallServiceImpl instance).
+class InstallElementAndApiInteractionBrowserTest
+    : public InstallElementBrowserTest {
+ public:
+  InstallElementAndApiInteractionBrowserTest() {
+    additional_features_.InitWithFeatures(
+        {blink::features::kWebAppInstallation}, {});
+  }
+
+ private:
+  base::test::ScopedFeatureList additional_features_;
+};
+
+// Regression test for crbug.com/487568011: triggered_from_element was set by
+// InstallFromElement() but never reset, causing subsequent Install() calls via
+// navigator.install() on the same document to bypass the permissions-policy and
+// permission prompt checks.
+IN_PROC_BROWSER_TEST_F(InstallElementAndApiInteractionBrowserTest,
+                       InstallApiRespectsPermissionsAfterElementInstall) {
+  // Navigate to a page with <install> elements.
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(
+      browser(), https_server()->GetURL(kInstallElementPageStartUrl)));
+
+  auto auto_accept = SetAutoAcceptPWAInstallConfirmationForTesting();
+
+  // Set the <install> element's installurl to a another test page.
+  const GURL element_install_url =
+      https_server()->GetURL(kCustomIdPageInstallUrl);
+  ASSERT_TRUE(SetButtonInstallUrl(element_install_url));
+
+  // Step 1: Click the <install> element. This calls InstallFromElement() on the
+  // browser-side WebInstallServiceImpl instance for this document, which passes
+  // triggered_from_element = true.
+  {
+    ui_test_utils::BrowserCreatedObserver browser_created_observer;
+    ASSERT_TRUE(ClickElementWithId(kInstallElementId));
+    Browser* web_app_browser = browser_created_observer.Wait();
+    WaitForPromptActionEvent(kInstallElementId);
+    ASSERT_TRUE(AppBrowserController::IsWebApp(web_app_browser));
+  }
+
+  // Step 2: From the SAME page (same WebInstallServiceImpl instance), call
+  // navigator.install(url, manifest_id) via JS. This uses the Install() Mojo
+  // method, NOT InstallFromElement(). The permission check should NOT be
+  // bypassed by the sticky triggered_from_element_ flag from step 1.
+  const GURL api_install_url =
+      https_server()->GetURL(kNoCustomIdPageInstallUrl);
+  const GURL api_manifest_id = https_server()->GetURL(kNoCustomIdPageId);
+
+  // Block the WEB_APP_INSTALLATION permission so that if the logic to prompt
+  // for permission is correctly reached, it will be denied. With the sticky
+  // state bug, the permission check would've been skipped entirely and the
+  // install would've succeeded.
+  BlockWebInstallPermission(api_install_url);
+
+  auto result = content::EvalJs(
+      web_contents(), "navigator.install('" + api_install_url.spec() + "', '" +
+                          api_manifest_id.spec() +
+                          "')"
+                          ".then(result => 'success')"
+                          ".catch(error => error.name)");
+
+  // The Install API call should fail because the WEB_APP_INSTALLATION
+  // permission was blocked. With the sticky state bug,
+  // triggered_from_element_ would still be true from step 1, causing
+  // Install() to skip all permission checks and auto-grant, resulting in
+  // 'success' instead.
+  EXPECT_EQ("AbortError", result.ExtractString());
+}
+
 }  // namespace web_app