[PeerConnection] Surface generated ICE candidates in localDescription.

CL https://chromium-review.googlesource.com/c/chromium/src/+/2323359
implemented the spec-compliant behavior of having internal slots for
SDP as to not prematurely surface descriptions of pending SLD, SRD or
addIceCandidate operations.

But that CL forgot the changes to the localDescription due to ICE
candidates being generated and "onicecandidate" firing. This caused a
regression in M87.

This CL adds test coverage for this in WPT and fixes the bug by
surfacing all descriptions at OnIceCandidate.

// Fippo approved. TBR while hta is OOO
TBR=hta@chromium.org

Bug: chromium:1124384
Change-Id: Ifb11d6416772b87b9076ff6238e909928c66af6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390747Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804267}

third_party/blink/renderer/modules/peerconnection/rtc_peer_connection_handler.cc

@@ -830,9 +830,19 @@ class RTCPeerConnectionHandler::Observer
  public:
   Observer(const base::WeakPtr<RTCPeerConnectionHandler>& handler,
            scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-      : handler_(handler), main_thread_(task_runner) {}
+      : handler_(handler),
+        main_thread_(task_runner),
+        native_peer_connection_(nullptr) {}
   ~Observer() override = default;
 
+  void Initialize() {
+    DCHECK(main_thread_->BelongsToCurrentThread());
+    DCHECK(!native_peer_connection_);
+    DCHECK(handler_);
+    native_peer_connection_ = handler_->native_peer_connection_;
+    DCHECK(native_peer_connection_);
+  }
+
   // When an RTC event log is sent back from PeerConnection, it arrives here.
   void OnWebRtcEventLogWrite(const WTF::Vector<uint8_t>& output) override {
     if (!main_thread_->BelongsToCurrentThread()) {
@@ -918,11 +928,28 @@ class RTCPeerConnectionHandler::Observer
   }
 
   void OnIceCandidate(const IceCandidateInterface* candidate) override {
+    DCHECK(native_peer_connection_);
     std::string sdp;
     if (!candidate->ToString(&sdp)) {
       NOTREACHED() << "OnIceCandidate: Could not get SDP string.";
       return;
     }
+    // The generated candidate may have been added to the pending or current
+    // local description, take a snapshot and surface them to the main thread.
+    // Remote descriptions are also surfaced because
+    // OnSessionDescriptionsUpdated() requires all four as arguments.
+    std::unique_ptr<webrtc::SessionDescriptionInterface>
+        pending_local_description = CopySessionDescription(
+            native_peer_connection_->pending_local_description());
+    std::unique_ptr<webrtc::SessionDescriptionInterface>
+        current_local_description = CopySessionDescription(
+            native_peer_connection_->current_local_description());
+    std::unique_ptr<webrtc::SessionDescriptionInterface>
+        pending_remote_description = CopySessionDescription(
+            native_peer_connection_->pending_remote_description());
+    std::unique_ptr<webrtc::SessionDescriptionInterface>
+        current_remote_description = CopySessionDescription(
+            native_peer_connection_->current_remote_description());
 
     PostCrossThreadTask(
         *main_thread_.get(), FROM_HERE,
@@ -931,7 +958,11 @@ class RTCPeerConnectionHandler::Observer
             WrapCrossThreadPersistent(this), String::FromUTF8(sdp),
             String::FromUTF8(candidate->sdp_mid()),
             candidate->sdp_mline_index(), candidate->candidate().component(),
-            candidate->candidate().address().family()));
+            candidate->candidate().address().family(),
+            std::move(pending_local_description),
+            std::move(current_local_description),
+            std::move(pending_remote_description),
+            std::move(current_remote_description)));
   }
 
   void OnIceCandidateError(const std::string& address,
@@ -960,9 +991,22 @@ class RTCPeerConnectionHandler::Observer
                           const String& sdp_mid,
                           int sdp_mline_index,
                           int component,
-                          int address_family) {
+                          int address_family,
+                          std::unique_ptr<webrtc::SessionDescriptionInterface>
+                              pending_local_description,
+                          std::unique_ptr<webrtc::SessionDescriptionInterface>
+                              current_local_description,
+                          std::unique_ptr<webrtc::SessionDescriptionInterface>
+                              pending_remote_description,
+                          std::unique_ptr<webrtc::SessionDescriptionInterface>
+                              current_remote_description) {
     DCHECK(main_thread_->BelongsToCurrentThread());
     if (handler_) {
+      handler_->OnSessionDescriptionsUpdated(
+          std::move(pending_local_description),
+          std::move(current_local_description),
+          std::move(pending_remote_description),
+          std::move(current_remote_description));
       handler_->OnIceCandidate(sdp, sdp_mid, sdp_mline_index, component,
                                address_family);
     }
@@ -1002,6 +1046,9 @@ class RTCPeerConnectionHandler::Observer
  private:
   const base::WeakPtr<RTCPeerConnectionHandler> handler_;
   const scoped_refptr<base::SingleThreadTaskRunner> main_thread_;
+  // A copy of |handler_->native_peer_connection_| for use on the WebRTC
+  // signaling thread.
+  scoped_refptr<webrtc::PeerConnectionInterface> native_peer_connection_;
 };
 
 RTCPeerConnectionHandler::RTCPeerConnectionHandler(
@@ -1094,11 +1141,11 @@ bool RTCPeerConnectionHandler::Initialize(
       MakeGarbageCollected<Observer>(weak_factory_.GetWeakPtr(), task_runner_);
   native_peer_connection_ = dependency_factory_->CreatePeerConnection(
       configuration_, frame_, peer_connection_observer_);
-
   if (!native_peer_connection_.get()) {
     LOG(ERROR) << "Failed to initialize native PeerConnection.";
     return false;
   }
+  peer_connection_observer_->Initialize();
 
   if (peer_connection_tracker_) {
     peer_connection_tracker_->RegisterPeerConnection(this, configuration_,
@@ -1130,6 +1177,7 @@ bool RTCPeerConnectionHandler::InitializeForTest(
     LOG(ERROR) << "Failed to initialize native PeerConnection.";
     return false;
   }
+  peer_connection_observer_->Initialize();
   peer_connection_tracker_ = peer_connection_tracker;
   return true;
 }

third_party/blink/web_tests/external/wpt/webrtc/RTCPeerConnection-candidate-in-sdp.https.html

+<!doctype html>
+<meta charset=utf-8>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+'use strict';
+
+promise_test(async t => {
+  const pc = new RTCPeerConnection();
+  t.add_cleanup(() => pc.close());
+  let resolveIceCandidatePromise = null;
+  const iceCandidatePromise = new Promise(r => resolveIceCandidatePromise = r);
+  pc.onicecandidate = e => {
+    resolveIceCandidatePromise(pc.localDescription.sdp);
+    pc.onicecandidate = null;
+  }
+  pc.addTransceiver("audio");
+  await pc.setLocalDescription(await pc.createOffer());
+  assert_false(pc.localDescription.sdp.includes("a=candidate:"),
+               "localDescription is missing candidate before onicecandidate");
+  // The localDescription at the time of the onicecandidate event.
+  const localDescriptionSdp = await iceCandidatePromise;
+  assert_true(localDescriptionSdp.includes("a=candidate:"),
+               "localDescription contains candidate after onicecandidate");
+}, 'localDescription contains candidates');
+</script>

third_party/blink/web_tests/virtual/webrtc-wpt-plan-b/external/wpt/webrtc/RTCPeerConnection-candidate-in-sdp.https-expected.txt

+This is a testharness.js-based test.
+FAIL localDescription contains candidates promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'."
+Harness: the test ran to completion.
+