From a8edb7e93c1475d79477e0cd032c0c2781991710 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 20 Nov 2025 13:46:32 -0800 Subject: [PATCH 1/2] Remove unimplemented (#54585) Summary: # Changelog: [Internal] There is no implementation for this method and it is unused. This is actually part of `HostAgent`. Differential Revision: D87220192 --- .../react-native/ReactCommon/jsinspector-modern/HostTarget.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index 72929a95a2058e..e315893e8e1157 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -326,11 +326,6 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis */ void emitTraceRecordingForFirstFuseboxClient(tracing::TraceRecordingState traceRecording) const; - /** - * Emits a system state changed event to all active sessions. - */ - void emitSystemStateChanged(bool isSingleHost) const; - private: /** * Constructs a new HostTarget. From e59647b922dc1db24544ff8a2a7a564e448bff91 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 20 Nov 2025 13:46:32 -0800 Subject: [PATCH 2/2] Define HostTargetTracingDelegate (#54622) Summary: # Changelog: [Internal] Introduces a TracingDelegate that will receive tracing-related events on the Host side. We can't give synchronous access to tracing state to Host directly, since there is no guarantee that this call will happen on the inspector thread. Example: - `HostTarget::tracingState()` call may happen on UI thread, or whatever thread the Host is using, which reads `traceRecoding_` value. - At the same time, the `stopTracing()` call that will invalidate `traceRecording_` could happen on inspector thread. This is a pre-requisite for a better threading model, since we are going to be doing much frequent write operations from the Host side to record frame timings or screenshots. Differential Revision: D86205686 --- .../jsinspector-modern/HostTarget.h | 42 +++++++++++++++ .../jsinspector-modern/HostTargetTracing.cpp | 13 ++++- .../tests/HostTargetTest.cpp | 53 +++++++++++++++++++ .../jsinspector-modern/tests/InspectorMocks.h | 20 +++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h index e315893e8e1157..56a0dab62e2b3a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h @@ -55,6 +55,36 @@ struct HostTargetMetadata { std::optional reactNativeVersion{}; }; +/** + * Receives any performance-related events from a HostTarget: could be Tracing, Performance Monitor, etc. + */ +class HostTargetTracingDelegate { + public: + HostTargetTracingDelegate() = default; + virtual ~HostTargetTracingDelegate() = default; + + /** + * Fired when the corresponding HostTarget started recording a tracing session. + * The tracing state is expected to be initialized at this point and the delegate should be able to record events + * through HostTarget. + */ + virtual void onTracingStarted(tracing::Mode /* tracingMode */, bool /* screenshotsCategoryEnabled */) {} + + /** + * Fired when the corresponding HostTarget is about to end recording a tracing session. + * The tracing state is expected to be still initialized during the call and the delegate should be able to record + * events through HostTarget. + * + * Any attempts to record events after this callback is finished will fail. + */ + virtual void onTracingStopped() {} + + HostTargetTracingDelegate(const HostTargetTracingDelegate &) = delete; + HostTargetTracingDelegate(HostTargetTracingDelegate &&) = delete; + HostTargetTracingDelegate &operator=(const HostTargetTracingDelegate &) = delete; + HostTargetTracingDelegate &operator=(HostTargetTracingDelegate &&) = delete; +}; + /** * Receives events from a HostTarget. This is a shared interface that each * React Native platform needs to implement in order to integrate with the @@ -161,6 +191,14 @@ class HostTargetDelegate : public LoadNetworkResourceDelegate { { return std::nullopt; } + + /** + * An optional delegate that will be used by HostTarget to notify about tracing-related events. + */ + virtual HostTargetTracingDelegate *getTracingDelegate() + { + return nullptr; + } }; /** @@ -230,12 +268,15 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis public: /** * Constructs a new HostTarget. + * * \param delegate The HostTargetDelegate that will * receive events from this HostTarget. The caller is responsible for ensuring * that the HostTargetDelegate outlives this object. + * * \param executor An executor that may be used to call methods on this * HostTarget while it exists. \c create additionally guarantees that the * executor will not be called after the HostTarget is destroyed. + * * \note Copies of the provided executor may be destroyed on arbitrary * threads, including after the HostTarget is destroyed. Callers must ensure * that such destructor calls are safe - e.g. if using a lambda as the @@ -330,6 +371,7 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis /** * Constructs a new HostTarget. * The caller must call setExecutor immediately afterwards. + * * \param delegate The HostTargetDelegate that will * receive events from this HostTarget. The caller is responsible for ensuring * that the HostTargetDelegate outlives this object. diff --git a/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.cpp b/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.cpp index 7d9ec16a3ea282..87bebd09a490fc 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.cpp @@ -35,23 +35,34 @@ bool HostTarget::startTracing( if (traceRecording_ != nullptr) { if (traceRecording_->isBackgroundInitiated() && tracingMode == tracing::Mode::CDP) { - traceRecording_.reset(); + stopTracing(); } else { return false; } } + auto screenshotsCategoryEnabled = + enabledCategories.contains(tracing::Category::Screenshot); + traceRecording_ = std::make_unique( *this, tracingMode, std::move(enabledCategories)); traceRecording_->setTracedInstance(currentInstance_.get()); traceRecording_->start(); + if (auto tracingDelegate = delegate_.getTracingDelegate()) { + tracingDelegate->onTracingStarted(tracingMode, screenshotsCategoryEnabled); + } + return true; } tracing::TraceRecordingState HostTarget::stopTracing() { assert(traceRecording_ != nullptr && "No tracing in progress"); + if (auto tracingDelegate = delegate_.getTracingDelegate()) { + tracingDelegate->onTracingStopped(); + } + auto state = traceRecording_->stop(); traceRecording_.reset(); diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/HostTargetTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/HostTargetTest.cpp index 88fef17ce041cb..5b600997ff89e2 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/HostTargetTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/HostTargetTest.cpp @@ -1526,4 +1526,57 @@ TEST_F(HostTargetTest, IOReadSizeValidation) { })"); } +TEST_F(HostTargetTest, TracingDelegateIsNotifiedOnCDPRequest) { + connect(); + InSequence s; + + EXPECT_CALL( + hostTargetDelegate_.getTracingDelegateMock(), + onTracingStarted(Eq(tracing::Mode::CDP), Eq(false))) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({ + "id": 1, + "result": {} + })"))); + toPage_->sendMessage(R"({ + "id": 1, + "method": "Tracing.start" + })"); + + EXPECT_CALL(hostTargetDelegate_.getTracingDelegateMock(), onTracingStopped()) + .Times(1) + .RetiresOnSaturation(); + EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({ + "id": 1, + "result": {} + })"))); + EXPECT_CALL( + fromPage(), + onMessage(JsonParsed( + testing::AllOf( + AtJsonPtr("/method", "Tracing.tracingComplete"), + AtJsonPtr("/params/dataLossOccurred", false))))); + toPage_->sendMessage(R"({ + "id": 1, + "method": "Tracing.end" + })"); +} + +TEST_F(HostTargetTest, TracingDelegateIsNotifiedOnDirectTracingCall) { + connect(); + + EXPECT_CALL( + hostTargetDelegate_.getTracingDelegateMock(), + onTracingStarted(Eq(tracing::Mode::Background), Eq(false))) + .Times(1) + .RetiresOnSaturation(); + page_->startTracing(tracing::Mode::Background, {}); + + EXPECT_CALL(hostTargetDelegate_.getTracingDelegateMock(), onTracingStopped()) + .Times(1) + .RetiresOnSaturation(); + page_->stopTracing(); +} + } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h index d83ed861884670..122b233f24a326 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h @@ -113,6 +113,12 @@ class MockInspectorPackagerConnectionDelegate : public InspectorPackagerConnecti folly::Executor &executor_; }; +class MockHostTargetTracingDelegate : public HostTargetTracingDelegate { + public: + MOCK_METHOD(void, onTracingStarted, (tracing::Mode tracingMode, bool screenshotsCategoryEnabled), (override)); + MOCK_METHOD(void, onTracingStopped, (), (override)); +}; + class MockHostTargetDelegate : public HostTargetDelegate { public: // HostTargetDelegate methods @@ -131,6 +137,20 @@ class MockHostTargetDelegate : public HostTargetDelegate { loadNetworkResource, (const LoadNetworkResourceRequest ¶ms, ScopedExecutor executor), (override)); + + HostTargetTracingDelegate *getTracingDelegate() override + { + return mockTracingDelegate_.get(); + } + + MockHostTargetTracingDelegate &getTracingDelegateMock() + { + return *mockTracingDelegate_; + } + + private: + std::unique_ptr mockTracingDelegate_ = + std::make_unique(); }; class MockInstanceTargetDelegate : public InstanceTargetDelegate {};