Skip to content

Commit

Permalink
Migrate from legacy to new Hermes CDP handler (#39367)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #39367

Updates React Native to use the new CDP handler provided by the Hermes engine instead of the legacy one (`Connection.cpp`) built into React Native. The new Hermes CDP handler has a simpler & safer design, new features (e.g. `console.log` buffering) and is under active development by the Hermes team.

NOTE: Both the legacy and modern handlers are Hermes-specific. In future work, React Native will *wrap* the modern Hermes handler in an engine-agnostic CDP layer implementing additional functionality and managing the lifecycle of debugging sessions more correctly. This diff is the first step of this larger migration.

Changelog: [General][Changed] Use new Hermes CDP handler implementation for debugging

Reviewed By: cipolleschi

Differential Revision: D48783980

fbshipit-source-id: 4d2ca3fa04e96e92a38d82c90737cb660ba56655
  • Loading branch information
huntie authored and facebook-github-bot committed Sep 14, 2023
1 parent f2884a7 commit 3e7a873
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
#include <hermes/hermes.h>
#include <jsi/decorator.h>

#include <hermes/inspector-modern/RuntimeAdapter.h>
#include <hermes/inspector-modern/chrome/Registration.h>
#include <hermes/inspector/RuntimeAdapter.h>

#include "JSITracing.h"

Expand All @@ -26,6 +26,8 @@ namespace facebook::react {

namespace {

#ifdef HERMES_ENABLE_DEBUGGER

class HermesExecutorRuntimeAdapter
: public facebook::hermes::inspector_modern::RuntimeAdapter {
public:
Expand Down Expand Up @@ -56,6 +58,8 @@ class HermesExecutorRuntimeAdapter
std::shared_ptr<MessageQueueThread> thread_;
};

#endif // HERMES_ENABLE_DEBUGGER

struct ReentrancyCheck {
// This is effectively a very subtle and complex assert, so only
// include it in builds which would include asserts.
Expand Down Expand Up @@ -139,6 +143,7 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator<ReentrancyCheck> {
const std::string& debuggerName)
: jsi::WithRuntimeDecorator<ReentrancyCheck>(*runtime, reentrancyCheck_),
runtime_(std::move(runtime)) {
#ifdef HERMES_ENABLE_DEBUGGER
enableDebugger_ = enableDebugger;
if (enableDebugger_) {
std::shared_ptr<HermesRuntime> rt(runtime_, &hermesRuntime);
Expand All @@ -147,12 +152,15 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator<ReentrancyCheck> {
debugToken_ = facebook::hermes::inspector_modern::chrome::enableDebugging(
std::move(adapter), debuggerName);
}
#endif // HERMES_ENABLE_DEBUGGER
}

~DecoratedRuntime() {
#ifdef HERMES_ENABLE_DEBUGGER
if (enableDebugger_) {
facebook::hermes::inspector_modern::chrome::disableDebugging(debugToken_);
}
#endif // HERMES_ENABLE_DEBUGGER
}

private:
Expand All @@ -165,8 +173,10 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator<ReentrancyCheck> {

std::shared_ptr<Runtime> runtime_;
ReentrancyCheck reentrancyCheck_;
#ifdef HERMES_ENABLE_DEBUGGER
bool enableDebugger_;
facebook::hermes::inspector_modern::chrome::DebugSessionToken debugToken_;
#endif // HERMES_ENABLE_DEBUGGER
};

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
*/

#include "ConnectionDemux.h"
#include "Connection.h"

#ifdef HERMES_ENABLE_DEBUGGER

#include <hermes/inspector/RuntimeAdapter.h>
#include <hermes/inspector/chrome/CDPHandler.h>

#include <jsinspector-modern/InspectorInterfaces.h>

Expand All @@ -24,20 +28,20 @@ namespace {
class LocalConnection : public ILocalConnection {
public:
LocalConnection(
std::shared_ptr<Connection> conn,
std::shared_ptr<hermes::inspector_modern::chrome::CDPHandler> conn,
std::shared_ptr<std::unordered_set<std::string>> inspectedContexts);
~LocalConnection();

void sendMessage(std::string message) override;
void disconnect() override;

private:
std::shared_ptr<Connection> conn_;
std::shared_ptr<hermes::inspector_modern::chrome::CDPHandler> conn_;
std::shared_ptr<std::unordered_set<std::string>> inspectedContexts_;
};

LocalConnection::LocalConnection(
std::shared_ptr<Connection> conn,
std::shared_ptr<hermes::inspector_modern::chrome::CDPHandler> conn,
std::shared_ptr<std::unordered_set<std::string>> inspectedContexts)
: conn_(conn), inspectedContexts_(inspectedContexts) {
inspectedContexts_->insert(conn->getTitle());
Expand All @@ -46,12 +50,12 @@ LocalConnection::LocalConnection(
LocalConnection::~LocalConnection() = default;

void LocalConnection::sendMessage(std::string str) {
conn_->sendMessage(std::move(str));
conn_->handle(std::move(str));
}

void LocalConnection::disconnect() {
inspectedContexts_->erase(conn_->getTitle());
conn_->disconnect();
conn_->unregisterCallbacks();
}

} // namespace
Expand Down Expand Up @@ -87,8 +91,8 @@ DebugSessionToken ConnectionDemux::enableDebugging(

auto waitForDebugger =
(inspectedContexts_->find(title) != inspectedContexts_->end());
return addPage(
std::make_shared<Connection>(std::move(adapter), title, waitForDebugger));
return addPage(hermes::inspector_modern::chrome::CDPHandler::create(
std::move(adapter), title, waitForDebugger));
}

void ConnectionDemux::disableDebugging(DebugSessionToken session) {
Expand All @@ -99,10 +103,19 @@ void ConnectionDemux::disableDebugging(DebugSessionToken session) {
removePage(session);
}

int ConnectionDemux::addPage(std::shared_ptr<Connection> conn) {
int ConnectionDemux::addPage(
std::shared_ptr<hermes::inspector_modern::chrome::CDPHandler> conn) {
auto connectFunc = [conn, this](std::unique_ptr<IRemoteConnection> remoteConn)
-> std::unique_ptr<ILocalConnection> {
if (!conn->connect(std::move(remoteConn))) {
// This cannot be unique_ptr as std::function is copyable but unique_ptr
// isn't. TODO: Change the CDPHandler API to accommodate this and not
// require a copyable callback?
std::shared_ptr<IRemoteConnection> sharedConn = std::move(remoteConn);
if (!conn->registerCallbacks(
[sharedConn](const std::string &message) {
sharedConn->onMessage(message);
},
[sharedConn]() { sharedConn->onDisconnect(); })) {
return nullptr;
}

Expand All @@ -122,11 +135,13 @@ void ConnectionDemux::removePage(int pageId) {
auto conn = conns_.at(pageId);
std::string title = conn->getTitle();
inspectedContexts_->erase(title);
conn->disconnect();
conn->unregisterCallbacks();
conns_.erase(pageId);
}

} // namespace chrome
} // namespace inspector_modern
} // namespace hermes
} // namespace facebook

#endif // HERMES_ENABLE_DEBUGGER
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@

#pragma once

#ifdef HERMES_ENABLE_DEBUGGER

#include <memory>
#include <mutex>
#include <string>
#include <unordered_map>
#include <unordered_set>

#include <hermes/hermes.h>
#include <hermes/inspector-modern/RuntimeAdapter.h>
#include <hermes/inspector-modern/chrome/Connection.h>
#include <hermes/inspector-modern/chrome/Registration.h>
#include <hermes/inspector/RuntimeAdapter.h>
#include <hermes/inspector/chrome/CDPHandler.h>
#include <jsinspector-modern/InspectorInterfaces.h>

namespace facebook {
Expand Down Expand Up @@ -44,17 +46,23 @@ class ConnectionDemux {
void disableDebugging(DebugSessionToken session);

private:
int addPage(std::shared_ptr<Connection> conn);
int addPage(
std::shared_ptr<hermes::inspector_modern::chrome::CDPHandler> conn);
void removePage(int pageId);

facebook::react::jsinspector_modern::IInspector &globalInspector_;

std::mutex mutex_;
std::unordered_map<int, std::shared_ptr<Connection>> conns_;
std::unordered_map<
int,
std::shared_ptr<hermes::inspector_modern::chrome::CDPHandler>>
conns_;
std::shared_ptr<std::unordered_set<std::string>> inspectedContexts_;
};

} // namespace chrome
} // namespace inspector_modern
} // namespace hermes
} // namespace facebook

#endif // HERMES_ENABLE_DEBUGGER
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include "Registration.h"
#include "ConnectionDemux.h"

#ifdef HERMES_ENABLE_DEBUGGER

namespace facebook {
namespace hermes {
namespace inspector_modern {
Expand Down Expand Up @@ -37,3 +39,5 @@ void disableDebugging(DebugSessionToken session) {
} // namespace inspector_modern
} // namespace hermes
} // namespace facebook

#endif // HERMES_ENABLE_DEBUGGER
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@

#pragma once

#ifdef HERMES_ENABLE_DEBUGGER

#include <memory>
#include <string>

#include <hermes/hermes.h>
#include <hermes/inspector-modern/RuntimeAdapter.h>
#include <hermes/inspector/RuntimeAdapter.h>

namespace facebook {
namespace hermes {
Expand Down Expand Up @@ -41,3 +43,5 @@ extern void disableDebugging(DebugSessionToken session);
} // namespace inspector_modern
} // namespace hermes
} // namespace facebook

#endif // HERMES_ENABLE_DEBUGGER
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,11 @@ target_link_libraries(bridgelesshermes
jsi
hermes_executor_common
)

if(${CMAKE_BUILD_TYPE} MATCHES Debug)
target_compile_options(
bridgelesshermes
PRIVATE
-DHERMES_ENABLE_DEBUGGER=1
)
endif()
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
#include <jsi/jsilib.h>

#ifdef HERMES_ENABLE_DEBUGGER
#include <hermes/inspector-modern/RuntimeAdapter.h>
#include <hermes/inspector-modern/chrome/Registration.h>
#include <hermes/inspector/RuntimeAdapter.h>
#include <jsi/decorator.h>
#endif

Expand Down

0 comments on commit 3e7a873

Please sign in to comment.