Skip to content

Commit 1b6d3c9

Browse files
hoxyqfacebook-github-bot
authored andcommitted
refactor: emit the trace recoding on ReactNativeApplication domain initialization (#53760)
Summary: Pull Request resolved: #53760 # Changelog: [Internal] This is a different approach from the one that I've introduced initially in [1]. This saves us from the scenario, where any local session could snatch the stashed trace recording. For example, if some session was created for a Runtime binding right after we've stashed the trace and before initializing real CDP session with the Frontend. Reviewed By: huntie Differential Revision: D82316584 fbshipit-source-id: 806a0f6dbdb4e4e928ce33af228cae86d43772e9
1 parent fedad12 commit 1b6d3c9

File tree

5 files changed

+37
-39
lines changed

5 files changed

+37
-39
lines changed

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,14 @@ class HostAgent::Impl final {
4141
HostTargetController& targetController,
4242
HostTargetMetadata hostMetadata,
4343
SessionState& sessionState,
44-
VoidExecutor executor,
45-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit)
44+
VoidExecutor executor)
4645
: frontendChannel_(frontendChannel),
4746
targetController_(targetController),
4847
hostMetadata_(std::move(hostMetadata)),
4948
sessionState_(sessionState),
5049
networkIOAgent_(NetworkIOAgent(frontendChannel, std::move(executor))),
51-
tracingAgent_(TracingAgent(
52-
frontendChannel,
53-
sessionState,
54-
targetController,
55-
std::move(traceRecordingToEmit))) {}
50+
tracingAgent_(
51+
TracingAgent(frontendChannel, sessionState, targetController)) {}
5652

5753
~Impl() {
5854
if (isPausedInDebuggerOverlayVisible_) {
@@ -201,6 +197,14 @@ class HostAgent::Impl final {
201197
"ReactNativeApplication.metadataUpdated",
202198
createHostMetadataPayload(hostMetadata_)));
203199

200+
auto stashedTraceRecording =
201+
targetController_.getDelegate()
202+
.unstable_getTraceRecordingThatWillBeEmittedOnInitialization();
203+
if (stashedTraceRecording.has_value()) {
204+
tracingAgent_.emitExternalTraceRecording(
205+
std::move(stashedTraceRecording.value()));
206+
}
207+
204208
return {
205209
.isFinishedHandlingRequest = true,
206210
.shouldSendOKResponse = true,
@@ -432,8 +436,7 @@ class HostAgent::Impl final {
432436
HostTargetController& targetController,
433437
HostTargetMetadata hostMetadata,
434438
SessionState& sessionState,
435-
VoidExecutor executor,
436-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit) {}
439+
VoidExecutor executor) {}
437440

438441
void handleRequest(const cdp::PreparsedRequest& req) {}
439442
void setCurrentInstanceAgent(std::shared_ptr<InstanceAgent> agent) {}
@@ -446,16 +449,14 @@ HostAgent::HostAgent(
446449
HostTargetController& targetController,
447450
HostTargetMetadata hostMetadata,
448451
SessionState& sessionState,
449-
VoidExecutor executor,
450-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit)
452+
VoidExecutor executor)
451453
: impl_(std::make_unique<Impl>(
452454
*this,
453455
frontendChannel,
454456
targetController,
455457
std::move(hostMetadata),
456458
sessionState,
457-
std::move(executor),
458-
std::move(traceRecordingToEmit))) {}
459+
std::move(executor))) {}
459460

460461
HostAgent::~HostAgent() = default;
461462

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,13 @@ class HostAgent final {
3636
* \param hostMetadata Metadata about the host that created this agent.
3737
* \param sessionState The state of the session that created this agent.
3838
* \param executor A void executor to be used by async-aware handlers.
39-
* \param traceRecordingToEmit If set, this is the trace that Host has
40-
* requested to display in the Frontend.
4139
*/
4240
HostAgent(
4341
const FrontendChannel& frontendChannel,
4442
HostTargetController& targetController,
4543
HostTargetMetadata hostMetadata,
4644
SessionState& sessionState,
47-
VoidExecutor executor,
48-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit);
45+
VoidExecutor executor);
4946

5047
HostAgent(const HostAgent&) = delete;
5148
HostAgent(HostAgent&&) = delete;

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ class HostTargetSession {
3434
std::unique_ptr<IRemoteConnection> remote,
3535
HostTargetController& targetController,
3636
HostTargetMetadata hostMetadata,
37-
VoidExecutor executor,
38-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit)
37+
VoidExecutor executor)
3938
: remote_(std::make_shared<RAIIRemoteConnection>(std::move(remote))),
4039
frontendChannel_(
4140
[remoteWeak = std::weak_ptr(remote_)](std::string_view message) {
@@ -48,8 +47,7 @@ class HostTargetSession {
4847
targetController,
4948
std::move(hostMetadata),
5049
state_,
51-
std::move(executor),
52-
std::move(traceRecordingToEmit)) {}
50+
std::move(executor)) {}
5351

5452
/**
5553
* Called by CallbackLocalConnection to send a message to this Session's
@@ -205,8 +203,7 @@ std::unique_ptr<ILocalConnection> HostTarget::connect(
205203
std::move(connectionToFrontend),
206204
controller_,
207205
delegate_.getMetadata(),
208-
makeVoidExecutor(executorFromThis()),
209-
delegate_.unstable_getTraceRecordingThatWillBeEmittedOnInitialization());
206+
makeVoidExecutor(executorFromThis()));
210207
session->setCurrentInstance(currentInstance_.get());
211208
sessions_.insert(std::weak_ptr(session));
212209
return std::make_unique<CallbackLocalConnection>(

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,10 @@ const uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 1;
3838
TracingAgent::TracingAgent(
3939
FrontendChannel frontendChannel,
4040
SessionState& sessionState,
41-
HostTargetController& hostTargetController,
42-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit)
41+
HostTargetController& hostTargetController)
4342
: frontendChannel_(std::move(frontendChannel)),
4443
sessionState_(sessionState),
45-
hostTargetController_(hostTargetController) {
46-
if (traceRecordingToEmit.has_value()) {
47-
frontendChannel_(
48-
cdp::jsonNotification("ReactNativeApplication.traceRequested"));
49-
emitTraceRecording(std::move(traceRecordingToEmit.value()));
50-
}
51-
}
44+
hostTargetController_(hostTargetController) {}
5245

5346
TracingAgent::~TracingAgent() {
5447
// Agents are owned by the session. If the agent is destroyed, it means that
@@ -100,15 +93,22 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) {
10093
return false;
10194
}
10295

96+
void TracingAgent::emitExternalTraceRecording(
97+
tracing::TraceRecordingState traceRecording) const {
98+
frontendChannel_(
99+
cdp::jsonNotification("ReactNativeApplication.traceRequested"));
100+
emitTraceRecording(std::move(traceRecording));
101+
}
102+
103103
void TracingAgent::emitTraceRecording(
104-
tracing::TraceRecordingState state) const {
104+
tracing::TraceRecordingState traceRecording) const {
105105
auto dataCollectedCallback = [this](folly::dynamic&& eventsChunk) {
106106
frontendChannel_(cdp::jsonNotification(
107107
"Tracing.dataCollected",
108108
folly::dynamic::object("value", std::move(eventsChunk))));
109109
};
110110
tracing::TraceRecordingStateSerializer::emitAsDataCollectedChunks(
111-
std::move(state),
111+
std::move(traceRecording),
112112
dataCollectedCallback,
113113
TRACE_EVENT_CHUNK_SIZE,
114114
PROFILE_TRACE_EVENT_CHUNK_SIZE);

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,11 @@ class TracingAgent {
2828
* \param hostTargetController An interface to the HostTarget that this agent
2929
* is attached to. The caller is responsible for ensuring that the
3030
* HostTargetDelegate and underlying HostTarget both outlive the agent.
31-
* \param traceRecordingToEmit If set, this is the trace that Host has
32-
* requested to display in the Frontend.
3331
*/
3432
TracingAgent(
3533
FrontendChannel frontendChannel,
3634
SessionState& sessionState,
37-
HostTargetController& hostTargetController,
38-
std::optional<tracing::TraceRecordingState> traceRecordingToEmit);
35+
HostTargetController& hostTargetController);
3936

4037
~TracingAgent();
4138

@@ -46,6 +43,12 @@ class TracingAgent {
4643
*/
4744
bool handleRequest(const cdp::PreparsedRequest& req);
4845

46+
/**
47+
* Emits the Trace Recording that was stashed externally by the HostTarget.
48+
*/
49+
void emitExternalTraceRecording(
50+
tracing::TraceRecordingState traceRecording) const;
51+
4952
private:
5053
/**
5154
* A channel used to send responses and events to the frontend.
@@ -60,7 +63,7 @@ class TracingAgent {
6063
* Emits the captured Trace Recording state in a series of
6164
* Tracing.dataCollected events, followed by a Tracing.tracingComplete event.
6265
*/
63-
void emitTraceRecording(tracing::TraceRecordingState state) const;
66+
void emitTraceRecording(tracing::TraceRecordingState traceRecording) const;
6467
};
6568

6669
} // namespace facebook::react::jsinspector_modern

0 commit comments

Comments
 (0)