From 60de2bc147335b3c8a6832715215072d6af585ee Mon Sep 17 00:00:00 2001
From: Lorenzo Blasa <lblasa@fb.com>
Date: Mon, 29 Nov 2021 03:49:20 -0800
Subject: [PATCH] Addresses a few issues raised during tests

Summary:
This change addresses a warning and provides a safer way of resolving the connection promise.

Also, fixes an issue whereas the websocket connection was not being reestablished as the listener is not notified when we manually disconnect.

Reviewed By: passy

Differential Revision: D32591859

fbshipit-source-id: 78ce4eac5414a924217867f2f47b04829da3b705
---
 android/src/main/cpp/sonar.cpp                  | 17 +++++++++++------
 .../flipper/android/FlipperSocketImpl.java      |  4 +---
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/android/src/main/cpp/sonar.cpp b/android/src/main/cpp/sonar.cpp
index 72d474899..0375d081f 100644
--- a/android/src/main/cpp/sonar.cpp
+++ b/android/src/main/cpp/sonar.cpp
@@ -251,7 +251,10 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
         connectionContextStore_(connectionContextStore) {}
 
   virtual ~JFlipperWebSocket() {
-    disconnect();
+    if (socket_ != nullptr) {
+      socket_->disconnect();
+      socket_ = nullptr;
+    }
   }
 
   virtual void setEventHandler(SocketEventHandler eventHandler) override {
@@ -282,21 +285,21 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
 
     auto secure = endpoint_.secure;
 
-    bool fullfilled = false;
     std::promise<bool> promise;
     auto connected = promise.get_future();
 
+    connecting_ = true;
+
     socket_ = make_global(JFlipperSocketImpl::create(connectionURL));
     socket_->setEventHandler(JFlipperSocketEventHandlerImpl::newObjectCxxArgs(
-        [&fullfilled, &promise, eventHandler = eventHandler_](
-            SocketEvent event) {
+        [this, &promise, eventHandler = eventHandler_](SocketEvent event) {
           /**
              Only fulfill the promise the first time the event handler is used.
              If the open event is received, then set the promise value to true.
              For any other event, consider a failure and set to false.
            */
-          if (!fullfilled) {
-            fullfilled = true;
+          if (this->connecting_) {
+            this->connecting_ = false;
             if (event == SocketEvent::OPEN) {
               promise.set_value(true);
             } else if (event == SocketEvent::SSL_ERROR) {
@@ -338,6 +341,7 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
     if (state == std::future_status::ready) {
       return connected.get();
     }
+
     disconnect();
     return false;
   }
@@ -399,6 +403,7 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
   facebook::flipper::SocketMessageHandler messageHandler_;
 
   jni::global_ref<JFlipperSocketImpl> socket_;
+  bool connecting_;
 };
 
 class JFlipperSocketProvider : public facebook::flipper::FlipperSocketProvider {
diff --git a/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java b/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java
index d03c37f6d..09286081d 100644
--- a/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java
+++ b/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java
@@ -148,9 +148,7 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
 
   @Override
   public void flipperDisconnect() {
-    /**
-     * Set an event handler that does nothing, not interested in getting more socket event messages.
-     */
+    this.mEventHandler.onConnectionEvent(FlipperSocketEventHandler.SocketEvent.CLOSE);
     this.mEventHandler =
         new FlipperSocketEventHandler() {
           @Override