diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 58425d281b5c..0bbfcc0f820c 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -135,3 +135,4 @@ fix_activate_background_material_on_windows.patch fix_move_autopipsettingshelper_behind_branding_buildflag.patch revert_remove_the_allowaggressivethrottlingwithwebsocket_feature.patch fix_handle_no_top_level_aura_window_in_webcontentsimpl.patch +ensure_messageports_get_gced_when_not_referenced.patch diff --git a/patches/chromium/ensure_messageports_get_gced_when_not_referenced.patch b/patches/chromium/ensure_messageports_get_gced_when_not_referenced.patch new file mode 100644 index 000000000000..d4fe394e4fc1 --- /dev/null +++ b/patches/chromium/ensure_messageports_get_gced_when_not_referenced.patch @@ -0,0 +1,73 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Yoav Weiss +Date: Mon, 9 Oct 2023 14:21:44 +0000 +Subject: Ensure MessagePorts get GCed when not referenced + +[1] regressed MessagePort memory and caused them to no longer be +collected after not being referenced. +This CL fixes that by turning the mutual pointers between attached +MessagePorts to be WeakMembers. + +[1] https://chromium-review.googlesource.com/4919476 + +Bug: 1487835 +Change-Id: Icd7eba09a217ad5d588958504d5c4794d7d8a295 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4919476 +Commit-Queue: Yoav Weiss +Reviewed-by: Michael Lippautz +Cr-Commit-Position: refs/heads/main@{#1207067} + +diff --git a/third_party/blink/renderer/core/messaging/message_port.cc b/third_party/blink/renderer/core/messaging/message_port.cc +index 0fd5bbf170efa02d3e710de3cb6733158faec858..15f5f0f6aa05d3b17adae87286c92df9cc26a712 100644 +--- a/third_party/blink/renderer/core/messaging/message_port.cc ++++ b/third_party/blink/renderer/core/messaging/message_port.cc +@@ -162,8 +162,10 @@ MessagePortChannel MessagePort::Disentangle() { + DCHECK(!IsNeutered()); + port_descriptor_.GiveDisentangledHandle(connector_->PassMessagePipe()); + connector_ = nullptr; +- if (initially_entangled_port_) { +- initially_entangled_port_->OnEntangledPortDisconnected(); ++ // Using a variable here places the WeakMember pointer on the stack, ensuring ++ // it doesn't get GCed while it's being used. ++ if (auto* entangled_port = initially_entangled_port_.Get()) { ++ entangled_port->OnEntangledPortDisconnected(); + } + OnEntangledPortDisconnected(); + return MessagePortChannel(std::move(port_descriptor_)); +@@ -340,7 +342,10 @@ bool MessagePort::Accept(mojo::Message* mojo_message) { + // lifetime of this function. + std::unique_ptr + task_attribution_scope; +- if (initially_entangled_port_ && message.sender_origin && ++ // Using a variable here places the WeakMember pointer on the stack, ensuring ++ // it doesn't get GCed while it's being used. ++ auto* entangled_port = initially_entangled_port_.Get(); ++ if (entangled_port && message.sender_origin && + message.sender_origin->IsSameOriginWith(context->GetSecurityOrigin()) && + context->IsSameAgentCluster(message.sender_agent_cluster_id) && + context->IsWindow()) { +@@ -364,9 +369,9 @@ bool MessagePort::Accept(mojo::Message* mojo_message) { + ThreadScheduler::Current()->GetTaskAttributionTracker()) { + // Since `initially_entangled_port_` is not nullptr, neither should be + // its `post_message_task_container_`. +- CHECK(initially_entangled_port_->post_message_task_container_); ++ CHECK(entangled_port->post_message_task_container_); + scheduler::TaskAttributionInfo* parent_task = +- initially_entangled_port_->post_message_task_container_ ++ entangled_port->post_message_task_container_ + ->GetAndDecrementPostMessageTask(message.parent_task_id); + task_attribution_scope = tracker->CreateTaskScope( + script_state, parent_task, +diff --git a/third_party/blink/renderer/core/messaging/message_port.h b/third_party/blink/renderer/core/messaging/message_port.h +index 98ed58a9a765f5101d9b421507bf25db4359d7e5..a178d15c11b1e5fb1ff74d182021fe39e9d9b107 100644 +--- a/third_party/blink/renderer/core/messaging/message_port.h ++++ b/third_party/blink/renderer/core/messaging/message_port.h +@@ -195,7 +195,7 @@ class CORE_EXPORT MessagePort : public EventTarget, + + // The entangled port. Only set on initial entanglement, and gets unset as + // soon as the ports are disentangled. +- Member initially_entangled_port_; ++ WeakMember initially_entangled_port_; + + Member post_message_task_container_; + }; diff --git a/spec/api-ipc-spec.ts b/spec/api-ipc-spec.ts index 594148162bdc..873511b6ea37 100644 --- a/spec/api-ipc-spec.ts +++ b/spec/api-ipc-spec.ts @@ -317,11 +317,7 @@ describe('ipc module', () => { await once(ipcMain, 'closed'); }); - // TODO(@vertedinde): This broke upstream in CL https://chromium-review.googlesource.com/c/chromium/src/+/4831380 - // The behavior seems to be an intentional change, we need to either A) implement the task_container_ model in - // our renderer message ports or B) patch how we handle renderer message ports being garbage collected - // crbug: https://bugs.chromium.org/p/chromium/issues/detail?id=1487835 - it.skip('is emitted when the other end of a port is garbage-collected', async () => { + it('is emitted when the other end of a port is garbage-collected', async () => { const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, contextIsolation: false } }); w.loadURL('about:blank'); await w.webContents.executeJavaScript(`(${async function () {