From ddd6831446e7201a6bcfdba994562e20078bb3dd Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 00:57:09 -0800 Subject: [PATCH 01/16] Use implicit (brace) constructor for getMethods() --- vnext/Shared/Modules/WebSocketModule.cpp | 25 ++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index e8ae6adf704..770e63455a5 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -50,7 +50,7 @@ std::vector WebSocketModule::getMeth { return { - Method( + { "connect", [this](dynamic args) // const string& url, dynamic protocols, dynamic options, int64_t id { @@ -80,8 +80,9 @@ std::vector WebSocketModule::getMeth { sharedWs->Connect(protocols, options); } - }), - Method( + } + }, + { "close", [this](dynamic args) // [int64_t code, string reason,] int64_t id { @@ -107,8 +108,9 @@ std::vector WebSocketModule::getMeth auto errorObj = dynamic::object("id", -1)("message", "Incorrect number of parameters"); this->SendEvent("websocketFailed", std::move(errorObj)); } - }), - Method( + } + }, + { "send", [this](dynamic args) // const string& message, int64_t id { @@ -117,8 +119,9 @@ std::vector WebSocketModule::getMeth { sharedWs->Send(jsArgAsString(args, 0)); } - }), - Method( + } + }, + { "sendBinary", [this](dynamic args) // const string& base64String, int64_t id { @@ -127,8 +130,9 @@ std::vector WebSocketModule::getMeth { sharedWs->SendBinary(jsArgAsString(args, 0)); } - }), - Method( + } + }, + { "ping", [this](dynamic args) // int64_t id { @@ -137,7 +141,8 @@ std::vector WebSocketModule::getMeth { sharedWs->Ping(); } - }) + } + } }; } // getMethods // clang-format on From 80baef13846d190c109376a79b62449cd0527527 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 01:03:29 -0800 Subject: [PATCH 02/16] Define CxxModuleHolder --- vnext/Shared/Modules/WebSocketModule.cpp | 8 +++++++- vnext/Shared/Modules/WebSocketModule.h | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index 770e63455a5..4be02d5cf50 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -30,7 +30,13 @@ constexpr char moduleName[] = "WebSocketModule"; namespace Microsoft::React { WebSocketModule::WebSocketModule() - : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }} {} + : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }}, m_holder{} { + m_holder->Module = this; +} + +WebSocketModule::~WebSocketModule() noexcept /*override*/{ + m_holder->Module = nullptr; +} void WebSocketModule::SetResourceFactory( std::function(const string &)> &&resourceFactory) { diff --git a/vnext/Shared/Modules/WebSocketModule.h b/vnext/Shared/Modules/WebSocketModule.h index 25fa33493b7..2996ef66035 100644 --- a/vnext/Shared/Modules/WebSocketModule.h +++ b/vnext/Shared/Modules/WebSocketModule.h @@ -18,6 +18,8 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { WebSocketModule(); + ~WebSocketModule() noexcept override; + #pragma region CxxModule overrides /// @@ -41,6 +43,11 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { void SetResourceFactory(std::function(const std::string &)> &&resourceFactory); private: + struct CxxModuleHolder { + facebook::xplat::module::CxxModule *Module{nullptr}; + }; + + /// /// Notifies an event to the current React Instance. /// @@ -61,6 +68,11 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { /// Generates IWebSocketResource instances, defaulting to IWebSocketResource::Make. /// std::function(std::string &&)> m_resourceFactory; + + /// + /// TODO + /// + std::shared_ptr m_holder; }; } // namespace Microsoft::React From 5a1c98512d4bfb114d7972215414b382c2cbf1c0 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 02:12:15 -0800 Subject: [PATCH 03/16] Make m_asyncStorageManager shared_ptr --- vnext/Shared/AsyncStorageModule.h | 2 +- vnext/Shared/CxxModuleHolder.h | 14 +++ vnext/Shared/Modules/AsyncStorageModule.cpp | 122 +++++++++++--------- vnext/Shared/Modules/WebSocketModule.cpp | 2 +- vnext/Shared/Modules/WebSocketModule.h | 6 +- vnext/Shared/Shared.vcxitems | 1 + vnext/Shared/Shared.vcxitems.filters | 3 + 7 files changed, 86 insertions(+), 64 deletions(-) create mode 100644 vnext/Shared/CxxModuleHolder.h diff --git a/vnext/Shared/AsyncStorageModule.h b/vnext/Shared/AsyncStorageModule.h index 8cc8cfa5e30..a47b02836ce 100644 --- a/vnext/Shared/AsyncStorageModule.h +++ b/vnext/Shared/AsyncStorageModule.h @@ -19,7 +19,7 @@ class AsyncStorageModule : public facebook::xplat::module::CxxModule { std::vector getMethods() override; private: - std::unique_ptr m_asyncStorageManager; + std::shared_ptr m_asyncStorageManager; }; } // namespace react } // namespace facebook diff --git a/vnext/Shared/CxxModuleHolder.h b/vnext/Shared/CxxModuleHolder.h new file mode 100644 index 00000000000..09942183333 --- /dev/null +++ b/vnext/Shared/CxxModuleHolder.h @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#pragma once + +#include + +namespace Microsoft::React { + + struct CxxModuleHolder { + facebook::xplat::module::CxxModule *Module{nullptr}; +}; + +} //namespace diff --git a/vnext/Shared/Modules/AsyncStorageModule.cpp b/vnext/Shared/Modules/AsyncStorageModule.cpp index df1db1b2be2..12bf213761b 100644 --- a/vnext/Shared/Modules/AsyncStorageModule.cpp +++ b/vnext/Shared/Modules/AsyncStorageModule.cpp @@ -7,83 +7,91 @@ #undef U -using namespace std; -using namespace folly; -using namespace facebook::xplat; +using facebook::xplat::module::CxxModule; +using std::map; +using std::string; +using std::vector; +using std::weak_ptr; + +namespace { +constexpr char moduleName[] = "AsyncLocalStorage"; +} namespace facebook { namespace react { AsyncStorageModule::AsyncStorageModule(const WCHAR *storageFileName) - : m_asyncStorageManager{make_unique(storageFileName)} {} + : m_asyncStorageManager{std::make_shared(storageFileName)} {} -std::string AsyncStorageModule::getName() { - return "AsyncLocalStorage"; +string AsyncStorageModule::getName() { + return moduleName; } -std::map AsyncStorageModule::getConstants() { +map AsyncStorageModule::getConstants() { return {}; } -std::vector AsyncStorageModule::getMethods() { +vector AsyncStorageModule::getMethods() { return { - Method( - "multiGet", - [this]( - dynamic args, - Callback jsCallback) // params - array Keys , - // Callback(error, returnValue) - { - m_asyncStorageManager->executeKVOperation( - AsyncStorageManager::AsyncStorageOperation::multiGet, args, jsCallback); - }), - - Method( - "multiSet", - [this]( - dynamic args, - Callback jsCallback) // params - array> - // KeyValuePairs , Callback(error) - { - m_asyncStorageManager->executeKVOperation( - AsyncStorageManager::AsyncStorageOperation::multiSet, args, jsCallback); - }), + {"multiGet", + [wpManager = weak_ptr(m_asyncStorageManager)]( + dynamic args, + Callback jsCallback) // params - array Keys , + // Callback(error, returnValue) + { + if (auto spManager = wpManager.lock()) { + spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::multiGet, args, jsCallback); + } + }}, + + {"multiSet", + [wpManager = weak_ptr(m_asyncStorageManager)]( + dynamic args, + Callback jsCallback) // params - array> + // KeyValuePairs , Callback(error) + { + if (auto spManager = wpManager.lock()) { + spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::multiSet, args, jsCallback); + } + }}, // The 'multiMerge' method is currently not implemented. We assume that // the ReactNative framework translates its absence into the AsyncStorage // JS object (see Libraries\Storage\AsyncStorage.js around line 485) which // in turn allows JS user code to test for support of merge methods. - Method( - "multiRemove", - [this](dynamic args, Callback jsCallback) // params - array - // Keys , Callback(error) - { - m_asyncStorageManager->executeKVOperation( - AsyncStorageManager::AsyncStorageOperation::multiRemove, args, jsCallback); - }), - - Method( - "clear", - [this]( - dynamic args, - Callback jsCallback) // params - args is unused , Callback(error) - { - m_asyncStorageManager->executeKVOperation( - AsyncStorageManager::AsyncStorageOperation::clear, args, jsCallback); - }), - - Method( - "getAllKeys", - [this](dynamic args, Callback jsCallback) // params - args is unused , - // Callback(error, returnValue) - { - m_asyncStorageManager->executeKVOperation( - AsyncStorageManager::AsyncStorageOperation::getAllKeys, args, jsCallback); - }), + {"multiRemove", + [wpManager = weak_ptr(m_asyncStorageManager)]( + dynamic args, Callback jsCallback) // params - array + // Keys , Callback(error) + { + if (auto spManager = wpManager.lock()) { + spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::multiRemove, args, jsCallback); + } + }}, + + {"clear", + [wpManager = weak_ptr(m_asyncStorageManager)]( + dynamic args, + Callback jsCallback) // params - args is unused , Callback(error) + { + if (auto spManager = wpManager.lock()) { + spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::clear, args, jsCallback); + } + }}, + + {"getAllKeys", + [wpManager = weak_ptr(m_asyncStorageManager)]( + dynamic args, Callback jsCallback) // params - args is unused , + // Callback(error, returnValue) + { + if (auto spManager = wpManager.lock()) { + spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::getAllKeys, args, jsCallback); + } + }}, }; } -std::unique_ptr CreateAsyncStorageModule(const WCHAR *storageFileName) noexcept { +std::unique_ptr CreateAsyncStorageModule(const WCHAR *storageFileName) noexcept { return std::make_unique(storageFileName); } diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index 4be02d5cf50..08b1800d2d1 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -34,7 +34,7 @@ WebSocketModule::WebSocketModule() m_holder->Module = this; } -WebSocketModule::~WebSocketModule() noexcept /*override*/{ +WebSocketModule::~WebSocketModule() noexcept /*override*/ { m_holder->Module = nullptr; } diff --git a/vnext/Shared/Modules/WebSocketModule.h b/vnext/Shared/Modules/WebSocketModule.h index 2996ef66035..ed1652f1078 100644 --- a/vnext/Shared/Modules/WebSocketModule.h +++ b/vnext/Shared/Modules/WebSocketModule.h @@ -4,6 +4,7 @@ #pragma once #include +#include "CxxModuleHolder.h" #include "IWebSocketResource.h" namespace Microsoft::React { @@ -43,11 +44,6 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { void SetResourceFactory(std::function(const std::string &)> &&resourceFactory); private: - struct CxxModuleHolder { - facebook::xplat::module::CxxModule *Module{nullptr}; - }; - - /// /// Notifies an event to the current React Instance. /// diff --git a/vnext/Shared/Shared.vcxitems b/vnext/Shared/Shared.vcxitems index f4929bd7296..75ba2b490ec 100644 --- a/vnext/Shared/Shared.vcxitems +++ b/vnext/Shared/Shared.vcxitems @@ -73,6 +73,7 @@ + diff --git a/vnext/Shared/Shared.vcxitems.filters b/vnext/Shared/Shared.vcxitems.filters index 31d69a43b7e..5b64c4ad897 100644 --- a/vnext/Shared/Shared.vcxitems.filters +++ b/vnext/Shared/Shared.vcxitems.filters @@ -381,6 +381,9 @@ Header Files + + Source Files\Modules + From 4b5eb210178c780518903e6c0a1c47ba15d00cea Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 02:13:02 -0800 Subject: [PATCH 04/16] Change files --- ...ative-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json diff --git a/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json b/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json new file mode 100644 index 00000000000..93547280d96 --- /dev/null +++ b/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Avoid capturing raw `this` pointer in WS and AsyncStorage'", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +} From 4331602b2c105b4d9d6c2734d9cf8f929c750437 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 15:55:50 -0800 Subject: [PATCH 05/16] Initialize m_holder --- vnext/Shared/CxxModuleHolder.h | 4 ++-- vnext/Shared/Modules/WebSocketModule.cpp | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/vnext/Shared/CxxModuleHolder.h b/vnext/Shared/CxxModuleHolder.h index 09942183333..c203a55e366 100644 --- a/vnext/Shared/CxxModuleHolder.h +++ b/vnext/Shared/CxxModuleHolder.h @@ -7,8 +7,8 @@ namespace Microsoft::React { - struct CxxModuleHolder { +struct CxxModuleHolder { facebook::xplat::module::CxxModule *Module{nullptr}; }; -} //namespace +} // namespace Microsoft::React diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index 08b1800d2d1..7cb831ee15b 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -30,7 +30,8 @@ constexpr char moduleName[] = "WebSocketModule"; namespace Microsoft::React { WebSocketModule::WebSocketModule() - : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }}, m_holder{} { + : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }}, + m_holder{std::make_shared()} { m_holder->Module = this; } From 5577952c4dffb7c4fa86dbacc2e86dba76a49a5e Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 19:41:36 -0800 Subject: [PATCH 06/16] Implement WebSocketModule::SharedState --- vnext/Shared/CxxModuleHolder.h | 14 -- vnext/Shared/Modules/WebSocketModule.cpp | 223 ++++++++++++----------- vnext/Shared/Modules/WebSocketModule.h | 28 +-- vnext/Shared/Shared.vcxitems | 1 - vnext/Shared/Shared.vcxitems.filters | 3 - 5 files changed, 129 insertions(+), 140 deletions(-) delete mode 100644 vnext/Shared/CxxModuleHolder.h diff --git a/vnext/Shared/CxxModuleHolder.h b/vnext/Shared/CxxModuleHolder.h deleted file mode 100644 index c203a55e366..00000000000 --- a/vnext/Shared/CxxModuleHolder.h +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -#pragma once - -#include - -namespace Microsoft::React { - -struct CxxModuleHolder { - facebook::xplat::module::CxxModule *Module{nullptr}; -}; - -} // namespace Microsoft::React diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index 7cb831ee15b..8dae59eda5d 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -14,7 +14,9 @@ #include using namespace facebook::xplat; -using namespace folly; + +using facebook::react::Instance; +using folly::dynamic; using Microsoft::Common::Unicode::Utf16ToUtf8; using Microsoft::Common::Unicode::Utf8ToUtf16; @@ -24,19 +26,110 @@ using std::string; using std::weak_ptr; namespace { +using Microsoft::React::IWebSocketResource; +using Microsoft::React::WebSocketModule; + constexpr char moduleName[] = "WebSocketModule"; + +static void SendEvent(weak_ptr weakInstance, string&& eventName, dynamic&& args) { + if (auto instance = weakInstance.lock()) { + instance->callJSFunction("RCTDeviceEventEmitter", "emit", dynamic::array(std::move(eventName), std::move(args))); + } +} + +static shared_ptr GetOrCreateWebSocket( + int64_t id, + string &&url, + weak_ptr weakState) { + + auto state = weakState.lock(); + if (!state) { + return nullptr; + } + + auto itr = state->ResourceMap.find(id); + if (itr == state->ResourceMap.end()) { + + if (!state->Module) { + return nullptr; + } + auto weakInstance = state->Module->getInstance(); + + shared_ptr ws; + try { + ws = IWebSocketResource::Make(std::move(url)); + } catch (const winrt::hresult_error &e) { + std::stringstream ss; + ss << "[" << std::hex << std::showbase << std::setw(8) << static_cast(e.code()) << "] " + << winrt::to_string(e.message()); + + SendEvent(weakInstance, "webSocketFailed", dynamic::object("id", id)("message", std::move(ss.str()))); + + return nullptr; + } catch (const std::exception &e) { + SendEvent(weakInstance, "webSocketFailed", dynamic::object("id", id)("message", e.what())); + + return nullptr; + } catch (...) { + SendEvent( + weakInstance, + "webSocketFailed", dynamic::object("id", id)("message", "Unidentified error creating IWebSocketResource")); + + return nullptr; + } + + ws->SetOnError([id, weakInstance](const IWebSocketResource::Error &err) { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto errorObj = dynamic::object("id", id)("message", err.Message); + SendEvent(weakInstance, "websocketFailed", std::move(errorObj)); + }); + ws->SetOnConnect([id, weakInstance]() { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto args = dynamic::object("id", id); + SendEvent(weakInstance, "websocketOpen", std::move(args)); + }); + ws->SetOnMessage([id, weakInstance](size_t length, const string &message, bool isBinary) { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto args = dynamic::object("id", id)("data", message)("type", isBinary ? "binary" : "text"); + SendEvent(weakInstance, "websocketMessage", std::move(args)); + }); + ws->SetOnClose([id, weakInstance](IWebSocketResource::CloseCode code, const string &reason) { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto args = dynamic::object("id", id)("code", static_cast(code))("reason", reason); + SendEvent(weakInstance, "websocketClosed", std::move(args)); + }); + + state->ResourceMap.emplace(id, ws); + return ws; + } + + return itr->second; +} + } // anonymous namespace namespace Microsoft::React { WebSocketModule::WebSocketModule() : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }}, - m_holder{std::make_shared()} { - m_holder->Module = this; + m_sharedState{std::make_shared()} { + m_sharedState->Module = this; } WebSocketModule::~WebSocketModule() noexcept /*override*/ { - m_holder->Module = nullptr; + m_sharedState->Module = nullptr; } void WebSocketModule::SetResourceFactory( @@ -59,7 +152,7 @@ std::vector WebSocketModule::getMeth { { "connect", - [this](dynamic args) // const string& url, dynamic protocols, dynamic options, int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // const string& url, dynamic protocols, dynamic options, int64_t id { IWebSocketResource::Protocols protocols; dynamic protocolsDynamic = jsArgAsDynamic(args, 1); @@ -82,7 +175,7 @@ std::vector WebSocketModule::getMeth } } - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 3), jsArgAsString(args, 0)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 3), jsArgAsString(args, 0), weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Connect(protocols, options); @@ -91,12 +184,12 @@ std::vector WebSocketModule::getMeth }, { "close", - [this](dynamic args) // [int64_t code, string reason,] int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // [int64_t code, string reason,] int64_t id { // See react-native\Libraries\WebSocket\WebSocket.js:_close if (args.size() == 3) // WebSocketModule.close(statusCode, closeReason, this._socketId); { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 2)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 2), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Close(static_cast(jsArgAsInt(args, 0)), jsArgAsString(args, 1)); @@ -104,7 +197,7 @@ std::vector WebSocketModule::getMeth } else if (args.size() == 1) // WebSocketModule.close(this._socketId); { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 0)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 0), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Close(IWebSocketResource::CloseCode::Normal, {}); @@ -112,16 +205,19 @@ std::vector WebSocketModule::getMeth } else { - auto errorObj = dynamic::object("id", -1)("message", "Incorrect number of parameters"); - this->SendEvent("websocketFailed", std::move(errorObj)); + auto state = weakState.lock(); + if (state && state->Module) { + auto errorObj = dynamic::object("id", -1)("message", "Incorrect number of parameters"); + SendEvent(state->Module->getInstance(), "websocketFailed", std::move(errorObj)); + } } } }, { "send", - [this](dynamic args) // const string& message, int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // const string& message, int64_t id { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 1)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 1), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Send(jsArgAsString(args, 0)); @@ -130,9 +226,9 @@ std::vector WebSocketModule::getMeth }, { "sendBinary", - [this](dynamic args) // const string& base64String, int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // const string& base64String, int64_t id { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 1)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 1), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->SendBinary(jsArgAsString(args, 0)); @@ -141,9 +237,9 @@ std::vector WebSocketModule::getMeth }, { "ping", - [this](dynamic args) // int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // int64_t id { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 0)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 0), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Ping(); @@ -154,99 +250,8 @@ std::vector WebSocketModule::getMeth } // getMethods // clang-format on -#pragma region private members - -void WebSocketModule::SendEvent(string &&eventName, dynamic &&args) { - auto weakInstance = this->getInstance(); - if (auto instance = weakInstance.lock()) { - instance->callJSFunction("RCTDeviceEventEmitter", "emit", dynamic::array(std::move(eventName), std::move(args))); - } -} - -// clang-format off -shared_ptr WebSocketModule::GetOrCreateWebSocket(int64_t id, string&& url) -{ - auto itr = m_webSockets.find(id); - if (itr == m_webSockets.end()) - { - shared_ptr ws; - try - { - ws = m_resourceFactory(std::move(url)); - } - catch (const winrt::hresult_error& e) - { - std::stringstream ss; - ss << "[" << std::hex << std::showbase << std::setw(8) << static_cast(e.code()) << "] " << - winrt::to_string(e.message()); - - SendEvent("webSocketFailed", dynamic::object("id", id)("message", std::move(ss.str()))); - - return nullptr; - } - catch (const std::exception& e) - { - SendEvent("webSocketFailed", dynamic::object("id", id)("message", e.what())); - - return nullptr; - } - catch (...) - { - SendEvent("webSocketFailed", dynamic::object("id", id)("message", "Unidentified error creating IWebSocketResource")); - - return nullptr; - } - - auto weakInstance = this->getInstance(); - ws->SetOnError([this, id, weakInstance](const IWebSocketResource::Error& err) - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto errorObj = dynamic::object("id", id)("message", err.Message); - this->SendEvent("websocketFailed", std::move(errorObj)); - }); - ws->SetOnConnect([this, id, weakInstance]() - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto args = dynamic::object("id", id); - this->SendEvent("websocketOpen", std::move(args)); - }); - ws->SetOnMessage([this, id, weakInstance](size_t length, const string& message, bool isBinary) - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto args = dynamic::object("id", id)("data", message)("type", isBinary ? "binary" : "text"); - this->SendEvent("websocketMessage", std::move(args)); - }); - ws->SetOnClose([this, id, weakInstance](IWebSocketResource::CloseCode code, const string& reason) - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto args = dynamic::object("id", id)("code", static_cast(code))("reason", reason); - this->SendEvent("websocketClosed", std::move(args)); - }); - - m_webSockets.emplace(id, ws); - return ws; - } - - return itr->second; -} -// clang-format on - -#pragma endregion private members - /*extern*/ std::unique_ptr CreateWebSocketModule() noexcept { - return make_unique(); + return std::make_unique(); } } // namespace Microsoft::React diff --git a/vnext/Shared/Modules/WebSocketModule.h b/vnext/Shared/Modules/WebSocketModule.h index ed1652f1078..7dd193e92e0 100644 --- a/vnext/Shared/Modules/WebSocketModule.h +++ b/vnext/Shared/Modules/WebSocketModule.h @@ -4,7 +4,6 @@ #pragma once #include -#include "CxxModuleHolder.h" #include "IWebSocketResource.h" namespace Microsoft::React { @@ -21,6 +20,19 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { ~WebSocketModule() noexcept override; + struct SharedState { + /// + /// Keeps IWebSocketResource instances identified by id. + /// As defined in WebSocket.js. + /// + std::map> ResourceMap{}; + + /// + /// Keeps a raw reference to the module object to lazily retrieve the React Instance as needed. + /// + CxxModule *Module{nullptr}; + }; + #pragma region CxxModule overrides /// @@ -44,16 +56,6 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { void SetResourceFactory(std::function(const std::string &)> &&resourceFactory); private: - /// - /// Notifies an event to the current React Instance. - /// - void SendEvent(std::string &&eventName, folly::dynamic &¶meters); - - /// - /// Creates or retrieves a raw IWebSocketResource pointer. - /// - std::shared_ptr GetOrCreateWebSocket(std::int64_t id, std::string &&url = {}); - /// /// Keeps IWebSocketResource instances identified by id. /// As defined in WebSocket.js. @@ -66,9 +68,9 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { std::function(std::string &&)> m_resourceFactory; /// - /// TODO + /// Keeps members that can be accessed threads other than this module's owner accessible. /// - std::shared_ptr m_holder; + std::shared_ptr m_sharedState; }; } // namespace Microsoft::React diff --git a/vnext/Shared/Shared.vcxitems b/vnext/Shared/Shared.vcxitems index 75ba2b490ec..f4929bd7296 100644 --- a/vnext/Shared/Shared.vcxitems +++ b/vnext/Shared/Shared.vcxitems @@ -73,7 +73,6 @@ - diff --git a/vnext/Shared/Shared.vcxitems.filters b/vnext/Shared/Shared.vcxitems.filters index 5b64c4ad897..31d69a43b7e 100644 --- a/vnext/Shared/Shared.vcxitems.filters +++ b/vnext/Shared/Shared.vcxitems.filters @@ -381,9 +381,6 @@ Header Files - - Source Files\Modules - From 3e8383faaa28e117e9cf122ace13a67f8840411e Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 19:59:55 -0800 Subject: [PATCH 07/16] Move ResourceFactory into shared state --- vnext/Shared/Modules/WebSocketModule.cpp | 22 +++++++++------------- vnext/Shared/Modules/WebSocketModule.h | 10 +++++----- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index 8dae59eda5d..d0b03bc368b 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -31,17 +31,14 @@ using Microsoft::React::WebSocketModule; constexpr char moduleName[] = "WebSocketModule"; -static void SendEvent(weak_ptr weakInstance, string&& eventName, dynamic&& args) { +static void SendEvent(weak_ptr weakInstance, string &&eventName, dynamic &&args) { if (auto instance = weakInstance.lock()) { instance->callJSFunction("RCTDeviceEventEmitter", "emit", dynamic::array(std::move(eventName), std::move(args))); } } -static shared_ptr GetOrCreateWebSocket( - int64_t id, - string &&url, - weak_ptr weakState) { - +static shared_ptr +GetOrCreateWebSocket(int64_t id, string &&url, weak_ptr weakState) { auto state = weakState.lock(); if (!state) { return nullptr; @@ -49,7 +46,6 @@ static shared_ptr GetOrCreateWebSocket( auto itr = state->ResourceMap.find(id); if (itr == state->ResourceMap.end()) { - if (!state->Module) { return nullptr; } @@ -57,7 +53,7 @@ static shared_ptr GetOrCreateWebSocket( shared_ptr ws; try { - ws = IWebSocketResource::Make(std::move(url)); + ws = state->ResourceFactory(std::move(url)); } catch (const winrt::hresult_error &e) { std::stringstream ss; ss << "[" << std::hex << std::showbase << std::setw(8) << static_cast(e.code()) << "] " @@ -73,7 +69,8 @@ static shared_ptr GetOrCreateWebSocket( } catch (...) { SendEvent( weakInstance, - "webSocketFailed", dynamic::object("id", id)("message", "Unidentified error creating IWebSocketResource")); + "webSocketFailed", + dynamic::object("id", id)("message", "Unidentified error creating IWebSocketResource")); return nullptr; } @@ -122,9 +119,8 @@ static shared_ptr GetOrCreateWebSocket( namespace Microsoft::React { -WebSocketModule::WebSocketModule() - : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }}, - m_sharedState{std::make_shared()} { +WebSocketModule::WebSocketModule() : m_sharedState{std::make_shared()} { + m_sharedState->ResourceFactory = [](string &&url) { return IWebSocketResource::Make(std::move(url)); }; m_sharedState->Module = this; } @@ -134,7 +130,7 @@ WebSocketModule::~WebSocketModule() noexcept /*override*/ { void WebSocketModule::SetResourceFactory( std::function(const string &)> &&resourceFactory) { - m_resourceFactory = std::move(resourceFactory); + m_sharedState->ResourceFactory = std::move(resourceFactory); } string WebSocketModule::getName() { diff --git a/vnext/Shared/Modules/WebSocketModule.h b/vnext/Shared/Modules/WebSocketModule.h index 7dd193e92e0..e9ecc504d50 100644 --- a/vnext/Shared/Modules/WebSocketModule.h +++ b/vnext/Shared/Modules/WebSocketModule.h @@ -27,6 +27,11 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { /// std::map> ResourceMap{}; + /// + /// Generates IWebSocketResource instances, defaulting to IWebSocketResource::Make. + /// + std::function(std::string &&)> ResourceFactory; + /// /// Keeps a raw reference to the module object to lazily retrieve the React Instance as needed. /// @@ -62,11 +67,6 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { /// std::map> m_webSockets; - /// - /// Generates IWebSocketResource instances, defaulting to IWebSocketResource::Make. - /// - std::function(std::string &&)> m_resourceFactory; - /// /// Keeps members that can be accessed threads other than this module's owner accessible. /// From a65c9672876eb65d015170b7250f5c296340aec7 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Wed, 8 Dec 2021 22:16:53 -0800 Subject: [PATCH 08/16] Revert AsyncStorage changes --- vnext/Shared/AsyncStorageModule.h | 2 +- vnext/Shared/Modules/AsyncStorageModule.cpp | 122 +++++++++----------- 2 files changed, 58 insertions(+), 66 deletions(-) diff --git a/vnext/Shared/AsyncStorageModule.h b/vnext/Shared/AsyncStorageModule.h index a47b02836ce..8cc8cfa5e30 100644 --- a/vnext/Shared/AsyncStorageModule.h +++ b/vnext/Shared/AsyncStorageModule.h @@ -19,7 +19,7 @@ class AsyncStorageModule : public facebook::xplat::module::CxxModule { std::vector getMethods() override; private: - std::shared_ptr m_asyncStorageManager; + std::unique_ptr m_asyncStorageManager; }; } // namespace react } // namespace facebook diff --git a/vnext/Shared/Modules/AsyncStorageModule.cpp b/vnext/Shared/Modules/AsyncStorageModule.cpp index 12bf213761b..df1db1b2be2 100644 --- a/vnext/Shared/Modules/AsyncStorageModule.cpp +++ b/vnext/Shared/Modules/AsyncStorageModule.cpp @@ -7,91 +7,83 @@ #undef U -using facebook::xplat::module::CxxModule; -using std::map; -using std::string; -using std::vector; -using std::weak_ptr; - -namespace { -constexpr char moduleName[] = "AsyncLocalStorage"; -} +using namespace std; +using namespace folly; +using namespace facebook::xplat; namespace facebook { namespace react { AsyncStorageModule::AsyncStorageModule(const WCHAR *storageFileName) - : m_asyncStorageManager{std::make_shared(storageFileName)} {} + : m_asyncStorageManager{make_unique(storageFileName)} {} -string AsyncStorageModule::getName() { - return moduleName; +std::string AsyncStorageModule::getName() { + return "AsyncLocalStorage"; } -map AsyncStorageModule::getConstants() { +std::map AsyncStorageModule::getConstants() { return {}; } -vector AsyncStorageModule::getMethods() { +std::vector AsyncStorageModule::getMethods() { return { - {"multiGet", - [wpManager = weak_ptr(m_asyncStorageManager)]( - dynamic args, - Callback jsCallback) // params - array Keys , - // Callback(error, returnValue) - { - if (auto spManager = wpManager.lock()) { - spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::multiGet, args, jsCallback); - } - }}, - - {"multiSet", - [wpManager = weak_ptr(m_asyncStorageManager)]( - dynamic args, - Callback jsCallback) // params - array> - // KeyValuePairs , Callback(error) - { - if (auto spManager = wpManager.lock()) { - spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::multiSet, args, jsCallback); - } - }}, + Method( + "multiGet", + [this]( + dynamic args, + Callback jsCallback) // params - array Keys , + // Callback(error, returnValue) + { + m_asyncStorageManager->executeKVOperation( + AsyncStorageManager::AsyncStorageOperation::multiGet, args, jsCallback); + }), + + Method( + "multiSet", + [this]( + dynamic args, + Callback jsCallback) // params - array> + // KeyValuePairs , Callback(error) + { + m_asyncStorageManager->executeKVOperation( + AsyncStorageManager::AsyncStorageOperation::multiSet, args, jsCallback); + }), // The 'multiMerge' method is currently not implemented. We assume that // the ReactNative framework translates its absence into the AsyncStorage // JS object (see Libraries\Storage\AsyncStorage.js around line 485) which // in turn allows JS user code to test for support of merge methods. - {"multiRemove", - [wpManager = weak_ptr(m_asyncStorageManager)]( - dynamic args, Callback jsCallback) // params - array - // Keys , Callback(error) - { - if (auto spManager = wpManager.lock()) { - spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::multiRemove, args, jsCallback); - } - }}, - - {"clear", - [wpManager = weak_ptr(m_asyncStorageManager)]( - dynamic args, - Callback jsCallback) // params - args is unused , Callback(error) - { - if (auto spManager = wpManager.lock()) { - spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::clear, args, jsCallback); - } - }}, - - {"getAllKeys", - [wpManager = weak_ptr(m_asyncStorageManager)]( - dynamic args, Callback jsCallback) // params - args is unused , - // Callback(error, returnValue) - { - if (auto spManager = wpManager.lock()) { - spManager->executeKVOperation(AsyncStorageManager::AsyncStorageOperation::getAllKeys, args, jsCallback); - } - }}, + Method( + "multiRemove", + [this](dynamic args, Callback jsCallback) // params - array + // Keys , Callback(error) + { + m_asyncStorageManager->executeKVOperation( + AsyncStorageManager::AsyncStorageOperation::multiRemove, args, jsCallback); + }), + + Method( + "clear", + [this]( + dynamic args, + Callback jsCallback) // params - args is unused , Callback(error) + { + m_asyncStorageManager->executeKVOperation( + AsyncStorageManager::AsyncStorageOperation::clear, args, jsCallback); + }), + + Method( + "getAllKeys", + [this](dynamic args, Callback jsCallback) // params - args is unused , + // Callback(error, returnValue) + { + m_asyncStorageManager->executeKVOperation( + AsyncStorageManager::AsyncStorageOperation::getAllKeys, args, jsCallback); + }), }; } -std::unique_ptr CreateAsyncStorageModule(const WCHAR *storageFileName) noexcept { +std::unique_ptr CreateAsyncStorageModule(const WCHAR *storageFileName) noexcept { return std::make_unique(storageFileName); } From d691029f745dee82d0967d7be83e7546c77c9da3 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 00:34:17 -0800 Subject: [PATCH 09/16] Add URL argument to IWebSocketResource::Connect --- .../WebSocketIntegrationTest.cpp | 16 ++++++++-------- .../WebSocketResourcePerformanceTests.cpp | 2 +- vnext/Desktop.UnitTests/WebSocketMocks.cpp | 7 +++++-- vnext/Desktop.UnitTests/WebSocketMocks.h | 4 ++-- vnext/Desktop.UnitTests/WebSocketModuleTest.cpp | 2 +- .../WinRTWebSocketResourceUnitTest.cpp | 4 ++-- vnext/Desktop/BeastWebSocketResource.h | 2 +- vnext/Shared/IWebSocketResource.h | 4 ++-- vnext/Shared/InspectorPackagerConnection.cpp | 2 +- vnext/Shared/Modules/WebSocketModule.cpp | 2 +- vnext/Shared/WinRTWebSocketResource.cpp | 11 ++++++----- vnext/Shared/WinRTWebSocketResource.h | 6 +++--- 12 files changed, 33 insertions(+), 29 deletions(-) diff --git a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp index 05f719b8085..face1a88cb0 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp @@ -61,7 +61,7 @@ TEST_CLASS (WebSocketIntegrationTest) server->Start(); string sent = "prefix"; auto expectedSize = sent.size(); - ws->Connect(); + ws->Connect(scheme + "://localhost:5556/"); ws->Send(std::move(sent)); // Block until response is received. Fail in case of a remote endpoint failure. @@ -105,7 +105,7 @@ TEST_CLASS (WebSocketIntegrationTest) }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556/"); ws->Close(CloseCode::Normal, "Closing"); server->Stop(); @@ -138,7 +138,7 @@ TEST_CLASS (WebSocketIntegrationTest) errorMessage = error.Message; }); - ws->Connect(); + ws->Connect("ws://localhost:5556"); ws->Close();//TODO: Either remove or rename test. } @@ -168,7 +168,7 @@ TEST_CLASS (WebSocketIntegrationTest) errorString = err.Message; }); - ws->Connect(); + ws->Connect("ws://localhost:5556"); ws->Ping(); auto pingFuture = pingPromise.get_future(); pingFuture.wait(); @@ -196,7 +196,7 @@ TEST_CLASS (WebSocketIntegrationTest) ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556"); char digits[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; #define LEN 4096 + 4096 * 2 + 1 @@ -251,7 +251,7 @@ TEST_CLASS (WebSocketIntegrationTest) ws->SetOnMessage([&response](size_t size, const string &message, bool isBinary) { response.set_value(message); }); server->Start(); - ws->Connect({}, {{L"Cookie", "JSESSIONID=AD9A320CC4034641997FF903F1D10906"}}); + ws->Connect("ws://localhost:5556/", {}, {{L"Cookie", "JSESSIONID=AD9A320CC4034641997FF903F1D10906"}}); ws->Send(""); auto future = response.get_future(); @@ -307,7 +307,7 @@ TEST_CLASS (WebSocketIntegrationTest) }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556"); // Send all but the last message. // Compare result with the next message in the sequence. @@ -356,7 +356,7 @@ TEST_CLASS (WebSocketIntegrationTest) }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556"); // Consecutive immediate writes should be enqueued. // The WebSocket library (WinRT or Beast) can't handle multiple write operations diff --git a/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp b/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp index 628576fc5b5..1c538d16c75 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp @@ -103,7 +103,7 @@ TEST_CLASS (WebSocketResourcePerformanceTest) { errorFound = true; errorMessage = error.Message; }); - ws->Connect(); + ws->Connect("ws://localhost:5555"); resources.push_back(std::move(ws)); } // Create and store WS resources. diff --git a/vnext/Desktop.UnitTests/WebSocketMocks.cpp b/vnext/Desktop.UnitTests/WebSocketMocks.cpp index 6d5f2b646ac..00301705917 100644 --- a/vnext/Desktop.UnitTests/WebSocketMocks.cpp +++ b/vnext/Desktop.UnitTests/WebSocketMocks.cpp @@ -10,10 +10,13 @@ MockWebSocketResource::~MockWebSocketResource() {} #pragma region IWebSocketResource overrides -void MockWebSocketResource::Connect(const Protocols &protocols, const Options &options) noexcept /*override*/ +void MockWebSocketResource::Connect( + string &&url, + const Protocols &protocols, + const Options &options) noexcept /*override*/ { if (Mocks.Connect) - return Mocks.Connect(protocols, options); + return Mocks.Connect(std::move(url), protocols, options); } void MockWebSocketResource::Ping() noexcept /*override*/ diff --git a/vnext/Desktop.UnitTests/WebSocketMocks.h b/vnext/Desktop.UnitTests/WebSocketMocks.h index 962af1eda72..70b972c374a 100644 --- a/vnext/Desktop.UnitTests/WebSocketMocks.h +++ b/vnext/Desktop.UnitTests/WebSocketMocks.h @@ -7,7 +7,7 @@ struct MockWebSocketResource : public IWebSocketResource { ~MockWebSocketResource(); struct Mocks { - std::function Connect; + std::function Connect; std::function Ping; std::function Send; std::function SendBinary; @@ -25,7 +25,7 @@ struct MockWebSocketResource : public IWebSocketResource { #pragma region IWebSocketResource overrides - void Connect(const Protocols &, const Options &) noexcept override; + void Connect(std::string &&url, const Protocols &, const Options &) noexcept override; void Ping() noexcept override; diff --git a/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp b/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp index 076f707b056..dd7b423efb0 100644 --- a/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp +++ b/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp @@ -74,7 +74,7 @@ TEST_CLASS (WebSocketModuleTest) { module->setInstance(instance); module->SetResourceFactory([](const string &) { auto rc = make_shared(); - rc->Mocks.Connect = [rc](const IWebSocketResource::Protocols &, const IWebSocketResource::Options &) { + rc->Mocks.Connect = [rc](string &&, const IWebSocketResource::Protocols &, const IWebSocketResource::Options &) { rc->OnConnect(); }; diff --git a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp index 71dcb31cdc5..c3ae0f2c93b 100644 --- a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp +++ b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp @@ -56,7 +56,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); - rc->Connect({}, {}); + rc->Connect("ws://host:0", {}, {}); rc->Close(CloseCode::Normal, {}); Assert::AreEqual({}, errorMessage); @@ -82,7 +82,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); - rc->Connect({}, {}); + rc->Connect("ws://host:0", {}, {}); rc->Close(CloseCode::Normal, {}); Assert::AreEqual({"[0x80004005] Expected Failure"}, errorMessage); diff --git a/vnext/Desktop/BeastWebSocketResource.h b/vnext/Desktop/BeastWebSocketResource.h index a471c1ec6b2..65bf9b57367 100644 --- a/vnext/Desktop/BeastWebSocketResource.h +++ b/vnext/Desktop/BeastWebSocketResource.h @@ -156,7 +156,7 @@ class BaseWebSocketResource : public IWebSocketResource { /// /// /// - void Connect(const Protocols &protocols, const Options &options) noexcept override; + void Connect(std::string &&url, const Protocols &protocols, const Options &options) noexcept override; /// /// diff --git a/vnext/Shared/IWebSocketResource.h b/vnext/Shared/IWebSocketResource.h index 75a3f10b72d..7a4fc000913 100644 --- a/vnext/Shared/IWebSocketResource.h +++ b/vnext/Shared/IWebSocketResource.h @@ -83,7 +83,7 @@ struct IWebSocketResource { /// WebSocket URL address the instance will connect to. /// The address's scheme can be either ws:// or wss://. /// - static std::shared_ptr Make(std::string &&url); + static std::shared_ptr Make(std::string &&url = {}); virtual ~IWebSocketResource() noexcept {} @@ -97,7 +97,7 @@ struct IWebSocketResource { /// HTTP header fields passed by the remote endpoint, to be used in the /// handshake process. /// - virtual void Connect(const Protocols &protocols = {}, const Options &options = {}) noexcept = 0; + virtual void Connect(std::string &&url, const Protocols &protocols = {}, const Options &options = {}) noexcept = 0; /// /// Sends a ping frame to the remote endpoint. diff --git a/vnext/Shared/InspectorPackagerConnection.cpp b/vnext/Shared/InspectorPackagerConnection.cpp index 84d91003776..24317efda0f 100644 --- a/vnext/Shared/InspectorPackagerConnection.cpp +++ b/vnext/Shared/InspectorPackagerConnection.cpp @@ -270,7 +270,7 @@ winrt::fire_and_forget InspectorPackagerConnection::connectAsync() { Microsoft::React::IWebSocketResource::Protocols protocols; Microsoft::React::IWebSocketResource::Options options; - m_packagerWebSocketConnection->Connect(protocols, options); + m_packagerWebSocketConnection->Connect(std::string{m_url}, protocols, options); co_return; } diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index d0b03bc368b..4cb8ed0af3e 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -174,7 +174,7 @@ std::vector WebSocketModule::getMeth weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 3), jsArgAsString(args, 0), weakState); if (auto sharedWs = weakWs.lock()) { - sharedWs->Connect(protocols, options); + sharedWs->Connect(jsArgAsString(args, 0), protocols, options); } } }, diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index 8d98c9e8213..2bf843df818 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -103,7 +103,7 @@ WinRTWebSocketResource::WinRTWebSocketResource( IDataWriter &&writer, Uri &&uri, vector &&certExceptions) - : m_uri{std::move(uri)}, m_socket{std::move(socket)}, m_writer{std::move(writer)} { + : m_socket{std::move(socket)}, m_writer{std::move(writer)} { m_socket.MessageReceived({this, &WinRTWebSocketResource::OnMessageReceived}); for (const auto &certException : certExceptions) { @@ -123,13 +123,14 @@ WinRTWebSocketResource::~WinRTWebSocketResource() noexcept /*override*/ #pragma region Private members -IAsyncAction WinRTWebSocketResource::PerformConnect() noexcept { +IAsyncAction WinRTWebSocketResource::PerformConnect(Uri&& uri) noexcept { auto self = shared_from_this(); + auto coUri = std::move(uri); co_await resume_background(); try { - auto async = self->m_socket.ConnectAsync(self->m_uri); + auto async = self->m_socket.ConnectAsync(coUri); co_await lessthrow_await_adapter{async}; @@ -334,7 +335,7 @@ void WinRTWebSocketResource::Synchronize() noexcept { #pragma region IWebSocketResource -void WinRTWebSocketResource::Connect(const Protocols &protocols, const Options &options) noexcept { +void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, const Options &options) noexcept { m_readyState = ReadyState::Connecting; for (const auto &header : options) { @@ -348,7 +349,7 @@ void WinRTWebSocketResource::Connect(const Protocols &protocols, const Options & } m_connectRequested = true; - PerformConnect(); + PerformConnect(Uri{winrt::to_hstring(std::move(url))}); } void WinRTWebSocketResource::Ping() noexcept { diff --git a/vnext/Shared/WinRTWebSocketResource.h b/vnext/Shared/WinRTWebSocketResource.h index 777d78de4fc..f8a96f4c72b 100644 --- a/vnext/Shared/WinRTWebSocketResource.h +++ b/vnext/Shared/WinRTWebSocketResource.h @@ -17,7 +17,6 @@ namespace Microsoft::React { class WinRTWebSocketResource : public IWebSocketResource, public std::enable_shared_from_this { - winrt::Windows::Foundation::Uri m_uri; winrt::Windows::Networking::Sockets::IMessageWebSocket m_socket; // TODO: Use or remove. winrt::Windows::Networking::Sockets::IMessageWebSocket::MessageReceived_revoker m_revoker; @@ -48,7 +47,8 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha winrt::Windows::Foundation::Uri &&uri, std::vector &&certExceptions); - winrt::Windows::Foundation::IAsyncAction PerformConnect() noexcept; + winrt::Windows::Foundation::IAsyncAction PerformConnect( + /*std::string &&url*/ winrt::Windows::Foundation::Uri &&uri) noexcept; winrt::fire_and_forget PerformPing() noexcept; winrt::fire_and_forget PerformWrite(std::string &&message, bool isBinary) noexcept; winrt::fire_and_forget PerformClose() noexcept; @@ -76,7 +76,7 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha /// /// /// - void Connect(const Protocols &protocols, const Options &options) noexcept override; + void Connect(std::string &&url, const Protocols &protocols, const Options &options) noexcept override; /// /// From 1bedb259b029c0f31bdd6d811e628bec3e8a944d Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 01:18:09 -0800 Subject: [PATCH 10/16] Drop URL from factory method --- vnext/Desktop.DLL/react-native-win32.x64.def | 2 +- vnext/Desktop.DLL/react-native-win32.x86.def | 2 +- .../WebSocketIntegrationTest.cpp | 16 ++++++++-------- .../WebSocketResourcePerformanceTests.cpp | 4 ++-- .../WinRTWebSocketResourceUnitTest.cpp | 6 +++--- vnext/Desktop/WebSocketResourceFactory.cpp | 4 ++-- vnext/Shared/IWebSocketResource.h | 10 +++++----- vnext/Shared/InspectorPackagerConnection.cpp | 2 +- vnext/Shared/Modules/WebSocketModule.cpp | 2 +- vnext/Shared/WinRTWebSocketResource.cpp | 7 ++----- vnext/Shared/WinRTWebSocketResource.h | 6 +----- 11 files changed, 27 insertions(+), 34 deletions(-) diff --git a/vnext/Desktop.DLL/react-native-win32.x64.def b/vnext/Desktop.DLL/react-native-win32.x64.def index 0bc740ebf47..bf32f6fc7c6 100644 --- a/vnext/Desktop.DLL/react-native-win32.x64.def +++ b/vnext/Desktop.DLL/react-native-win32.x64.def @@ -22,7 +22,7 @@ EXPORTS ?GetConstants@ViewManagerBase@react@facebook@@UEBA?AUdynamic@folly@@XZ ?InitializeLogging@react@facebook@@YAX$$QEAV?$function@$$A6AXW4RCTLogLevel@react@facebook@@PEBD@Z@std@@@Z ?InitializeTracing@react@facebook@@YAXPEAUINativeTraceHandler@12@@Z -?Make@IWebSocketResource@React@Microsoft@@SA?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@$$QEAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@5@@Z +?Make@IWebSocketResource@React@Microsoft@@SA?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@XZ ?Make@JSBigAbiString@react@facebook@@SA?AV?$unique_ptr@$$CBUJSBigAbiString@react@facebook@@U?$default_delete@$$CBUJSBigAbiString@react@facebook@@@std@@@std@@$$QEAV?$unique_ptr@U?$IAbiArray@D@AbiSafe@@UAbiObjectDeleter@2@@5@@Z ?at@dynamic@folly@@QEGBAAEBU12@V?$Range@PEBD@2@@Z ?atImpl@dynamic@folly@@AEGBAAEBU12@AEBU12@@Z diff --git a/vnext/Desktop.DLL/react-native-win32.x86.def b/vnext/Desktop.DLL/react-native-win32.x86.def index f77e326570d..d3487e29742 100644 --- a/vnext/Desktop.DLL/react-native-win32.x86.def +++ b/vnext/Desktop.DLL/react-native-win32.x86.def @@ -22,7 +22,7 @@ EXPORTS ?GetConstants@ViewManagerBase@react@facebook@@UBE?AUdynamic@folly@@XZ ?InitializeLogging@react@facebook@@YGX$$QAV?$function@$$A6GXW4RCTLogLevel@react@facebook@@PBD@Z@std@@@Z ?InitializeTracing@react@facebook@@YGXPAUINativeTraceHandler@12@@Z -?Make@IWebSocketResource@React@Microsoft@@SG?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@$$QAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@5@@Z +?Make@IWebSocketResource@React@Microsoft@@SG?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@XZ ?Make@JSBigAbiString@react@facebook@@SG?AV?$unique_ptr@$$CBUJSBigAbiString@react@facebook@@U?$default_delete@$$CBUJSBigAbiString@react@facebook@@@std@@@std@@$$QAV?$unique_ptr@U?$IAbiArray@D@AbiSafe@@UAbiObjectDeleter@2@@5@@Z ?moduleNames@ModuleRegistry@react@facebook@@QAE?AV?$vector@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@V?$allocator@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@@std@@XZ ?at@dynamic@folly@@QGBEABU12@V?$Range@PBD@2@@Z diff --git a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp index face1a88cb0..a540ad82bf0 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp @@ -39,7 +39,7 @@ TEST_CLASS (WebSocketIntegrationTest) string scheme = "ws"; if (isSecure) scheme += "s"; - auto ws = IWebSocketResource::Make(scheme + "://localhost:5556/"); + auto ws = IWebSocketResource::Make(); promise sentSizePromise; ws->SetOnSend([&sentSizePromise](size_t size) { @@ -85,7 +85,7 @@ TEST_CLASS (WebSocketIntegrationTest) TEST_METHOD(ConnectClose) { auto server = make_shared(5556); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); Assert::IsFalse(nullptr == ws); bool connected = false; bool closed = false; @@ -124,7 +124,7 @@ TEST_CLASS (WebSocketIntegrationTest) // IWebSocketResource scope. Ensures object is closed implicitly by destructor. { - auto ws = IWebSocketResource::Make("ws://localhost:5556"); + auto ws = IWebSocketResource::Make(); ws->SetOnConnect([&connected]() { connected = true; @@ -156,7 +156,7 @@ TEST_CLASS (WebSocketIntegrationTest) auto server = make_shared(5556); server->Start(); - auto ws = IWebSocketResource::Make("ws://localhost:5556"); + auto ws = IWebSocketResource::Make(); promise pingPromise; ws->SetOnPing([&pingPromise]() { @@ -189,7 +189,7 @@ TEST_CLASS (WebSocketIntegrationTest) TEST_METHOD(SendReceiveLargeMessage) { auto server = make_shared(5556); server->SetMessageFactory([](string &&message) { return message + "_response"; }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); promise response; ws->SetOnMessage([&response](size_t size, const string &message, bool isBinary) { response.set_value(message); }); string errorMessage; @@ -246,7 +246,7 @@ TEST_CLASS (WebSocketIntegrationTest) auto cookie = response[boost::beast::http::field::cookie].to_string(); server->SetMessageFactory([cookie](string &&) { return cookie; }); }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); promise response; ws->SetOnMessage([&response](size_t size, const string &message, bool isBinary) { response.set_value(message); }); @@ -285,7 +285,7 @@ TEST_CLASS (WebSocketIntegrationTest) return message; }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); std::vector messages{ "AQ==", // [ 01 ] @@ -336,7 +336,7 @@ TEST_CLASS (WebSocketIntegrationTest) { return message; }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); string expected = "ABCDEFGHIJ"; string result(expected.size(), '0'); diff --git a/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp b/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp index 1c538d16c75..ff708605652 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp @@ -71,7 +71,7 @@ TEST_CLASS (WebSocketResourcePerformanceTest) { { vector> resources; for (int i = 0; i < resourceTotal; i++) { - auto ws = IWebSocketResource::Make("ws://localhost:5555/"); // TODO: Switch to port 5556 + auto ws = IWebSocketResource::Make(); ws->SetOnMessage([this, &threadCount, &errorFound](size_t size, const string &message, bool isBinary) { if (errorFound) return; @@ -103,7 +103,7 @@ TEST_CLASS (WebSocketResourcePerformanceTest) { errorFound = true; errorMessage = error.Message; }); - ws->Connect("ws://localhost:5555"); + ws->Connect("ws://localhost:5555"); // TODO: Switch to port 5556 resources.push_back(std::move(ws)); } // Create and store WS resources. diff --git a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp index c3ae0f2c93b..18696dad39b 100644 --- a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp +++ b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp @@ -52,7 +52,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { // Test APIs auto rc = - make_shared(std::move(imws), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{}); + make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); @@ -78,7 +78,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { // Test APIs auto rc = - make_shared(std::move(imws), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{}); + make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); @@ -95,7 +95,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { auto lambda = [&rc]() mutable { rc = make_shared( - winrt::make(), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{}); + winrt::make(), MockDataWriter{}, CertExceptions{}); }; Assert::ExpectException(lambda); diff --git a/vnext/Desktop/WebSocketResourceFactory.cpp b/vnext/Desktop/WebSocketResourceFactory.cpp index a9e782f20cf..16ca740f338 100644 --- a/vnext/Desktop/WebSocketResourceFactory.cpp +++ b/vnext/Desktop/WebSocketResourceFactory.cpp @@ -15,7 +15,7 @@ namespace Microsoft::React { #pragma region IWebSocketResource static members /*static*/ -shared_ptr IWebSocketResource::Make(string &&urlString) { +shared_ptr IWebSocketResource::Make() { #if ENABLE_BEAST if (GetRuntimeOptionBool("UseBeastWebSocket")) { Url url(std::move(urlString)); @@ -42,7 +42,7 @@ shared_ptr IWebSocketResource::Make(string &&urlString) { certExceptions.emplace_back( winrt::Windows::Security::Cryptography::Certificates::ChainValidationResult::InvalidName); } - return make_shared(std::move(urlString), std::move(certExceptions)); + return make_shared(std::move(certExceptions)); #if ENABLE_BEAST } #endif // ENABLE_BEAST diff --git a/vnext/Shared/IWebSocketResource.h b/vnext/Shared/IWebSocketResource.h index 7a4fc000913..6c27c72755f 100644 --- a/vnext/Shared/IWebSocketResource.h +++ b/vnext/Shared/IWebSocketResource.h @@ -79,17 +79,17 @@ struct IWebSocketResource { /// /// Creates an IWebSocketResource instance. /// - /// - /// WebSocket URL address the instance will connect to. - /// The address's scheme can be either ws:// or wss://. - /// - static std::shared_ptr Make(std::string &&url = {}); + static std::shared_ptr Make(); virtual ~IWebSocketResource() noexcept {} /// /// Establishes a continuous connection with the remote endpoint. /// + /// + /// WebSocket URL address the instance will connect to. + /// The address's scheme can be either ws:// or wss://. + /// /// /// Currently unused /// diff --git a/vnext/Shared/InspectorPackagerConnection.cpp b/vnext/Shared/InspectorPackagerConnection.cpp index 24317efda0f..770a0a82ace 100644 --- a/vnext/Shared/InspectorPackagerConnection.cpp +++ b/vnext/Shared/InspectorPackagerConnection.cpp @@ -205,7 +205,7 @@ winrt::fire_and_forget InspectorPackagerConnection::connectAsync() { std::vector certExceptions; m_packagerWebSocketConnection = - std::make_shared(m_url, std::move(certExceptions)); + std::make_shared(std::move(certExceptions)); m_packagerWebSocketConnection->SetOnError([](const Microsoft::React::IWebSocketResource::Error &err) { facebook::react::tracing::error(err.Message.c_str()); diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index 4cb8ed0af3e..3e70ebe8c67 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -120,7 +120,7 @@ GetOrCreateWebSocket(int64_t id, string &&url, weak_ptr()} { - m_sharedState->ResourceFactory = [](string &&url) { return IWebSocketResource::Make(std::move(url)); }; + m_sharedState->ResourceFactory = [](string &&url) { return IWebSocketResource::Make(); }; m_sharedState->Module = this; } diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index 2bf843df818..d45b013cdcb 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -90,18 +90,15 @@ namespace Microsoft::React { // private WinRTWebSocketResource::WinRTWebSocketResource( IMessageWebSocket &&socket, - Uri &&uri, vector &&certExceptions) : WinRTWebSocketResource( std::move(socket), DataWriter{socket.OutputStream()}, - std::move(uri), std::move(certExceptions)) {} WinRTWebSocketResource::WinRTWebSocketResource( IMessageWebSocket &&socket, IDataWriter &&writer, - Uri &&uri, vector &&certExceptions) : m_socket{std::move(socket)}, m_writer{std::move(writer)} { m_socket.MessageReceived({this, &WinRTWebSocketResource::OnMessageReceived}); @@ -111,8 +108,8 @@ WinRTWebSocketResource::WinRTWebSocketResource( } } -WinRTWebSocketResource::WinRTWebSocketResource(const string &urlString, vector &&certExceptions) - : WinRTWebSocketResource(MessageWebSocket{}, Uri{winrt::to_hstring(urlString)}, std::move(certExceptions)) {} +WinRTWebSocketResource::WinRTWebSocketResource(vector &&certExceptions) + : WinRTWebSocketResource(MessageWebSocket{}, std::move(certExceptions)) {} WinRTWebSocketResource::~WinRTWebSocketResource() noexcept /*override*/ { diff --git a/vnext/Shared/WinRTWebSocketResource.h b/vnext/Shared/WinRTWebSocketResource.h index f8a96f4c72b..f75ae809614 100644 --- a/vnext/Shared/WinRTWebSocketResource.h +++ b/vnext/Shared/WinRTWebSocketResource.h @@ -44,11 +44,9 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha WinRTWebSocketResource( winrt::Windows::Networking::Sockets::IMessageWebSocket &&socket, - winrt::Windows::Foundation::Uri &&uri, std::vector &&certExceptions); - winrt::Windows::Foundation::IAsyncAction PerformConnect( - /*std::string &&url*/ winrt::Windows::Foundation::Uri &&uri) noexcept; + winrt::Windows::Foundation::IAsyncAction PerformConnect(winrt::Windows::Foundation::Uri &&uri) noexcept; winrt::fire_and_forget PerformPing() noexcept; winrt::fire_and_forget PerformWrite(std::string &&message, bool isBinary) noexcept; winrt::fire_and_forget PerformClose() noexcept; @@ -62,11 +60,9 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha WinRTWebSocketResource( winrt::Windows::Networking::Sockets::IMessageWebSocket &&socket, winrt::Windows::Storage::Streams::IDataWriter &&writer, - winrt::Windows::Foundation::Uri &&uri, std::vector &&certExceptions); WinRTWebSocketResource( - const std::string &urlString, std::vector &&certExceptions); ~WinRTWebSocketResource() noexcept override; From 7d7bdcd813faed16573c1d4c7806dc5336547d76 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 03:36:37 -0800 Subject: [PATCH 11/16] Update BeastWebSocketResource --- .../Desktop.UnitTests/BaseWebSocketTests.cpp | 8 +- .../WinRTWebSocketResourceUnitTest.cpp | 6 +- vnext/Desktop/BeastWebSocketResource.cpp | 89 ++++++++++++++++++- vnext/Desktop/BeastWebSocketResource.h | 83 ++++++++++++++++- vnext/Desktop/WebSocketResourceFactory.cpp | 19 +--- vnext/Shared/InspectorPackagerConnection.cpp | 3 +- vnext/Shared/WinRTWebSocketResource.cpp | 7 +- 7 files changed, 180 insertions(+), 35 deletions(-) diff --git a/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp b/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp index ae3aa109be4..4e6b96a1764 100644 --- a/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp +++ b/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp @@ -48,7 +48,7 @@ TEST_CLASS(BaseWebSocketTest){ ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); - ws->Connect({}, {}); + ws->Connect("ws://localhost", {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreEqual({}, errorMessage); @@ -65,7 +65,7 @@ TEST_CLASS(BaseWebSocketTest){ return make_error_code(errc::state_not_recoverable); }); - ws->Connect({}, {}); + ws->Connect("ws://localhost", {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreNotEqual({}, errorMessage); @@ -85,7 +85,7 @@ TEST_CLASS(BaseWebSocketTest){ return make_error_code(errc::state_not_recoverable); }); - ws->Connect({}, {}); + ws->Connect("ws://localhost", {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreNotEqual({}, errorMessage); @@ -108,7 +108,7 @@ TEST_CLASS(BaseWebSocketTest){ ws->SetCloseResult( []() -> error_code { return make_error_code(errc::success); }); - ws->Connect({}, {}); + ws->Connect("ws://localhost", {}, {}); connected.get_future().wait(); ws->Close(CloseCode::Normal, "Normal"); diff --git a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp index 18696dad39b..4ff96d6bc9c 100644 --- a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp +++ b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp @@ -51,8 +51,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { mws->Mocks.Close = [](uint16_t, const hstring &) {}; // Test APIs - auto rc = - make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); + auto rc = make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); @@ -77,8 +76,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { mws->Mocks.Close = [](uint16_t, const hstring &) {}; // Test APIs - auto rc = - make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); + auto rc = make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); diff --git a/vnext/Desktop/BeastWebSocketResource.cpp b/vnext/Desktop/BeastWebSocketResource.cpp index 4eb476400e8..95c89bee9a9 100644 --- a/vnext/Desktop/BeastWebSocketResource.cpp +++ b/vnext/Desktop/BeastWebSocketResource.cpp @@ -30,6 +30,90 @@ using std::unique_ptr; namespace Microsoft::React { +#pragma region BeastWebSocketResource + +BeastWebSocketResource::BeastWebSocketResource() noexcept {} + +#pragma region IWebSocketResource members + +void BeastWebSocketResource::Connect( + std::string &&urlString, + const Protocols &protocols, + const Options &options) noexcept { + Url url(std::move(urlString)); + + if (url.scheme == "ws") { + if (url.port.empty()) + url.port = "80"; + + m_concreteResource = std::make_shared(std::move(url)); + } else if (url.scheme == "wss") { + if (url.port.empty()) + url.port = "443"; + + m_concreteResource = std::make_shared(std::move(url)); + } + + m_concreteResource->SetOnClose(std::move(m_closeHandler)); + m_concreteResource->SetOnConnect(std::move(m_connectHandler)); + m_concreteResource->SetOnError(std::move(m_errorHandler)); + m_concreteResource->SetOnMessage(std::move(m_readHandler)); + m_concreteResource->SetOnPing(std::move(m_pingHandler)); + m_concreteResource->SetOnSend(std::move(m_writeHandler)); +} + +void BeastWebSocketResource::Close(CloseCode code, const string &reason) noexcept { + m_concreteResource->Close(code, reason); +} + +void BeastWebSocketResource::Send(string &&message) noexcept { + m_concreteResource->Send(std::move(message)); +} + +void BeastWebSocketResource::SendBinary(string &&base64String) noexcept { + m_concreteResource->SendBinary(std::move(base64String)); +} + +void BeastWebSocketResource::Ping() noexcept { + m_concreteResource->Ping(); +} + +#pragma endregion IWebSocketResource members + +#pragma region Handler setters + +void BeastWebSocketResource::SetOnConnect(function &&handler) noexcept { + m_connectHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnPing(function &&handler) noexcept { + m_pingHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnSend(function &&handler) noexcept { + m_writeHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnMessage(function &&handler) noexcept { + m_readHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnClose(function &&handler) noexcept { + m_closeHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnError(function &&handler) noexcept { + m_errorHandler = std::move(handler); +} + +IWebSocketResource::ReadyState BeastWebSocketResource::GetReadyState() const noexcept { + return m_concreteResource->GetReadyState(); +} + +#pragma endregion Handler setters + +#pragma endregion BeastWebSocketResource + namespace Beast { #pragma region BaseWebSocketResource members @@ -314,7 +398,10 @@ websocket::close_code BaseWebSocketResource::ToBeastCloseCo #pragma region IWebSocketResource members template -void BaseWebSocketResource::Connect(const Protocols &protocols, const Options &options) noexcept { +void BaseWebSocketResource::Connect( + std::string &&url, + const Protocols &protocols, + const Options &options) noexcept { // "Cannot call Connect more than once"); assert(ReadyState::Connecting == m_readyState); diff --git a/vnext/Desktop/BeastWebSocketResource.h b/vnext/Desktop/BeastWebSocketResource.h index 65bf9b57367..aba52553e21 100644 --- a/vnext/Desktop/BeastWebSocketResource.h +++ b/vnext/Desktop/BeastWebSocketResource.h @@ -13,7 +13,9 @@ #include "IWebSocketResource.h" #include "Utils.h" -namespace Microsoft::React::Beast { +namespace Microsoft::React { + +namespace Beast { template < typename SocketLayer = boost::beast::tcp_stream, @@ -341,4 +343,81 @@ class TestWebSocketResource : public BaseWebSocketResource< } // namespace Test -} // namespace Microsoft::React::Beast +} // namespace Beast + +class BeastWebSocketResource : public IWebSocketResource, public std::enable_shared_from_this { + std::function m_connectHandler; + std::function m_pingHandler; + std::function m_writeHandler; + std::function m_readHandler; + std::function m_closeHandler; + std::function m_errorHandler; + + std::shared_ptr m_concreteResource; + + public: + BeastWebSocketResource() noexcept; + +#pragma region IWebSocketResource + + /// + /// + /// + void Connect(std::string &&url, const Protocols &protocols, const Options &options) noexcept override; + + /// + /// + /// + void Ping() noexcept override; + + /// + /// + /// + void Send(std::string &&message) noexcept override; + + /// + /// + /// + void SendBinary(std::string &&base64String) noexcept override; + + /// + /// + /// + void Close(CloseCode code, const std::string &reason) noexcept override; + + ReadyState GetReadyState() const noexcept override; + + /// + /// + /// + void SetOnConnect(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnPing(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnSend(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnMessage(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnClose(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnError(std::function &&handler) noexcept override; + +#pragma endregion IWebSocketResource +}; + +} // namespace Microsoft::React diff --git a/vnext/Desktop/WebSocketResourceFactory.cpp b/vnext/Desktop/WebSocketResourceFactory.cpp index 16ca740f338..3a89a0b6f94 100644 --- a/vnext/Desktop/WebSocketResourceFactory.cpp +++ b/vnext/Desktop/WebSocketResourceFactory.cpp @@ -7,7 +7,6 @@ #include #include "BeastWebSocketResource.h" -using std::make_shared; using std::shared_ptr; using std::string; @@ -18,21 +17,7 @@ namespace Microsoft::React { shared_ptr IWebSocketResource::Make() { #if ENABLE_BEAST if (GetRuntimeOptionBool("UseBeastWebSocket")) { - Url url(std::move(urlString)); - - if (url.scheme == "ws") { - if (url.port.empty()) - url.port = "80"; - - return make_shared(std::move(url)); - } else if (url.scheme == "wss") { - if (url.port.empty()) - url.port = "443"; - - return make_shared(std::move(url)); - } else { - throw std::invalid_argument((string("Incorrect URL scheme: ") + url.scheme).c_str()); - } + return std::make_shared(); } else { #endif // ENABLE_BEAST std::vector certExceptions; @@ -42,7 +27,7 @@ shared_ptr IWebSocketResource::Make() { certExceptions.emplace_back( winrt::Windows::Security::Cryptography::Certificates::ChainValidationResult::InvalidName); } - return make_shared(std::move(certExceptions)); + return std::make_shared(std::move(certExceptions)); #if ENABLE_BEAST } #endif // ENABLE_BEAST diff --git a/vnext/Shared/InspectorPackagerConnection.cpp b/vnext/Shared/InspectorPackagerConnection.cpp index 770a0a82ace..481067b0dc7 100644 --- a/vnext/Shared/InspectorPackagerConnection.cpp +++ b/vnext/Shared/InspectorPackagerConnection.cpp @@ -204,8 +204,7 @@ winrt::fire_and_forget InspectorPackagerConnection::connectAsync() { co_await winrt::resume_background(); std::vector certExceptions; - m_packagerWebSocketConnection = - std::make_shared(std::move(certExceptions)); + m_packagerWebSocketConnection = std::make_shared(std::move(certExceptions)); m_packagerWebSocketConnection->SetOnError([](const Microsoft::React::IWebSocketResource::Error &err) { facebook::react::tracing::error(err.Message.c_str()); diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index d45b013cdcb..df24a9f9b49 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -91,10 +91,7 @@ namespace Microsoft::React { WinRTWebSocketResource::WinRTWebSocketResource( IMessageWebSocket &&socket, vector &&certExceptions) - : WinRTWebSocketResource( - std::move(socket), - DataWriter{socket.OutputStream()}, - std::move(certExceptions)) {} + : WinRTWebSocketResource(std::move(socket), DataWriter{socket.OutputStream()}, std::move(certExceptions)) {} WinRTWebSocketResource::WinRTWebSocketResource( IMessageWebSocket &&socket, @@ -120,7 +117,7 @@ WinRTWebSocketResource::~WinRTWebSocketResource() noexcept /*override*/ #pragma region Private members -IAsyncAction WinRTWebSocketResource::PerformConnect(Uri&& uri) noexcept { +IAsyncAction WinRTWebSocketResource::PerformConnect(Uri &&uri) noexcept { auto self = shared_from_this(); auto coUri = std::move(uri); From 564a504e355b345dc9c2e11d09c2372a5948a31b Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 04:16:50 -0800 Subject: [PATCH 12/16] Handle malformed Uris within ::Connect() --- vnext/Shared/WinRTWebSocketResource.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index df24a9f9b49..baf1ec3a501 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -343,7 +343,24 @@ void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, c } m_connectRequested = true; - PerformConnect(Uri{winrt::to_hstring(std::move(url))}); + + Uri uri{nullptr}; + try { + uri = Uri{winrt::to_hstring(std::move(url))}; + } catch (hresult_error const &e) { + if (m_errorHandler) { + m_errorHandler({HResultToString(e), ErrorType::Connection}); + } + + // Abort - Mark connection as concluded. + SetEvent(m_connectPerformed.get()); + m_connectPerformedPromise.set_value(); + m_connectRequested = false; + + return; + } + + PerformConnect(std::move(uri)); } void WinRTWebSocketResource::Ping() noexcept { From 8d2502d52b7f6b7096274007958368f3b1f9f821 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 04:48:04 -0800 Subject: [PATCH 13/16] Remove urlString argument from IWebSocketResource::Make --- vnext/Microsoft.ReactNative/Modules/CreateModules.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp b/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp index f5d0b565247..8b2a22e9b14 100644 --- a/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp +++ b/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp @@ -19,9 +19,9 @@ using winrt::Microsoft::ReactNative::implementation::QuirkSettings; namespace Microsoft::React { -std::shared_ptr IWebSocketResource::Make(std::string &&urlString) { +std::shared_ptr IWebSocketResource::Make() { std::vector certExceptions; - return std::make_shared(std::move(urlString), std::move(certExceptions)); + return std::make_shared(std::move(certExceptions)); } } // namespace Microsoft::React From ee1c2a10672af338559dc08962a3b978487394a3 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 14:56:00 -0800 Subject: [PATCH 14/16] Use constexpr for commont test URLs --- .../Desktop.UnitTests/BaseWebSocketTests.cpp | 24 ++++++++++++------- .../WinRTWebSocketResourceUnitTest.cpp | 6 +++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp b/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp index 4e6b96a1764..c5e68faa19a 100644 --- a/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp +++ b/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp @@ -15,6 +15,12 @@ using std::make_unique; using std::promise; using std::string; +namespace { + +constexpr char testUrl[] = "ws://localhost"; + +} // namespace + namespace Microsoft::React::Test { using CloseCode = IWebSocketResource::CloseCode; @@ -30,7 +36,7 @@ TEST_CLASS(BaseWebSocketTest){ END_TEST_CLASS_ATTRIBUTE() TEST_METHOD(CreateAndSetHandlers){ - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); Assert::IsFalse(nullptr == ws); ws->SetOnConnect([]() {}); @@ -44,11 +50,11 @@ TEST_CLASS(BaseWebSocketTest){ TEST_METHOD(ConnectSucceeds) { string errorMessage; bool connected = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); - ws->Connect("ws://localhost", {}, {}); + ws->Connect(testUrl, {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreEqual({}, errorMessage); @@ -58,14 +64,14 @@ TEST_CLASS(BaseWebSocketTest){ TEST_METHOD(ConnectFails) { string errorMessage; bool connected = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); ws->SetConnectResult([]() -> error_code { return make_error_code(errc::state_not_recoverable); }); - ws->Connect("ws://localhost", {}, {}); + ws->Connect(testUrl, {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreNotEqual({}, errorMessage); @@ -78,14 +84,14 @@ TEST_CLASS(BaseWebSocketTest){ TEST_METHOD(HandshakeFails) { string errorMessage; bool connected = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); ws->SetHandshakeResult([](string, string) -> error_code { return make_error_code(errc::state_not_recoverable); }); - ws->Connect("ws://localhost", {}, {}); + ws->Connect(testUrl, {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreNotEqual({}, errorMessage); @@ -99,7 +105,7 @@ TEST_CLASS(BaseWebSocketTest){ string errorMessage; promise connected; bool closed = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected.set_value(); }); ws->SetOnClose( @@ -108,7 +114,7 @@ TEST_CLASS(BaseWebSocketTest){ ws->SetCloseResult( []() -> error_code { return make_error_code(errc::success); }); - ws->Connect("ws://localhost", {}, {}); + ws->Connect(testUrl, {}, {}); connected.get_future().wait(); ws->Close(CloseCode::Normal, "Normal"); diff --git a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp index 4ff96d6bc9c..f13ba0964ca 100644 --- a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp +++ b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp @@ -32,6 +32,8 @@ IAsyncAction ThrowAsync() { co_return; } +constexpr char testUrl[] = "ws://host:0"; + } // namespace namespace Microsoft::React::Test { @@ -55,7 +57,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); - rc->Connect("ws://host:0", {}, {}); + rc->Connect(testUrl, {}, {}); rc->Close(CloseCode::Normal, {}); Assert::AreEqual({}, errorMessage); @@ -80,7 +82,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); - rc->Connect("ws://host:0", {}, {}); + rc->Connect(testUrl, {}, {}); rc->Close(CloseCode::Normal, {}); Assert::AreEqual({"[0x80004005] Expected Failure"}, errorMessage); From 699e44568a3ee3da1c1a51ba045e7dcc50ed83da Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 16:36:53 -0800 Subject: [PATCH 15/16] Remove redundant move in to_hstring --- vnext/Shared/WinRTWebSocketResource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index baf1ec3a501..84d173d504d 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -346,7 +346,7 @@ void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, c Uri uri{nullptr}; try { - uri = Uri{winrt::to_hstring(std::move(url))}; + uri = Uri{winrt::to_hstring(url)}; } catch (hresult_error const &e) { if (m_errorHandler) { m_errorHandler({HResultToString(e), ErrorType::Connection}); From 494666da6e473c02ee109cd49b3f2994760cb4c1 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Thu, 9 Dec 2021 16:46:01 -0800 Subject: [PATCH 16/16] Add/remove std::move where applicable --- vnext/Desktop/BeastWebSocketResource.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/vnext/Desktop/BeastWebSocketResource.cpp b/vnext/Desktop/BeastWebSocketResource.cpp index 95c89bee9a9..2951bd61562 100644 --- a/vnext/Desktop/BeastWebSocketResource.cpp +++ b/vnext/Desktop/BeastWebSocketResource.cpp @@ -407,7 +407,7 @@ void BaseWebSocketResource::Connect( // TODO: Enable? // m_stream->set_option(websocket::stream_base::timeout::suggested(role_type::client)); - m_stream->set_option(websocket::stream_base::decorator([options = std::move(options)](websocket::request_type &req) { + m_stream->set_option(websocket::stream_base::decorator([options = options](websocket::request_type &req) { // Collect headers for (const auto &header : options) { req.insert(Microsoft::Common::Unicode::Utf16ToUtf8(header.first), header.second); @@ -520,34 +520,34 @@ void BaseWebSocketResource::Ping() noexcept { template void BaseWebSocketResource::SetOnConnect(function &&handler) noexcept { - m_connectHandler = handler; + m_connectHandler = std::move(handler); } template void BaseWebSocketResource::SetOnPing(function &&handler) noexcept { - m_pingHandler = handler; + m_pingHandler = std::move(handler); } template void BaseWebSocketResource::SetOnSend(function &&handler) noexcept { - m_writeHandler = handler; + m_writeHandler = std::move(handler); } template void BaseWebSocketResource::SetOnMessage( function &&handler) noexcept { - m_readHandler = handler; + m_readHandler = std::move(handler); } template void BaseWebSocketResource::SetOnClose( function &&handler) noexcept { - m_closeHandler = handler; + m_closeHandler = std::move(handler); } template void BaseWebSocketResource::SetOnError(function &&handler) noexcept { - m_errorHandler = handler; + m_errorHandler = std::move(handler); } template