Skip to content

Commit aa68540

Browse files
motiz88meta-codesync[bot]
authored andcommitted
Check Network domain state in RuntimeTargetNetwork (#54082)
Summary: Pull Request resolved: #54082 Changelog: [Internal] Followup from D83238216. Adds bookkeeping in `RuntimeTarget` to allow checking from the JS thread whether there is a session with the `Network` CDP enabled. We then use this in the implementation of `__NETWORK_REPORTER__.createDevToolsRequestId` as a more precise alternative to checking the `NetworkHandler` singleton's state. This is a first step towards breaking up the `NetworkHandler` singleton, subsuming its responsibilities into the Target/Agent architecture of `jsinspector-modern` and fixing a variety of bugs in the process. NOTE: The additions to `RuntimeTarget` are more general than just tracking the `Network` domain - I'll use them in an upcoming diff to fix a concrete bug related to concurrent session support in `DebuggerSessionObserver`. Reviewed By: huntie Differential Revision: D83847356 fbshipit-source-id: 3384e3feb44adbd9ed4d54a20875669d63ae8764
1 parent a4be9b2 commit aa68540

5 files changed

Lines changed: 143 additions & 14 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#pragma once
9+
10+
#include <array>
11+
#include <limits>
12+
13+
namespace facebook::react::jsinspector_modern {
14+
15+
/**
16+
* A statically-sized array with an enum class as the index type.
17+
* Values are value-initialized (i.e. zero-initialized for integral types).
18+
* Requires that the enum class has a kMaxValue member.
19+
*/
20+
template <class IndexType, class ValueType>
21+
requires std::is_enum_v<IndexType> &&
22+
std::is_same_v<std::underlying_type_t<IndexType>, int> &&
23+
requires { IndexType::kMaxValue; } &&
24+
(static_cast<int>(IndexType::kMaxValue) < std::numeric_limits<int>::max())
25+
26+
class EnumArray {
27+
public:
28+
constexpr ValueType& operator[](IndexType i) {
29+
return array_[static_cast<int>(i)];
30+
}
31+
32+
constexpr const ValueType& operator[](IndexType i) const {
33+
return array_[static_cast<int>(i)];
34+
}
35+
36+
constexpr int size() const {
37+
return size_;
38+
}
39+
40+
private:
41+
constexpr static int size_ = static_cast<int>(IndexType::kMaxValue) + 1;
42+
43+
std::array<ValueType, size_> array_{};
44+
};
45+
} // namespace facebook::react::jsinspector_modern

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ RuntimeAgent::RuntimeAgent(
3333
sessionState_.isLogDomainEnabled) {
3434
targetController_.notifyDebuggerSessionCreated();
3535
}
36+
37+
if (sessionState_.isNetworkDomainEnabled) {
38+
targetController_.notifyDomainStateChanged(
39+
RuntimeTargetController::Domain::Network, true, *this);
40+
}
3641
}
3742

3843
bool RuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) {
@@ -67,6 +72,17 @@ bool RuntimeAgent::handleRequest(const cdp::PreparsedRequest& req) {
6772
if (req.method == "Log.disable" && sessionState_.isRuntimeDomainEnabled) {
6873
targetController_.notifyDebuggerSessionDestroyed();
6974
}
75+
76+
if (req.method == "Network.enable" || req.method == "Network.disable") {
77+
targetController_.notifyDomainStateChanged(
78+
RuntimeTargetController::Domain::Network,
79+
sessionState_.isNetworkDomainEnabled,
80+
*this);
81+
82+
// We are not responding to this request, just processing a side effect.
83+
return false;
84+
}
85+
7086
if (delegate_) {
7187
return delegate_->handleRequest(req);
7288
}
@@ -108,6 +124,10 @@ RuntimeAgent::~RuntimeAgent() {
108124
sessionState_.isLogDomainEnabled) {
109125
targetController_.notifyDebuggerSessionDestroyed();
110126
}
127+
if (sessionState_.isNetworkDomainEnabled) {
128+
targetController_.notifyDomainStateChanged(
129+
RuntimeTargetController::Domain::Network, false, *this);
130+
}
111131

112132
// TODO: Eventually, there may be more than one Runtime per Page, and we'll
113133
// need to store multiple agent states here accordingly. For now let's do

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

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,34 @@ void RuntimeTarget::emitDebuggerSessionDestroyed() {
163163
});
164164
}
165165

166+
void RuntimeTarget::enableSamplingProfiler() {
167+
delegate_.enableSamplingProfiler();
168+
}
169+
170+
void RuntimeTarget::disableSamplingProfiler() {
171+
delegate_.disableSamplingProfiler();
172+
}
173+
174+
tracing::RuntimeSamplingProfile RuntimeTarget::collectSamplingProfile() {
175+
return delegate_.collectSamplingProfile();
176+
}
177+
178+
void RuntimeTarget::notifyDomainStateChanged(
179+
Domain domain,
180+
bool enabled,
181+
const RuntimeAgent& notifyingAgent) {
182+
if (enabled) {
183+
agentsByEnabledDomain_[domain].insert(&notifyingAgent);
184+
} else {
185+
agentsByEnabledDomain_[domain].erase(&notifyingAgent);
186+
}
187+
threadSafeDomainStatus_[domain] = !agentsByEnabledDomain_[domain].empty();
188+
}
189+
190+
bool RuntimeTarget::isDomainEnabled(Domain domain) const {
191+
return threadSafeDomainStatus_[domain];
192+
}
193+
166194
RuntimeTargetController::RuntimeTargetController(RuntimeTarget& target)
167195
: target_(target) {}
168196

@@ -192,16 +220,11 @@ RuntimeTargetController::collectSamplingProfile() {
192220
return target_.collectSamplingProfile();
193221
}
194222

195-
void RuntimeTarget::enableSamplingProfiler() {
196-
delegate_.enableSamplingProfiler();
197-
}
198-
199-
void RuntimeTarget::disableSamplingProfiler() {
200-
delegate_.disableSamplingProfiler();
201-
}
202-
203-
tracing::RuntimeSamplingProfile RuntimeTarget::collectSamplingProfile() {
204-
return delegate_.collectSamplingProfile();
223+
void RuntimeTargetController::notifyDomainStateChanged(
224+
Domain domain,
225+
bool enabled,
226+
const RuntimeAgent& notifyingAgent) {
227+
target_.notifyDomainStateChanged(domain, enabled, notifyingAgent);
205228
}
206229

207230
} // namespace facebook::react::jsinspector_modern

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#pragma once
99

1010
#include "ConsoleMessage.h"
11+
#include "EnumArray.h"
1112
#include "ExecutionContext.h"
1213
#include "InspectorInterfaces.h"
1314
#include "RuntimeAgent.h"
@@ -123,6 +124,8 @@ class RuntimeTargetDelegate {
123124
*/
124125
class RuntimeTargetController {
125126
public:
127+
enum class Domain { Network, kMaxValue };
128+
126129
explicit RuntimeTargetController(RuntimeTarget& target);
127130

128131
/**
@@ -144,6 +147,15 @@ class RuntimeTargetController {
144147
*/
145148
void notifyDebuggerSessionDestroyed();
146149

150+
/**
151+
* Notifies the target that an agent has received an enable or disable
152+
* message for the given domain.
153+
*/
154+
void notifyDomainStateChanged(
155+
Domain domain,
156+
bool enabled,
157+
const RuntimeAgent& notifyingAgent);
158+
147159
/**
148160
* Start sampling profiler for the corresponding RuntimeTarget.
149161
*/
@@ -239,6 +251,8 @@ class JSINSPECTOR_EXPORT RuntimeTarget
239251
tracing::RuntimeSamplingProfile collectSamplingProfile();
240252

241253
private:
254+
using Domain = RuntimeTargetController::Domain;
255+
242256
/**
243257
* Constructs a new RuntimeTarget. The caller must call setExecutor
244258
* immediately afterwards.
@@ -267,6 +281,19 @@ class JSINSPECTOR_EXPORT RuntimeTarget
267281
WeakList<RuntimeAgent> agents_;
268282
RuntimeTargetController controller_{*this};
269283

284+
/**
285+
* Keeps track of the agents that have enabled various domains.
286+
*/
287+
EnumArray<Domain, std::unordered_set<const RuntimeAgent*>>
288+
agentsByEnabledDomain_;
289+
290+
/**
291+
* For each Domain, contains true if the domain has been enabled by any
292+
* active agent. Unlike agentsByEnabledDomain_, this is safe to read from any
293+
* thread. \see isDomainEnabled.
294+
*/
295+
EnumArray<Domain, std::atomic<bool>> threadSafeDomainStatus_{};
296+
270297
/**
271298
* This TracingAgent is owned by the InstanceTracingAgent, both are bound to
272299
* the lifetime of their corresponding targets and the lifetime of the tracing
@@ -323,6 +350,23 @@ class JSINSPECTOR_EXPORT RuntimeTarget
323350
*/
324351
std::string createNetworkRequestId();
325352

353+
/**
354+
* Notifies the target that an agent has received an enable or disable
355+
* message for the given domain.
356+
*/
357+
void notifyDomainStateChanged(
358+
Domain domain,
359+
bool enabled,
360+
const RuntimeAgent& notifyingAgent);
361+
362+
/**
363+
* Checks whether the given domain is enabled in at least one session
364+
* that is currently connected. This may be called from any thread, with
365+
* the caveat that the result can change at arbitrary times unless the caller
366+
* is on the inspector thread.
367+
*/
368+
bool isDomainEnabled(Domain domain) const;
369+
326370
// Necessary to allow RuntimeAgent to access RuntimeTarget's internals in a
327371
// controlled way (i.e. only RuntimeTargetController gets friend access, while
328372
// RuntimeAgent itself doesn't).

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,7 @@ void RuntimeTarget::installNetworkReporterAPI() {
5252
std::optional<std::string> devToolsRequestId;
5353
tryExecuteSync(selfWeak, [&](RuntimeTarget& self) {
5454
devToolsRequestId = self.createNetworkRequestId();
55-
// TODO(moti): Instead of checking the singleton state,
56-
// directly check whether the current target has a session
57-
// with the Network domain enabled.
58-
if (NetworkHandler::getInstance().isEnabled()) {
55+
if (self.isDomainEnabled(Domain::Network)) {
5956
// Q: Why is it safe to use self.delegate_ here?
6057
// A: Because the caller of InspectorTarget::registerRuntime
6158
// is explicitly required to guarantee that the delegate not

0 commit comments

Comments
 (0)