Identify debug sessions with a token

Summary:
The `RuntimeAdapter` may be used after `disableDebugging` has been called. To ensure the `RuntimeAdapter` is alive long enough for this use, the Inspector should continue to control `RuntimeAdapter`'s lifetime (which is currently done via a `unique_ptr` that's moved into the Inspector).

Callers need a way to identify the `RuntimeAdapter` after it has been moved into the Inspector so that debugging can be disabled via `disableDebugging`.

This could be done by switching from a `unique_ptr` to a `shared_ptr` (so the caller can keep a copy), but consumers don't really have a reason to hang onto the `RuntimeAdapter` instance. Instead, leave the `RuntimeAdapter` inside a `unique_ptr`, and have consumers use a token to identify instances.

Update all consumers of this API to use this new token interface.

Changelog: [Internal]

Reviewed By: jpporto

Differential Revision: D38513256

fbshipit-source-id: 33580747cd8365d25dbddbe289f0c41141e3bc6a

ReactCommon/hermes/executor/HermesExecutorFactory.cpp

@@ -149,21 +149,18 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator<ReentrancyCheck> {
       HermesRuntime &hermesRuntime,
       std::shared_ptr<MessageQueueThread> jsQueue)
       : jsi::WithRuntimeDecorator<ReentrancyCheck>(*runtime, reentrancyCheck_),
-        runtime_(std::move(runtime)),
-        hermesRuntime_(hermesRuntime) {
+        runtime_(std::move(runtime)) {
 #ifdef HERMES_ENABLE_DEBUGGER
     std::shared_ptr<HermesRuntime> rt(runtime_, &hermesRuntime);
     auto adapter = std::make_unique<HermesExecutorRuntimeAdapter>(rt, jsQueue);
-    facebook::hermes::inspector::chrome::enableDebugging(
+    debugToken_ = facebook::hermes::inspector::chrome::enableDebugging(
         std::move(adapter), "Hermes React Native");
-#else
-    (void)hermesRuntime_;
 #endif
   }
 
   ~DecoratedRuntime() {
 #ifdef HERMES_ENABLE_DEBUGGER
-    facebook::hermes::inspector::chrome::disableDebugging(hermesRuntime_);
+    facebook::hermes::inspector::chrome::disableDebugging(debugToken_);
 #endif
   }
 
@@ -177,7 +174,9 @@ class DecoratedRuntime : public jsi::WithRuntimeDecorator<ReentrancyCheck> {
 
   std::shared_ptr<Runtime> runtime_;
   ReentrancyCheck reentrancyCheck_;
-  HermesRuntime &hermesRuntime_;
+#ifdef HERMES_ENABLE_DEBUGGER
+  facebook::hermes::inspector::chrome::DebugSessionToken debugToken_;
+#endif
 };
 
 } // namespace

ReactCommon/hermes/inspector/chrome/ConnectionDemux.cpp

@@ -63,7 +63,7 @@ ConnectionDemux::ConnectionDemux(facebook::react::IInspector &inspector)
 
 ConnectionDemux::~ConnectionDemux() = default;
 
-int ConnectionDemux::enableDebugging(
+DebugSessionToken ConnectionDemux::enableDebugging(
     std::unique_ptr<RuntimeAdapter> adapter,
     const std::string &title) {
   std::lock_guard<std::mutex> lock(mutex_);
@@ -94,21 +94,12 @@ int ConnectionDemux::enableDebugging(
       std::make_shared<Connection>(std::move(adapter), title, waitForDebugger));
 }
 
-void ConnectionDemux::disableDebugging(HermesRuntime &runtime) {
+void ConnectionDemux::disableDebugging(DebugSessionToken session) {
   std::lock_guard<std::mutex> lock(mutex_);
-
-  for (auto &it : conns_) {
-    int pageId = it.first;
-    auto &conn = it.second;
-
-    if (&(conn->getRuntime()) == &runtime) {
-      removePage(pageId);
-
-      // must break here. removePage mutates conns_, so range-for iterator is
-      // now invalid.
-      break;
-    }
+  if (conns_.find(session) == conns_.end()) {
+    return;
   }
+  removePage(session);
 }
 
 int ConnectionDemux::addPage(std::shared_ptr<Connection> conn) {

ReactCommon/hermes/inspector/chrome/ConnectionDemux.h

@@ -16,6 +16,7 @@
 #include <hermes/hermes.h>
 #include <hermes/inspector/RuntimeAdapter.h>
 #include <hermes/inspector/chrome/Connection.h>
+#include <hermes/inspector/chrome/Registration.h>
 #include <jsinspector/InspectorInterfaces.h>
 
 namespace facebook {
@@ -36,10 +37,10 @@ class ConnectionDemux {
   ConnectionDemux(const ConnectionDemux &) = delete;
   ConnectionDemux &operator=(const ConnectionDemux &) = delete;
 
-  int enableDebugging(
+  DebugSessionToken enableDebugging(
       std::unique_ptr<RuntimeAdapter> adapter,
       const std::string &title);
-  void disableDebugging(HermesRuntime &runtime);
+  void disableDebugging(DebugSessionToken session);
 
  private:
   int addPage(std::shared_ptr<Connection> conn);

ReactCommon/hermes/inspector/chrome/Registration.cpp

@@ -22,14 +22,14 @@ ConnectionDemux &demux() {
 
 } // namespace
 
-void enableDebugging(
+DebugSessionToken enableDebugging(
     std::unique_ptr<RuntimeAdapter> adapter,
     const std::string &title) {
-  demux().enableDebugging(std::move(adapter), title);
+  return demux().enableDebugging(std::move(adapter), title);
 }
 
-void disableDebugging(HermesRuntime &runtime) {
-  demux().disableDebugging(runtime);
+void disableDebugging(DebugSessionToken session) {
+  demux().disableDebugging(session);
 }
 
 } // namespace chrome

ReactCommon/hermes/inspector/chrome/Registration.h

@@ -18,20 +18,24 @@ namespace hermes {
 namespace inspector {
 namespace chrome {
 
+using DebugSessionToken = int;
+
 /*
  * enableDebugging adds this runtime to the list of debuggable JS targets
- * (called "pages" in the higher-leavel React Native API) in this process. It
- * should be called before any JS runs in the runtime.
+ * (called "pages" in the higher-level React Native API) in this process. It
+ * should be called before any JS runs in the runtime. The returned token
+ * can be used to disable debugging for this runtime.
  */
-extern void enableDebugging(
+extern DebugSessionToken enableDebugging(
     std::unique_ptr<RuntimeAdapter> adapter,
     const std::string &title);
 
 /*
  * disableDebugging removes this runtime from the list of debuggable JS targets
- * in this process.
+ * in this process. The runtime to remove is identified by the token returned
+ * from enableDebugging.
  */
-extern void disableDebugging(HermesRuntime &runtime);
+extern void disableDebugging(DebugSessionToken session);
 
 } // namespace chrome
 } // namespace inspector

ReactCommon/hermes/inspector/chrome/tests/ConnectionDemuxTests.cpp

@@ -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(*runtime2);
+  demux.disableDebugging(id2);
   expectPages(*inspector, {{id1, "page1"}});
   remoteData2->expectDisconnected();