RequestServer: Transfer ownership of Protocols to all_protocols map

It's no change in application behavior to have these objects owned by
the function-scope static map in Protocol.cpp, while allowing us to
remove some ugly FIXMEs from time immemorial.

Ladybird/Android/src/main/cpp/RequestServerService.cpp

@@ -37,10 +37,9 @@ ErrorOr<int> service_main(int ipc_socket, int fd_passing_socket)
 
     Core::EventLoop event_loop;
 
-    // FIXME: Don't leak these :V
+    RequestServer::GeminiProtocol::install();
-    [[maybe_unused]] auto* gemini = new RequestServer::GeminiProtocol;
+    RequestServer::HttpProtocol::install();
-    [[maybe_unused]] auto* http = new RequestServer::HttpProtocol;
+    RequestServer::HttpsProtocol::install();
-    [[maybe_unused]] auto* https = new RequestServer::HttpsProtocol;
 
     auto socket = TRY(Core::LocalSocket::adopt_fd(ipc_socket));
     auto client = TRY(RequestServer::ConnectionFromClient::try_create(move(socket)));

Ladybird/RequestServer/main.cpp

@@ -51,17 +51,12 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     Core::EventLoop event_loop;
 
-    [[maybe_unused]] auto gemini = make<RequestServer::GeminiProtocol>();
+    RequestServer::GeminiProtocol::install();
-    [[maybe_unused]] auto http = make<RequestServer::HttpProtocol>();
+    RequestServer::HttpProtocol::install();
-    [[maybe_unused]] auto https = make<RequestServer::HttpsProtocol>();
+    RequestServer::HttpsProtocol::install();
 
     auto client = TRY(IPC::take_over_accepted_client_from_system_server<RequestServer::ConnectionFromClient>());
     client->set_fd_passing_socket(TRY(Core::LocalSocket::adopt_fd(fd_passing_socket)));
 
-    auto result = event_loop.exec();
+    return event_loop.exec();
-
-    // FIXME: We exit instead of returning, so that protocol destructors don't get called.
-    //        The Protocol base class should probably do proper de-registration instead of
-    //        just VERIFY_NOT_REACHED().
-    exit(result);
 }

Userland/Services/RequestServer/GeminiProtocol.cpp

@@ -38,4 +38,9 @@ OwnPtr<Request> GeminiProtocol::start_request(i32 request_id, ConnectionFromClie
     return protocol_request;
 }
 
+void GeminiProtocol::install()
+{
+    Protocol::install(adopt_own(*new GeminiProtocol()));
+}
+
 }

Userland/Services/RequestServer/GeminiProtocol.h

@@ -12,9 +12,13 @@ namespace RequestServer {
 
 class GeminiProtocol final : public Protocol {
 public:
-    GeminiProtocol();
     virtual ~GeminiProtocol() override = default;
 
+    static void install();
+
+private:
+    GeminiProtocol();
+
     virtual OwnPtr<Request> start_request(i32, ConnectionFromClient&, ByteString const& method, const URL&, HashMap<ByteString, ByteString> const&, ReadonlyBytes body, Core::ProxyData proxy_data = {}) override;
 };
 

Userland/Services/RequestServer/HttpProtocol.cpp

@@ -27,4 +27,9 @@ OwnPtr<Request> HttpProtocol::start_request(i32 request_id, ConnectionFromClient
     return Detail::start_request(Badge<HttpProtocol> {}, request_id, client, method, url, headers, body, get_pipe_for_request(), proxy_data);
 }
 
+void HttpProtocol::install()
+{
+    Protocol::install(adopt_own(*new HttpProtocol()));
+}
+
 }

Userland/Services/RequestServer/HttpProtocol.h

@@ -24,9 +24,13 @@ public:
     using JobType = HTTP::Job;
     using RequestType = HttpRequest;
 
-    HttpProtocol();
     ~HttpProtocol() override = default;
 
+    static void install();
+
+private:
+    HttpProtocol();
+
     virtual OwnPtr<Request> start_request(i32, ConnectionFromClient&, ByteString const& method, const URL&, HashMap<ByteString, ByteString> const& headers, ReadonlyBytes body, Core::ProxyData proxy_data = {}) override;
 };
 

Userland/Services/RequestServer/HttpsProtocol.cpp

@@ -27,4 +27,9 @@ OwnPtr<Request> HttpsProtocol::start_request(i32 request_id, ConnectionFromClien
     return Detail::start_request(Badge<HttpsProtocol> {}, request_id, client, method, url, headers, body, get_pipe_for_request(), proxy_data);
 }
 
+void HttpsProtocol::install()
+{
+    Protocol::install(adopt_own(*new HttpsProtocol()));
+}
+
 }

Userland/Services/RequestServer/HttpsProtocol.h

@@ -24,9 +24,13 @@ public:
     using JobType = HTTP::HttpsJob;
     using RequestType = HttpsRequest;
 
-    HttpsProtocol();
     ~HttpsProtocol() override = default;
 
+    static void install();
+
+private:
+    HttpsProtocol();
+
     virtual OwnPtr<Request> start_request(i32, ConnectionFromClient&, ByteString const& method, const URL&, HashMap<ByteString, ByteString> const& headers, ReadonlyBytes body, Core::ProxyData proxy_data = {}) override;
 };
 

Userland/Services/RequestServer/Protocol.cpp

@@ -5,6 +5,7 @@
  */
 
 #include <AK/HashMap.h>
+#include <AK/NonnullOwnPtr.h>
 #include <RequestServer/Protocol.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -13,26 +14,20 @@
 
 namespace RequestServer {
 
-static HashMap<ByteString, Protocol*>& all_protocols()
+static HashMap<ByteString, NonnullOwnPtr<Protocol>>& all_protocols()
 {
-    static HashMap<ByteString, Protocol*> map;
+    static HashMap<ByteString, NonnullOwnPtr<Protocol>> map;
     return map;
 }
 
 Protocol* Protocol::find_by_name(ByteString const& name)
 {
-    return all_protocols().get(name).value_or(nullptr);
+    return all_protocols().get(name).map([](auto& p) -> Protocol* { return p; }).value_or(nullptr);
 }
 
 Protocol::Protocol(ByteString const& name)
+    : m_name(name)
 {
-    all_protocols().set(name, this);
-}
-
-Protocol::~Protocol()
-{
-    // FIXME: Do proper de-registration.
-    VERIFY_NOT_REACHED();
 }
 
 ErrorOr<Protocol::Pipe> Protocol::get_pipe_for_request()
@@ -47,4 +42,10 @@ ErrorOr<Protocol::Pipe> Protocol::get_pipe_for_request()
     return Pipe { fd_pair[0], fd_pair[1] };
 }
 
+void Protocol::install(NonnullOwnPtr<Protocol> protocol)
+{
+    auto name = protocol->name();
+    all_protocols().set(move(name), move(protocol));
+}
+
 }

Userland/Services/RequestServer/Protocol.h

@@ -15,7 +15,7 @@ namespace RequestServer {
 
 class Protocol {
 public:
-    virtual ~Protocol();
+    virtual ~Protocol() = default;
 
     ByteString const& name() const { return m_name; }
     virtual OwnPtr<Request> start_request(i32, ConnectionFromClient&, ByteString const& method, const URL&, HashMap<ByteString, ByteString> const& headers, ReadonlyBytes body, Core::ProxyData proxy_data = {}) = 0;
@@ -30,6 +30,8 @@ protected:
     };
     static ErrorOr<Pipe> get_pipe_for_request();
 
+    static void install(NonnullOwnPtr<Protocol>);
+
 private:
     ByteString m_name;
 };

Userland/Services/RequestServer/main.cpp

@@ -45,16 +45,11 @@ ErrorOr<int> serenity_main(Main::Arguments)
         TRY(Core::System::unveil("/home/anon", "rwc"));
     TRY(Core::System::unveil(nullptr, nullptr));
 
-    [[maybe_unused]] auto gemini = make<RequestServer::GeminiProtocol>();
+    RequestServer::GeminiProtocol::install();
-    [[maybe_unused]] auto http = make<RequestServer::HttpProtocol>();
+    RequestServer::HttpProtocol::install();
-    [[maybe_unused]] auto https = make<RequestServer::HttpsProtocol>();
+    RequestServer::HttpsProtocol::install();
 
     auto client = TRY(IPC::take_over_accepted_client_from_system_server<RequestServer::ConnectionFromClient>());
 
-    auto result = event_loop.exec();
+    return event_loop.exec();
-
-    // FIXME: We exit instead of returning, so that protocol destructors don't get called.
-    //        The Protocol base class should probably do proper de-registration instead of
-    //        just VERIFY_NOT_REACHED().
-    exit(result);
 }