diff --git a/ReactCommon/hermes/executor/HermesExecutorFactory.cpp b/ReactCommon/hermes/executor/HermesExecutorFactory.cpp index bd58a483112..3bb4e0c7b08 100644 --- a/ReactCommon/hermes/executor/HermesExecutorFactory.cpp +++ b/ReactCommon/hermes/executor/HermesExecutorFactory.cpp @@ -149,38 +149,35 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator { HermesRuntime &hermesRuntime, std::shared_ptr jsQueue) : jsi::WithRuntimeDecorator(*runtime, reentrancyCheck_), - runtime_(std::move(runtime)) -#ifdef HERMES_ENABLE_DEBUGGER - , - adapter_( - std::shared_ptr(runtime_, &hermesRuntime), - jsQueue) -#endif - { + runtime_(std::move(runtime)), + hermesRuntime_(hermesRuntime) { #ifdef HERMES_ENABLE_DEBUGGER + std::shared_ptr rt(runtime_, &hermesRuntime); + auto adapter = std::make_unique(rt, jsQueue); facebook::hermes::inspector::chrome::enableDebugging( - adapter_, "Hermes React Native"); + std::move(adapter), "Hermes React Native"); +#else + (void)hermesRuntime_; #endif } ~DecoratedRuntime() { #ifdef HERMES_ENABLE_DEBUGGER - facebook::hermes::inspector::chrome::disableDebugging(adapter_); + facebook::hermes::inspector::chrome::disableDebugging(hermesRuntime_); #endif } private: // runtime_ is a potentially decorated Runtime. + // hermesRuntime is a reference to a HermesRuntime managed by runtime_. // // HermesExecutorRuntimeAdapter requirements are kept, because the // dtor will disable debugging on the HermesRuntime before the // member managing it is destroyed. std::shared_ptr runtime_; -#ifdef HERMES_ENABLE_DEBUGGER - HermesExecutorRuntimeAdapter adapter_; -#endif ReentrancyCheck reentrancyCheck_; + HermesRuntime &hermesRuntime_; }; } // namespace diff --git a/ReactCommon/hermes/inspector/Inspector.cpp b/ReactCommon/hermes/inspector/Inspector.cpp index c1942d0495c..9aafecb214d 100644 --- a/ReactCommon/hermes/inspector/Inspector.cpp +++ b/ReactCommon/hermes/inspector/Inspector.cpp @@ -104,16 +104,16 @@ static constexpr bool kShouldLog = false; } while (0) Inspector::Inspector( - RuntimeAdapter &adapter, + std::shared_ptr adapter, InspectorObserver &observer, bool pauseOnFirstStatement) : adapter_(adapter), - debugger_(adapter.getRuntime().getDebugger()), + debugger_(adapter->getRuntime().getDebugger()), observer_(observer), executor_(std::make_unique("hermes-inspector")) { // TODO (t26491391): make tickleJs a real Hermes runtime API std::string src = "function __tickleJs() { return Math.random(); }"; - adapter.getRuntime().evaluateJavaScript( + adapter->getRuntime().evaluateJavaScript( std::make_shared(src), "__tickleJsHackUrl"); { @@ -163,7 +163,7 @@ void Inspector::installConsoleFunction( std::shared_ptr &originalConsole, const std::string &name, const std::string &chromeTypeDefault = "") { - jsi::Runtime &rt = adapter_.getRuntime(); + jsi::Runtime &rt = adapter_->getRuntime(); auto chromeType = chromeTypeDefault == "" ? name : chromeTypeDefault; auto nameID = jsi::PropNameID::forUtf8(rt, name); auto weakInspector = std::weak_ptr(shared_from_this()); @@ -222,7 +222,7 @@ void Inspector::installConsoleFunction( } void Inspector::installLogHandler() { - jsi::Runtime &rt = adapter_.getRuntime(); + jsi::Runtime &rt = adapter_->getRuntime(); auto console = jsi::Object(rt); auto val = rt.global().getProperty(rt, "console"); std::shared_ptr originalConsole; @@ -261,8 +261,9 @@ void Inspector::triggerAsyncPause(bool andTickle) { if (andTickle) { // We run the dummy JS on a background thread to avoid any reentrancy issues // in case this thread is called with the inspector mutex held. + std::shared_ptr adapter = adapter_; detail::Thread tickleJsLater( - "inspectorTickleJs", [&adapter = adapter_]() { adapter.tickleJs(); }); + "inspectorTickleJs", [adapter]() { adapter->tickleJs(); }); tickleJsLater.detach(); } } @@ -705,8 +706,8 @@ void Inspector::alertIfPausedInSupersededFile() { "=true to " "suppress this warning. Filename: " + info.fileName + ")."; - jsi::Array jsiArray(adapter_.getRuntime(), 1); - jsiArray.setValueAtIndex(adapter_.getRuntime(), 0, warning); + jsi::Array jsiArray(adapter_->getRuntime(), 1); + jsiArray.setValueAtIndex(adapter_->getRuntime(), 0, warning); ConsoleMessageInfo logMessage("warning", std::move(jsiArray)); observer_.onMessageAdded(*this, logMessage); @@ -714,7 +715,7 @@ void Inspector::alertIfPausedInSupersededFile() { } bool Inspector::shouldSuppressAlertAboutSupersededFiles() { - jsi::Runtime &rt = adapter_.getRuntime(); + jsi::Runtime &rt = adapter_->getRuntime(); jsi::Value setting = rt.global().getProperty(rt, kSuppressionVariable); if (setting.isUndefined() || !setting.isBool()) diff --git a/ReactCommon/hermes/inspector/Inspector.h b/ReactCommon/hermes/inspector/Inspector.h index e6447f6c350..648499207f6 100644 --- a/ReactCommon/hermes/inspector/Inspector.h +++ b/ReactCommon/hermes/inspector/Inspector.h @@ -106,7 +106,7 @@ class Inspector : public facebook::hermes::debugger::EventObserver, * provided runtime before any JS executes in the runtime. */ Inspector( - RuntimeAdapter &adapter, + std::shared_ptr adapter, InspectorObserver &observer, bool pauseOnFirstStatement); ~Inspector(); @@ -317,7 +317,7 @@ class Inspector : public facebook::hermes::debugger::EventObserver, const std::string &name, const std::string &chromeType); - RuntimeAdapter &adapter_; + std::shared_ptr adapter_; facebook::hermes::debugger::Debugger &debugger_; InspectorObserver &observer_; diff --git a/ReactCommon/hermes/inspector/chrome/Connection.cpp b/ReactCommon/hermes/inspector/chrome/Connection.cpp index 6fbadf2fcbb..ac7b36cc35a 100644 --- a/ReactCommon/hermes/inspector/chrome/Connection.cpp +++ b/ReactCommon/hermes/inspector/chrome/Connection.cpp @@ -48,11 +48,13 @@ static const char *const kBeforeScriptWithSourceMapExecution = class Connection::Impl : public inspector::InspectorObserver, public message::RequestHandler { public: - Impl(RuntimeAdapter &adapter, const std::string &title, bool waitForDebugger); + Impl( + std::unique_ptr adapter, + const std::string &title, + bool waitForDebugger); ~Impl(); jsi::Runtime &getRuntime(); - RuntimeAdapter &getRuntimeAdapter(); std::string getTitle() const; bool connect(std::unique_ptr remoteConn); @@ -141,7 +143,7 @@ class Connection::Impl : public inspector::InspectorObserver, folly::via(executor_.get(), [cb = std::move(callback)]() { cb(); }); } - RuntimeAdapter &runtimeAdapter_; + std::shared_ptr runtimeAdapter_; std::string title_; // connected_ is protected by connectionMutex_. @@ -183,10 +185,10 @@ class Connection::Impl : public inspector::InspectorObserver, }; Connection::Impl::Impl( - RuntimeAdapter &adapter, + std::unique_ptr adapter, const std::string &title, bool waitForDebugger) - : runtimeAdapter_(adapter), + : runtimeAdapter_(std::move(adapter)), title_(title), connected_(false), executor_(std::make_unique( @@ -202,11 +204,7 @@ Connection::Impl::Impl( Connection::Impl::~Impl() = default; jsi::Runtime &Connection::Impl::getRuntime() { - return runtimeAdapter_.getRuntime(); -} - -RuntimeAdapter &Connection::Impl::getRuntimeAdapter() { - return runtimeAdapter_; + return runtimeAdapter_->getRuntime(); } std::string Connection::Impl::getTitle() const { @@ -1573,10 +1571,11 @@ void Connection::Impl::sendNotificationToClientViaExecutor( * Connection */ Connection::Connection( - RuntimeAdapter &adapter, + std::unique_ptr adapter, const std::string &title, bool waitForDebugger) - : impl_(std::make_unique(adapter, title, waitForDebugger)) {} + : impl_( + std::make_unique(std::move(adapter), title, waitForDebugger)) {} Connection::~Connection() = default; @@ -1584,10 +1583,6 @@ jsi::Runtime &Connection::getRuntime() { return impl_->getRuntime(); } -RuntimeAdapter &Connection::getRuntimeAdapter() { - return impl_->getRuntimeAdapter(); -} - std::string Connection::getTitle() const { return impl_->getTitle(); } diff --git a/ReactCommon/hermes/inspector/chrome/Connection.h b/ReactCommon/hermes/inspector/chrome/Connection.h index 967d5857139..92dbbdf4a3a 100644 --- a/ReactCommon/hermes/inspector/chrome/Connection.h +++ b/ReactCommon/hermes/inspector/chrome/Connection.h @@ -29,7 +29,7 @@ class INSPECTOR_EXPORT Connection { /// Connection constructor enables the debugger on the provided runtime. This /// should generally called before you start running any JS in the runtime. Connection( - RuntimeAdapter &adapter, + std::unique_ptr adapter, const std::string &title, bool waitForDebugger = false); ~Connection(); @@ -37,9 +37,6 @@ class INSPECTOR_EXPORT Connection { /// getRuntime returns the underlying runtime being debugged. jsi::Runtime &getRuntime(); - /// getRuntimeAdapter returns the runtime adapter being debugged. - RuntimeAdapter &getRuntimeAdapter(); - /// getTitle returns the name of the friendly name of the runtime that's shown /// to users in Nuclide. std::string getTitle() const; diff --git a/ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp b/ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp index 1f06dacdc3e..9aaee2ca13a 100644 --- a/ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp +++ b/ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp @@ -64,7 +64,7 @@ ConnectionDemux::ConnectionDemux(facebook::react::IInspector &inspector) ConnectionDemux::~ConnectionDemux() = default; int ConnectionDemux::enableDebugging( - RuntimeAdapter &adapter, + std::unique_ptr adapter, const std::string &title) { std::lock_guard lock(mutex_); @@ -90,17 +90,18 @@ int ConnectionDemux::enableDebugging( (inspectedContexts_->find(title) != inspectedContexts_->end()) || isNetworkInspected(title, "app_name", "device_name"); - return addPage(std::make_shared(adapter, title, waitForDebugger)); + return addPage( + std::make_shared(std::move(adapter), title, waitForDebugger)); } -void ConnectionDemux::disableDebugging(RuntimeAdapter &adapter) { +void ConnectionDemux::disableDebugging(HermesRuntime &runtime) { std::lock_guard lock(mutex_); for (auto &it : conns_) { int pageId = it.first; auto &conn = it.second; - if (&(conn->getRuntimeAdapter()) == &adapter) { + if (&(conn->getRuntime()) == &runtime) { removePage(pageId); // must break here. removePage mutates conns_, so range-for iterator is diff --git a/ReactCommon/hermes/inspector/chrome/ConnectionDemux.h b/ReactCommon/hermes/inspector/chrome/ConnectionDemux.h index f2d4f5ceb39..d7697bc6b4f 100644 --- a/ReactCommon/hermes/inspector/chrome/ConnectionDemux.h +++ b/ReactCommon/hermes/inspector/chrome/ConnectionDemux.h @@ -36,8 +36,10 @@ class ConnectionDemux { ConnectionDemux(const ConnectionDemux &) = delete; ConnectionDemux &operator=(const ConnectionDemux &) = delete; - int enableDebugging(RuntimeAdapter &adapter, const std::string &title); - void disableDebugging(RuntimeAdapter &adapter); + int enableDebugging( + std::unique_ptr adapter, + const std::string &title); + void disableDebugging(HermesRuntime &runtime); private: int addPage(std::shared_ptr conn); diff --git a/ReactCommon/hermes/inspector/chrome/Registration.cpp b/ReactCommon/hermes/inspector/chrome/Registration.cpp index 42778c1fd5a..d64a08ebbfc 100644 --- a/ReactCommon/hermes/inspector/chrome/Registration.cpp +++ b/ReactCommon/hermes/inspector/chrome/Registration.cpp @@ -22,12 +22,14 @@ ConnectionDemux &demux() { } // namespace -void enableDebugging(RuntimeAdapter &adapter, const std::string &title) { - demux().enableDebugging(adapter, title); +void enableDebugging( + std::unique_ptr adapter, + const std::string &title) { + demux().enableDebugging(std::move(adapter), title); } -void disableDebugging(RuntimeAdapter &adapter) { - demux().disableDebugging(adapter); +void disableDebugging(HermesRuntime &runtime) { + demux().disableDebugging(runtime); } } // namespace chrome diff --git a/ReactCommon/hermes/inspector/chrome/Registration.h b/ReactCommon/hermes/inspector/chrome/Registration.h index 7dd862f145c..9acff574df8 100644 --- a/ReactCommon/hermes/inspector/chrome/Registration.h +++ b/ReactCommon/hermes/inspector/chrome/Registration.h @@ -23,13 +23,15 @@ namespace chrome { * (called "pages" in the higher-leavel React Native API) in this process. It * should be called before any JS runs in the runtime. */ -extern void enableDebugging(RuntimeAdapter &adapter, const std::string &title); +extern void enableDebugging( + std::unique_ptr adapter, + const std::string &title); /* * disableDebugging removes this runtime from the list of debuggable JS targets * in this process. */ -extern void disableDebugging(RuntimeAdapter &adapter); +extern void disableDebugging(HermesRuntime &runtime); } // namespace chrome } // namespace inspector diff --git a/ReactCommon/hermes/inspector/chrome/cli/main.cpp b/ReactCommon/hermes/inspector/chrome/cli/main.cpp index e14cdc1779a..4795231afd9 100644 --- a/ReactCommon/hermes/inspector/chrome/cli/main.cpp +++ b/ReactCommon/hermes/inspector/chrome/cli/main.cpp @@ -214,9 +214,10 @@ static void runDebuggerLoop( static void runScript(const std::string &scriptSource, const std::string &url) { std::shared_ptr runtime( fbhermes::makeHermesRuntime()); - fbhermes::inspector::SharedRuntimeAdapter adapter(runtime); + auto adapter = + std::make_unique(runtime); fbhermes::inspector::chrome::Connection conn( - adapter, "hermes-chrome-debug-server"); + std::move(adapter), "hermes-chrome-debug-server"); std::thread debuggerLoop(runDebuggerLoop, std::ref(conn), scriptSource); fbhermes::HermesRuntime::DebugFlags flags{}; diff --git a/ReactCommon/hermes/inspector/chrome/tests/ConnectionDemuxTests.cpp b/ReactCommon/hermes/inspector/chrome/tests/ConnectionDemuxTests.cpp index a434a7e55fa..08c86996dc9 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/ConnectionDemuxTests.cpp +++ b/ReactCommon/hermes/inspector/chrome/tests/ConnectionDemuxTests.cpp @@ -97,10 +97,10 @@ TEST(ConnectionDemuxTests, TestEnableDisable) { ConnectionDemux demux{*inspector}; - SharedRuntimeAdapter adapter1(runtime1); - int id1 = demux.enableDebugging(adapter1, "page1"); - SharedRuntimeAdapter adapter2(runtime2); - int id2 = demux.enableDebugging(adapter2, "page2"); + int id1 = demux.enableDebugging( + std::make_unique(runtime1), "page1"); + int id2 = demux.enableDebugging( + std::make_unique(runtime2), "page2"); expectPages(*inspector, {{id1, "page1"}, {id2, "page2"}}); @@ -124,7 +124,7 @@ TEST(ConnectionDemuxTests, TestEnableDisable) { // Disable debugging on runtime2. This should remove its page from the list // and call onDisconnect on its remoteConn - demux.disableDebugging(adapter2); + demux.disableDebugging(*runtime2); expectPages(*inspector, {{id1, "page1"}}); remoteData2->expectDisconnected(); diff --git a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp index c2aef9634b8..57223de420c 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp +++ b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.cpp @@ -55,8 +55,10 @@ class SyncConnection::RemoteConnnection : public IRemoteConnection { SyncConnection::SyncConnection( std::shared_ptr runtime, bool waitForDebugger) - : runtimeAdapter_(runtime), - connection_(runtimeAdapter_, "testConn", waitForDebugger) { + : connection_( + std::make_unique(runtime), + "testConn", + waitForDebugger) { connection_.connect(std::make_unique(*this)); } diff --git a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h index 7ee9080486f..d50018a3d24 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h +++ b/ReactCommon/hermes/inspector/chrome/tests/SyncConnection.h @@ -54,7 +54,6 @@ class SyncConnection { void onReply(const std::string &message); - SharedRuntimeAdapter runtimeAdapter_; Connection connection_; std::mutex mutex_; diff --git a/ReactCommon/hermes/inspector/tests/InspectorTests.cpp b/ReactCommon/hermes/inspector/tests/InspectorTests.cpp index 0fcc8d604e7..0be1a7cb652 100644 --- a/ReactCommon/hermes/inspector/tests/InspectorTests.cpp +++ b/ReactCommon/hermes/inspector/tests/InspectorTests.cpp @@ -92,8 +92,10 @@ struct HermesDebugContext { InspectorObserver &observer, folly::Future &&finished) : runtime(makeHermesRuntime()), - adapter(runtime), - inspector(adapter, observer, false), + inspector( + std::make_shared(runtime), + observer, + false), stopFlag(false), finished(std::move(finished)) { runtime->global().setProperty( @@ -122,7 +124,6 @@ struct HermesDebugContext { } std::shared_ptr runtime; - SharedRuntimeAdapter adapter; Inspector inspector; std::atomic stopFlag{}; folly::Future finished;