LibWeb: Don't drop messages received before MessagePort is enabled

This change implements following behavior defined in the spec:
https://html.spec.whatwg.org/multipage/web-messaging.html#examples-5
> The start() method, whether called explicitly or implicitly (by
setting onmessage), starts the flow of messages: messages posted on
message ports are initially paused, so that they don't get dropped on
the floor before the script has had a chance to set up its handlers.

Now we don't read bytes from the underlying transport socket until the
message port transitions to the enabled state. This required the
following places to explicitly enable the message port, because now,
when it actually matters, we must do so, or otherwise sent messages will
get stuck:
- `onmessage` attribute setter in DedicatedWorkerGlobalScope, because
  it implicitly sets the onmessage handler for the worker's underlying
  port.
- Stream API operations where the message port enabling steps were
  previously marked as FIXMEs.

Libraries/LibWeb/HTML/DedicatedWorkerGlobalScope.cpp

@@ -66,17 +66,31 @@ WebIDL::ExceptionOr<void> DedicatedWorkerGlobalScope::post_message(JS::Value mes
     return m_internal_port->post_message(message, transfer);
 }
 
-#undef __ENUMERATE
-#define __ENUMERATE(attribute_name, event_name)                                        \
-    void DedicatedWorkerGlobalScope::set_##attribute_name(WebIDL::CallbackType* value) \
-    {                                                                                  \
-        set_event_handler_attribute(event_name, move(value));                          \
-    }                                                                                  \
-    WebIDL::CallbackType* DedicatedWorkerGlobalScope::attribute_name()                 \
-    {                                                                                  \
-        return event_handler_attribute(event_name);                                    \
+WebIDL::CallbackType* DedicatedWorkerGlobalScope::onmessage()
+{
+    return event_handler_attribute(EventNames::message);
+}
+
+void DedicatedWorkerGlobalScope::set_onmessage(WebIDL::CallbackType* callback)
+{
+    set_event_handler_attribute(EventNames::message, callback);
+
+    // NOTE: This onmessage attribute setter implicitly sets worker's underlying MessagePort's onmessage attribute, so this
+    //       spec behavior also applies here:
+    // https://html.spec.whatwg.org/multipage/web-messaging.html#message-ports:handler-messageeventtarget-onmessage
+    // The first time a MessagePort object's onmessage IDL attribute is set, the port's port message queue must be enabled,
+    // as if the start() method had been called.
+    m_internal_port->start();
+}
+
+void DedicatedWorkerGlobalScope::set_onmessageerror(WebIDL::CallbackType* callback)
+{
+    set_event_handler_attribute(EventNames::messageerror, callback);
+}
+
+WebIDL::CallbackType* DedicatedWorkerGlobalScope::onmessageerror()
+{
+    return event_handler_attribute(EventNames::messageerror);
 }
-ENUMERATE_DEDICATED_WORKER_GLOBAL_SCOPE_EVENT_HANDLERS(__ENUMERATE)
-#undef __ENUMERATE
 
 }

Libraries/LibWeb/HTML/DedicatedWorkerGlobalScope.h

@@ -10,10 +10,6 @@
 #include <LibWeb/Bindings/WorkerGlobalScopePrototype.h>
 #include <LibWeb/HTML/WorkerGlobalScope.h>
 
-#define ENUMERATE_DEDICATED_WORKER_GLOBAL_SCOPE_EVENT_HANDLERS(E) \
-    E(onmessage, HTML::EventNames::message)                       \
-    E(onmessageerror, HTML::EventNames::messageerror)
-
 namespace Web::HTML {
 
 class DedicatedWorkerGlobalScope
@@ -30,12 +26,11 @@ public:
 
     void close();
 
-#undef __ENUMERATE
-#define __ENUMERATE(attribute_name, event_name)       \
-    void set_##attribute_name(WebIDL::CallbackType*); \
-    WebIDL::CallbackType* attribute_name();
-    ENUMERATE_DEDICATED_WORKER_GLOBAL_SCOPE_EVENT_HANDLERS(__ENUMERATE)
-#undef __ENUMERATE
+    WebIDL::CallbackType* onmessage();
+    void set_onmessage(WebIDL::CallbackType* callback);
+
+    WebIDL::CallbackType* onmessageerror();
+    void set_onmessageerror(WebIDL::CallbackType* callback);
 
     virtual void finalize() override;
 

Libraries/LibWeb/HTML/MessagePort.cpp

@@ -135,6 +135,7 @@ WebIDL::ExceptionOr<void> MessagePort::transfer_receiving_steps(HTML::TransferDa
         m_transport = make<IPC::Transport>(MUST(Core::LocalSocket::adopt_fd(fd.take_fd())));
 
         m_transport->set_up_read_hook([strong_this = GC::make_root(this)]() {
+            if (strong_this->m_enabled)
                 strong_this->read_from_transport();
         });
     } else if (fd_tag != 0) {
@@ -200,10 +201,12 @@ void MessagePort::entangle_with(MessagePort& remote_port)
     m_remote_port->m_transport = make<IPC::Transport>(move(sockets[1]));
 
     m_transport->set_up_read_hook([strong_this = GC::make_root(this)]() {
+        if (strong_this->m_enabled)
             strong_this->read_from_transport();
     });
 
     m_remote_port->m_transport->set_up_read_hook([remote_port = GC::make_root(m_remote_port)]() {
+        if (remote_port->m_enabled)
             remote_port->read_from_transport();
     });
 }
@@ -297,6 +300,8 @@ void MessagePort::post_port_message(SerializedTransferRecord serialize_with_tran
 
 void MessagePort::read_from_transport()
 {
+    VERIFY(m_enabled);
+
     auto schedule_shutdown = m_transport->read_as_many_messages_as_possible_without_blocking([this](auto&& raw_message) {
         FixedMemoryStream stream { raw_message.bytes.span(), FixedMemoryStream::Mode::ReadOnly };
         IPC::Decoder decoder { stream, raw_message.fds };
@@ -317,6 +322,8 @@ void MessagePort::read_from_transport()
 
 void MessagePort::post_message_task_steps(SerializedTransferRecord& serialize_with_transfer_result)
 {
+    VERIFY(m_enabled);
+
     // 1. Let finalTargetPort be the MessagePort in whose port message queue the task now finds itself.
     // NOTE: This can be different from targetPort, if targetPort itself was transferred and thus all its tasks moved along with it.
     auto* final_target_port = this;
@@ -368,6 +375,14 @@ void MessagePort::post_message_task_steps(SerializedTransferRecord& serialize_wi
     message_event_target->dispatch_event(event);
 }
 
+void MessagePort::enable()
+{
+    if (!m_enabled) {
+        m_enabled = true;
+        read_from_transport();
+    }
+}
+
 // https://html.spec.whatwg.org/multipage/web-messaging.html#dom-messageport-start
 void MessagePort::start()
 {
@@ -376,7 +391,8 @@ void MessagePort::start()
 
     VERIFY(m_transport);
 
-    // TODO: The start() method steps are to enable this's port message queue, if it is not already enabled.
+    // The start() method steps are to enable this's port message queue, if it is not already enabled.
+    enable();
 }
 
 // https://html.spec.whatwg.org/multipage/web-messaging.html#dom-messageport-close

Libraries/LibWeb/HTML/MessagePort.h

@@ -47,6 +47,7 @@ public:
     // https://html.spec.whatwg.org/multipage/web-messaging.html#dom-messageport-postmessage-options
     WebIDL::ExceptionOr<void> post_message(JS::Value message, StructuredSerializeOptions const& options);
 
+    void enable();
     void start();
 
     void close();
@@ -91,6 +92,8 @@ private:
     OwnPtr<IPC::Transport> m_transport;
 
     GC::Ptr<DOM::EventTarget> m_worker_event_target;
+
+    bool m_enabled { false };
 };
 
 }

Libraries/LibWeb/HTML/WorkerAgentParent.cpp

@@ -33,6 +33,11 @@ void WorkerAgentParent::initialize(JS::Realm& realm)
     TransferDataHolder data_holder;
     MUST(m_message_port->transfer_steps(data_holder));
 
+    // FIXME: Specification says this supposed to happen in step 11 of onComplete handler defined in https://html.spec.whatwg.org/multipage/workers.html#run-a-worker
+    //        but that would require introducing a new IPC message type to communicate this from WebWorker to WebContent process,
+    //        so let's do it here for now.
+    m_outside_port->start();
+
     // NOTE: This blocking IPC call may launch another process.
     //    If spinning the event loop for this can cause other javascript to execute, we're in trouble.
     auto worker_socket_file = Bindings::principal_host_defined_page(realm).client().request_worker_agent(m_agent_type);

Libraries/LibWeb/Streams/AbstractOperations.cpp

@@ -219,7 +219,8 @@ void set_up_cross_realm_transform_readable(JS::Realm& realm, ReadableStream& str
             port.disentangle();
         });
 
-    // FIXME: 5. Enable port’s port message queue.
+    // 5. Enable port’s port message queue.
+    port.enable();
 
     // 6. Let startAlgorithm be an algorithm that returns undefined.
     auto start_algorithm = GC::create_function(realm.heap(), []() -> WebIDL::ExceptionOr<JS::Value> {
@@ -333,7 +334,8 @@ void set_up_cross_realm_transform_writable(JS::Realm& realm, WritableStream& str
             port.disentangle();
         });
 
-    // FIXME: 6. Enable port’s port message queue.
+    // 6. Enable port’s port message queue.
+    port.enable();
 
     // 7. Let startAlgorithm be an algorithm that returns undefined.
     auto start_algorithm = GC::create_function(realm.heap(), []() -> WebIDL::ExceptionOr<JS::Value> {

Services/WebWorker/WorkerHost.cpp

@@ -208,7 +208,7 @@ void WorkerHost::run(GC::Ref<Web::Page> page, Web::HTML::TransferDataHolder mess
 
         // 12. If is shared is false, enable the port message queue of the worker's implicit port.
         if (!is_shared) {
-            inside_port->start();
+            inside_port->enable();
         }
 
         // 13. If is shared is true, then queue a global task on DOM manipulation task source given worker