Skip to content

Commit

Permalink
Change HermesRuntime to jsi::Runtime in RuntimeAdapter
Browse files Browse the repository at this point in the history
Summary:
Changelog: [Internal]

The inspector API doesn't really need a `HermesRuntime`, all it needs is a `jsi::Runtime` and a `Debugger &`.
Change the return type of `RuntimeAdapter::getRuntime` to be `jsi::Runtime`.
This will allow the inspector to use the tracing runtime instead of the direct hermes runtime.

Reviewed By: willholen

Differential Revision: D18973867

fbshipit-source-id: 6809e52452a35e62be9ca8143aeaba8964c98eaa
  • Loading branch information
Riley Dulin authored and facebook-github-bot committed Jan 23, 2020
1 parent f92b161 commit e6f3388
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,21 @@ class HermesExecutorRuntimeAdapter

virtual ~HermesExecutorRuntimeAdapter() = default;

HermesRuntime &getRuntime() override {
return hermesRuntime_;
jsi::Runtime &getRuntime() override {
return *runtime_;
}

debugger::Debugger &getDebugger() override {
return hermesRuntime_.getDebugger();
}

void tickleJs() override {
// The queue will ensure that runtime_ is still valid when this
// gets invoked.
// clang-format off
thread_->runOnQueue([&runtime = hermesRuntime_]() {
// clang-format on
auto func = runtime.global().getPropertyAsFunction(runtime, "__tickleJs");
func.call(runtime);
thread_->runOnQueue([&runtime = runtime_]() {
auto func =
runtime->global().getPropertyAsFunction(*runtime, "__tickleJs");
func.call(*runtime);
});
}

Expand Down Expand Up @@ -193,7 +196,7 @@ std::unique_ptr<JSExecutor> HermesExecutorFactory::createJSExecutor(
std::shared_ptr<MessageQueueThread> jsQueue) {
std::unique_ptr<HermesRuntime> hermesRuntime =
makeHermesRuntimeSystraced(runtimeConfig_);
HermesRuntime& hermesRuntimeRef = *hermesRuntime;
HermesRuntime &hermesRuntimeRef = *hermesRuntime;
auto decoratedRuntime = std::make_shared<DecoratedRuntime>(
makeTracingHermesRuntime(std::move(hermesRuntime), runtimeConfig_),
hermesRuntimeRef,
Expand Down
7 changes: 4 additions & 3 deletions ReactCommon/hermes/inspector/Inspector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,13 @@ Inspector::Inspector(
InspectorObserver &observer,
bool pauseOnFirstStatement)
: adapter_(adapter),
debugger_(adapter->getRuntime().getDebugger()),
debugger_(adapter->getDebugger()),
observer_(observer),
executor_(std::make_unique<detail::SerialExecutor>("hermes-inspector")) {
// TODO (t26491391): make tickleJs a real Hermes runtime API
const char *src = "function __tickleJs() { return Math.random(); }";
adapter->getRuntime().debugJavaScript(src, "__tickleJsHackUrl", {});
std::string src = "function __tickleJs() { return Math.random(); }";
adapter->getRuntime().evaluateJavaScript(
std::make_shared<jsi::StringBuffer>(src), "__tickleJsHackUrl");

{
std::lock_guard<std::mutex> lock(mutex_);
Expand Down
11 changes: 8 additions & 3 deletions ReactCommon/hermes/inspector/RuntimeAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,20 @@ RuntimeAdapter::~RuntimeAdapter() = default;
void RuntimeAdapter::tickleJs() {}

SharedRuntimeAdapter::SharedRuntimeAdapter(
std::shared_ptr<HermesRuntime> runtime)
: runtime_(std::move(runtime)) {}
std::shared_ptr<jsi::Runtime> runtime,
debugger::Debugger &debugger)
: runtime_(std::move(runtime)), debugger_(debugger) {}

SharedRuntimeAdapter::~SharedRuntimeAdapter() = default;

HermesRuntime &SharedRuntimeAdapter::getRuntime() {
jsi::Runtime &SharedRuntimeAdapter::getRuntime() {
return *runtime_;
}

debugger::Debugger &SharedRuntimeAdapter::getDebugger() {
return debugger_;
}

} // namespace inspector
} // namespace hermes
} // namespace facebook
15 changes: 10 additions & 5 deletions ReactCommon/hermes/inspector/RuntimeAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ class RuntimeAdapter {
public:
virtual ~RuntimeAdapter() = 0;

/// getRuntime should return the Hermes runtime encapsulated by this adapter.
virtual HermesRuntime &getRuntime() = 0;
/// getRuntime should return the runtime encapsulated by this adapter.
virtual jsi::Runtime &getRuntime() = 0;
virtual debugger::Debugger &getDebugger() = 0;

/// tickleJs is a method that subclasses can choose to override to make the
/// inspector more responsive. If overridden, it should call the "__tickleJs"
Expand All @@ -50,13 +51,17 @@ class RuntimeAdapter {
*/
class SharedRuntimeAdapter : public RuntimeAdapter {
public:
SharedRuntimeAdapter(std::shared_ptr<HermesRuntime> runtime);
SharedRuntimeAdapter(
std::shared_ptr<jsi::Runtime> runtime,
debugger::Debugger &debugger);
virtual ~SharedRuntimeAdapter();

HermesRuntime &getRuntime() override;
jsi::Runtime &getRuntime() override;
debugger::Debugger &getDebugger() override;

private:
std::shared_ptr<HermesRuntime> runtime_;
std::shared_ptr<jsi::Runtime> runtime_;
debugger::Debugger &debugger_;
};

} // namespace inspector
Expand Down
8 changes: 4 additions & 4 deletions ReactCommon/hermes/inspector/chrome/Connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Connection::Impl : public inspector::InspectorObserver,
bool waitForDebugger);
~Impl();

HermesRuntime &getRuntime();
jsi::Runtime &getRuntime();
std::string getTitle() const;

bool connect(std::unique_ptr<IRemoteConnection> remoteConn);
Expand Down Expand Up @@ -151,7 +151,7 @@ Connection::Impl::Impl(

Connection::Impl::~Impl() = default;

HermesRuntime &Connection::Impl::getRuntime() {
jsi::Runtime &Connection::Impl::getRuntime() {
return runtimeAdapter_->getRuntime();
}

Expand Down Expand Up @@ -617,7 +617,7 @@ Connection::Impl::makePropsFromValue(
std::vector<m::runtime::PropertyDescriptor> result;

if (value.isObject()) {
HermesRuntime &runtime = getRuntime();
jsi::Runtime &runtime = getRuntime();
jsi::Object obj = value.getObject(runtime);

// TODO(hypuk): obj.getPropertyNames only returns enumerable properties.
Expand Down Expand Up @@ -768,7 +768,7 @@ Connection::Connection(

Connection::~Connection() = default;

HermesRuntime &Connection::getRuntime() {
jsi::Runtime &Connection::getRuntime() {
return impl_->getRuntime();
}

Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/hermes/inspector/chrome/Connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Connection {
~Connection();

/// getRuntime returns the underlying runtime being debugged.
HermesRuntime &getRuntime();
jsi::Runtime &getRuntime();

/// getTitle returns the name of the friendly name of the runtime that's shown
/// to users in Nuclide.
Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/hermes/inspector/chrome/MessageConverters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ m::debugger::CallFrame m::debugger::makeCallFrame(
const h::debugger::CallFrameInfo &callFrameInfo,
const h::debugger::LexicalInfo &lexicalInfo,
RemoteObjectsTable &objTable,
HermesRuntime &runtime,
jsi::Runtime &runtime,
const facebook::hermes::debugger::ProgramState &state) {
m::debugger::CallFrame result;

Expand Down Expand Up @@ -115,7 +115,7 @@ m::debugger::CallFrame m::debugger::makeCallFrame(
std::vector<m::debugger::CallFrame> m::debugger::makeCallFrames(
const h::debugger::ProgramState &state,
RemoteObjectsTable &objTable,
HermesRuntime &runtime) {
jsi::Runtime &runtime) {
const h::debugger::StackTrace &stackTrace = state.getStackTrace();
uint32_t count = stackTrace.callFrameCount();

Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/hermes/inspector/chrome/MessageConverters.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ CallFrame makeCallFrame(
const facebook::hermes::debugger::CallFrameInfo &callFrameInfo,
const facebook::hermes::debugger::LexicalInfo &lexicalInfo,
facebook::hermes::inspector::chrome::RemoteObjectsTable &objTable,
HermesRuntime &runtime,
jsi::Runtime &runtime,
const facebook::hermes::debugger::ProgramState &state);

std::vector<CallFrame> makeCallFrames(
const facebook::hermes::debugger::ProgramState &state,
facebook::hermes::inspector::chrome::RemoteObjectsTable &objTable,
HermesRuntime &runtime);
jsi::Runtime &runtime);

} // namespace debugger

Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/hermes/inspector/chrome/cli/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ static void runDebuggerLoop(
static void runScript(const std::string &scriptSource, const std::string &url) {
std::shared_ptr<fbhermes::HermesRuntime> runtime(
fbhermes::makeHermesRuntime());
auto adapter =
std::make_unique<fbhermes::inspector::SharedRuntimeAdapter>(runtime);
auto adapter = std::make_unique<fbhermes::inspector::SharedRuntimeAdapter>(
runtime, runtime->getDebugger());
fbhermes::inspector::chrome::Connection conn(
std::move(adapter), "hermes-chrome-debug-server");
std::thread debuggerLoop(runDebuggerLoop, std::ref(conn), scriptSource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,11 @@ TEST(ConnectionDemuxTests, TestEnableDisable) {
ConnectionDemux demux{*inspector};

int id1 = demux.enableDebugging(
std::make_unique<SharedRuntimeAdapter>(runtime1), "page1");
std::make_unique<SharedRuntimeAdapter>(runtime1, runtime1->getDebugger()),
"page1");
int id2 = demux.enableDebugging(
std::make_unique<SharedRuntimeAdapter>(runtime2), "page2");
std::make_unique<SharedRuntimeAdapter>(runtime2, runtime2->getDebugger()),
"page2");

expectPages(*inspector, {{id1, "page1"}, {id2, "page2"}});

Expand Down
4 changes: 3 additions & 1 deletion ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class SyncConnection::RemoteConnnection : public IRemoteConnection {

SyncConnection::SyncConnection(std::shared_ptr<HermesRuntime> runtime)
: connection_(
std::make_unique<SharedRuntimeAdapter>(std::move(runtime)),
std::make_unique<SharedRuntimeAdapter>(
runtime,
runtime->getDebugger()),
"testConn") {
connection_.connect(std::make_unique<RemoteConnnection>(*this));
}
Expand Down
4 changes: 3 additions & 1 deletion ReactCommon/hermes/inspector/tests/InspectorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ struct HermesDebugContext {
folly::Future<Unit> &&finished)
: runtime(makeHermesRuntime()),
inspector(
std::make_shared<SharedRuntimeAdapter>(runtime),
std::make_shared<SharedRuntimeAdapter>(
runtime,
runtime->getDebugger()),
observer,
false),
stopFlag(false),
Expand Down

0 comments on commit e6f3388

Please sign in to comment.