From 73ba32063f33fda20a4894457419edf4ade7d0da Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 27 Jan 2026 06:42:28 -0800 Subject: [PATCH 1/5] Add missing mutex lock to NetworkHandler::disable() (#55241) Summary: Changelog: [Internal] TSIA Reviewed By: hoxyq Differential Revision: D90998980 --- .../ReactCommon/jsinspector-modern/network/NetworkHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp index 0025a731b4542b..3e8c6d9f8eefe5 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp @@ -56,6 +56,7 @@ bool NetworkHandler::disable() { } enabled_.store(false, std::memory_order_release); + std::lock_guard lock(requestBodyMutex_); responseBodyBuffer_.clear(); return true; } From 02343e7eb3f6c1becbd68fb4dd6f65fe824f8287 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 27 Jan 2026 06:42:28 -0800 Subject: [PATCH 2/5] Support using the Network domain in multiple concurrent sessions (#55240) Summary: Changelog: [General][Fixed] Send network events to all CDP clients with the Network domain enabled Reviewed By: hoxyq Differential Revision: D90888854 --- .../jsinspector-modern/NetworkIOAgent.cpp | 15 ++++-- .../jsinspector-modern/NetworkIOAgent.h | 13 +++++ .../network/NetworkHandler.cpp | 47 ++++++++++--------- .../network/NetworkHandler.h | 36 ++++++++------ .../tests/NetworkReporterTest.cpp | 34 ++++++++++++++ 5 files changed, 105 insertions(+), 40 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.cpp index b914998e96cf81..c18795b7e5297a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.cpp @@ -259,6 +259,13 @@ class Stream : public NetworkRequestListener, }; } // namespace +NetworkIOAgent::~NetworkIOAgent() { + if (networkAgentId_) { + NetworkHandler::getInstance().disableAgent(*networkAgentId_); + networkAgentId_ = std::nullopt; + } +} + bool NetworkIOAgent::handleRequest( const cdp::PreparsedRequest& req, LoadNetworkResourceDelegate& delegate) { @@ -278,15 +285,17 @@ bool NetworkIOAgent::handleRequest( // @cdp Network.enable support is experimental. if (req.method == "Network.enable") { - networkHandler.setFrontendChannel(frontendChannel_); - networkHandler.enable(); + networkAgentId_ = networkHandler.enableAgent(frontendChannel_); // NOTE: Domain enable/disable responses are sent by HostAgent. return false; } // @cdp Network.disable support is experimental. if (req.method == "Network.disable") { - networkHandler.disable(); + if (networkAgentId_) { + networkHandler.disableAgent(*networkAgentId_); + networkAgentId_ = std::nullopt; + } // NOTE: Domain enable/disable responses are sent by HostAgent. return false; } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h index 05fa053e1bc221..c0561f5e4af358 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/NetworkIOAgent.h @@ -211,6 +211,13 @@ class NetworkIOAgent { { } + ~NetworkIOAgent(); + + NetworkIOAgent(const NetworkIOAgent &) = delete; + NetworkIOAgent &operator=(const NetworkIOAgent &) = delete; + NetworkIOAgent(NetworkIOAgent &&) = delete; + NetworkIOAgent &operator=(NetworkIOAgent &&) = delete; + /** * Handle a CDP request. The response will be sent over the provided * \c FrontendChannel synchronously or asynchronously. @@ -247,6 +254,12 @@ class NetworkIOAgent { */ unsigned long nextStreamId_{0}; + /** + * If non-nullopt, indicates that this agent has enabled the Network domain + * via NetworkHandler, storing the agent ID for cleanup. + */ + std::optional networkAgentId_; + /** * Begin loading an HTTP resource, delegating platform-specific * implementation, responding to the frontend on headers received or on error. diff --git a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp index 3e8c6d9f8eefe5..9dbe9bfbaa8abb 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.cpp @@ -37,28 +37,31 @@ NetworkHandler& NetworkHandler::getInstance() { return instance; } -void NetworkHandler::setFrontendChannel(FrontendChannel frontendChannel) { - frontendChannel_ = std::move(frontendChannel); +size_t NetworkHandler::enableAgent(FrontendChannel frontendChannel) { + std::lock_guard lock(agentsMutex_); + size_t id = nextAgentId_++; + agents_.push_back({id, std::move(frontendChannel)}); + enabled_.store(true, std::memory_order_release); + return id; } -bool NetworkHandler::enable() { - if (enabled_.load(std::memory_order_acquire)) { - return false; - } +void NetworkHandler::disableAgent(size_t agentId) { + std::lock_guard lock(agentsMutex_); + agents_.remove_if( + [agentId](const AgentRecord& r) { return r.id == agentId; }); + if (agents_.empty()) { + enabled_.store(false, std::memory_order_release); - enabled_.store(true, std::memory_order_release); - return true; + std::lock_guard lock2(requestBodyMutex_); + responseBodyBuffer_.clear(); + } } -bool NetworkHandler::disable() { - if (!enabled_.load(std::memory_order_acquire)) { - return false; +void NetworkHandler::sendToAllAgents(std::string_view message) { + std::lock_guard lock(agentsMutex_); + for (auto& agent : agents_) { + agent.channel(message); } - - enabled_.store(false, std::memory_order_release); - std::lock_guard lock(requestBodyMutex_); - responseBodyBuffer_.clear(); - return true; } void NetworkHandler::onRequestWillBeSent( @@ -89,7 +92,7 @@ void NetworkHandler::onRequestWillBeSent( .redirectResponse = redirectResponse, }; - frontendChannel_( + sendToAllAgents( cdp::jsonNotification("Network.requestWillBeSent", params.toDynamic())); } @@ -106,7 +109,7 @@ void NetworkHandler::onRequestWillBeSentExtraInfo( .connectTiming = {.requestTime = getCurrentUnixTimestampSeconds()}, }; - frontendChannel_( + sendToAllAgents( cdp::jsonNotification( "Network.requestWillBeSentExtraInfo", params.toDynamic())); } @@ -133,7 +136,7 @@ void NetworkHandler::onResponseReceived( .hasExtraInfo = false, }; - frontendChannel_( + sendToAllAgents( cdp::jsonNotification("Network.responseReceived", params.toDynamic())); } @@ -152,7 +155,7 @@ void NetworkHandler::onDataReceived( .encodedDataLength = encodedDataLength, }; - frontendChannel_( + sendToAllAgents( cdp::jsonNotification("Network.dataReceived", params.toDynamic())); } @@ -169,7 +172,7 @@ void NetworkHandler::onLoadingFinished( .encodedDataLength = encodedDataLength, }; - frontendChannel_( + sendToAllAgents( cdp::jsonNotification("Network.loadingFinished", params.toDynamic())); } @@ -192,7 +195,7 @@ void NetworkHandler::onLoadingFailed( .canceled = cancelled, }; - frontendChannel_( + sendToAllAgents( cdp::jsonNotification("Network.loadingFailed", params.toDynamic())); } } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h index f451ffa99d92a8..aaa43aad2b4755 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/network/NetworkHandler.h @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -36,24 +37,17 @@ class NetworkHandler { static NetworkHandler &getInstance(); /** - * Set the channel used to send CDP events to the frontend. This should be - * supplied before calling `enable`. + * Register a frontend channel for receiving Network domain events. + * Implicitly enables the domain if this is the first agent. + * \returns An agent ID to be passed to disableAgent() on cleanup. */ - void setFrontendChannel(FrontendChannel frontendChannel); + size_t enableAgent(FrontendChannel frontendChannel); /** - * Enable network debugging. Returns `false` if already enabled. - * - * @cdp Network.enable - */ - bool enable(); - - /** - * Disable network debugging. Returns `false` if not initially enabled. - * - * @cdp Network.disable + * Unregister a frontend channel by its ID. + * Implicitly disables the domain if this was the last agent. */ - bool disable(); + void disableAgent(size_t agentId); /** * Returns whether network debugging is currently enabled. @@ -134,7 +128,19 @@ class NetworkHandler { std::optional consumeStoredRequestInitiator(const std::string &requestId); - FrontendChannel frontendChannel_; + /** + * Send a message to all registered frontend channels. + */ + void sendToAllAgents(std::string_view message); + + struct AgentRecord { + size_t id; + FrontendChannel channel; + }; + + std::list agents_; + size_t nextAgentId_{0}; + std::mutex agentsMutex_; std::map resourceTypeMap_{}; std::map requestInitiatorById_{}; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp index 05fa77c7c95b97..6b77a04935674f 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/NetworkReporterTest.cpp @@ -588,6 +588,40 @@ TEST_P(NetworkReporterTest, testCreateRequestIdWithoutNetworkDomain) { EXPECT_NE(id1, id2); } +TEST_P(NetworkReporterTest, testTwoSessionsReceiveNetworkEvents) { + auto secondary = this->connectSecondary(); + + this->expectMessageFromPage(JsonEq(R"({"id": 1, "result": {}})")); + this->toPage_->sendMessage(R"({"id": 1, "method": "Network.enable"})"); + + EXPECT_CALL( + secondary.fromPage(), onMessage(JsonEq(R"({"id": 2, "result": {}})"))); + secondary.toPage().sendMessage(R"({"id": 2, "method": "Network.enable"})"); + + // Both sessions should receive the network event + this->expectMessageFromPage(JsonParsed(AllOf( + AtJsonPtr("/method", "Network.requestWillBeSent"), + AtJsonPtr("/params/requestId", "multi-session-request")))); + EXPECT_CALL( + secondary.fromPage(), + onMessage(JsonParsed(AllOf( + AtJsonPtr("/method", "Network.requestWillBeSent"), + AtJsonPtr("/params/requestId", "multi-session-request"))))); + + RequestInfo requestInfo; + requestInfo.url = "https://example.com/test"; + requestInfo.httpMethod = "GET"; + NetworkReporter::getInstance().reportRequestStart( + "multi-session-request", requestInfo, 0, std::nullopt); + + this->expectMessageFromPage(JsonEq(R"({"id": 3, "result": {}})")); + this->toPage_->sendMessage(R"({"id": 3, "method": "Network.disable"})"); + + EXPECT_CALL( + secondary.fromPage(), onMessage(JsonEq(R"({"id": 4, "result": {}})"))); + secondary.toPage().sendMessage(R"({"id": 4, "method": "Network.disable"})"); +} + struct NetworkReporterTracingTestParams { bool enableNetworkEventReporting; bool enableNetworkDomain; From d0f258e181d7b0dd03b17ed55847717ffcf5dd69 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 27 Jan 2026 06:42:28 -0800 Subject: [PATCH 3/5] Add tests for multi-session Tracing domain support (first to trace wins) (#55287) Summary: Changelog: [Internal] Adds tests (and a doc comment) codifying the existing, correct behavior of the CDP `Tracing.start` command when there is already a tracing session in progress. Reviewed By: hoxyq Differential Revision: D90888857 --- .../jsinspector-modern/TracingAgent.cpp | 2 + .../jsinspector-modern/tests/TracingTest.cpp | 72 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp index 956f6246c8b7d0..3fc0957c9010de 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp @@ -90,6 +90,8 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) { bool didNotHaveAlreadyRunningRecording = hostTargetController_.startTracing( tracing::Mode::CDP, std::move(enabledCategories)); if (!didNotHaveAlreadyRunningRecording) { + // @cdp Tracing.start fails if there is a tracing session already running + // in the current target. This matches Chrome's behavior. frontendChannel_( cdp::jsonError( req.id, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp index 18ff1f9244e289..b04290604c9a46 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp @@ -113,4 +113,76 @@ TEST_F(TracingTest, EmitsScreenshotEventWhenScreenshotValuePassed) { EXPECT_THAT(allTraceEvents, Contains(AtJsonPtr("/name", "Screenshot"))); } +TEST_F( + TracingTest, + SecondSessionTracingStartIsRejectedWhileFirstSessionIsTracing) { + auto secondary = connectSecondary(); + InSequence s; + + // Session 1 starts tracing successfully + startTracing(); + + // Session 2 tries to start tracing - should get error + EXPECT_CALL( + secondary.fromPage(), + onMessage(JsonParsed(AllOf( + AtJsonPtr("/id", 2), + AtJsonPtr("/error/message", "Tracing has already been started"))))); + secondary.toPage().sendMessage(R"({"id": 2, "method": "Tracing.start"})"); + + // Session 1 ends tracing normally + endTracingAndCollectEvents(); + + // Now Session 2 can start tracing + EXPECT_CALL( + secondary.fromPage(), onMessage(JsonEq(R"({"id": 3, "result": {}})"))); + secondary.toPage().sendMessage(R"({"id": 3, "method": "Tracing.start"})"); + + // Clean up - end secondary's tracing + EXPECT_CALL( + secondary.fromPage(), onMessage(JsonEq(R"({"id": 4, "result": {}})"))); + EXPECT_CALL( + secondary.fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.dataCollected")))) + .Times(AtLeast(1)); + EXPECT_CALL( + secondary.fromPage(), + onMessage(JsonParsed(AtJsonPtr("/method", "Tracing.tracingComplete")))); + secondary.toPage().sendMessage(R"({"id": 4, "method": "Tracing.end"})"); +} + +TEST_F(TracingTest, CDPTracingPreemptsBackgroundTracing) { + InSequence s; + + // Start background tracing directly + page_->startTracing(tracing::Mode::Background, {}); + + // CDP Tracing.start should preempt background (succeed, not fail) + startTracing(); + + // End tracing normally + endTracingAndCollectEvents(); +} + +TEST_F(TracingTest, BackgroundTracingIsRejectedWhileCDPTracingIsRunning) { + InSequence s; + + // Start CDP tracing + startTracing(); + + // Background tracing should be rejected + bool started = page_->startTracing(tracing::Mode::Background, {}); + EXPECT_FALSE(started); + + // End CDP tracing + endTracingAndCollectEvents(); + + // Now background tracing should succeed + started = page_->startTracing(tracing::Mode::Background, {}); + EXPECT_TRUE(started); + + // Clean up + page_->stopTracing(); +} + } // namespace facebook::react::jsinspector_modern From feb84fef42594436afea7073910f6a8fec6a90ac Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 27 Jan 2026 06:42:28 -0800 Subject: [PATCH 4/5] Remove const qualifier from side-effectful methods of Agents and Targets Summary: TSIA - minor refactor for correctness and clarity before making changes to some of these methods up the stack. Changelog: [Internal] Reviewed By: hoxyq Differential Revision: D91140491 --- .../ReactCommon/jsinspector-modern/HostAgent.cpp | 7 +++---- .../ReactCommon/jsinspector-modern/HostAgent.h | 4 ++-- .../ReactCommon/jsinspector-modern/HostTarget.cpp | 5 ++--- .../ReactCommon/jsinspector-modern/HostTarget.h | 2 +- .../ReactCommon/jsinspector-modern/TracingAgent.cpp | 4 ++-- .../ReactCommon/jsinspector-modern/TracingAgent.h | 4 ++-- 6 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp index 41d81767df350d..888d27936e90ee 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.cpp @@ -385,8 +385,7 @@ class HostAgent::Impl final { return fuseboxClientType_ == FuseboxClientType::Fusebox; } - void emitExternalTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) { assert( hasFuseboxClientConnected() && "Attempted to emit a trace recording to a non-Fusebox client"); @@ -543,11 +542,11 @@ bool HostAgent::hasFuseboxClientConnected() const { } void HostAgent::emitExternalTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { impl_->emitExternalTracingProfile(std::move(tracingProfile)); } -void HostAgent::emitSystemStateChanged(bool isSingleHost) const { +void HostAgent::emitSystemStateChanged(bool isSingleHost) { impl_->emitSystemStateChanged(isSingleHost); } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h index 8789cb5d32c165..3046a7788c9686 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h @@ -77,13 +77,13 @@ class HostAgent final { * Emits the HostTracingProfile that was captured externally, not via the * CDP-initiated request. */ - void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) const; + void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile); /** * Emits a system state changed event when the number of ReactHost instances * changes. */ - void emitSystemStateChanged(bool isSingleHost) const; + void emitSystemStateChanged(bool isSingleHost); private: // We use the private implementation idiom to ensure this class has the same diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp index e34ca99bfad2eb..0e3cca677f86ce 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp @@ -111,8 +111,7 @@ class HostTargetSession { return hostAgent_.hasFuseboxClientConnected(); } - void emitHostTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile) { hostAgent_.emitExternalTracingProfile(std::move(tracingProfile)); } @@ -384,7 +383,7 @@ bool HostTarget::hasActiveSessionWithFuseboxClient() const { } void HostTarget::emitTracingProfileForFirstFuseboxClient( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { bool emitted = false; sessions_.forEach([&](HostTargetSession& session) { if (emitted) { diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index aed1396b14e6f3..b1640f31a47383 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -363,7 +363,7 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis * * @see \c hasActiveFrontendSession */ - void emitTracingProfileForFirstFuseboxClient(tracing::HostTracingProfile tracingProfile) const; + void emitTracingProfileForFirstFuseboxClient(tracing::HostTracingProfile tracingProfile); /** * An endpoint for the Host to report frame timings that will be recorded if and only if there is currently an active diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp index 3fc0957c9010de..4a876eb1b02dad 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp @@ -120,14 +120,14 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) { } void TracingAgent::emitExternalHostTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { frontendChannel_( cdp::jsonNotification("ReactNativeApplication.traceRequested")); emitHostTracingProfile(std::move(tracingProfile)); } void TracingAgent::emitHostTracingProfile( - tracing::HostTracingProfile tracingProfile) const { + tracing::HostTracingProfile tracingProfile) { auto dataCollectedCallback = [this](folly::dynamic&& eventsChunk) { frontendChannel_( cdp::jsonNotification( diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h index d0f474f1234bca..3b2a6325989880 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.h @@ -44,7 +44,7 @@ class TracingAgent { /** * Emits the HostTracingProfile that was stashed externally by the HostTarget. */ - void emitExternalHostTracingProfile(tracing::HostTracingProfile tracingProfile) const; + void emitExternalHostTracingProfile(tracing::HostTracingProfile tracingProfile); private: /** @@ -60,7 +60,7 @@ class TracingAgent { * Emits captured HostTracingProfile in a series of * Tracing.dataCollected events, followed by a Tracing.tracingComplete event. */ - void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile) const; + void emitHostTracingProfile(tracing::HostTracingProfile tracingProfile); }; } // namespace facebook::react::jsinspector_modern From 6158e049148533c3757a7c9d850e05c6cd7d2deb Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 27 Jan 2026 06:42:28 -0800 Subject: [PATCH 5/5] WeakList: add anyOf + const-propagating forEach Summary: Changelog: [Internal] TSIA - minor refactor for convenience and correctness. NOTE: When we get C++23, we can deduplicate the identical const/non-const method bodies using the nifty [deducing `this`](https://en.cppreference.com/w/cpp/language/member_functions.html#Explicit_object_member_functions) feature. Reviewed By: hoxyq Differential Revision: D91140490 --- .../jsinspector-modern/HostTarget.cpp | 4 +- .../ReactCommon/jsinspector-modern/WeakList.h | 61 ++++++++++++++++++- .../jsinspector-modern/tests/WeakListTest.cpp | 32 +++++++++- 3 files changed, 93 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp index 0e3cca677f86ce..fae9d52605ff6a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.cpp @@ -376,7 +376,7 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) { bool HostTarget::hasActiveSessionWithFuseboxClient() const { bool hasActiveFuseboxSession = false; - sessions_.forEach([&](HostTargetSession& session) { + sessions_.forEach([&](auto& session) { hasActiveFuseboxSession |= session.hasFuseboxClient(); }); return hasActiveFuseboxSession; @@ -385,7 +385,7 @@ bool HostTarget::hasActiveSessionWithFuseboxClient() const { void HostTarget::emitTracingProfileForFirstFuseboxClient( tracing::HostTracingProfile tracingProfile) { bool emitted = false; - sessions_.forEach([&](HostTargetSession& session) { + sessions_.forEach([&](auto& session) { if (emitted) { /** * HostTracingProfile object is not copiable for performance reasons, diff --git a/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h b/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h index 00338c344ab59c..8b299742449e55 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/WeakList.h @@ -30,7 +30,7 @@ class WeakList { * to destroyed elements) will be removed during iteration. */ template - void forEach(Fn &&fn) const + void forEach(Fn &&fn) { for (auto it = ptrs_.begin(); it != ptrs_.end();) { if (auto ptr = it->lock()) { @@ -42,6 +42,65 @@ class WeakList { } } + /** + * Call the given function for every element in the list, ensuring the element + * is not destroyed for the duration of the call. Elements are visited in the + * order they were inserted. + * + * As a side effect, any null pointers in the underlying list (corresponding + * to destroyed elements) will be removed during iteration. + */ + template + void forEach(Fn &&fn) const + { + for (auto it = ptrs_.cbegin(); it != ptrs_.cend();) { + if (auto ptr = it->lock()) { + fn(*ptr); + ++it; + } else { + it = ptrs_.erase(it); + } + } + } + + /** + * Returns true if the given function returns true for any element in the + * list, ensuring the element is not destroyed for the duration of the call. + * + * As a side effect, any null pointers in the underlying list (corresponding + * to destroyed elements) will be removed during iteration. + */ + template + bool anyOf(Fn &&fn) + { + bool found = false; + forEach([&](auto &element) { + if (!found && fn(element)) { + found = true; + } + }); + return found; + } + + /** + * Returns true if the given function returns true for any element in the + * list, ensuring the element is not destroyed for the duration of the call. + * + * As a side effect, any null pointers in the underlying list (corresponding + * to destroyed elements) will be removed during iteration. + */ + template + bool anyOf(Fn &&fn) const + { + bool found = false; + forEach([&](const auto &element) { + if (!found && fn(element)) { + found = true; + } + }); + return found; + } + /** * Returns the number of (non-null) elements in the list. The count will only * remain accurate as long as the list is not modified and elements are diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp index ee86a8e68a9435..d228c7cc0ff2b3 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/WeakListTest.cpp @@ -70,7 +70,7 @@ TEST(WeakListTest, ForEach) { EXPECT_THAT(visited, ElementsAre(1, 3)); } -TEST(WeakListTest, ElementsAreAliveDuringCallback) { +TEST(WeakListTest, ElementsAreAliveDuringForEachCallback) { WeakList list; auto p1 = std::make_shared(1); // A separate weak_ptr to observe the lifetime of `p1`. @@ -88,4 +88,34 @@ TEST(WeakListTest, ElementsAreAliveDuringCallback) { EXPECT_THAT(visited, ElementsAre(1)); } +TEST(WeakListTest, AnyOf) { + WeakList list; + auto p1 = std::make_shared(1); + auto p2 = std::make_shared(2); + auto p3 = std::make_shared(3); + list.insert(p1); + list.insert(p2); + list.insert(p3); + + EXPECT_TRUE(list.anyOf([](const int& value) { return value == 2; })); + EXPECT_FALSE(list.anyOf([](const int& value) { return value == 4; })); +} + +TEST(WeakListTest, ElementsAreAliveDuringAnyOfCallback) { + WeakList list; + auto p1 = std::make_shared(42); + // A separate weak_ptr to observe the lifetime of `p1`. + std::weak_ptr wp1 = p1; + list.insert(p1); + + bool result = list.anyOf([&](const int& value) { + p1.reset(); + EXPECT_FALSE(wp1.expired()); + return value == 42; + }); + + EXPECT_TRUE(result); + EXPECT_TRUE(wp1.expired()); +} + } // namespace facebook::react::jsinspector_modern