Move SessionMetadata to HostTargetDelegate (#44878)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44878

A refactor moving `SessionMetadata` (now renamed as `HostTargetMetadata`) out of `inspectorTarget->connect()` calls into a `HostTargetDelegate::getMetadata` method. This provides a cleaner interface and location for extending metadata fields in future.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D58288491

fbshipit-source-id: 67e8b9a3fb6d0b7966187fa98d9852222f242b9d
This commit is contained in:
Alex Hunt
2024-06-12 09:39:08 -07:00
committed by Facebook GitHub Bot
parent 18be49e7bb
commit 28ded2c6cd
14 changed files with 66 additions and 50 deletions
@@ -197,6 +197,13 @@ class RCTBridgeHostTargetDelegate : public facebook::react::jsinspector_modern::
{
}
facebook::react::jsinspector_modern::HostTargetMetadata getMetadata() override
{
return {
.integrationName = "iOS Bridge (RCTBridge)",
};
}
void onReload(const PageReloadRequest &request) override
{
RCTAssertMainQueue();
@@ -458,11 +465,7 @@ RCT_NOT_IMPLEMENTED(-(instancetype)init)
// This can happen if we're about to be dealloc'd. Reject the connection.
return nullptr;
}
return strongSelf->_inspectorTarget->connect(
std::move(remote),
{
.integrationName = "iOS Bridge (RCTBridge)",
});
return strongSelf->_inspectorTarget->connect(std::move(remote));
},
{.nativePageReloads = true, .prefersFuseboxFrontend = true});
}
@@ -56,12 +56,7 @@ ReactInstanceManagerInspectorTarget::ReactInstanceManagerInspectorTarget(
[inspectorTarget =
inspectorTarget_](std::unique_ptr<IRemoteConnection> remote)
-> std::unique_ptr<ILocalConnection> {
return inspectorTarget->connect(
std::move(remote),
{
.integrationName =
"Android Bridge (ReactInstanceManagerInspectorTarget)",
});
return inspectorTarget->connect(std::move(remote));
},
{.nativePageReloads = true, .prefersFuseboxFrontend = true});
}
@@ -103,6 +98,13 @@ void ReactInstanceManagerInspectorTarget::registerNatives() {
});
}
jsinspector_modern::HostTargetMetadata
ReactInstanceManagerInspectorTarget::getMetadata() {
return {
.integrationName = "Android Bridge (ReactInstanceManagerInspectorTarget)",
};
}
void ReactInstanceManagerInspectorTarget::onReload(
const PageReloadRequest& /*request*/) {
delegate_->onReload();
@@ -51,6 +51,7 @@ class ReactInstanceManagerInspectorTarget
jsinspector_modern::HostTarget* getInspectorTarget();
// HostTargetDelegate methods
jsinspector_modern::HostTargetMetadata getMetadata() override;
void onReload(const PageReloadRequest& request) override;
void onSetPausedInDebuggerMessage(
const OverlaySetPausedInDebuggerMessageRequest&) override;
@@ -40,11 +40,7 @@ JReactHostInspectorTarget::JReactHostInspectorTarget(
std::unique_ptr<IRemoteConnection> remote)
-> std::unique_ptr<ILocalConnection> {
if (auto inspectorTarget = inspectorTargetWeak.lock()) {
return inspectorTarget->connect(
std::move(remote),
{
.integrationName = "Android Bridgeless (ReactHostImpl)",
});
return inspectorTarget->connect(std::move(remote));
}
// Reject the connection.
return nullptr;
@@ -86,6 +82,13 @@ void JReactHostInspectorTarget::registerNatives() {
});
}
jsinspector_modern::HostTargetMetadata
JReactHostInspectorTarget::getMetadata() {
return {
.integrationName = "Android Bridgeless (ReactHostImpl)",
};
}
void JReactHostInspectorTarget::onReload(const PageReloadRequest& request) {
javaReactHostImpl_->reload("CDP Page.reload");
}
@@ -58,6 +58,7 @@ class JReactHostInspectorTarget
jsinspector_modern::HostTarget* getInspectorTarget();
// HostTargetDelegate methods
jsinspector_modern::HostTargetMetadata getMetadata() override;
void onReload(const PageReloadRequest& request) override;
void onSetPausedInDebuggerMessage(
const OverlaySetPausedInDebuggerMessageRequest&) override;
@@ -27,11 +27,11 @@ namespace facebook::react::jsinspector_modern {
HostAgent::HostAgent(
FrontendChannel frontendChannel,
HostTargetController& targetController,
HostTarget::SessionMetadata sessionMetadata,
HostTargetMetadata hostMetadata,
SessionState& sessionState)
: frontendChannel_(frontendChannel),
targetController_(targetController),
sessionMetadata_(std::move(sessionMetadata)),
hostMetadata_(std::move(hostMetadata)),
sessionState_(sessionState) {}
void HostAgent::handleRequest(const cdp::PreparsedRequest& req) {
@@ -49,10 +49,10 @@ void HostAgent::handleRequest(const cdp::PreparsedRequest& req) {
}
// Send a log entry with the integration name.
if (sessionMetadata_.integrationName) {
if (hostMetadata_.integrationName) {
sendInfoLogEntry(
ANSI_COLOR_BG_YELLOW "Debugger integration: " +
*sessionMetadata_.integrationName);
*hostMetadata_.integrationName);
}
shouldSendOKResponse = true;
@@ -37,13 +37,13 @@ class HostAgent final {
* \param targetController An interface to the HostTarget that this agent is
* attached to. The caller is responsible for ensuring that the
* HostTargetDelegate and underlying HostTarget both outlive the agent.
* \param sessionMetadata Metadata about the session that created this agent.
* \param hostMetadata Metadata about the host that created this agent.
* \param sessionState The state of the session that created this agent.
*/
HostAgent(
FrontendChannel frontendChannel,
HostTargetController& targetController,
HostTarget::SessionMetadata sessionMetadata,
HostTargetMetadata hostMetadata,
SessionState& sessionState);
HostAgent(const HostAgent&) = delete;
@@ -94,7 +94,7 @@ class HostAgent final {
FrontendChannel frontendChannel_;
HostTargetController& targetController_;
const HostTarget::SessionMetadata sessionMetadata_;
const HostTargetMetadata hostMetadata_;
std::shared_ptr<InstanceAgent> instanceAgent_;
FuseboxClientType fuseboxClientType_{FuseboxClientType::Unknown};
bool isPausedInDebuggerOverlayVisible_{false};
@@ -29,7 +29,7 @@ class HostTargetSession {
explicit HostTargetSession(
std::unique_ptr<IRemoteConnection> remote,
HostTargetController& targetController,
HostTarget::SessionMetadata sessionMetadata)
HostTargetMetadata hostMetadata)
: remote_(std::make_shared<RAIIRemoteConnection>(std::move(remote))),
frontendChannel_(
[remoteWeak = std::weak_ptr(remote_)](std::string_view message) {
@@ -40,7 +40,7 @@ class HostTargetSession {
hostAgent_(
frontendChannel_,
targetController,
std::move(sessionMetadata),
std::move(hostMetadata),
state_) {}
/**
@@ -146,10 +146,9 @@ HostTarget::HostTarget(HostTargetDelegate& delegate)
executionContextManager_{std::make_shared<ExecutionContextManager>()} {}
std::unique_ptr<ILocalConnection> HostTarget::connect(
std::unique_ptr<IRemoteConnection> connectionToFrontend,
SessionMetadata sessionMetadata) {
std::unique_ptr<IRemoteConnection> connectionToFrontend) {
auto session = std::make_shared<HostTargetSession>(
std::move(connectionToFrontend), controller_, std::move(sessionMetadata));
std::move(connectionToFrontend), controller_, delegate_.getMetadata());
session->setCurrentInstance(currentInstance_.get());
sessions_.insert(std::weak_ptr(session));
return std::make_unique<CallbackLocalConnection>(
@@ -36,6 +36,10 @@ class HostAgent;
class HostCommandSender;
class HostTarget;
struct HostTargetMetadata {
std::optional<std::string> integrationName;
};
/**
* Receives events from a HostTarget. This is a shared interface that each
* React Native platform needs to implement in order to integrate with the
@@ -85,6 +89,11 @@ class HostTargetDelegate {
virtual ~HostTargetDelegate();
/**
* Returns a metadata object describing the host.
*/
virtual HostTargetMetadata getMetadata() = 0;
/**
* Called when the debugger requests a reload of the page. This is called on
* the thread on which messages are dispatched to the session (that is, where
@@ -150,10 +159,6 @@ class HostTargetController final {
class JSINSPECTOR_EXPORT HostTarget
: public EnableExecutorFromThis<HostTarget> {
public:
struct SessionMetadata {
std::optional<std::string> integrationName;
};
/**
* Constructs a new HostTarget.
* \param delegate The HostTargetDelegate that will
@@ -185,8 +190,7 @@ class JSINSPECTOR_EXPORT HostTarget
* destructor execute.
*/
std::unique_ptr<ILocalConnection> connect(
std::unique_ptr<IRemoteConnection> connectionToFrontend,
SessionMetadata sessionMetadata = {});
std::unique_ptr<IRemoteConnection> connectionToFrontend);
/**
* Registers an instance with this HostTarget.
@@ -49,9 +49,7 @@ class HostTargetTest : public Test {
std::pair<std::unique_ptr<ILocalConnection>, MockRemoteConnection&>
makeConnection() {
size_t connectionIndex = remoteConnections_.objectsVended();
auto toPage = page_->connect(
remoteConnections_.make_unique(),
{.integrationName = "HostTargetTest"});
auto toPage = page_->connect(remoteConnections_.make_unique());
// We'll always get an onDisconnect call when we tear
// down the test. Expect it in order to satisfy the strict mock.
@@ -118,6 +118,9 @@ class MockInspectorPackagerConnectionDelegate
class MockHostTargetDelegate : public HostTargetDelegate {
public:
// HostTargetDelegate methods
HostTargetMetadata getMetadata() override {
return {.integrationName = "MockHostTargetDelegate"};
}
MOCK_METHOD(void, onReload, (const PageReloadRequest& request), (override));
MOCK_METHOD(
void,
@@ -91,9 +91,7 @@ class JsiIntegrationPortableTest : public ::testing::Test,
void connect() {
ASSERT_FALSE(toPage_) << "Can only connect once in a JSI integration test.";
toPage_ = page_->connect(
remoteConnections_.make_unique(),
{.integrationName = "JsiIntegrationTest"});
toPage_ = page_->connect(remoteConnections_.make_unique());
using namespace ::testing;
// Default to ignoring console messages originating inside the backend.
@@ -179,6 +177,10 @@ class JsiIntegrationPortableTest : public ::testing::Test,
private:
// HostTargetDelegate methods
HostTargetMetadata getMetadata() override {
return {.integrationName = "JsiIntegrationTest"};
}
void onReload(const PageReloadRequest& request) override {
(void)request;
reload();
@@ -94,11 +94,8 @@ void ReactInstanceIntegrationTest::SetUp() {
"mock-vm",
[hostTargetIfModernCDP](std::unique_ptr<IRemoteConnection> remote)
-> std::unique_ptr<ILocalConnection> {
auto localConnection = hostTargetIfModernCDP->connect(
std::move(remote),
{
.integrationName = "ReactInstanceIntegrationTest",
});
auto localConnection =
hostTargetIfModernCDP->connect(std::move(remote));
return localConnection;
},
// TODO: Allow customisation of InspectorTargetCapabilities
@@ -40,6 +40,13 @@ class RCTHostHostTargetDelegate : public facebook::react::jsinspector_modern::Ho
{
}
jsinspector_modern::HostTargetMetadata getMetadata() override
{
return {
.integrationName = "iOS Bridgeless (RCTHost)",
};
}
void onReload(const PageReloadRequest &request) override
{
RCTAssertMainQueue();
@@ -233,11 +240,7 @@ class RCTHostHostTargetDelegate : public facebook::react::jsinspector_modern::Ho
// This can happen if we're about to be dealloc'd. Reject the connection.
return nullptr;
}
return strongSelf->_inspectorTarget->connect(
std::move(remote),
{
.integrationName = "iOS Bridgeless (RCTHost)",
});
return strongSelf->_inspectorTarget->connect(std::move(remote));
},
{.nativePageReloads = true, .prefersFuseboxFrontend = true});
}