Browse Source

chore: cherry-pick ca2b108a0f1f from chromium (#37058)

* chore: [20-x-y] cherry-pick ca2b108a0f1f from chromium

* chore: update patches

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Keeley Hammond 2 years ago
parent
commit
03193df4d3
2 changed files with 117 additions and 0 deletions
  1. 1 0
      patches/chromium/.patches
  2. 116 0
      patches/chromium/cherry-pick-ca2b108a0f1f.patch

+ 1 - 0
patches/chromium/.patches

@@ -158,3 +158,4 @@ mojo_disable_sync_call_interrupts_in_the_browser.patch
 mojo_validate_that_a_message_is_allowed_to_use_the_sync_flag.patch
 cherry-pick-819d876e1bb8.patch
 cherry-pick-43637378b14e.patch
+cherry-pick-ca2b108a0f1f.patch

+ 116 - 0
patches/chromium/cherry-pick-ca2b108a0f1f.patch

@@ -0,0 +1,116 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Harald Alvestrand <[email protected]>
+Date: Wed, 18 Jan 2023 08:02:48 +0000
+Subject: Delete PeerConnectionHandler in PeerConnection finalizer
+
+Also guard against removal of PC during PeerConnectionHandler
+call that may cause garbage collection.
+
+(cherry picked from commit 5066dd66309d884762e5fb9be04b59582893d09a)
+
+Bug: chromium:1405256
+Change-Id: I9adf7b219e2026e07ccc0868c1a85f3b35cd9d26
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4154578
+Commit-Queue: Harald Alvestrand <[email protected]>
+Reviewed-by: Guido Urdaneta <[email protected]>
+Commit-Queue: Guido Urdaneta <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1091801}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4176372
+Auto-Submit: Harald Alvestrand <[email protected]>
+Cr-Commit-Position: refs/branch-heads/5481@{#418}
+Cr-Branched-From: 130f3e4d850f4bc7387cfb8d08aa993d288a67a9-refs/heads/main@{#1084008}
+
+diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
+index bcb9e9f13cfa525499023e369cefe0ed23f47abc..cf01e7144fc2901c88f320ea7b1c093dfb6ab8b1 100644
+--- a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
++++ b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc
+@@ -819,10 +819,11 @@ RTCPeerConnection::~RTCPeerConnection() {
+ }
+ 
+ void RTCPeerConnection::Dispose() {
+-  // Promptly clears the handler's pointer to |this|
++  // Promptly clears the handler
+   // so that content/ doesn't access it in a lazy sweeping phase.
++  // Other references to the handler use a weak pointer, preventing access.
+   if (peer_handler_) {
+-    peer_handler_->CloseAndUnregister();
++    peer_handler_.reset();
+   }
+ }
+ 
+diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
+index 7662f2cda29b6265705a4f83dbf76ccaf50ab576..48bd8a4eee90b07a38c7f09f260da30c9e7e34bc 100644
+--- a/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
++++ b/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc
+@@ -771,6 +771,8 @@ class RTCPeerConnectionHandler::WebRtcSetDescriptionObserverImpl
+     if (handler_) {
+       handler_->OnModifySctpTransport(std::move(states.sctp_transport_state));
+     }
++    // Since OnSessionDescriptionsUpdated can fire events, it may cause
++    // garbage collection. Ensure that handler_ is still valid.
+     if (handler_) {
+       handler_->OnModifyTransceivers(
+           states.signaling_state, std::move(states.transceiver_states),
+@@ -1128,6 +1130,8 @@ bool RTCPeerConnectionHandler::Initialize(
+   CHECK(!initialize_called_);
+   initialize_called_ = true;
+ 
++  // Prevent garbage collection of client_ during processing.
++  auto* client_on_stack = client_;
+   peer_connection_tracker_ = PeerConnectionTracker::From(*frame);
+ 
+   configuration_ = server_configuration;
+@@ -1165,8 +1169,8 @@ bool RTCPeerConnectionHandler::Initialize(
+     peer_connection_tracker_->RegisterPeerConnection(this, configuration_,
+                                                      options, frame_);
+   }
+-
+-  return true;
++  // Gratuitous usage of client_on_stack to prevent compiler errors.
++  return !!client_on_stack;
+ }
+ 
+ bool RTCPeerConnectionHandler::InitializeForTest(
+@@ -2229,9 +2233,11 @@ void RTCPeerConnectionHandler::OnSessionDescriptionsUpdated(
+         pending_remote_description,
+     std::unique_ptr<webrtc::SessionDescriptionInterface>
+         current_remote_description) {
++  // Prevent garbage collection of client_ during processing.
++  auto* client_on_stack = client_;
+   if (!client_ || is_closed_)
+     return;
+-  client_->DidChangeSessionDescriptions(
++  client_on_stack->DidChangeSessionDescriptions(
+       pending_local_description
+           ? CreateWebKitSessionDescription(pending_local_description.get())
+           : nullptr,
+@@ -2552,8 +2558,12 @@ void RTCPeerConnectionHandler::OnIceCandidate(const String& sdp,
+                                               int sdp_mline_index,
+                                               int component,
+                                               int address_family) {
++  // In order to ensure that the RTCPeerConnection is not garbage collected
++  // from under the function, we keep a pointer to it on the stack.
++  auto* client_on_stack = client_;
+   DCHECK(task_runner_->RunsTasksInCurrentSequence());
+   TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::OnIceCandidateImpl");
++  // This line can cause garbage collection.
+   auto* platform_candidate = MakeGarbageCollected<RTCIceCandidatePlatform>(
+       sdp, sdp_mid, sdp_mline_index);
+   if (peer_connection_tracker_) {
+@@ -2573,7 +2583,7 @@ void RTCPeerConnectionHandler::OnIceCandidate(const String& sdp,
+     }
+   }
+   if (!is_closed_)
+-    client_->DidGenerateICECandidate(platform_candidate);
++    client_on_stack->DidGenerateICECandidate(platform_candidate);
+ }
+ 
+ void RTCPeerConnectionHandler::OnIceCandidateError(
+@@ -2585,7 +2595,6 @@ void RTCPeerConnectionHandler::OnIceCandidateError(
+     const String& error_text) {
+   DCHECK(task_runner_->RunsTasksInCurrentSequence());
+   TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::OnIceCandidateError");
+-
+   if (peer_connection_tracker_) {
+     peer_connection_tracker_->TrackIceCandidateError(
+         this, address, port, host_candidate, url, error_code, error_text);