Fix app lagging while attempting a connection to Metro (facebook#48642)

Summary:

Changelog:
[General][Breaking][Fixed] - removed a long-running loop causing the app to lag while attempting a connection to Metro

D65952134 fixed the auto-reconnection between Metro and the device.

There's an existing "constructed = connected" contract as [discussed](https://www.internalfb.com/diff/D65952134?dst_version_fbid=3741052436109227&transaction_fbid=581445277659906):

https://www.internalfb.com/code/fbsource/[1592525fbcbb]/xplat/js/react-native-github/packages/react-native/ReactCommon/jsinspector-modern/WebSocketInterfaces.h?lines=16-20

In compliance, busy-waiting was [introduced](https://www.internalfb.com/diff/D65952134?dst_version_fbid=896147259315683&transaction_fbid=427393513742494) in V4 to wait for the connection result in the constructor.

xArthasx [discovered](https://www.internalfb.com/diff/D65952134?dst_version_fbid=896147259315683&transaction_fbid=1406890420289706) a performance issue from this impl via a profiling result.

In favour of async connection results, we're going back to V3 design with the imperative `isConnected()` check to the interface. xArthasx has confirmed this fixes the perf issue.

While I haven't found a compelling reason against removing this contract from the initial design in D52134592, please let me know if I've missed one.

This also means there was a scenario where messages were sent before the websocket is open. Those were dropped silently previously (before the busy-waiting while loop was introduced):

https://www.internalfb.com/code/fbsource/[f7113e167ee1]/fbobjc/VendorLib/SocketRocket/src/SocketRocket/SRWebSocket.m?lines=630-637

This means message senders must now consider the connection state, e.g. by maintaining a pre-connection message queue, if they need to guarantee the messages to be sent.

Reviewed By: christophpurrer

Differential Revision: D68023397

packages/react-native/React/Inspector/RCTCxxInspectorWebSocketAdapter.mm

@@ -19,8 +19,6 @@
 
 using namespace facebook::react::jsinspector_modern;
 
-static const uint64_t MAX_CONNECTING_TIME_NS = 2 * 1000000000L;
-
 namespace {
 NSString *NSStringFromUTF8StringView(std::string_view view)
 {
@@ -41,15 +39,6 @@ - (instancetype)initWithURL:(const std::string &)url delegate:(std::weak_ptr<IWe
     _webSocket = [[SRWebSocket alloc] initWithURL:[NSURL URLWithString:NSStringFromUTF8StringView(url)]];
     _webSocket.delegate = self;
     [_webSocket open];
-    uint64_t startTime = clock_gettime_nsec_np(CLOCK_MONOTONIC_RAW);
-    while ([_webSocket readyState] == SR_CONNECTING) {
-      if ((clock_gettime_nsec_np(CLOCK_MONOTONIC_RAW) - startTime) > MAX_CONNECTING_TIME_NS) {
-        break;
-      }
-    }
-    if ([_webSocket readyState] != SR_OPEN) {
-      return nil;
-    }
   }
   return self;
 }
@@ -71,6 +60,14 @@ - (void)close
   [_webSocket closeWithCode:1000 reason:@"End of session"];
 }
 
+- (void)webSocketDidOpen:(__unused SRWebSocket *)webSocket
+{
+  // NOTE: We are on the main queue here, per SRWebSocket's defaults.
+  if (auto delegate = _delegate.lock()) {
+    delegate->didOpen();
+  }
+}
+
 - (void)webSocket:(__unused SRWebSocket *)webSocket didFailWithError:(NSError *)error
 {
   // NOTE: We are on the main queue here, per SRWebSocket's defaults.

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/CxxInspectorPackagerConnection.java

@@ -50,6 +50,8 @@ private static class WebSocketDelegate implements Closeable {
 
     public native void didReceiveMessage(String message);
 
+    public native void didOpen();
+
     public native void didClose();
 
     /**
@@ -126,6 +128,17 @@ public void run() {
                       0);
                 }
 
+                @Override
+                public void onOpen(WebSocket _webSocket, Response response) {
+                  scheduleCallback(
+                      new Runnable() {
+                        public void run() {
+                          delegate.didOpen();
+                        }
+                      },
+                      0);
+                }
+
                 @Override
                 public void onClosed(WebSocket _unused, int code, String reason) {
                   scheduleCallback(

packages/react-native/ReactAndroid/src/main/jni/react/devsupport/JCxxInspectorPackagerConnectionWebSocketDelegate.cpp

@@ -40,6 +40,12 @@ void JCxxInspectorPackagerConnectionWebSocketDelegate::didReceiveMessage(
   }
 }
 
+void JCxxInspectorPackagerConnectionWebSocketDelegate::didOpen() {
+  if (auto delegate = cxxDelegate_.lock()) {
+    delegate->didOpen();
+  }
+}
+
 void JCxxInspectorPackagerConnectionWebSocketDelegate::didClose() {
   if (auto delegate = cxxDelegate_.lock()) {
     delegate->didClose();

packages/react-native/ReactAndroid/src/main/jni/react/devsupport/JCxxInspectorPackagerConnectionWebSocketDelegate.h

@@ -32,6 +32,8 @@ class JCxxInspectorPackagerConnectionWebSocketDelegate
 
   void didReceiveMessage(const std::string& message);
 
+  void didOpen();
+
   void didClose();
 
   static void registerNatives();

packages/react-native/ReactCommon/jsinspector-modern/InspectorPackagerConnection.cpp

@@ -198,7 +198,12 @@ void InspectorPackagerConnection::Impl::didReceiveMessage(
   handleProxyMessage(std::move(parsedJSON));
 }
 
+void InspectorPackagerConnection::Impl::didOpen() {
+  connected_ = true;
+}
+
 void InspectorPackagerConnection::Impl::didClose() {
+  connected_ = false;
   webSocket_.reset();
   closeAllConnections();
   if (!closed_) {
@@ -214,7 +219,7 @@ void InspectorPackagerConnection::Impl::onPageRemoved(int pageId) {
 }
 
 bool InspectorPackagerConnection::Impl::isConnected() const {
-  return webSocket_ != nullptr;
+  return webSocket_ != nullptr && connected_;
 }
 
 void InspectorPackagerConnection::Impl::connect() {
@@ -241,13 +246,22 @@ void InspectorPackagerConnection::Impl::reconnect() {
     suppressConnectionErrors_ = true;
   }
 
+  if (isConnected()) {
+    return;
+  }
+
   reconnectPending_ = true;
 
   delegate_->scheduleCallback(
       [weakSelf = weak_from_this()] {
         auto strongSelf = weakSelf.lock();
         if (strongSelf && !strongSelf->closed_) {
           strongSelf->reconnectPending_ = false;
+
+          if (strongSelf->isConnected()) {
+            return;
+          }
+
           strongSelf->connect();
 
           if (!strongSelf->isConnected()) {

packages/react-native/ReactCommon/jsinspector-modern/InspectorPackagerConnectionImpl.h

@@ -87,6 +87,7 @@ class InspectorPackagerConnection::Impl
   virtual void didFailWithError(std::optional<int> posixCode, std::string error)
       override;
   virtual void didReceiveMessage(std::string_view message) override;
+  virtual void didOpen() override;
   virtual void didClose() override;
 
   // IPageStatusListener methods
@@ -99,6 +100,7 @@ class InspectorPackagerConnection::Impl
 
   std::unordered_map<std::string, Session> inspectorSessions_;
   std::unique_ptr<IWebSocket> webSocket_;
+  bool connected_{false};
   bool closed_{false};
   bool suppressConnectionErrors_{false};
 

packages/react-native/ReactCommon/jsinspector-modern/WebSocketInterfaces.h

@@ -15,7 +15,6 @@ namespace facebook::react::jsinspector_modern {
 
 /**
  * Simplified interface to a WebSocket connection.
- * The socket MUST be initially open when constructed.
  */
 class IWebSocket {
  public:
@@ -56,6 +55,13 @@ class IWebSocketDelegate {
    */
   virtual void didReceiveMessage(std::string_view message) = 0;
 
+  /**
+   * Called when the socket has been opened.
+   * This method must be called on the inspector queue, and the
+   * WebSocketDelegate may not be destroyed while it is executing.
+   */
+  virtual void didOpen() = 0;
+
   /**
    * Called when the socket has been closed. The call is not required if
    * didFailWithError was called instead.