Return proxied object in OnTransceiver

This makes it possible to invoke methods on the transceiver object
from any thread.

Also makes a few of the mock observer objects thread-safe, to allow
testing when the main thread is not the signaling thread.

Bug: webrtc:13183
Change-Id: Ic97efef71a21c3075700a028103061032f8d2bcc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232120
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35010}

pc/BUILD.gn

@@ -1053,6 +1053,7 @@ if (rtc_include_tests && !build_with_chromium) {
       "rtp_sender_receiver_unittest.cc",
       "rtp_transceiver_unittest.cc",
       "sctp_utils_unittest.cc",
+      "sdp_offer_answer_unittest.cc",
       "sdp_serializer_unittest.cc",
       "stats_collector_unittest.cc",
       "test/fake_audio_capture_module_unittest.cc",

pc/sdp_offer_answer.cc

@@ -1725,7 +1725,9 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription(
           RTC_LOG(LS_INFO)
               << "Processing the addition of a remote track for MID="
               << content->name << ".";
-          now_receiving_transceivers.push_back(transceiver);
+          // Since the transceiver is passed to the user in an
+          // OnTrack event, we must use the proxied transceiver.
+          now_receiving_transceivers.push_back(transceiver_ext);
         }
       }
       // 2.2.8.1.9: If direction is "sendonly" or "inactive", and transceiver's

pc/sdp_offer_answer_unittest.cc

@@ -0,0 +1,140 @@
+/*
+ *  Copyright 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <stdint.h>
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "absl/types/optional.h"
+#include "api/audio/audio_mixer.h"
+#include "api/audio_codecs/audio_decoder_factory.h"
+#include "api/audio_codecs/audio_encoder_factory.h"
+#include "api/audio_codecs/builtin_audio_decoder_factory.h"
+#include "api/audio_codecs/builtin_audio_encoder_factory.h"
+#include "api/create_peerconnection_factory.h"
+#include "api/jsep.h"
+#include "api/media_stream_interface.h"
+#include "api/media_types.h"
+#include "api/peer_connection_interface.h"
+#include "api/rtc_error.h"
+#include "api/rtp_parameters.h"
+#include "api/rtp_receiver_interface.h"
+#include "api/rtp_sender_interface.h"
+#include "api/rtp_transceiver_interface.h"
+#include "api/scoped_refptr.h"
+#include "api/set_remote_description_observer_interface.h"
+#include "api/uma_metrics.h"
+#include "api/video_codecs/builtin_video_decoder_factory.h"
+#include "api/video_codecs/builtin_video_encoder_factory.h"
+#include "api/video_codecs/video_decoder_factory.h"
+#include "api/video_codecs/video_encoder_factory.h"
+#include "media/base/stream_params.h"
+#include "modules/audio_device/include/audio_device.h"
+#include "modules/audio_processing/include/audio_processing.h"
+#include "p2p/base/port_allocator.h"
+#include "pc/media_session.h"
+#include "pc/peer_connection_wrapper.h"
+#include "pc/sdp_utils.h"
+#include "pc/session_description.h"
+#include "pc/test/fake_audio_capture_module.h"
+#include "pc/test/mock_peer_connection_observers.h"
+#include "rtc_base/checks.h"
+#include "rtc_base/gunit.h"
+#include "rtc_base/ref_counted_object.h"
+#include "rtc_base/rtc_certificate_generator.h"
+#include "rtc_base/thread.h"
+#include "system_wrappers/include/metrics.h"
+#include "test/gmock.h"
+#include "test/gtest.h"
+
+// This file contains unit tests that relate to the behavior of the
+// SdpOfferAnswer module.
+// Tests are writen as integration tests with PeerConnection, since the
+// behaviors are still linked so closely that it is hard to test them in
+// isolation.
+
+namespace webrtc {
+
+using RTCConfiguration = PeerConnectionInterface::RTCConfiguration;
+
+namespace {
+
+std::unique_ptr<rtc::Thread> CreateAndStartThread() {
+  auto thread = rtc::Thread::Create();
+  thread->Start();
+  return thread;
+}
+
+}  // namespace
+
+class SdpOfferAnswerTest : public ::testing::Test {
+ public:
+  SdpOfferAnswerTest()
+      // Note: We use a PeerConnectionFactory with a distinct
+      // signaling thread, so that thread handling can be tested.
+      : signaling_thread_(CreateAndStartThread()),
+        pc_factory_(
+            CreatePeerConnectionFactory(nullptr,
+                                        nullptr,
+                                        signaling_thread_.get(),
+                                        FakeAudioCaptureModule::Create(),
+                                        CreateBuiltinAudioEncoderFactory(),
+                                        CreateBuiltinAudioDecoderFactory(),
+                                        CreateBuiltinVideoEncoderFactory(),
+                                        CreateBuiltinVideoDecoderFactory(),
+                                        nullptr /* audio_mixer */,
+                                        nullptr /* audio_processing */)) {
+    webrtc::metrics::Reset();
+  }
+
+  std::unique_ptr<PeerConnectionWrapper> CreatePeerConnection() {
+    RTCConfiguration config;
+    config.sdp_semantics = SdpSemantics::kUnifiedPlan;
+    return CreatePeerConnection(config);
+  }
+
+  std::unique_ptr<PeerConnectionWrapper> CreatePeerConnection(
+      const RTCConfiguration& config) {
+    auto observer = std::make_unique<MockPeerConnectionObserver>();
+    auto pc = pc_factory_->CreatePeerConnection(config, nullptr, nullptr,
+                                                observer.get());
+    EXPECT_TRUE(pc.get());
+    observer->SetPeerConnectionInterface(pc.get());
+    return std::make_unique<PeerConnectionWrapper>(pc_factory_, pc,
+                                                   std::move(observer));
+  }
+
+ protected:
+  std::unique_ptr<rtc::Thread> signaling_thread_;
+  rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory_;
+
+ private:
+};
+
+TEST_F(SdpOfferAnswerTest, OnTrackReturnsProxiedObject) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+
+  auto audio_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
+
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  // Verify that caller->observer->OnTrack() has been called with a
+  // proxied transceiver object.
+  ASSERT_EQ(callee->observer()->on_track_transceivers_.size(), 1u);
+  auto transceiver = callee->observer()->on_track_transceivers_[0];
+  // Since the signaling thread is not the current thread,
+  // this will DCHECK if the transceiver is not proxied.
+  transceiver->stopped();
+}
+
+}  // namespace webrtc

pc/test/mock_peer_connection_observers.h

@@ -259,25 +259,38 @@ class MockCreateSessionDescriptionObserver
         error_("MockCreateSessionDescriptionObserver not called") {}
   virtual ~MockCreateSessionDescriptionObserver() {}
   void OnSuccess(SessionDescriptionInterface* desc) override {
+    MutexLock lock(&mutex_);
     called_ = true;
     error_ = "";
     desc_.reset(desc);
   }
   void OnFailure(webrtc::RTCError error) override {
+    MutexLock lock(&mutex_);
     called_ = true;
     error_ = error.message();
   }
-  bool called() const { return called_; }
-  bool result() const { return error_.empty(); }
-  const std::string& error() const { return error_; }
+  bool called() const {
+    MutexLock lock(&mutex_);
+    return called_;
+  }
+  bool result() const {
+    MutexLock lock(&mutex_);
+    return error_.empty();
+  }
+  const std::string& error() const {
+    MutexLock lock(&mutex_);
+    return error_;
+  }
   std::unique_ptr<SessionDescriptionInterface> MoveDescription() {
+    MutexLock lock(&mutex_);
     return std::move(desc_);
   }
 
  private:
-  bool called_;
-  std::string error_;
-  std::unique_ptr<SessionDescriptionInterface> desc_;
+  mutable Mutex mutex_;
+  bool called_ RTC_GUARDED_BY(mutex_);
+  std::string error_ RTC_GUARDED_BY(mutex_);
+  std::unique_ptr<SessionDescriptionInterface> desc_ RTC_GUARDED_BY(mutex_);
 };
 
 class MockSetSessionDescriptionObserver
@@ -292,19 +305,32 @@ class MockSetSessionDescriptionObserver
         error_("MockSetSessionDescriptionObserver not called") {}
   ~MockSetSessionDescriptionObserver() override {}
   void OnSuccess() override {
+    MutexLock lock(&mutex_);
+
     called_ = true;
     error_ = "";
   }
   void OnFailure(webrtc::RTCError error) override {
+    MutexLock lock(&mutex_);
     called_ = true;
     error_ = error.message();
   }
 
-  bool called() const { return called_; }
-  bool result() const { return error_.empty(); }
-  const std::string& error() const { return error_; }
+  bool called() const {
+    MutexLock lock(&mutex_);
+    return called_;
+  }
+  bool result() const {
+    MutexLock lock(&mutex_);
+    return error_.empty();
+  }
+  const std::string& error() const {
+    MutexLock lock(&mutex_);
+    return error_;
+  }
 
  private:
+  mutable Mutex mutex_;
   bool called_;
   std::string error_;
 };