Skip to content

Commit e59647b

Browse files
hoxyqfacebook-github-bot
authored andcommitted
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
1 parent a8edb7e commit e59647b

File tree

4 files changed

+127
-1
lines changed

4 files changed

+127
-1
lines changed

packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,36 @@ struct HostTargetMetadata {
5555
std::optional<std::string> reactNativeVersion{};
5656
};
5757

58+
/**
59+
* Receives any performance-related events from a HostTarget: could be Tracing, Performance Monitor, etc.
60+
*/
61+
class HostTargetTracingDelegate {
62+
public:
63+
HostTargetTracingDelegate() = default;
64+
virtual ~HostTargetTracingDelegate() = default;
65+
66+
/**
67+
* Fired when the corresponding HostTarget started recording a tracing session.
68+
* The tracing state is expected to be initialized at this point and the delegate should be able to record events
69+
* through HostTarget.
70+
*/
71+
virtual void onTracingStarted(tracing::Mode /* tracingMode */, bool /* screenshotsCategoryEnabled */) {}
72+
73+
/**
74+
* Fired when the corresponding HostTarget is about to end recording a tracing session.
75+
* The tracing state is expected to be still initialized during the call and the delegate should be able to record
76+
* events through HostTarget.
77+
*
78+
* Any attempts to record events after this callback is finished will fail.
79+
*/
80+
virtual void onTracingStopped() {}
81+
82+
HostTargetTracingDelegate(const HostTargetTracingDelegate &) = delete;
83+
HostTargetTracingDelegate(HostTargetTracingDelegate &&) = delete;
84+
HostTargetTracingDelegate &operator=(const HostTargetTracingDelegate &) = delete;
85+
HostTargetTracingDelegate &operator=(HostTargetTracingDelegate &&) = delete;
86+
};
87+
5888
/**
5989
* Receives events from a HostTarget. This is a shared interface that each
6090
* React Native platform needs to implement in order to integrate with the
@@ -161,6 +191,14 @@ class HostTargetDelegate : public LoadNetworkResourceDelegate {
161191
{
162192
return std::nullopt;
163193
}
194+
195+
/**
196+
* An optional delegate that will be used by HostTarget to notify about tracing-related events.
197+
*/
198+
virtual HostTargetTracingDelegate *getTracingDelegate()
199+
{
200+
return nullptr;
201+
}
164202
};
165203

166204
/**
@@ -230,12 +268,15 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis<HostTarget>
230268
public:
231269
/**
232270
* Constructs a new HostTarget.
271+
*
233272
* \param delegate The HostTargetDelegate that will
234273
* receive events from this HostTarget. The caller is responsible for ensuring
235274
* that the HostTargetDelegate outlives this object.
275+
*
236276
* \param executor An executor that may be used to call methods on this
237277
* HostTarget while it exists. \c create additionally guarantees that the
238278
* executor will not be called after the HostTarget is destroyed.
279+
*
239280
* \note Copies of the provided executor may be destroyed on arbitrary
240281
* threads, including after the HostTarget is destroyed. Callers must ensure
241282
* that such destructor calls are safe - e.g. if using a lambda as the
@@ -330,6 +371,7 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis<HostTarget>
330371
/**
331372
* Constructs a new HostTarget.
332373
* The caller must call setExecutor immediately afterwards.
374+
*
333375
* \param delegate The HostTargetDelegate that will
334376
* receive events from this HostTarget. The caller is responsible for ensuring
335377
* that the HostTargetDelegate outlives this object.

packages/react-native/ReactCommon/jsinspector-modern/HostTargetTracing.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,34 @@ bool HostTarget::startTracing(
3535
if (traceRecording_ != nullptr) {
3636
if (traceRecording_->isBackgroundInitiated() &&
3737
tracingMode == tracing::Mode::CDP) {
38-
traceRecording_.reset();
38+
stopTracing();
3939
} else {
4040
return false;
4141
}
4242
}
4343

44+
auto screenshotsCategoryEnabled =
45+
enabledCategories.contains(tracing::Category::Screenshot);
46+
4447
traceRecording_ = std::make_unique<HostTargetTraceRecording>(
4548
*this, tracingMode, std::move(enabledCategories));
4649
traceRecording_->setTracedInstance(currentInstance_.get());
4750
traceRecording_->start();
4851

52+
if (auto tracingDelegate = delegate_.getTracingDelegate()) {
53+
tracingDelegate->onTracingStarted(tracingMode, screenshotsCategoryEnabled);
54+
}
55+
4956
return true;
5057
}
5158

5259
tracing::TraceRecordingState HostTarget::stopTracing() {
5360
assert(traceRecording_ != nullptr && "No tracing in progress");
5461

62+
if (auto tracingDelegate = delegate_.getTracingDelegate()) {
63+
tracingDelegate->onTracingStopped();
64+
}
65+
5566
auto state = traceRecording_->stop();
5667
traceRecording_.reset();
5768

packages/react-native/ReactCommon/jsinspector-modern/tests/HostTargetTest.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,4 +1526,57 @@ TEST_F(HostTargetTest, IOReadSizeValidation) {
15261526
})");
15271527
}
15281528

1529+
TEST_F(HostTargetTest, TracingDelegateIsNotifiedOnCDPRequest) {
1530+
connect();
1531+
InSequence s;
1532+
1533+
EXPECT_CALL(
1534+
hostTargetDelegate_.getTracingDelegateMock(),
1535+
onTracingStarted(Eq(tracing::Mode::CDP), Eq(false)))
1536+
.Times(1)
1537+
.RetiresOnSaturation();
1538+
EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({
1539+
"id": 1,
1540+
"result": {}
1541+
})")));
1542+
toPage_->sendMessage(R"({
1543+
"id": 1,
1544+
"method": "Tracing.start"
1545+
})");
1546+
1547+
EXPECT_CALL(hostTargetDelegate_.getTracingDelegateMock(), onTracingStopped())
1548+
.Times(1)
1549+
.RetiresOnSaturation();
1550+
EXPECT_CALL(fromPage(), onMessage(JsonEq(R"({
1551+
"id": 1,
1552+
"result": {}
1553+
})")));
1554+
EXPECT_CALL(
1555+
fromPage(),
1556+
onMessage(JsonParsed(
1557+
testing::AllOf(
1558+
AtJsonPtr("/method", "Tracing.tracingComplete"),
1559+
AtJsonPtr("/params/dataLossOccurred", false)))));
1560+
toPage_->sendMessage(R"({
1561+
"id": 1,
1562+
"method": "Tracing.end"
1563+
})");
1564+
}
1565+
1566+
TEST_F(HostTargetTest, TracingDelegateIsNotifiedOnDirectTracingCall) {
1567+
connect();
1568+
1569+
EXPECT_CALL(
1570+
hostTargetDelegate_.getTracingDelegateMock(),
1571+
onTracingStarted(Eq(tracing::Mode::Background), Eq(false)))
1572+
.Times(1)
1573+
.RetiresOnSaturation();
1574+
page_->startTracing(tracing::Mode::Background, {});
1575+
1576+
EXPECT_CALL(hostTargetDelegate_.getTracingDelegateMock(), onTracingStopped())
1577+
.Times(1)
1578+
.RetiresOnSaturation();
1579+
page_->stopTracing();
1580+
}
1581+
15291582
} // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ class MockInspectorPackagerConnectionDelegate : public InspectorPackagerConnecti
113113
folly::Executor &executor_;
114114
};
115115

116+
class MockHostTargetTracingDelegate : public HostTargetTracingDelegate {
117+
public:
118+
MOCK_METHOD(void, onTracingStarted, (tracing::Mode tracingMode, bool screenshotsCategoryEnabled), (override));
119+
MOCK_METHOD(void, onTracingStopped, (), (override));
120+
};
121+
116122
class MockHostTargetDelegate : public HostTargetDelegate {
117123
public:
118124
// HostTargetDelegate methods
@@ -131,6 +137,20 @@ class MockHostTargetDelegate : public HostTargetDelegate {
131137
loadNetworkResource,
132138
(const LoadNetworkResourceRequest &params, ScopedExecutor<NetworkRequestListener> executor),
133139
(override));
140+
141+
HostTargetTracingDelegate *getTracingDelegate() override
142+
{
143+
return mockTracingDelegate_.get();
144+
}
145+
146+
MockHostTargetTracingDelegate &getTracingDelegateMock()
147+
{
148+
return *mockTracingDelegate_;
149+
}
150+
151+
private:
152+
std::unique_ptr<MockHostTargetTracingDelegate> mockTracingDelegate_ =
153+
std::make_unique<MockHostTargetTracingDelegate>();
134154
};
135155

136156
class MockInstanceTargetDelegate : public InstanceTargetDelegate {};

0 commit comments

Comments
 (0)