From 01795a1b2287a694f37eef6164a169e78708f576 Mon Sep 17 00:00:00 2001
From: Nate Chapin <[email protected]>
Date: Fri, 09 Oct 2026 16:25:23 -0700
Subject: [PATCH] Cache InstalledWebappProvider rules in a lock-protected OriginValueMap

Populate and refresh InstalledWebappProvider's rule snapshot on the UI thread into content_settings::OriginValueMap so GetRuleIterator() and GetRule() satisfy the ProviderInterface thread-safety and post-ShutdownOnUIThread contracts purely in C++ without entering JNI on background worker threads.

Bug: 568900845
Change-Id: I03e797c36950ca6cf3d4adc6b492c44dee49cf9e
Fixed: 568900845
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8543372
Commit-Queue: Nate Chapin <[email protected]>
Reviewed-by: Daniel Murphy <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1716146}
---
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappBridge.java
index 4f9475e..c4efe674 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappBridge.java
@@ -10,6 +10,7 @@
 import org.jni_zero.JniType;
 import org.jni_zero.NativeMethods;
 
+import org.chromium.base.ThreadUtils;
 import org.chromium.build.annotations.NullMarked;
 import org.chromium.components.content_settings.ContentSetting;
 import org.chromium.components.content_settings.ContentSettingsType;
@@ -53,6 +54,7 @@
     }
 
     public static void notifyPermissionsChange(@ContentSettingsType.EnumType int type) {
+        ThreadUtils.assertOnUiThread();
         if (sNativeInstalledWebappProvider == 0) return;
 
         InstalledWebappBridgeJni.get()
@@ -68,6 +70,9 @@
     @CalledByNative
     private static void setInstalledWebappProvider(long provider) {
         sNativeInstalledWebappProvider = provider;
+        if (provider != 0) {
+            InstalledWebappPermissionManager.ensureActivityStateListenerRegistered();
+        }
     }
 
     @CalledByNative
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManager.java b/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManager.java
index 94f6ce31..564dabc3 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManager.java
@@ -21,9 +21,13 @@
 import androidx.annotation.VisibleForTesting;
 import androidx.browser.trusted.Token;
 
+import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.task.PostTask;
+import org.chromium.base.task.TaskTraits;
 import org.chromium.build.annotations.NullMarked;
 import org.chromium.build.annotations.Nullable;
 import org.chromium.chrome.browser.customtabs.CustomTabActivity;
@@ -48,12 +52,40 @@
 public class InstalledWebappPermissionManager {
     private static final String TAG = "PermissionManager";
 
+    private static boolean sIsRunningTwa;
+    private static boolean sActivityStateListenerRegistered;
+
     private InstalledWebappPermissionManager() {}
 
     private static InstalledWebappPermissionStore getStore() {
         return WebappRegistry.getInstance().getPermissionStore();
     }
 
+    static void ensureActivityStateListenerRegistered() {
+        ThreadUtils.assertOnUiThread();
+        if (sActivityStateListenerRegistered || !ApplicationStatus.isInitialized()) {
+            return;
+        }
+        sActivityStateListenerRegistered = true;
+        sIsRunningTwa = getLastTrackedFocusedTwaCustomTabActivity() != null;
+        ApplicationStatus.registerStateListenerForAllActivities(
+                (activity, newState) -> {
+                    boolean prevRunningTwa = sIsRunningTwa;
+                    boolean nowRunningTwa = getLastTrackedFocusedTwaCustomTabActivity() != null;
+                    sIsRunningTwa = nowRunningTwa;
+                    // Refresh if TWA active status changed, or if a TWA resumed (switching to an
+                    // external app like Android Settings doesn't clear ApplicationStatus's last
+                    // focused activity, and the client app's OS location permission may have
+                    // changed while paused). Only geolocation permissions depend on whether a TWA
+                    // is currently running; notification permissions apply regardless of activity
+                    // state.
+                    if (prevRunningTwa != nowRunningTwa
+                            || (nowRunningTwa && newState == ActivityState.RESUMED)) {
+                        InstalledWebappBridge.notifyPermissionsChange(getGeolocationType());
+                    }
+                });
+    }
+
     static boolean isRunningTwa() {
         CustomTabActivity customTabActivity = getLastTrackedFocusedTwaCustomTabActivity();
         return customTabActivity != null;
@@ -115,6 +147,7 @@
             @Nullable String packageName,
             @ContentSettingsType.EnumType int type,
             @ContentSetting int settingValue) {
+        ThreadUtils.assertOnUiThread();
         if (packageName == null) return;
 
         String appName = getAppNameForPackage(packageName);
@@ -138,6 +171,7 @@
 
     @UiThread
     static void unregister(Origin origin) {
+        ThreadUtils.assertOnUiThread();
         getStore().removeOrigin(origin);
 
         NotificationChannelPreserver.restoreChannelIfNeeded(origin);
@@ -149,6 +183,7 @@
 
     @UiThread
     static void resetStoredPermission(Origin origin, @ContentSettingsType.EnumType int type) {
+        ThreadUtils.assertOnUiThread();
         getStore().resetPermission(origin, type);
         InstalledWebappBridge.notifyPermissionsChange(type);
     }
@@ -231,7 +266,19 @@
                     @ContentSetting
                     int settingValue = enabled ? ContentSetting.ALLOW : ContentSetting.BLOCK;
 
-                    updatePermission(origin, packageName, getGeolocationType(), settingValue);
+                    final @ContentSettingsType.EnumType int geolocationType = getGeolocationType();
+                    if (ThreadUtils.runningOnUiThread()) {
+                        updatePermission(origin, packageName, geolocationType, settingValue);
+                    } else {
+                        PostTask.postTask(
+                                TaskTraits.UI_DEFAULT,
+                                () ->
+                                        updatePermission(
+                                                origin,
+                                                packageName,
+                                                geolocationType,
+                                                settingValue));
+                    }
 
                     return settingValue;
                 }
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManagerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManagerTest.java
index 6ecb066c..b584a0a 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManagerTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/browserservices/permissiondelegation/InstalledWebappPermissionManagerTest.java
@@ -8,10 +8,12 @@
 import static android.Manifest.permission.ACCESS_FINE_LOCATION;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -31,12 +33,14 @@
 import org.robolectric.ParameterizedRobolectricTestRunner;
 import org.robolectric.Robolectric;
 import org.robolectric.RuntimeEnvironment;
+import org.robolectric.shadows.ShadowLooper;
 import org.robolectric.shadows.ShadowPackageManager;
 import org.robolectric.util.ReflectionHelpers;
 
 import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.FeatureOverrides;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.BaseRobolectricTestRule;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.customtabs.CustomTabActivity;
@@ -49,6 +53,8 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /** Tests for {@link InstalledWebappPermissionManager}. */
 @RunWith(ParameterizedRobolectricTestRunner.class)
@@ -330,4 +336,38 @@
             ApplicationStatus.onStateChangeForTesting(activity, ActivityState.DESTROYED);
         }
     }
+
+    @Test
+    @Feature("TrustedWebActivities")
+    public void getPermission_offUiThread_postsUpdateToUiThread() throws Exception {
+        setClientLocationPermission(true);
+        setStoredLocationPermission(ContentSetting.BLOCK);
+
+        AtomicInteger resultOnBackgroundThread = new AtomicInteger(ContentSetting.DEFAULT);
+        AtomicBoolean storeUpdatedOffUiThread = new AtomicBoolean(false);
+        doAnswer(
+                        invocation -> {
+                            storeUpdatedOffUiThread.set(!ThreadUtils.runningOnUiThread());
+                            return true;
+                        })
+                .when(mStore)
+                .setStateForOrigin(any(), anyString(), anyString(), anyInt(), anyInt());
+
+        Thread bgThread =
+                new Thread(
+                        () ->
+                                resultOnBackgroundThread.set(
+                                        InstalledWebappPermissionManager.getPermission(
+                                                mType, mOrigin)));
+        bgThread.start();
+        bgThread.join();
+
+        assertEquals(ContentSetting.ALLOW, resultOnBackgroundThread.get());
+        verifyPermissionNotUpdated();
+
+        ShadowLooper.runUiThreadTasks();
+
+        verifyLocationPermissionUpdated(ContentSetting.ALLOW);
+        assertFalse(storeUpdatedOffUiThread.get());
+    }
 }
diff --git a/chrome/browser/webapps/installable/installed_webapp_provider.cc b/chrome/browser/webapps/installable/installed_webapp_provider.cc
index 87010237f..40e1cd7 100644
--- a/chrome/browser/webapps/installable/installed_webapp_provider.cc
+++ b/chrome/browser/webapps/installable/installed_webapp_provider.cc
@@ -8,7 +8,9 @@
 #include <utility>
 #include <vector>
 
+#include "base/auto_reset.h"
 #include "base/feature_list.h"
+#include "base/synchronization/lock.h"
 #include "base/values.h"
 #include "chrome/browser/webapps/installable/installed_webapp_bridge.h"
 #include "components/content_settings/core/browser/content_settings_rule.h"
@@ -24,40 +26,6 @@
 
 namespace {
 
-class InstalledWebappIterator : public content_settings::RuleIterator {
- public:
-  explicit InstalledWebappIterator(InstalledWebappProvider::RuleList rules,
-                                   ContentSettingsType type)
-      : rules_(std::move(rules)),
-        info_(content_settings::PermissionSettingsRegistry::GetInstance()->Get(
-            type)) {}
-
-  InstalledWebappIterator(const InstalledWebappIterator&) = delete;
-  InstalledWebappIterator& operator=(const InstalledWebappIterator&) = delete;
-
-  ~InstalledWebappIterator() override = default;
-
-  bool HasNext() const override { return index_ < rules_.size(); }
-
-  std::unique_ptr<content_settings::Rule> Next() override {
-    DCHECK(HasNext());
-    const GURL& origin = rules_[index_].first;
-    PermissionSetting setting = rules_[index_].second;
-    DCHECK(info_->delegate().IsValid(setting)) << setting;
-    index_++;
-
-    return std::make_unique<content_settings::Rule>(
-        ContentSettingsPattern::FromURLNoWildcard(origin),
-        ContentSettingsPattern::Wildcard(), info_->delegate().ToValue(setting),
-        content_settings::RuleMetaData{});
-  }
-
- private:
-  size_t index_ = 0;
-  const InstalledWebappProvider::RuleList rules_;
-  const raw_ptr<const content_settings::PermissionSettingsInfo> info_;
-};
-
 bool IsSupportedContentType(ContentSettingsType content_type) {
   switch (content_type) {
     case ContentSettingsType::NOTIFICATIONS:
@@ -77,6 +45,9 @@
 
 InstalledWebappProvider::InstalledWebappProvider() {
   InstalledWebappBridge::SetProviderInstance(this);
+  RefreshRulesForType(ContentSettingsType::NOTIFICATIONS);
+  RefreshRulesForType(ContentSettingsType::GEOLOCATION);
+  RefreshRulesForType(ContentSettingsType::GEOLOCATION_WITH_OPTIONS);
 }
 InstalledWebappProvider::~InstalledWebappProvider() {
   InstalledWebappBridge::SetProviderInstance(nullptr);
@@ -85,15 +56,22 @@
 std::unique_ptr<RuleIterator> InstalledWebappProvider::GetRuleIterator(
     ContentSettingsType content_type,
     bool incognito) const {
-  if (incognito)
+  if (incognito || !IsSupportedContentType(content_type)) {
     return nullptr;
-
-  if (IsSupportedContentType(content_type)) {
-    return std::make_unique<InstalledWebappIterator>(
-        InstalledWebappBridge::GetInstalledWebappPermissions(content_type),
-        content_type);
   }
-  return nullptr;
+  return value_map_.GetRuleIterator(content_type);
+}
+
+std::unique_ptr<content_settings::Rule> InstalledWebappProvider::GetRule(
+    const GURL& primary_url,
+    const GURL& secondary_url,
+    ContentSettingsType content_type,
+    bool off_the_record) const {
+  if (off_the_record || !IsSupportedContentType(content_type)) {
+    return nullptr;
+  }
+  base::AutoLock auto_lock(value_map_.GetLock());
+  return value_map_.GetRule(primary_url, secondary_url, content_type);
 }
 
 bool InstalledWebappProvider::SetWebsiteSetting(
@@ -112,11 +90,42 @@
 }
 
 void InstalledWebappProvider::ShutdownOnUIThread() {
-  DCHECK(CalledOnValidThread());
+  CHECK(CalledOnValidThread());
   RemoveAllObservers();
 }
 
 void InstalledWebappProvider::Notify(ContentSettingsType content_type) {
+  CHECK(CalledOnValidThread());
+  if (is_refreshing_) {
+    return;
+  }
+  RefreshRulesForType(content_type);
   NotifyObservers(ContentSettingsPattern::Wildcard(),
                   ContentSettingsPattern::Wildcard(), content_type);
 }
+
+void InstalledWebappProvider::RefreshRulesForType(
+    ContentSettingsType content_type) {
+  CHECK(CalledOnValidThread());
+  if (!IsSupportedContentType(content_type) || is_refreshing_) {
+    return;
+  }
+
+  base::AutoReset<bool> reset_refreshing(&is_refreshing_, true);
+  RuleList rules =
+      InstalledWebappBridge::GetInstalledWebappPermissions(content_type);
+  const auto* info =
+      content_settings::PermissionSettingsRegistry::GetInstance()->Get(
+          content_type);
+  CHECK(info);
+
+  base::AutoLock auto_lock(value_map_.GetLock());
+  value_map_.DeleteValues(content_type);
+  for (const auto& [origin, setting] : rules) {
+    DCHECK(info->delegate().IsValid(setting)) << setting;
+    value_map_.SetValue(ContentSettingsPattern::FromURLNoWildcard(origin),
+                        ContentSettingsPattern::Wildcard(), content_type,
+                        info->delegate().ToValue(setting),
+                        content_settings::RuleMetaData{});
+  }
+}
diff --git a/chrome/browser/webapps/installable/installed_webapp_provider.h b/chrome/browser/webapps/installable/installed_webapp_provider.h
index 6146bca..7da2a03d 100644
--- a/chrome/browser/webapps/installable/installed_webapp_provider.h
+++ b/chrome/browser/webapps/installable/installed_webapp_provider.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "components/content_settings/core/browser/content_settings_observable_provider.h"
+#include "components/content_settings/core/browser/content_settings_origin_value_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "url/gurl.h"
@@ -33,6 +34,12 @@
       ContentSettingsType content_type,
       bool incognito) const override;
 
+  std::unique_ptr<content_settings::Rule> GetRule(
+      const GURL& primary_url,
+      const GURL& secondary_url,
+      ContentSettingsType content_type,
+      bool off_the_record) const override;
+
   bool SetWebsiteSetting(
       const ContentSettingsPattern& primary_pattern,
       const ContentSettingsPattern& secondary_pattern,
@@ -44,6 +51,12 @@
   void ShutdownOnUIThread() override;
 
   void Notify(ContentSettingsType content_type);
+
+ private:
+  void RefreshRulesForType(ContentSettingsType content_type);
... (file diff truncated)