LibCore+Userland+Tests: Convert Stream APIs to construct on heap

As per previous discussion, it was decided that the Stream classes
should be constructed on the heap.

While I don't personally agree with this change, it does have the
benefit of avoiding Function object reconstructions due to the lambda
passed to Notifier pointing to a stale object reference. This also has
the benefit of not having to "box" objects for virtual usage, as the
objects come pre-boxed.

However, it means that we now hit the heap everytime we construct a
TCPSocket for instance, which might not be desirable.

8 files changed, 43 insertions, 42 deletions

diff --git a/Userland/Services/EchoServer/Client.cpp b/Userland/Services/EchoServer/Client.cpp
index d92ac912d4..66cec114f9 100644
--- a/Userland/Services/EchoServer/Client.cpp
+++ b/Userland/Services/EchoServer/Client.cpp
@@ -7,12 +7,12 @@
 #include "Client.h"
 #include "LibCore/EventLoop.h"
 
-Client::Client(int id, Core::Stream::TCPSocket socket)
+Client::Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket)
     : m_id(id)
     , m_socket(move(socket))
 {
-    m_socket.on_ready_to_read = [this] {
-        if (m_socket.is_eof())
+    m_socket->on_ready_to_read = [this] {
+        if (m_socket->is_eof())
             return;
 
         auto result = drain_socket();
@@ -32,17 +32,17 @@ ErrorOr<void> Client::drain_socket()
         return ENOMEM;
     auto buffer = maybe_buffer.release_value();
 
-    while (TRY(m_socket.can_read_without_blocking())) {
-        auto nread = TRY(m_socket.read(buffer));
+    while (TRY(m_socket->can_read_without_blocking())) {
+        auto nread = TRY(m_socket->read(buffer));
 
         dbgln("Read {} bytes.", nread);
 
-        if (m_socket.is_eof()) {
+        if (m_socket->is_eof()) {
             Core::deferred_invoke([this, strong_this = NonnullRefPtr(*this)] { quit(); });
             break;
         }
 
-        TRY(m_socket.write({ buffer.data(), nread }));
+        TRY(m_socket->write({ buffer.data(), nread }));
     }
 
     return {};
@@ -50,7 +50,7 @@ ErrorOr<void> Client::drain_socket()
 
 void Client::quit()
 {
-    m_socket.close();
+    m_socket->close();
     if (on_exit)
         on_exit();
 }
diff --git a/Userland/Services/EchoServer/Client.h b/Userland/Services/EchoServer/Client.h
index 8342096242..6a1478fa1e 100644
--- a/Userland/Services/EchoServer/Client.h
+++ b/Userland/Services/EchoServer/Client.h
@@ -10,7 +10,7 @@
 
 class Client : public RefCounted<Client> {
 public:
-    static NonnullRefPtr<Client> create(int id, Core::Stream::TCPSocket socket)
+    static NonnullRefPtr<Client> create(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket)
     {
         return adopt_ref(*new Client(id, move(socket)));
     }
@@ -18,12 +18,12 @@ public:
     Function<void()> on_exit;
 
 protected:
-    Client(int id, Core::Stream::TCPSocket socket);
+    Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket);
 
     ErrorOr<void> drain_socket();
     void quit();
 
 private:
     int m_id { 0 };
-    Core::Stream::TCPSocket m_socket;
+    NonnullOwnPtr<Core::Stream::TCPSocket> m_socket;
 };
diff --git a/Userland/Services/TelnetServer/Client.cpp b/Userland/Services/TelnetServer/Client.cpp
index 2a3e6b866d..619652c061 100644
--- a/Userland/Services/TelnetServer/Client.cpp
+++ b/Userland/Services/TelnetServer/Client.cpp
@@ -5,6 +5,7 @@
  */
 
 #include "Client.h"
+
 #include <AK/ByteBuffer.h>
 #include <AK/MemoryStream.h>
 #include <AK/String.h>
@@ -16,13 +17,13 @@
 #include <stdio.h>
 #include <unistd.h>
 
-Client::Client(int id, Core::Stream::TCPSocket socket, int ptm_fd)
+Client::Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd)
     : m_id(id)
     , m_socket(move(socket))
     , m_ptm_fd(ptm_fd)
     , m_ptm_notifier(Core::Notifier::construct(ptm_fd, Core::Notifier::Read))
 {
-    m_socket.on_ready_to_read = [this] {
+    m_socket->on_ready_to_read = [this] {
         auto result = drain_socket();
         if (result.is_error()) {
             dbgln("Failed to drain the socket: {}", result.error());
@@ -50,7 +51,7 @@ Client::Client(int id, Core::Stream::TCPSocket socket, int ptm_fd)
     m_parser.on_error = [this]() { handle_error(); };
 }
 
-ErrorOr<NonnullRefPtr<Client>> Client::create(int id, Core::Stream::TCPSocket socket, int ptm_fd)
+ErrorOr<NonnullRefPtr<Client>> Client::create(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd)
 {
     auto client = adopt_ref(*new Client(id, move(socket), ptm_fd));
 
@@ -77,12 +78,12 @@ ErrorOr<void> Client::drain_socket()
         return ENOMEM;
     auto buffer = maybe_buffer.release_value();
 
-    while (TRY(m_socket.can_read_without_blocking())) {
-        auto nread = TRY(m_socket.read(buffer));
+    while (TRY(m_socket->can_read_without_blocking())) {
+        auto nread = TRY(m_socket->read(buffer));
 
         m_parser.write({ buffer.data(), nread });
 
-        if (m_socket.is_eof()) {
+        if (m_socket->is_eof()) {
             Core::deferred_invoke([this, strong_this = NonnullRefPtr(*this)] { quit(); });
             break;
         }
@@ -162,7 +163,7 @@ ErrorOr<void> Client::send_data(StringView data)
     }
 
     if (fast) {
-        TRY(m_socket.write({ data.characters_without_null_termination(), data.length() }));
+        TRY(m_socket->write({ data.characters_without_null_termination(), data.length() }));
         return {};
     }
 
@@ -184,7 +185,7 @@ ErrorOr<void> Client::send_data(StringView data)
     }
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
     return {};
 }
 
@@ -205,7 +206,7 @@ ErrorOr<void> Client::send_commands(Vector<Command> commands)
         stream << (u8)IAC << command.command << command.subcommand;
 
     VERIFY(stream.is_end());
-    TRY(m_socket.write({ buffer.data(), buffer.size() }));
+    TRY(m_socket->write({ buffer.data(), buffer.size() }));
     return {};
 }
 
@@ -213,7 +214,7 @@ void Client::quit()
 {
     m_ptm_notifier->set_enabled(false);
     close(m_ptm_fd);
-    m_socket.close();
+    m_socket->close();
     if (on_exit)
         on_exit();
 }
diff --git a/Userland/Services/TelnetServer/Client.h b/Userland/Services/TelnetServer/Client.h
index 4b9225738c..70a2bb1106 100644
--- a/Userland/Services/TelnetServer/Client.h
+++ b/Userland/Services/TelnetServer/Client.h
@@ -17,12 +17,12 @@
 
 class Client : public RefCounted<Client> {
 public:
-    static ErrorOr<NonnullRefPtr<Client>> create(int id, Core::Stream::TCPSocket socket, int ptm_fd);
+    static ErrorOr<NonnullRefPtr<Client>> create(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd);
 
     Function<void()> on_exit;
 
 private:
-    Client(int id, Core::Stream::TCPSocket socket, int ptm_fd);
+    Client(int id, NonnullOwnPtr<Core::Stream::TCPSocket> socket, int ptm_fd);
 
     ErrorOr<void> drain_socket();
     ErrorOr<void> drain_pty();
@@ -38,7 +38,7 @@ private:
     // client id
     int m_id { 0 };
     // client resources
-    Core::Stream::TCPSocket m_socket;
+    NonnullOwnPtr<Core::Stream::TCPSocket> m_socket;
     Parser m_parser;
     // pty resources
     int m_ptm_fd { -1 };
diff --git a/Userland/Services/TelnetServer/main.cpp b/Userland/Services/TelnetServer/main.cpp
index 3859d27297..fa22b97182 100644
--- a/Userland/Services/TelnetServer/main.cpp
+++ b/Userland/Services/TelnetServer/main.cpp
@@ -106,7 +106,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     server->on_ready_to_accept = [&next_id, &clients, &server, command] {
         int id = next_id++;
 
-        ErrorOr<Core::Stream::TCPSocket> maybe_client_socket = server->accept();
+        auto maybe_client_socket = server->accept();
         if (maybe_client_socket.is_error()) {
             warnln("accept: {}", maybe_client_socket.error());
             return;
@@ -116,17 +116,17 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         int ptm_fd = posix_openpt(O_RDWR);
         if (ptm_fd < 0) {
             perror("posix_openpt");
-            client_socket.close();
+            client_socket->close();
             return;
         }
         if (grantpt(ptm_fd) < 0) {
             perror("grantpt");
-            client_socket.close();
+            client_socket->close();
             return;
         }
         if (unlockpt(ptm_fd) < 0) {
             perror("unlockpt");
-            client_socket.close();
+            client_socket->close();
             return;
         }
 
diff --git a/Userland/Services/WebServer/Client.cpp b/Userland/Services/WebServer/Client.cpp
index 7c2869eadd..99f066a6a9 100644
--- a/Userland/Services/WebServer/Client.cpp
+++ b/Userland/Services/WebServer/Client.cpp
@@ -28,7 +28,7 @@
 
 namespace WebServer {
 
-Client::Client(Core::Stream::BufferedTCPSocket socket, Core::Object* parent)
+Client::Client(NonnullOwnPtr<Core::Stream::BufferedTCPSocket> socket, Core::Object* parent)
     : Core::Object(parent)
     , m_socket(move(socket))
 {
@@ -36,16 +36,16 @@ Client::Client(Core::Stream::BufferedTCPSocket socket, Core::Object* parent)
 
 void Client::die()
 {
-    m_socket.close();
+    m_socket->close();
     deferred_invoke([this] { remove_from_parent(); });
 }
 
 void Client::start()
 {
-    m_socket.on_ready_to_read = [this] {
+    m_socket->on_ready_to_read = [this] {
         StringBuilder builder;
 
-        auto maybe_buffer = ByteBuffer::create_uninitialized(m_socket.buffer_size());
+        auto maybe_buffer = ByteBuffer::create_uninitialized(m_socket->buffer_size());
         if (!maybe_buffer.has_value()) {
             warnln("Could not create buffer for client (possibly out of memory)");
             die();
@@ -54,7 +54,7 @@ void Client::start()
 
         auto buffer = maybe_buffer.release_value();
         for (;;) {
-            auto maybe_can_read = m_socket.can_read_without_blocking();
+            auto maybe_can_read = m_socket->can_read_without_blocking();
             if (maybe_can_read.is_error()) {
                 warnln("Failed to get the blocking status for the socket: {}", maybe_can_read.error());
                 die();
@@ -64,14 +64,14 @@ void Client::start()
             if (!maybe_can_read.value())
                 break;
 
-            auto maybe_nread = m_socket.read_until_any_of(buffer, Array { "\r"sv, "\n"sv, "\r\n"sv });
+            auto maybe_nread = m_socket->read_until_any_of(buffer, Array { "\r"sv, "\n"sv, "\r\n"sv });
             if (maybe_nread.is_error()) {
                 warnln("Failed to read a line from the request: {}", maybe_nread.error());
                 die();
                 return;
             }
 
-            if (m_socket.is_eof()) {
+            if (m_socket->is_eof()) {
                 die();
                 break;
             }
@@ -182,7 +182,7 @@ ErrorOr<void> Client::send_response(InputStream& response, HTTP::HttpRequest con
     builder.append("\r\n");
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
     log_response(200, request);
 
     char buffer[PAGE_SIZE];
@@ -193,7 +193,7 @@ ErrorOr<void> Client::send_response(InputStream& response, HTTP::HttpRequest con
 
         ReadonlyBytes write_buffer { buffer, size };
         while (!write_buffer.is_empty()) {
-            auto nwritten = TRY(m_socket.write(write_buffer));
+            auto nwritten = TRY(m_socket->write(write_buffer));
 
             if (nwritten == 0) {
                 dbgln("EEEEEE got 0 bytes written!");
@@ -216,7 +216,7 @@ ErrorOr<void> Client::send_redirect(StringView redirect_path, HTTP::HttpRequest
     builder.append("\r\n");
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
 
     log_response(301, request);
     return {};
@@ -341,7 +341,7 @@ ErrorOr<void> Client::send_error_response(unsigned code, HTTP::HttpRequest const
     builder.append("</h1></body></html>");
 
     auto builder_contents = builder.to_byte_buffer();
-    TRY(m_socket.write(builder_contents));
+    TRY(m_socket->write(builder_contents));
 
     log_response(code, request);
     return {};
diff --git a/Userland/Services/WebServer/Client.h b/Userland/Services/WebServer/Client.h
index a8a306f3ed..98ddfdc654 100644
--- a/Userland/Services/WebServer/Client.h
+++ b/Userland/Services/WebServer/Client.h
@@ -20,7 +20,7 @@ public:
     void start();
 
 private:
-    Client(Core::Stream::BufferedTCPSocket, Core::Object* parent);
+    Client(NonnullOwnPtr<Core::Stream::BufferedTCPSocket>, Core::Object* parent);
 
     ErrorOr<bool> handle_request(ReadonlyBytes);
     ErrorOr<void> send_response(InputStream&, HTTP::HttpRequest const&, String const& content_type);
@@ -31,7 +31,7 @@ private:
     ErrorOr<void> handle_directory_listing(String const& requested_path, String const& real_path, HTTP::HttpRequest const&);
     bool verify_credentials(Vector<HTTP::HttpRequest::Header> const&);
 
-    Core::Stream::BufferedTCPSocket m_socket;
+    NonnullOwnPtr<Core::Stream::BufferedTCPSocket> m_socket;
 };
 
 }
diff --git a/Userland/Services/WebServer/main.cpp b/Userland/Services/WebServer/main.cpp
index f5f02cbb6b..567f512316 100644
--- a/Userland/Services/WebServer/main.cpp
+++ b/Userland/Services/WebServer/main.cpp
@@ -85,7 +85,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         }
 
         // FIXME: Propagate errors
-        MUST(maybe_buffered_socket.value().set_blocking(true));
+        MUST(maybe_buffered_socket.value()->set_blocking(true));
         auto client = WebServer::Client::construct(maybe_buffered_socket.release_value(), server);
         client->start();
     };