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
This commit is contained in:
Lorenzo Blasa
2021-11-29 03:49:20 -08:00
committed by Facebook GitHub Bot
parent 18188e8a9c
commit 60de2bc147
2 changed files with 12 additions and 9 deletions

View File

@@ -251,7 +251,10 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
connectionContextStore_(connectionContextStore) {} connectionContextStore_(connectionContextStore) {}
virtual ~JFlipperWebSocket() { virtual ~JFlipperWebSocket() {
disconnect(); if (socket_ != nullptr) {
socket_->disconnect();
socket_ = nullptr;
}
} }
virtual void setEventHandler(SocketEventHandler eventHandler) override { virtual void setEventHandler(SocketEventHandler eventHandler) override {
@@ -282,21 +285,21 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
auto secure = endpoint_.secure; auto secure = endpoint_.secure;
bool fullfilled = false;
std::promise<bool> promise; std::promise<bool> promise;
auto connected = promise.get_future(); auto connected = promise.get_future();
connecting_ = true;
socket_ = make_global(JFlipperSocketImpl::create(connectionURL)); socket_ = make_global(JFlipperSocketImpl::create(connectionURL));
socket_->setEventHandler(JFlipperSocketEventHandlerImpl::newObjectCxxArgs( socket_->setEventHandler(JFlipperSocketEventHandlerImpl::newObjectCxxArgs(
[&fullfilled, &promise, eventHandler = eventHandler_]( [this, &promise, eventHandler = eventHandler_](SocketEvent event) {
SocketEvent event) {
/** /**
Only fulfill the promise the first time the event handler is used. 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. If the open event is received, then set the promise value to true.
For any other event, consider a failure and set to false. For any other event, consider a failure and set to false.
*/ */
if (!fullfilled) { if (this->connecting_) {
fullfilled = true; this->connecting_ = false;
if (event == SocketEvent::OPEN) { if (event == SocketEvent::OPEN) {
promise.set_value(true); promise.set_value(true);
} else if (event == SocketEvent::SSL_ERROR) { } else if (event == SocketEvent::SSL_ERROR) {
@@ -338,6 +341,7 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
if (state == std::future_status::ready) { if (state == std::future_status::ready) {
return connected.get(); return connected.get();
} }
disconnect(); disconnect();
return false; return false;
} }
@@ -399,6 +403,7 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
facebook::flipper::SocketMessageHandler messageHandler_; facebook::flipper::SocketMessageHandler messageHandler_;
jni::global_ref<JFlipperSocketImpl> socket_; jni::global_ref<JFlipperSocketImpl> socket_;
bool connecting_;
}; };
class JFlipperSocketProvider : public facebook::flipper::FlipperSocketProvider { class JFlipperSocketProvider : public facebook::flipper::FlipperSocketProvider {

View File

@@ -148,9 +148,7 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
@Override @Override
public void flipperDisconnect() { public void flipperDisconnect() {
/** this.mEventHandler.onConnectionEvent(FlipperSocketEventHandler.SocketEvent.CLOSE);
* Set an event handler that does nothing, not interested in getting more socket event messages.
*/
this.mEventHandler = this.mEventHandler =
new FlipperSocketEventHandler() { new FlipperSocketEventHandler() {
@Override @Override