From b10fee00eb133da7f12fc48417f9cd15b7e463ef Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Wed, 20 Dec 2023 13:47:01 -0700 Subject: [PATCH] LibWeb+WebWorker: Convert Workers to use MessagePorts for postMessage This aligns Workers and Window and MessagePorts to all use the same mechanism for transferring serialized messages across realms. It also allows transferring more message ports into a worker. Re-enable the Worker-echo test, as none of the MessagePort tests have themselves been flaky, and those are now using the same underlying implementation. --- Base/res/html/misc/worker.js | 4 +- Tests/LibWeb/TestConfig.ini | 2 - .../Text/expected/Worker/Worker-echo.txt | 3 +- .../LibWeb/Text/input/Worker/Worker-echo.html | 26 ++++-- Tests/LibWeb/Text/input/Worker/worker.js | 14 +++- Userland/Libraries/LibWeb/Forward.h | 1 + .../Libraries/LibWeb/HTML/MessagePort.cpp | 24 +++++- Userland/Libraries/LibWeb/HTML/MessagePort.h | 4 + Userland/Libraries/LibWeb/HTML/Worker.cpp | 82 +++---------------- Userland/Libraries/LibWeb/HTML/Worker.h | 17 ++-- Userland/Libraries/LibWeb/HTML/Worker.idl | 5 +- .../Libraries/LibWeb/HTML/WorkerAgent.cpp | 26 ++++-- Userland/Libraries/LibWeb/HTML/WorkerAgent.h | 11 +-- .../LibWeb/HTML/WorkerGlobalScope.cpp | 75 ++--------------- .../Libraries/LibWeb/HTML/WorkerGlobalScope.h | 19 ++--- .../LibWeb/HTML/WorkerGlobalScope.idl | 6 +- .../LibWeb/Worker/WebWorkerServer.ipc | 3 +- .../WebWorker/ConnectionFromClient.cpp | 9 +- .../Services/WebWorker/ConnectionFromClient.h | 2 +- .../WebWorker/DedicatedWorkerHost.cpp | 40 ++++----- .../Services/WebWorker/DedicatedWorkerHost.h | 8 +- 21 files changed, 159 insertions(+), 222 deletions(-) diff --git a/Base/res/html/misc/worker.js b/Base/res/html/misc/worker.js index f0e63dfdef..ba16d4937a 100644 --- a/Base/res/html/misc/worker.js +++ b/Base/res/html/misc/worker.js @@ -1,10 +1,10 @@ onmessage = evt => { console.log("In Worker - Got message:", JSON.stringify(evt.data)); - postMessage(evt.data, null); + postMessage(evt.data); }; console.log("In Worker - Loaded", this); console.log("Keys: ", JSON.stringify(Object.keys(this))); -postMessage("loaded", null); +postMessage("loaded"); diff --git a/Tests/LibWeb/TestConfig.ini b/Tests/LibWeb/TestConfig.ini index def2307567..12dfc314b4 100644 --- a/Tests/LibWeb/TestConfig.ini +++ b/Tests/LibWeb/TestConfig.ini @@ -1,3 +1 @@ [Skipped] -Text/input/Worker/Worker-echo.html - diff --git a/Tests/LibWeb/Text/expected/Worker/Worker-echo.txt b/Tests/LibWeb/Text/expected/Worker/Worker-echo.txt index 35844ec531..0edbc85457 100644 --- a/Tests/LibWeb/Text/expected/Worker/Worker-echo.txt +++ b/Tests/LibWeb/Text/expected/Worker/Worker-echo.txt @@ -1,3 +1,4 @@ Got message from worker: "loaded" -Got message from worker: {"msg":"marco"} +Got message from port: "Worker got message port!" +Got message from port: "Extra Port got message: \"Hello from port2\"" DONE diff --git a/Tests/LibWeb/Text/input/Worker/Worker-echo.html b/Tests/LibWeb/Text/input/Worker/Worker-echo.html index c0fe7ca263..b869526462 100644 --- a/Tests/LibWeb/Text/input/Worker/Worker-echo.html +++ b/Tests/LibWeb/Text/input/Worker/Worker-echo.html @@ -2,17 +2,31 @@ diff --git a/Tests/LibWeb/Text/input/Worker/worker.js b/Tests/LibWeb/Text/input/Worker/worker.js index 4b9a531037..8c88966e3d 100644 --- a/Tests/LibWeb/Text/input/Worker/worker.js +++ b/Tests/LibWeb/Text/input/Worker/worker.js @@ -1,4 +1,14 @@ +let extraPort = null; + onmessage = evt => { - postMessage(evt.data, null); + if (evt.ports.length > 0) { + extraPort = evt.ports[0]; + extraPort.onmessage = evt => { + extraPort.postMessage("Extra Port got message: " + JSON.stringify(evt.data)); + }; + extraPort.postMessage("Worker got message port!"); + } else { + postMessage(evt.data); + } }; -postMessage("loaded", null); +postMessage("loaded"); diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index 7884cd4f8a..4fa70adcb6 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -446,6 +446,7 @@ class Timer; class TimeRanges; class ToggleEvent; class TrackEvent; +struct TransferDataHolder; class TraversableNavigable; class VideoTrack; class VideoTrackList; diff --git a/Userland/Libraries/LibWeb/HTML/MessagePort.cpp b/Userland/Libraries/LibWeb/HTML/MessagePort.cpp index 0d71a6e7e5..9688e65c7f 100644 --- a/Userland/Libraries/LibWeb/HTML/MessagePort.cpp +++ b/Userland/Libraries/LibWeb/HTML/MessagePort.cpp @@ -19,6 +19,7 @@ #include #include #include +#include namespace Web::HTML { @@ -53,6 +54,11 @@ void MessagePort::visit_edges(Cell::Visitor& visitor) visitor.visit(m_remote_port); } +void MessagePort::set_worker_event_target(JS::NonnullGCPtr target) +{ + m_worker_event_target = target; +} + // https://html.spec.whatwg.org/multipage/web-messaging.html#message-ports:transfer-steps WebIDL::ExceptionOr MessagePort::transfer_steps(HTML::TransferDataHolder& data_holder) { @@ -107,6 +113,10 @@ WebIDL::ExceptionOr MessagePort::transfer_receiving_steps(HTML::TransferDa VERIFY(fd_tag == IPC_FILE_TAG); fd = data_holder.fds.take_first(); m_fd_passing_socket = MUST(Core::LocalSocket::adopt_fd(fd.take_fd(), Core::LocalSocket::PreventSIGPIPE::Yes)); + + m_socket->on_ready_to_read = [strong_this = JS::make_handle(this)]() { + strong_this->read_from_socket(); + }; } else if (fd_tag != 0) { dbgln("Unexpected byte {:x} in MessagePort transfer data", fd_tag); VERIFY_NOT_REACHED(); @@ -348,6 +358,16 @@ void MessagePort::post_message_task_steps(SerializedTransferRecord& serialize_wi // 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; + // IMPLEMENTATION DEFINED: + // https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the-worker-interface + // Worker objects act as if they had an implicit MessagePort associated with them. + // All messages received by that port must immediately be retargeted at the Worker object. + // We therefore set a special event target for those implicit ports on the Worker and the WorkerGlobalScope objects + EventTarget* message_event_target = final_target_port; + if (m_worker_event_target != nullptr) { + message_event_target = m_worker_event_target; + } + // 2. Let targetRealm be finalTargetPort's relevant realm. auto& target_realm = relevant_realm(*final_target_port); auto& target_vm = target_realm.vm(); @@ -359,7 +379,7 @@ void MessagePort::post_message_task_steps(SerializedTransferRecord& serialize_wi // If this throws an exception, catch it, fire an event named messageerror at finalTargetPort, using MessageEvent, and then return. auto exception = deserialize_record_or_error.release_error(); MessageEventInit event_init {}; - final_target_port->dispatch_event(MessageEvent::create(target_realm, HTML::EventNames::messageerror, event_init)); + message_event_target->dispatch_event(MessageEvent::create(target_realm, HTML::EventNames::messageerror, event_init)); return; } auto deserialize_record = deserialize_record_or_error.release_value(); @@ -380,7 +400,7 @@ void MessagePort::post_message_task_steps(SerializedTransferRecord& serialize_wi MessageEventInit event_init {}; event_init.data = message_clone; event_init.ports = move(new_ports); - final_target_port->dispatch_event(MessageEvent::create(target_realm, HTML::EventNames::message, event_init)); + message_event_target->dispatch_event(MessageEvent::create(target_realm, HTML::EventNames::message, event_init)); } // https://html.spec.whatwg.org/multipage/web-messaging.html#dom-messageport-start diff --git a/Userland/Libraries/LibWeb/HTML/MessagePort.h b/Userland/Libraries/LibWeb/HTML/MessagePort.h index 13a36ba460..10caba667a 100644 --- a/Userland/Libraries/LibWeb/HTML/MessagePort.h +++ b/Userland/Libraries/LibWeb/HTML/MessagePort.h @@ -61,6 +61,8 @@ public: virtual WebIDL::ExceptionOr transfer_receiving_steps(HTML::TransferDataHolder&) override; virtual HTML::TransferType primary_interface() const override { return HTML::TransferType::MessagePort; } + void set_worker_event_target(JS::NonnullGCPtr); + private: explicit MessagePort(JS::Realm&); @@ -91,6 +93,8 @@ private: Error, } m_socket_state { SocketState::Header }; size_t m_socket_incoming_message_size { 0 }; + + JS::GCPtr m_worker_event_target; }; } diff --git a/Userland/Libraries/LibWeb/HTML/Worker.cpp b/Userland/Libraries/LibWeb/HTML/Worker.cpp index 93df48289a..19f5ff2d95 100644 --- a/Userland/Libraries/LibWeb/HTML/Worker.cpp +++ b/Userland/Libraries/LibWeb/HTML/Worker.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include namespace Web::HTML { @@ -19,7 +18,7 @@ namespace Web::HTML { JS_DEFINE_ALLOCATOR(Worker); // https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the-worker-interface -Worker::Worker(String const& script_url, WorkerOptions const options, DOM::Document& document) +Worker::Worker(String const& script_url, WorkerOptions const& options, DOM::Document& document) : DOM::EventTarget(document.realm()) , m_script_url(script_url) , m_options(options) @@ -42,7 +41,7 @@ void Worker::visit_edges(Cell::Visitor& visitor) } // https://html.spec.whatwg.org/multipage/workers.html#dom-worker -WebIDL::ExceptionOr> Worker::create(String const& script_url, WorkerOptions const options, DOM::Document& document) +WebIDL::ExceptionOr> Worker::create(String const& script_url, WorkerOptions const& options, DOM::Document& document) { dbgln_if(WEB_WORKER_DEBUG, "WebWorker: Creating worker with script_url = {}", script_url); @@ -79,6 +78,7 @@ WebIDL::ExceptionOr> Worker::create(String const& scrip // 8. Associate the outside port with worker worker->m_outside_port = outside_port; + worker->m_outside_port->set_worker_event_target(worker); // 9. Run this step in parallel: // 1. Run a worker given worker, worker URL, outside settings, outside port, and options. @@ -89,7 +89,7 @@ WebIDL::ExceptionOr> Worker::create(String const& scrip } // https://html.spec.whatwg.org/multipage/workers.html#run-a-worker -void Worker::run_a_worker(AK::URL& url, EnvironmentSettingsObject& outside_settings, MessagePort&, WorkerOptions const& options) +void Worker::run_a_worker(AK::URL& url, EnvironmentSettingsObject& outside_settings, JS::GCPtr port, WorkerOptions const& options) { // 1. Let is shared be true if worker is a SharedWorker object, and false otherwise. // FIXME: SharedWorker support @@ -110,55 +110,7 @@ void Worker::run_a_worker(AK::URL& url, EnvironmentSettingsObject& outside_setti // and is shared. Run the rest of these steps in that agent. // Note: This spawns a new process to act as the 'agent' for the worker. - m_agent = heap().allocate_without_realm(url, options); - - auto& socket = m_agent->socket(); - // FIXME: Hide this logic in MessagePort - socket.set_notifications_enabled(true); - socket.on_ready_to_read = [this] { - auto& socket = this->m_agent->socket(); - auto& vm = this->vm(); - auto& realm = this->realm(); - - auto num_bytes_ready = MUST(socket.pending_bytes()); - switch (m_outside_port_state) { - case PortState::Header: { - if (num_bytes_ready < 8) - break; - auto const magic = MUST(socket.read_value()); - if (magic != 0xDEADBEEF) { - m_outside_port_state = PortState::Error; - break; - } - m_outside_port_incoming_message_size = MUST(socket.read_value()); - num_bytes_ready -= 8; - m_outside_port_state = PortState::Data; - } - [[fallthrough]]; - case PortState::Data: { - if (num_bytes_ready < m_outside_port_incoming_message_size) - break; - SerializationRecord rec; // FIXME: Keep in class scope - rec.resize(m_outside_port_incoming_message_size / sizeof(u32)); - - MUST(socket.read_until_filled(to_bytes(rec.span()))); - - TemporaryExecutionContext cxt(relevant_settings_object(*this)); - VERIFY(&realm == vm.current_realm()); - MessageEventInit event_init {}; - event_init.data = MUST(structured_deserialize(vm, rec, realm, {})); - // FIXME: Fill in the rest of the info from MessagePort - - this->dispatch_event(MessageEvent::create(realm, EventNames::message, event_init)); - - m_outside_port_state = PortState::Header; - break; - } - case PortState::Error: - VERIFY_NOT_REACHED(); - break; - } - }; + m_agent = heap().allocate(outside_settings.realm(), url, options, port); } // https://html.spec.whatwg.org/multipage/workers.html#dom-worker-terminate @@ -170,29 +122,15 @@ WebIDL::ExceptionOr Worker::terminate() } // https://html.spec.whatwg.org/multipage/workers.html#dom-worker-postmessage -WebIDL::ExceptionOr Worker::post_message(JS::Value message, JS::Value) +WebIDL::ExceptionOr Worker::post_message(JS::Value message, StructuredSerializeOptions const& options) { dbgln_if(WEB_WORKER_DEBUG, "WebWorker: Post Message: {}", message.to_string_without_side_effects()); - // FIXME: 1. Let targetPort be the port with which this is entangled, if any; otherwise let it be null. - // FIXME: 2. Let options be «[ "transfer" → transfer ]». - // FIXME: 3. Run the message port post message steps providing this, targetPort, message and options. + // The postMessage(message, transfer) and postMessage(message, options) methods on Worker objects act as if, + // when invoked, they immediately invoked the respective postMessage(message, transfer) and + // postMessage(message, options) on the port, with the same arguments, and returned the same return value. - auto& realm = this->realm(); - auto& vm = this->vm(); - - // FIXME: Use the with-transfer variant, which should(?) prepend the magic + size at the front - auto data = TRY(structured_serialize(vm, message)); - - Array header = { 0xDEADBEEF, static_cast(data.size() * sizeof(u32)) }; - - if (auto const err = m_agent->socket().write_until_depleted(to_readonly_bytes(header.span())); err.is_error()) - return WebIDL::DataCloneError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted("{}", err.error()))); - - if (auto const err = m_agent->socket().write_until_depleted(to_readonly_bytes(data.span())); err.is_error()) - return WebIDL::DataCloneError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted("{}", err.error()))); - - return {}; + return m_outside_port->post_message(message, options); } #undef __ENUMERATE diff --git a/Userland/Libraries/LibWeb/HTML/Worker.h b/Userland/Libraries/LibWeb/HTML/Worker.h index 6d0a9b6bb9..5b79a36d2e 100644 --- a/Userland/Libraries/LibWeb/HTML/Worker.h +++ b/Userland/Libraries/LibWeb/HTML/Worker.h @@ -32,8 +32,8 @@ class Worker : public DOM::EventTarget { JS_DECLARE_ALLOCATOR(Worker); public: - static WebIDL::ExceptionOr> create(String const& script_url, WorkerOptions const options, DOM::Document& document); - static WebIDL::ExceptionOr> construct_impl(JS::Realm& realm, String const& script_url, WorkerOptions const options) + static WebIDL::ExceptionOr> create(String const& script_url, WorkerOptions const& options, DOM::Document& document); + static WebIDL::ExceptionOr> construct_impl(JS::Realm& realm, String const& script_url, WorkerOptions const& options) { auto& window = verify_cast(realm.global_object()); return Worker::create(script_url, options, window.associated_document()); @@ -41,7 +41,7 @@ public: WebIDL::ExceptionOr terminate(); - WebIDL::ExceptionOr post_message(JS::Value message, JS::Value transfer); + WebIDL::ExceptionOr post_message(JS::Value message, StructuredSerializeOptions const&); virtual ~Worker() = default; @@ -55,7 +55,7 @@ public: #undef __ENUMERATE protected: - Worker(String const&, const WorkerOptions, DOM::Document&); + Worker(String const&, WorkerOptions const&, DOM::Document&); private: virtual void initialize(JS::Realm&) override; @@ -66,17 +66,10 @@ private: JS::GCPtr m_document; JS::GCPtr m_outside_port; - // FIXME: Move tihs state into the message port (and actually use it :) ) - enum class PortState : u8 { - Header, - Data, - Error, - } m_outside_port_state { PortState::Header }; - size_t m_outside_port_incoming_message_size { 0 }; JS::GCPtr m_agent; - void run_a_worker(AK::URL& url, EnvironmentSettingsObject& outside_settings, MessagePort& outside_port, WorkerOptions const& options); + void run_a_worker(AK::URL& url, EnvironmentSettingsObject& outside_settings, JS::GCPtr outside_port, WorkerOptions const& options); }; } diff --git a/Userland/Libraries/LibWeb/HTML/Worker.idl b/Userland/Libraries/LibWeb/HTML/Worker.idl index 30d6e602e0..2f97c0c561 100644 --- a/Userland/Libraries/LibWeb/HTML/Worker.idl +++ b/Userland/Libraries/LibWeb/HTML/Worker.idl @@ -1,5 +1,6 @@ #import #import +#import // https://html.spec.whatwg.org/#worker [Exposed=(Window)] @@ -7,7 +8,9 @@ interface Worker : EventTarget { constructor(DOMString scriptURL, optional WorkerOptions options = {}); undefined terminate(); - undefined postMessage(any message, optional any transfer); + // FIXME: IDL overload issue here + // FIXME: undefined postMessage(any message, sequence transfer); + undefined postMessage(any message, optional StructuredSerializeOptions options = {}); attribute EventHandler onmessage; attribute EventHandler onmessageerror; diff --git a/Userland/Libraries/LibWeb/HTML/WorkerAgent.cpp b/Userland/Libraries/LibWeb/HTML/WorkerAgent.cpp index a64b8289c8..d657fca385 100644 --- a/Userland/Libraries/LibWeb/HTML/WorkerAgent.cpp +++ b/Userland/Libraries/LibWeb/HTML/WorkerAgent.cpp @@ -95,24 +95,38 @@ namespace Web::HTML { JS_DEFINE_ALLOCATOR(WorkerAgent); -WorkerAgent::WorkerAgent(AK::URL url, WorkerOptions const& options) +WorkerAgent::WorkerAgent(AK::URL url, WorkerOptions const& options, JS::GCPtr outside_port) : m_worker_options(options) , m_url(move(url)) + , m_outside_port(outside_port) { +} + +void WorkerAgent::initialize(JS::Realm& realm) +{ + Base::initialize(realm); + + m_message_port = MessagePort::create(realm); + m_message_port->entangle_with(*m_outside_port); + #ifndef AK_OS_SERENITY - // FIXME: Add factory function auto paths = MUST(get_paths_for_helper_process("WebWorker"sv)); m_worker_ipc = MUST(launch_web_worker_process(paths)); #else m_worker_ipc = MUST(Web::HTML::WebWorkerClient::try_create()); #endif - int fds[2] = {}; - MUST(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, fds)); + TransferDataHolder data_holder; + MUST(m_message_port->transfer_steps(data_holder)); - m_socket = MUST(Core::BufferedLocalSocket::create(MUST(Core::LocalSocket::adopt_fd(fds[0])))); + m_worker_ipc->async_start_dedicated_worker(m_url, m_worker_options.type, m_worker_options.credentials, m_worker_options.name, move(data_holder)); +} - m_worker_ipc->async_start_dedicated_worker(m_url, options.type, options.credentials, options.name, fds[1]); +void WorkerAgent::visit_edges(Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_message_port); + visitor.visit(m_outside_port); } } diff --git a/Userland/Libraries/LibWeb/HTML/WorkerAgent.h b/Userland/Libraries/LibWeb/HTML/WorkerAgent.h index fb102ceb59..57f34078ae 100644 --- a/Userland/Libraries/LibWeb/HTML/WorkerAgent.h +++ b/Userland/Libraries/LibWeb/HTML/WorkerAgent.h @@ -26,18 +26,19 @@ struct WorkerAgent : JS::Cell { JS_CELL(Agent, JS::Cell); JS_DECLARE_ALLOCATOR(WorkerAgent); - WorkerAgent(AK::URL url, WorkerOptions const& options); + WorkerAgent(AK::URL url, WorkerOptions const& options, JS::GCPtr outside_port); RefPtr m_worker_ipc; - Core::BufferedLocalSocket& socket() const { return *m_socket; } - private: + virtual void initialize(JS::Realm&) override; + virtual void visit_edges(Cell::Visitor&) override; + WorkerOptions m_worker_options; AK::URL m_url; - // FIXME: associate with MessagePorts - OwnPtr m_socket; + JS::GCPtr m_message_port; + JS::GCPtr m_outside_port; }; } diff --git a/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp b/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp index 6ce9fe4ea7..2d328b831a 100644 --- a/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp +++ b/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.cpp @@ -4,26 +4,24 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include #include #include #include #include -#include #include #include #include -#include #include #include #include #include +#include namespace Web::HTML { JS_DEFINE_ALLOCATOR(WorkerGlobalScope); -WorkerGlobalScope::WorkerGlobalScope(JS::Realm& realm, Web::Page& page) +WorkerGlobalScope::WorkerGlobalScope(JS::Realm& realm, JS::NonnullGCPtr page) : DOM::EventTarget(realm) , m_page(page) { @@ -53,55 +51,14 @@ void WorkerGlobalScope::visit_edges(Cell::Visitor& visitor) visitor.visit(m_location); visitor.visit(m_navigator); + visitor.visit(m_internal_port); + visitor.visit(m_page); } -void WorkerGlobalScope::set_outside_port(NonnullOwnPtr port) +void WorkerGlobalScope::set_internal_port(JS::NonnullGCPtr port) { - m_outside_port = move(port); - - // FIXME: Hide this logic in MessagePort - m_outside_port->set_notifications_enabled(true); - m_outside_port->on_ready_to_read = [this] { - auto& vm = this->vm(); - auto& realm = this->realm(); - - auto num_bytes_ready = MUST(m_outside_port->pending_bytes()); - switch (m_outside_port_state) { - case PortState::Header: { - if (num_bytes_ready < 8) - break; - auto const magic = MUST(m_outside_port->read_value()); - if (magic != 0xDEADBEEF) { - m_outside_port_state = PortState::Error; - break; - } - m_outside_port_incoming_message_size = MUST(m_outside_port->read_value()); - num_bytes_ready -= 8; - m_outside_port_state = PortState::Data; - } - [[fallthrough]]; - case PortState::Data: { - if (num_bytes_ready < m_outside_port_incoming_message_size) - break; - SerializationRecord rec; // FIXME: Keep in class scope - rec.resize(m_outside_port_incoming_message_size / sizeof(u32)); - MUST(m_outside_port->read_until_filled(to_bytes(rec.span()))); - - TemporaryExecutionContext cxt(relevant_settings_object(*this)); - MessageEventInit event_init {}; - event_init.data = MUST(structured_deserialize(vm, rec, realm, {})); - // FIXME: Fill in the rest of the info from MessagePort - - this->dispatch_event(MessageEvent::create(realm, EventNames::message, event_init)); - - m_outside_port_state = PortState::Header; - break; - } - case PortState::Error: - VERIFY_NOT_REACHED(); - break; - } - }; + m_internal_port = port; + m_internal_port->set_worker_event_target(*this); } // https://html.spec.whatwg.org/multipage/workers.html#importing-scripts-and-libraries @@ -146,23 +103,9 @@ JS::NonnullGCPtr WorkerGlobalScope::navigator() const return *m_navigator; } -WebIDL::ExceptionOr WorkerGlobalScope::post_message(JS::Value message, JS::Value) +WebIDL::ExceptionOr WorkerGlobalScope::post_message(JS::Value message, StructuredSerializeOptions const& options) { - auto& realm = this->realm(); - auto& vm = this->vm(); - - // FIXME: Use the with-transfer variant, which should(?) prepend the magic + size at the front - auto data = TRY(structured_serialize(vm, message)); - - Array header = { 0xDEADBEEF, static_cast(data.size() * sizeof(u32)) }; - - if (auto const err = m_outside_port->write_until_depleted(to_readonly_bytes(header.span())); err.is_error()) - return WebIDL::DataCloneError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted("{}", err.error()))); - - if (auto const err = m_outside_port->write_until_depleted(to_readonly_bytes(data.span())); err.is_error()) - return WebIDL::DataCloneError::create(realm, TRY_OR_THROW_OOM(vm, String::formatted("{}", err.error()))); - - return {}; + return m_internal_port->post_message(message, options); } #undef __ENUMERATE diff --git a/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.h b/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.h index eb60c929bd..cc75d33e41 100644 --- a/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.h +++ b/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.h @@ -73,7 +73,7 @@ public: ENUMERATE_WORKER_GLOBAL_SCOPE_EVENT_HANDLERS(__ENUMERATE) #undef __ENUMERATE - WebIDL::ExceptionOr post_message(JS::Value message, JS::Value transfer); + WebIDL::ExceptionOr post_message(JS::Value message, StructuredSerializeOptions const&); // Non-IDL public methods @@ -84,14 +84,14 @@ public: // this is not problematic as it cannot be observed from script. void set_location(JS::NonnullGCPtr loc) { m_location = move(loc); } - void set_outside_port(NonnullOwnPtr port); + void set_internal_port(JS::NonnullGCPtr port); void initialize_web_interfaces(Badge); - Web::Page* page() { return &m_page; } + Web::Page* page() { return m_page.ptr(); } protected: - explicit WorkerGlobalScope(JS::Realm&, Web::Page&); + explicit WorkerGlobalScope(JS::Realm&, JS::NonnullGCPtr); private: virtual void visit_edges(Cell::Visitor&) override; @@ -99,13 +99,8 @@ private: JS::GCPtr m_location; JS::GCPtr m_navigator; - OwnPtr m_outside_port; - enum class PortState : u8 { - Header, - Data, - Error, - } m_outside_port_state { PortState::Header }; - size_t m_outside_port_incoming_message_size { 0 }; + JS::NonnullGCPtr m_page; + JS::GCPtr m_internal_port; // FIXME: Add all these internal slots @@ -138,8 +133,6 @@ private: // https://html.spec.whatwg.org/multipage/workers.html#concept-workerglobalscope-cross-origin-isolated-capability bool m_cross_origin_isolated_capability { false }; - - Web::Page& m_page; }; } diff --git a/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.idl b/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.idl index 61c43856fe..58d25184d9 100644 --- a/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.idl +++ b/Userland/Libraries/LibWeb/HTML/WorkerGlobalScope.idl @@ -3,6 +3,7 @@ #import #import #import +#import // https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope [Exposed=Worker] @@ -19,8 +20,9 @@ interface WorkerGlobalScope : EventTarget { attribute EventHandler onrejectionhandled; attribute EventHandler onunhandledrejection; - // FIXME: This belongs on the subclasses of WorkerGlobalScope - undefined postMessage(any message, any transfer); + // FIXME: IDL overload issue here + // FIXME: undefined postMessage(any message, sequence transfer); + undefined postMessage(any message, optional StructuredSerializeOptions options = {}); attribute EventHandler onmessage; attribute EventHandler onmessageerror; }; diff --git a/Userland/Libraries/LibWeb/Worker/WebWorkerServer.ipc b/Userland/Libraries/LibWeb/Worker/WebWorkerServer.ipc index 53fb5483fc..42610e2ea8 100644 --- a/Userland/Libraries/LibWeb/Worker/WebWorkerServer.ipc +++ b/Userland/Libraries/LibWeb/Worker/WebWorkerServer.ipc @@ -1,9 +1,10 @@ #include #include +#include endpoint WebWorkerServer { - start_dedicated_worker(URL url, String type, String credentials, String name, IPC::File message_port) =| + start_dedicated_worker(URL url, String type, String credentials, String name, Web::HTML::TransferDataHolder message_port) =| handle_file_return(i32 error, Optional file, i32 request_id) =| } diff --git a/Userland/Services/WebWorker/ConnectionFromClient.cpp b/Userland/Services/WebWorker/ConnectionFromClient.cpp index 122dcfcd2b..b1db664960 100644 --- a/Userland/Services/WebWorker/ConnectionFromClient.cpp +++ b/Userland/Services/WebWorker/ConnectionFromClient.cpp @@ -52,11 +52,12 @@ Web::Page const& ConnectionFromClient::page() const return m_page_host->page(); } -void ConnectionFromClient::start_dedicated_worker(AK::URL const& url, String const& type, String const&, String const&, IPC::File const& implicit_port) +void ConnectionFromClient::start_dedicated_worker(AK::URL const& url, String const& type, String const&, String const&, Web::HTML::TransferDataHolder const& implicit_port) { - m_worker_host = make_ref_counted(page(), url, type, implicit_port.take_fd()); - - m_worker_host->run(); + m_worker_host = make_ref_counted(url, type); + // FIXME: Yikes, const_cast to move? Feels like a LibIPC bug. + // We should be able to move non-copyable types from a Message type. + m_worker_host->run(page(), move(const_cast(implicit_port))); } void ConnectionFromClient::handle_file_return(i32 error, Optional const& file, i32 request_id) diff --git a/Userland/Services/WebWorker/ConnectionFromClient.h b/Userland/Services/WebWorker/ConnectionFromClient.h index 4c971129ee..4a44ba2ebe 100644 --- a/Userland/Services/WebWorker/ConnectionFromClient.h +++ b/Userland/Services/WebWorker/ConnectionFromClient.h @@ -39,7 +39,7 @@ private: Web::Page& page(); Web::Page const& page() const; - virtual void start_dedicated_worker(AK::URL const& url, String const&, String const&, String const&, IPC::File const&) override; + virtual void start_dedicated_worker(AK::URL const& url, String const&, String const&, String const&, Web::HTML::TransferDataHolder const&) override; virtual void handle_file_return(i32 error, Optional const& file, i32 request_id) override; JS::Handle m_page_host; diff --git a/Userland/Services/WebWorker/DedicatedWorkerHost.cpp b/Userland/Services/WebWorker/DedicatedWorkerHost.cpp index 85498bddf6..54873c100d 100644 --- a/Userland/Services/WebWorker/DedicatedWorkerHost.cpp +++ b/Userland/Services/WebWorker/DedicatedWorkerHost.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -17,35 +18,30 @@ namespace WebWorker { -DedicatedWorkerHost::DedicatedWorkerHost(Web::Page& page, AK::URL url, String type, int outside_port) - : m_page(page) - , m_url(move(url)) +DedicatedWorkerHost::DedicatedWorkerHost(AK::URL url, String type) + : m_url(move(url)) , m_type(move(type)) - , m_outside_port(outside_port) { } -DedicatedWorkerHost::~DedicatedWorkerHost() -{ - ::close(m_outside_port); -} +DedicatedWorkerHost::~DedicatedWorkerHost() = default; // https://html.spec.whatwg.org/multipage/workers.html#run-a-worker // FIXME: Extract out into a helper for both shared and dedicated workers -void DedicatedWorkerHost::run() +void DedicatedWorkerHost::run(JS::NonnullGCPtr page, Web::HTML::TransferDataHolder message_port_data) { bool const is_shared = false; // 7. Let realm execution context be the result of creating a new JavaScript realm given agent and the following customizations: auto realm_execution_context = Web::Bindings::create_a_new_javascript_realm( Web::Bindings::main_thread_vm(), - [this](JS::Realm& realm) -> JS::Object* { + [page](JS::Realm& realm) -> JS::Object* { // 7a. For the global object, if is shared is true, create a new SharedWorkerGlobalScope object. // 7b. Otherwise, create a new DedicatedWorkerGlobalScope object. // FIXME: Proper support for both SharedWorkerGlobalScope and DedicatedWorkerGlobalScope if (is_shared) TODO(); - return Web::Bindings::main_thread_vm().heap().allocate_without_realm(realm, m_page); + return Web::Bindings::main_thread_vm().heap().allocate_without_realm(realm, page); }, nullptr); @@ -55,7 +51,7 @@ void DedicatedWorkerHost::run() // 9. Set up a worker environment settings object with realm execution context, // outside settings, and unsafeWorkerCreationTime, and let inside settings be the result. - auto inner_settings = Web::HTML::WorkerEnvironmentSettingsObject::setup(m_page, move(realm_execution_context)); + auto inner_settings = Web::HTML::WorkerEnvironmentSettingsObject::setup(page, move(realm_execution_context)); auto& console_object = *inner_settings->realm().intrinsics().console_object(); m_console = adopt_ref(*new Web::HTML::WorkerDebugConsoleClient(console_object.console())); @@ -132,7 +128,7 @@ void DedicatedWorkerHost::run() }; auto perform_fetch = Web::HTML::create_perform_the_fetch_hook(inner_settings->heap(), move(perform_fetch_function)); - auto on_complete_function = [inner_settings, worker_global_scope, outside_port = m_outside_port, url = m_url](JS::GCPtr script) { + auto on_complete_function = [inner_settings, worker_global_scope, message_port_data = move(message_port_data), url = m_url](JS::GCPtr script) mutable { auto& realm = inner_settings->realm(); // 1. If script is null or if script's error to rethrow is non-null, then: if (!script || !script->error_to_rethrow().is_null()) { @@ -148,11 +144,14 @@ void DedicatedWorkerHost::run() // FIXME: 2. Associate worker with worker global scope. // What does this even mean? - // FIXME: 3. Let inside port be a new MessagePort object in inside settings's Realm. - // FIXME: 4. Associate inside port with worker global scope. - // FIXME: 5. Entangle outside port and inside port. - // This is a hack, move to a real MessagePort object per above FIXMEs. - worker_global_scope->set_outside_port(MUST(Core::BufferedLocalSocket::create(MUST(Core::LocalSocket::adopt_fd(outside_port))))); + // 3. Let inside port be a new MessagePort object in inside settings's Realm. + auto inside_port = Web::HTML::MessagePort::create(realm); + + // 4. Associate inside port with worker global scope. + worker_global_scope->set_internal_port(inside_port); + + // 5. Entangle outside port and inside port. + MUST(inside_port->transfer_receiving_steps(message_port_data)); // 6. Create a new WorkerLocation object and associate it with worker global scope. worker_global_scope->set_location(realm.heap().allocate(realm, *worker_global_scope)); @@ -178,7 +177,10 @@ void DedicatedWorkerHost::run() // FIXME: 11. Enable outside port's port message queue. - // FIXME: 12. If is shared is false, enable the port message queue of the worker's implicit port. + // 12. If is shared is false, enable the port message queue of the worker's implicit port. + if (!is_shared) { + inside_port->start(); + } // FIXME: 13. If is shared is true, then queue a global task on DOM manipulation task source given worker // global scope to fire an event named connect at worker global scope, using MessageEvent, diff --git a/Userland/Services/WebWorker/DedicatedWorkerHost.h b/Userland/Services/WebWorker/DedicatedWorkerHost.h index 37785a8f6e..d2fd05cfa0 100644 --- a/Userland/Services/WebWorker/DedicatedWorkerHost.h +++ b/Userland/Services/WebWorker/DedicatedWorkerHost.h @@ -10,24 +10,22 @@ #include #include #include +#include namespace WebWorker { class DedicatedWorkerHost : public RefCounted { public: - explicit DedicatedWorkerHost(Web::Page&, AK::URL url, String type, int outside_port); + explicit DedicatedWorkerHost(AK::URL url, String type); ~DedicatedWorkerHost(); - void run(); + void run(JS::NonnullGCPtr, Web::HTML::TransferDataHolder message_port_data); private: RefPtr m_console; - Web::Page& m_page; AK::URL m_url; String m_type; - - int m_outside_port { -1 }; }; }