Avoid capturing raw this pointer in WebSocket module

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"
+}

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

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

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<size_t> sentSizePromise;
     ws->SetOnSend([&sentSizePromise](size_t size)
     {
@@ -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.
@@ -85,7 +85,7 @@ TEST_CLASS (WebSocketIntegrationTest)
   TEST_METHOD(ConnectClose)
   {
     auto server = make_shared<Test::WebSocketServer>(5556);
-    auto ws = IWebSocketResource::Make("ws://localhost:5556/");
+    auto ws = IWebSocketResource::Make();
     Assert::IsFalse(nullptr == ws);
     bool connected = false;
     bool closed = false;
@@ -105,7 +105,7 @@ TEST_CLASS (WebSocketIntegrationTest)
     });
 
     server->Start();
-    ws->Connect();
+    ws->Connect("ws://localhost:5556/");
     ws->Close(CloseCode::Normal, "Closing");
     server->Stop();
 
@@ -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;
@@ -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.
     }
 
@@ -156,7 +156,7 @@ TEST_CLASS (WebSocketIntegrationTest)
     auto server = make_shared<Test::WebSocketServer>(5556);
     server->Start();
 
-    auto ws = IWebSocketResource::Make("ws://localhost:5556");
+    auto ws = IWebSocketResource::Make();
     promise<bool> pingPromise;
     ws->SetOnPing([&pingPromise]()
     {
@@ -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();
@@ -189,14 +189,14 @@ TEST_CLASS (WebSocketIntegrationTest)
   TEST_METHOD(SendReceiveLargeMessage) {
     auto server = make_shared<Test::WebSocketServer>(5556);
     server->SetMessageFactory([](string &&message) { return message + "_response"; });
-    auto ws = IWebSocketResource::Make("ws://localhost:5556/");
+    auto ws = IWebSocketResource::Make();
     promise<string> response;
     ws->SetOnMessage([&response](size_t size, const string &message, bool isBinary) { response.set_value(message); });
     string errorMessage;
     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
@@ -246,12 +246,12 @@ 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<string> response;
     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();
@@ -285,7 +285,7 @@ TEST_CLASS (WebSocketIntegrationTest)
       return message;
     });
 
-    auto ws = IWebSocketResource::Make("ws://localhost:5556/");
+    auto ws = IWebSocketResource::Make();
 
     std::vector<string> messages{
         "AQ==",     // [ 01 ]
@@ -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.
@@ -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');
@@ -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

vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp

@@ -71,7 +71,7 @@ TEST_CLASS (WebSocketResourcePerformanceTest) {
     {
       vector<shared_ptr<IWebSocketResource>> 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->Connect("ws://localhost:5555"); // TODO: Switch to port 5556
 
         resources.push_back(std::move(ws));
       } // Create and store WS resources.

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<TestWebSocketResource>(Url("ws://localhost"));
+    auto ws = make_unique<TestWebSocketResource>(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<TestWebSocketResource>(Url("ws://localhost"));
+    auto ws = make_unique<TestWebSocketResource>(Url(testUrl));
     ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; });
     ws->SetOnConnect([&connected]() { connected = true; });
 
-    ws->Connect({}, {});
+    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<TestWebSocketResource>(Url("ws://localhost"));
+    auto ws = make_unique<TestWebSocketResource>(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->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<TestWebSocketResource>(Url("ws://localhost"));
+    auto ws = make_unique<TestWebSocketResource>(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->Connect(testUrl, {}, {});
     ws->Close(CloseCode::Normal, {});
 
     Assert::AreNotEqual({}, errorMessage);
@@ -99,7 +105,7 @@ TEST_CLASS(BaseWebSocketTest){
     string errorMessage;
     promise<void> connected;
     bool closed = false;
-    auto ws = make_unique<TestWebSocketResource>(Url("ws://localhost"));
+    auto ws = make_unique<TestWebSocketResource>(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->Connect(testUrl, {}, {});
     connected.get_future().wait();
 
     ws->Close(CloseCode::Normal, "Normal");

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*/

vnext/Desktop.UnitTests/WebSocketMocks.h

@@ -7,7 +7,7 @@ struct MockWebSocketResource : public IWebSocketResource {
   ~MockWebSocketResource();
 
   struct Mocks {
-    std::function<void(const Protocols &, const Options &)> Connect;
+    std::function<void(std::string &&url, const Protocols &, const Options &)> Connect;
     std::function<void()> Ping;
     std::function<void(const std::string &)> Send;
     std::function<void(const std::string &)> 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;
 

vnext/Desktop.UnitTests/WebSocketModuleTest.cpp

@@ -74,7 +74,7 @@ TEST_CLASS (WebSocketModuleTest) {
     module->setInstance(instance);
     module->SetResourceFactory([](const string &) {
       auto rc = make_shared<MockWebSocketResource>();
-      rc->Mocks.Connect = [rc](const IWebSocketResource::Protocols &, const IWebSocketResource::Options &) {
+      rc->Mocks.Connect = [rc](string &&, const IWebSocketResource::Protocols &, const IWebSocketResource::Options &) {
         rc->OnConnect();
       };
 

vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp

@@ -32,6 +32,8 @@ IAsyncAction ThrowAsync() {
   co_return;
 }
 
+constexpr char testUrl[] = "ws://host:0";
+
 } // namespace
 
 namespace Microsoft::React::Test {
@@ -51,12 +53,11 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) {
     mws->Mocks.Close = [](uint16_t, const hstring &) {};
 
     // Test APIs
-    auto rc =
-        make_shared<WinRTWebSocketResource>(std::move(imws), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{});
+    auto rc = make_shared<WinRTWebSocketResource>(std::move(imws), MockDataWriter{}, CertExceptions{});
     rc->SetOnConnect([&connected]() { connected = true; });
     rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; });
 
-    rc->Connect({}, {});
+    rc->Connect(testUrl, {}, {});
     rc->Close(CloseCode::Normal, {});
 
     Assert::AreEqual({}, errorMessage);
@@ -77,12 +78,11 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) {
     mws->Mocks.Close = [](uint16_t, const hstring &) {};
 
     // Test APIs
-    auto rc =
-        make_shared<WinRTWebSocketResource>(std::move(imws), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{});
+    auto rc = make_shared<WinRTWebSocketResource>(std::move(imws), MockDataWriter{}, CertExceptions{});
     rc->SetOnConnect([&connected]() { connected = true; });
     rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; });
 
-    rc->Connect({}, {});
+    rc->Connect(testUrl, {}, {});
     rc->Close(CloseCode::Normal, {});
 
     Assert::AreEqual({"[0x80004005] Expected Failure"}, errorMessage);
@@ -95,7 +95,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) {
 
     auto lambda = [&rc]() mutable {
       rc = make_shared<WinRTWebSocketResource>(
-          winrt::make<ThrowingMessageWebSocket>(), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{});
+          winrt::make<ThrowingMessageWebSocket>(), MockDataWriter{}, CertExceptions{});
     };
 
     Assert::ExpectException<winrt::hresult_error>(lambda);