From b122b4a0d4587946fd2bffdd5b7c72479d5668e5 Mon Sep 17 00:00:00 2001
From: Richard Nichols <[email protected]>
Date: Tue, 28 Jul 2026 07:23:01 -0700
Subject: [PATCH] [chromecast] Make CastActivityUrlFilterManager thread-safe

CastURLLoaderThrottleProvider and CastWebSocketHandshakeThrottleProvider
are cloned to worker threads where they call
GetActivityUrlFilterForRenderFrameToken() while the main thread adds and
removes entries via OnRenderFrameCreated() and OnRenderFrameRemoved().
Guard the filter map with a lock so lookups never observe a map that is
being mutated, and hold the receivers via raw_ptr.

Adds a cast_renderer_unittests target with coverage for the concurrent
lookup path.

Bug: 495515356
Change-Id: Idedffe49e1849b521c465396611d40e3f1accca3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8108180
Reviewed-by: Shawn Quereshi <[email protected]>
Reviewed-by: Simeon Anfinrud <[email protected]>
Commit-Queue: Richard Nichols <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1669459}
---
diff --git a/chromecast/renderer/cast_activity_url_filter_manager.cc b/chromecast/renderer/cast_activity_url_filter_manager.cc
index ef750b8..e484b63 100644
--- a/chromecast/renderer/cast_activity_url_filter_manager.cc
+++ b/chromecast/renderer/cast_activity_url_filter_manager.cc
@@ -74,7 +74,8 @@
 ActivityUrlFilter*
 CastActivityUrlFilterManager::GetActivityUrlFilterForRenderFrameToken(
     const blink::LocalFrameToken& frame_token) {
-  const auto& it = activity_url_filters_.find(frame_token);
+  base::AutoLock lock(filters_lock_);
+  auto it = activity_url_filters_.find(frame_token);
   if (it == activity_url_filters_.end())
     return nullptr;
 
@@ -91,6 +92,7 @@
       base::BindOnce(&CastActivityUrlFilterManager::OnRenderFrameRemoved,
                      weak_this_, frame_token));
 
+  base::AutoLock lock(filters_lock_);
   auto result = activity_url_filters_.emplace(frame_token, filter_receiver);
 
   if (!result.second)
@@ -101,7 +103,8 @@
 
 void CastActivityUrlFilterManager::OnRenderFrameRemoved(
     const blink::LocalFrameToken& frame_token) {
-  const auto& it = activity_url_filters_.find(frame_token);
+  base::AutoLock lock(filters_lock_);
+  auto it = activity_url_filters_.find(frame_token);
 
   if (it != activity_url_filters_.end())
     activity_url_filters_.erase(it);
diff --git a/chromecast/renderer/cast_activity_url_filter_manager.h b/chromecast/renderer/cast_activity_url_filter_manager.h
index 948bb60..4ed10951 100644
--- a/chromecast/renderer/cast_activity_url_filter_manager.h
+++ b/chromecast/renderer/cast_activity_url_filter_manager.h
@@ -9,6 +9,9 @@
 #include <string>
 
 #include "base/containers/flat_map.h"
+#include "base/memory/raw_ptr.h"
+#include "base/synchronization/lock.h"
+#include "base/thread_annotations.h"
 #include "chromecast/common/activity_url_filter.h"
 #include "chromecast/common/mojom/activity_url_filter.mojom.h"
 #include "content/public/renderer/render_frame_observer.h"
@@ -33,6 +36,7 @@
   ~CastActivityUrlFilterManager();
 
   // Returns nullptr if no Activity URL filter exists for the render frame.
+  // May be called from any thread.
   ActivityUrlFilter* GetActivityUrlFilterForRenderFrameToken(
       const blink::LocalFrameToken& frame_token);
 
@@ -40,6 +44,8 @@
   void OnRenderFrameRemoved(const blink::LocalFrameToken& frame_token);
 
  private:
+  friend class CastActivityUrlFilterManagerTest;
+
   class UrlFilterReceiver
       : public content::RenderFrameObserver,
         public chromecast::mojom::ActivityUrlFilterConfiguration {
@@ -82,8 +88,9 @@
     base::WeakPtrFactory<UrlFilterReceiver> weak_factory_;
   };
 
-  base::flat_map<blink::LocalFrameToken, UrlFilterReceiver*>
-      activity_url_filters_;
+  base::Lock filters_lock_;
+  base::flat_map<blink::LocalFrameToken, raw_ptr<UrlFilterReceiver>>
+      activity_url_filters_ GUARDED_BY(filters_lock_);
 
   base::WeakPtr<CastActivityUrlFilterManager> weak_this_;
   base::WeakPtrFactory<CastActivityUrlFilterManager> weak_factory_;
diff --git a/chromecast/renderer/cast_activity_url_filter_manager_unittest.cc b/chromecast/renderer/cast_activity_url_filter_manager_unittest.cc
new file mode 100644
index 0000000..4dc89b4
--- /dev/null
+++ b/chromecast/renderer/cast_activity_url_filter_manager_unittest.cc
@@ -0,0 +1,95 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/renderer/cast_activity_url_filter_manager.h"
+
+#include <atomic>
+#include <vector>
+
+#include "base/memory/raw_ptr.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/thread_annotations.h"
+#include "base/threading/simple_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/tokens/tokens.h"
+
+namespace chromecast {
+
+class CastActivityUrlFilterManagerTest : public testing::Test {
+ protected:
+  void AddFilterForToken(const blink::LocalFrameToken& token)
+      NO_THREAD_SAFETY_ANALYSIS {
+    base::AutoLock lock(manager_.filters_lock_);
+    manager_.activity_url_filters_.emplace(token, nullptr);
+  }
+
+  CastActivityUrlFilterManager manager_;
+};
+
+namespace {
+
+class LookupRunner : public base::DelegateSimpleThread::Delegate {
+ public:
+  LookupRunner(CastActivityUrlFilterManager* manager,
+               const blink::LocalFrameToken& token,
+               base::WaitableEvent* started,
+               std::atomic<bool>* stop)
+      : manager_(manager), token_(token), started_(started), stop_(stop) {}
+
+  void Run() override {
+    started_->Signal();
+    while (!stop_->load(std::memory_order_relaxed)) {
+      EXPECT_EQ(nullptr,
+                manager_->GetActivityUrlFilterForRenderFrameToken(token_));
+    }
+  }
+
+ private:
+  const raw_ptr<CastActivityUrlFilterManager> manager_;
+  const blink::LocalFrameToken token_;
+  const raw_ptr<base::WaitableEvent> started_;
+  const raw_ptr<std::atomic<bool>> stop_;
+};
+
+}  // namespace
+
+TEST_F(CastActivityUrlFilterManagerTest, LookupForUnknownTokenReturnsNull) {
+  EXPECT_EQ(nullptr, manager_.GetActivityUrlFilterForRenderFrameToken(
+                         blink::LocalFrameToken()));
+}
+
+// CastURLLoaderThrottleProvider clones may look up filters from worker
+// threads while frames are being created and destroyed on the main thread,
+// so the manager must allow concurrent lookups and map mutations.
+TEST_F(CastActivityUrlFilterManagerTest, ConcurrentLookupAndModification) {
+  constexpr size_t kEntryCount = 8;
+  std::vector<blink::LocalFrameToken> tokens(kEntryCount);
+  for (const auto& token : tokens) {
+    AddFilterForToken(token);
+  }
+
+  blink::LocalFrameToken missing_token;
+  base::WaitableEvent started;
+  std::atomic<bool> stop(false);
+  LookupRunner runner(&manager_, missing_token, &started, &stop);
+  base::DelegateSimpleThread thread(&runner, "FilterLookup");
+  thread.Start();
+  started.Wait();
+
+  constexpr size_t kIterations = 4096;
+  for (size_t i = 0; i < kIterations; ++i) {
+    AddFilterForToken(blink::LocalFrameToken());
+  }
+  for (const auto& token : tokens) {
+    manager_.OnRenderFrameRemoved(token);
+  }
+  EXPECT_EQ(nullptr,
+            manager_.GetActivityUrlFilterForRenderFrameToken(tokens[0]));
+
+  stop.store(true, std::memory_order_relaxed);
+  thread.Join();
+}
+
+}  // namespace chromecast
diff --git a/chromecast/BUILD.gn b/chromecast/BUILD.gn
index 4627748..f3c54fa 100644
--- a/chromecast/BUILD.gn
+++ b/chromecast/BUILD.gn
@@ -101,6 +101,7 @@
 
   if (!is_fuchsia) {
     tests += [
+      "//chromecast/renderer:cast_renderer_unittests",
       "//chromecast/ui/display_settings:cast_display_settings_unittests",
       "//content/test:content_unittests",
     ]
diff --git a/chromecast/renderer/BUILD.gn b/chromecast/renderer/BUILD.gn
index 6f8da8fd..c98b375 100644
--- a/chromecast/renderer/BUILD.gn
+++ b/chromecast/renderer/BUILD.gn
@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import("//chromecast/chromecast.gni")
+import("//testing/test.gni")
 import("//tools/grit/grit_rule.gni")
 
 cast_source_set("renderer") {
@@ -81,6 +82,19 @@
   }
 }
 
+test("cast_renderer_unittests") {
+  sources = [ "cast_activity_url_filter_manager_unittest.cc" ]
+
+  deps = [
+    ":renderer",
+    "//base",
+    "//base/test:run_all_unittests",
+    "//base/test:test_support",
+    "//testing/gtest",
+    "//third_party/blink/public/common",
+  ]
+}
+
 cast_source_set("simple_client") {
   sources = [ "cast_content_renderer_client_simple.cc" ]
 

diff --git a/chromecast/renderer/cast_activity_url_filter_manager_unittest.cc b/chromecast/renderer/cast_activity_url_filter_manager_unittest.cc
new file mode 100644
index 0000000..4dc89b4
--- /dev/null
+++ b/chromecast/renderer/cast_activity_url_filter_manager_unittest.cc
@@ -0,0 +1,95 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/renderer/cast_activity_url_filter_manager.h"
+
+#include <atomic>
+#include <vector>
+
+#include "base/memory/raw_ptr.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/thread_annotations.h"
+#include "base/threading/simple_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/tokens/tokens.h"
+
+namespace chromecast {
+
+class CastActivityUrlFilterManagerTest : public testing::Test {
+ protected:
+  void AddFilterForToken(const blink::LocalFrameToken& token)
+      NO_THREAD_SAFETY_ANALYSIS {
+    base::AutoLock lock(manager_.filters_lock_);
+    manager_.activity_url_filters_.emplace(token, nullptr);
+  }
+
+  CastActivityUrlFilterManager manager_;
+};
+
+namespace {
+
+class LookupRunner : public base::DelegateSimpleThread::Delegate {
+ public:
+  LookupRunner(CastActivityUrlFilterManager* manager,
+               const blink::LocalFrameToken& token,
+               base::WaitableEvent* started,
+               std::atomic<bool>* stop)
+      : manager_(manager), token_(token), started_(started), stop_(stop) {}
+
+  void Run() override {
+    started_->Signal();
+    while (!stop_->load(std::memory_order_relaxed)) {
+      EXPECT_EQ(nullptr,
+                manager_->GetActivityUrlFilterForRenderFrameToken(token_));
+    }
+  }
+
+ private:
+  const raw_ptr<CastActivityUrlFilterManager> manager_;
+  const blink::LocalFrameToken token_;
+  const raw_ptr<base::WaitableEvent> started_;
+  const raw_ptr<std::atomic<bool>> stop_;
+};
+
+}  // namespace
+
+TEST_F(CastActivityUrlFilterManagerTest, LookupForUnknownTokenReturnsNull) {
+  EXPECT_EQ(nullptr, manager_.GetActivityUrlFilterForRenderFrameToken(
+                         blink::LocalFrameToken()));
+}
+
+// CastURLLoaderThrottleProvider clones may look up filters from worker
+// threads while frames are being created and destroyed on the main thread,
+// so the manager must allow concurrent lookups and map mutations.
+TEST_F(CastActivityUrlFilterManagerTest, ConcurrentLookupAndModification) {
+  constexpr size_t kEntryCount = 8;
+  std::vector<blink::LocalFrameToken> tokens(kEntryCount);
+  for (const auto& token : tokens) {
+    AddFilterForToken(token);
+  }
+
+  blink::LocalFrameToken missing_token;
+  base::WaitableEvent started;
+  std::atomic<bool> stop(false);
+  LookupRunner runner(&manager_, missing_token, &started, &stop);
+  base::DelegateSimpleThread thread(&runner, "FilterLookup");
+  thread.Start();
+  started.Wait();
+
+  constexpr size_t kIterations = 4096;
+  for (size_t i = 0; i < kIterations; ++i) {
+    AddFilterForToken(blink::LocalFrameToken());
+  }
+  for (const auto& token : tokens) {
+    manager_.OnRenderFrameRemoved(token);
+  }
+  EXPECT_EQ(nullptr,
+            manager_.GetActivityUrlFilterForRenderFrameToken(tokens[0]));
+
+  stop.store(true, std::memory_order_relaxed);
+  thread.Join();
+}
+
+}  // namespace chromecast