fix: ensure MessagePorts get GCed when not referenced (#40201)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

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

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 <yoavweiss@chromium.org>
+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 <yoavweiss@chromium.org>
+Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
+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<scheduler::TaskAttributionTracker::TaskScope>
+       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<MessagePort> initially_entangled_port_;
++  WeakMember<MessagePort> initially_entangled_port_;
+
+   Member<PostMessageTaskContainer> post_message_task_container_;
+ };

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 () {