Skip to content

Commit

Permalink
Create PageTarget{Delegate,Controller}, respond to Page.reload (#42587)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #42587

Changelog: [Internal]

* Introduces the Target Delegate and Target Controller concepts (see `CONCEPTS.md`).
* Introduces the `PageTargetDelegate` interface and `PageTargetController` class (see doc comments).
* Uses the above infra to implement support for the `Page.reload` CDP command. Each integration provides its own `PageTargetDelegate` that knows how to trigger a reload in a platform- and architecture-specific way.
  * iOS Bridge/Bridgeless and `PageTargetTest` are the only integrations that exist as of this diff, and are all updated here; Android will follow later.

NOTE: `RCTBridge` = iOS Bridge, `RCTHost` = iOS Bridgeless.

## Object lifetimes

`PageAgent` holds a raw `PageTargetController&` reference to a member of `PageTarget`, through which it gets access to that target's `PageTargetDelegate&` (another raw reference).

Here's what makes this safe:

1. **`PageTargetDelegate` outlives `PageTarget`** - this is the responsibility of the platform integration ( = the code that instantiates `PageTarget`).
2. **`PageTarget` outlives its Sessions and Agents** - this is `PageTarget`'s "moral" responsibility, even though it doesn't own its Sessions outright (`InspectorPackagerConnection` does). We add an assertion in `PageTarget`'s destructor to catch violations, and document that the integrator must call `getInspectorInstance().removePage` (which terminates all remaining sessions) before destroying the corresponding `PageTarget`.

NOTE: In upcoming diffs we'll use the new Target→Session references, currently used only for the assertion in (2), to power actual functionality (e.g. dispatching CDP events to the frontend when some imperative method is called on `PageTarget`).

## Thread safety

`PageTargetDelegate::onReload` is guaranteed to be called synchronously on the thread where messages are dispatched to `PageTargetSession`, which on iOS is the main (UI) thread.

Reviewed By: huntie

Differential Revision: D51164125

fbshipit-source-id: 4c3eeb81a8df9677c173588eb5acfd686722c3c9
  • Loading branch information
motiz88 authored and facebook-github-bot committed Jan 24, 2024
1 parent b8778ab commit be441f8
Show file tree
Hide file tree
Showing 10 changed files with 271 additions and 11 deletions.
18 changes: 17 additions & 1 deletion packages/react-native/React/Base/RCTBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,27 @@ void RCTUIManagerSetDispatchAccessibilityManagerInitOntoMain(BOOL enabled)
kDispatchAccessibilityManagerInitOntoMain = enabled;
}

class RCTBridgePageTargetDelegate : public facebook::react::jsinspector_modern::PageTargetDelegate {
public:
RCTBridgePageTargetDelegate(RCTBridge *bridge) : bridge_(bridge) {}

void onReload(const PageReloadRequest &request) override
{
RCTAssertMainQueue();
[bridge_ reload];
}

private:
__weak RCTBridge *bridge_;
};

@interface RCTBridge () <RCTReloadListener>
@end

@implementation RCTBridge {
NSURL *_delegateBundleURL;

std::unique_ptr<RCTBridgePageTargetDelegate> _inspectorPageDelegate;
std::unique_ptr<facebook::react::jsinspector_modern::PageTarget> _inspectorTarget;
std::optional<int> _inspectorPageId;
}
Expand Down Expand Up @@ -244,6 +259,7 @@ - (instancetype)initWithDelegate:(id<RCTBridgeDelegate>)delegate
_bundleURL = bundleURL;
_moduleProvider = block;
_launchOptions = [launchOptions copy];
_inspectorPageDelegate = std::make_unique<RCTBridgePageTargetDelegate>(self);

[self setUp];
}
Expand Down Expand Up @@ -394,7 +410,7 @@ - (void)setUp

auto &inspectorFlags = facebook::react::jsinspector_modern::InspectorFlags::getInstance();
if (inspectorFlags.getEnableModernCDPRegistry() && !_inspectorPageId.has_value()) {
_inspectorTarget = std::make_unique<facebook::react::jsinspector_modern::PageTarget>();
_inspectorTarget = std::make_unique<facebook::react::jsinspector_modern::PageTarget>(*_inspectorPageDelegate);
__weak RCTBridge *weakSelf = self;
_inspectorPageId = facebook::react::jsinspector_modern::getInspectorInstance().addPage(
"React Native Bridge (Experimental)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@

A debuggable entity that a debugger frontend can connect to.

### Target Delegate

An interface between a Target class and the underlying debuggable entity. For example, PageTargetDelegate is used by PageTarget to send page-related events to the native platform implementation.

### Target Controller

A private interface exposed by a Target class to its Sessions/Agents. For example, PageTargetController is used by PageAgent to safely access the page's PageTargetDelegate, without exposing PageTarget's other private state.

### Session

A single connection between a debugger frontend and a target. There can be multiple active sessions connected to the same target.
Expand Down
21 changes: 21 additions & 0 deletions packages/react-native/ReactCommon/jsinspector-modern/PageAgent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <folly/dynamic.h>
#include <folly/json.h>
#include <jsinspector-modern/PageAgent.h>
#include <jsinspector-modern/PageTarget.h>

#include <chrono>

Expand All @@ -29,8 +30,10 @@ static constexpr auto kModernCDPBackendNotice =

PageAgent::PageAgent(
FrontendChannel frontendChannel,
PageTargetController& targetController,
PageTarget::SessionMetadata sessionMetadata)
: frontendChannel_(frontendChannel),
targetController_(targetController),
sessionMetadata_(std::move(sessionMetadata)) {}

void PageAgent::handleRequest(const cdp::PreparsedRequest& req) {
Expand All @@ -49,6 +52,24 @@ void PageAgent::handleRequest(const cdp::PreparsedRequest& req) {

return;
}

if (req.method == "Page.reload") {
targetController_.getDelegate().onReload({
.ignoreCache = req.params.isObject() && req.params.count("ignoreCache")
? std::optional(req.params.at("ignoreCache").asBool())
: std::nullopt,
.scriptToEvaluateOnLoad =
req.params.isObject() && req.params.count("scriptToEvaluateOnLoad")
? std::optional(req.params.at("scriptToEvaluateOnLoad").asString())
: std::nullopt,
});
folly::dynamic res = folly::dynamic::object("id", req.id)(
"result", folly::dynamic::object());
std::string json = folly::toJson(res);
frontendChannel_(json);
return;
}

folly::dynamic res = folly::dynamic::object("id", req.id)(
"error",
folly::dynamic::object("code", -32601)(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,27 @@

namespace facebook::react::jsinspector_modern {

class PageTarget;

/**
* An Agent that handles requests from the Chrome DevTools Protocol for the
* given page.
* The constructor, destructor and all public methods must be called on the
* same thread.
* same thread, which is also the thread where the associated PageTarget is
* constructed and managed.
*/
class PageAgent {
public:
/**
* \param frontendChannel A channel used to send responses and events to the
* frontend.
* \param targetController An interface to the PageTarget that this agent is
* attached to. The caller is responsible for ensuring that the
* PageTargetDelegate and underlying PageTarget both outlive the agent.
*/
PageAgent(
FrontendChannel frontendChannel,
PageTargetController& targetController,
PageTarget::SessionMetadata sessionMetadata);

/**
Expand All @@ -53,6 +60,7 @@ class PageAgent {
void sendInfoLogEntry(std::string_view text);

FrontendChannel frontendChannel_;
PageTargetController& targetController_;
const PageTarget::SessionMetadata sessionMetadata_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

namespace facebook::react::jsinspector_modern {

namespace {

/**
* A Session connected to a PageTarget, passing CDP messages to and from a
* PageAgent which it owns.
Expand All @@ -27,6 +25,7 @@ class PageTargetSession {
public:
explicit PageTargetSession(
std::unique_ptr<IRemoteConnection> remote,
PageTargetController& targetController,
PageTarget::SessionMetadata sessionMetadata)
: remote_(std::make_shared<RAIIRemoteConnection>(std::move(remote))),
frontendChannel_(
Expand All @@ -35,7 +34,11 @@ class PageTargetSession {
remote->onMessage(std::string(message));
}
}),
pageAgent_(frontendChannel_, std::move(sessionMetadata)) {}
pageAgent_(
frontendChannel_,
targetController,
std::move(sessionMetadata)) {}

/**
* Called by CallbackLocalConnection to send a message to this Session's
* Agent.
Expand Down Expand Up @@ -76,13 +79,40 @@ class PageTargetSession {
PageAgent pageAgent_;
};

} // namespace
PageTarget::PageTarget(PageTargetDelegate& delegate) : delegate_(delegate) {}

std::unique_ptr<ILocalConnection> PageTarget::connect(
std::unique_ptr<IRemoteConnection> connectionToFrontend,
SessionMetadata sessionMetadata) {
return std::make_unique<CallbackLocalConnection>(PageTargetSession(
std::move(connectionToFrontend), std::move(sessionMetadata)));
auto session = std::make_shared<PageTargetSession>(
std::move(connectionToFrontend), controller_, std::move(sessionMetadata));
sessions_.push_back(std::weak_ptr(session));
return std::make_unique<CallbackLocalConnection>(
[session](std::string message) { (*session)(message); });
}

void PageTarget::removeExpiredSessions() {
// Remove all expired sessions.
forEachSession([](auto&) {});
}

PageTarget::~PageTarget() {
removeExpiredSessions();

// Sessions are owned by InspectorPackagerConnection, not by PageTarget, but
// they hold a PageTarget& that we must guarantee is valid.
assert(
sessions_.empty() &&
"PageTargetSession objects must be destroyed before their PageTarget. Did you call getInspectorInstance().removePage()?");
}

PageTargetDelegate::~PageTargetDelegate() {}

PageTargetController::PageTargetController(PageTarget& target)
: target_(target) {}

PageTargetDelegate& PageTargetController::getDelegate() {
return target_.getDelegate();
}

} // namespace facebook::react::jsinspector_modern
108 changes: 108 additions & 0 deletions packages/react-native/ReactCommon/jsinspector-modern/PageTarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <jsinspector-modern/InspectorInterfaces.h>

#include <list>
#include <optional>
#include <string>

Expand All @@ -26,6 +27,66 @@

namespace facebook::react::jsinspector_modern {

class PageTargetSession;
class PageAgent;
class PageTarget;

/**
* Receives events from a PageTarget. This is a shared interface that each
* React Native platform needs to implement in order to integrate with the
* debugging stack.
*/
class PageTargetDelegate {
public:
PageTargetDelegate() = default;
PageTargetDelegate(const PageTargetDelegate&) = delete;
PageTargetDelegate(PageTargetDelegate&&) = default;
PageTargetDelegate& operator=(const PageTargetDelegate&) = delete;
PageTargetDelegate& operator=(PageTargetDelegate&&) = default;

// TODO(moti): This is 1:1 the shape of the corresponding CDP message -
// consider reusing typed/generated CDP interfaces when we have those.
struct PageReloadRequest {
// It isn't clear what the ignoreCache parameter of @cdp Page.reload should
// mean in React Native. We parse it, but don't do anything with it yet.
std::optional<bool> ignoreCache;

// TODO: Implement scriptToEvaluateOnLoad parameter of @cdp Page.reload.
std::optional<std::string> scriptToEvaluateOnLoad;

/**
* Equality operator, useful for unit tests
*/
inline bool operator==(const PageReloadRequest& rhs) const {
return ignoreCache == rhs.ignoreCache &&
scriptToEvaluateOnLoad == rhs.scriptToEvaluateOnLoad;
}
};

virtual ~PageTargetDelegate();

/**
* 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
* ILocalConnection::sendMessage was called).
*/
virtual void onReload(const PageReloadRequest& request) = 0;
};

/**
* The limited interface that PageTarget exposes to its associated
* sessions/agents.
*/
class PageTargetController {
public:
explicit PageTargetController(PageTarget& target);

PageTargetDelegate& getDelegate();

private:
PageTarget& target_;
};

/**
* The top-level Target in a React Native app. This is equivalent to the
* "Host" in React Native's architecture - the entity that manages the
Expand All @@ -37,6 +98,20 @@ class JSINSPECTOR_EXPORT PageTarget {
std::optional<std::string> integrationName;
};

/**
* Constructs a new PageTarget.
* \param delegate The PageTargetDelegate that will receive events from
* this PageTarget. The caller is responsible for ensuring that the
* PageTargetDelegate outlives this object.
*/
explicit PageTarget(PageTargetDelegate& delegate);

PageTarget(const PageTarget&) = delete;
PageTarget(PageTarget&&) = default;
PageTarget& operator=(const PageTarget&) = delete;
PageTarget& operator=(PageTarget&&) = delete;
~PageTarget();

/**
* Creates a new Session connected to this PageTarget, wrapped in an
* interface which is compatible with \c IInspector::addPage.
Expand All @@ -47,6 +122,39 @@ class JSINSPECTOR_EXPORT PageTarget {
std::unique_ptr<ILocalConnection> connect(
std::unique_ptr<IRemoteConnection> connectionToFrontend,
SessionMetadata sessionMetadata = {});

private:
std::list<std::weak_ptr<PageTargetSession>> sessions_;

PageTargetDelegate& delegate_;
PageTargetController controller_{*this};

/**
* Call the given function for every active session, and clean up any
* references to inactive sessions.
*/
template <typename Fn>
void forEachSession(Fn&& fn) {
for (auto it = sessions_.begin(); it != sessions_.end();) {
if (auto session = it->lock()) {
fn(*session);
++it;
} else {
it = sessions_.erase(it);
}
}
}

void removeExpiredSessions();

inline PageTargetDelegate& getDelegate() {
return delegate_;
}

// Necessary to allow PageAgent to access PageTarget's internals in a
// controlled way (i.e. only PageTargetController gets friend access, while
// PageAgent itself doesn't).
friend class PageTargetController;
};

} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ Pod::Spec.new do |s|
s.dependency "glog"
s.dependency "RCT-Folly", folly_version
s.dependency "React-nativeconfig"
s.dependency "DoubleConversion"
end
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <jsinspector-modern/InspectorInterfaces.h>
#include <jsinspector-modern/InspectorPackagerConnection.h>
#include <jsinspector-modern/ReactCdp.h>

#include <chrono>
#include <functional>
Expand Down Expand Up @@ -114,4 +115,10 @@ class MockInspectorPackagerConnectionDelegate
folly::Executor& executor_;
};

class MockPageTargetDelegate : public PageTargetDelegate {
public:
// PageTargetDelegate methods
MOCK_METHOD(void, onReload, (const PageReloadRequest& request), (override));
};

} // namespace facebook::react::jsinspector_modern
Loading

0 comments on commit be441f8

Please sign in to comment.