Revert "SetRemoteDescriptionObserverInterface added."
This reverts commit 6c7ec32bd63ab2b45d4d83ae1de817ee946b4d72.
Reason for revert: Third party project breaks due to use-after-free
in the callback. I suspect this is because the adapter is processing
the async callback instead of the pc, i.e. callback is called from
SetRemoteDescriptionObserverAdapter::OnMessage instead of from
PeerConnection::OnMessage. This makes it possible for the callback to
be invoked after the PC is destroyed.
I argue this is how it should be done, and that if you're using a raw
pointer in an async callback you're doing it wrong, but I will reland
this CL with the callback processed in PeerConnection::OnMessage
instead as to not change the behavior of the old SRD signature.
Original change's description:
> SetRemoteDescriptionObserverInterface added.
>
> The new observer replaced SetSessionDescriptionObserver for
> SetRemoteDescription. Unlike SetSessionDescriptionObserver,
> SetRemoteDescriptionObserverInterface is invoked synchronously so
> that the you can rely on the state of the PeerConnection to represent
> the result of the SetRemoteDescription call in the callback.
>
> The new observer succeeds or fails with an RTCError.
>
> This deprecates the need for PeerConnectionObserver::OnAdd/RemoveTrack
> and SetSessionDescriptionObserver, with the benefit that all media
> object changes can be processed in a single callback by the application
> in a synchronous callback. This will help Chromium keep objects in-sync
> across layers and threads in a non-racy and straight-forward way, see
> design doc (Proposal 2):
> https://docs.google.com/a/google.com/document/d/1-cDDC82mgU5zrHacfFz720p3xwRtuBkOPSRchh07Ho0/edit?usp=sharing
>
> An adapter for SetSessionDescriptionObserver is added to allow calling
> the old SetRemoteDescription signature and get the old behavior
> (OnSuccess/OnFailure callback in a Post) until third parties switch.
>
> Bug: webrtc:8473
> Change-Id: I3d4eb60da6dd34615f2c9f384aeaf4634e648c99
> Reviewed-on: https://webrtc-review.googlesource.com/17523
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
> Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20841}
TBR=hbos@webrtc.org,hta@webrtc.org,pthatcher@webrtc.org,guidou@webrtc.org
Change-Id: I715555e084d9aae49ee2a8831c70dc006dbdb74c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8473
Reviewed-on: https://webrtc-review.googlesource.com/25580
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20850}diff --git a/api/BUILD.gn b/api/BUILD.gn
index 278ab15..03460f6 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -63,8 +63,6 @@
"rtpreceiverinterface.h",
"rtpsenderinterface.h",
"rtptransceiverinterface.h",
- "setremotedescriptionobserverinterface.cc",
- "setremotedescriptionobserverinterface.h",
"statstypes.cc",
"statstypes.h",
"turncustomizer.h",
@@ -381,7 +379,6 @@
deps = [
":array_view",
":libjingle_peerconnection_api",
- ":libjingle_peerconnection_test_api",
":optional",
":ortc_api",
"../rtc_base:rtc_base_approved",
diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
index 0cff5df..a682459 100644
--- a/api/peerconnectioninterface.h
+++ b/api/peerconnectioninterface.h
@@ -82,7 +82,6 @@
#include "api/rtceventlogoutput.h"
#include "api/rtpreceiverinterface.h"
#include "api/rtpsenderinterface.h"
-#include "api/setremotedescriptionobserverinterface.h"
#include "api/stats/rtcstatscollectorcallback.h"
#include "api/statstypes.h"
#include "api/turncustomizer.h"
@@ -727,18 +726,8 @@
// Sets the remote session description.
// The PeerConnection takes the ownership of |desc| even if it fails.
// The |observer| callback will be called when done.
- // TODO(hbos): Remove when Chrome implements the new signature.
virtual void SetRemoteDescription(SetSessionDescriptionObserver* observer,
- SessionDescriptionInterface* desc) {
- SetRemoteDescription(
- std::unique_ptr<SessionDescriptionInterface>(desc),
- rtc::scoped_refptr<SetRemoteDescriptionObserverInterface>(
- new SetRemoteDescriptionObserverAdapter(observer)));
- }
- // TODO(hbos): Make pure virtual when Chrome has updated its signature.
- virtual void SetRemoteDescription(
- std::unique_ptr<SessionDescriptionInterface> desc,
- rtc::scoped_refptr<SetRemoteDescriptionObserverInterface> observer) {}
+ SessionDescriptionInterface* desc) = 0;
// Deprecated; Replaced by SetConfiguration.
// TODO(deadbeef): Remove once Chrome is moved over to SetConfiguration.
virtual bool UpdateIce(const IceServers& configuration,
diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h
index bdd9fcb..36ee50f 100644
--- a/api/peerconnectionproxy.h
+++ b/api/peerconnectionproxy.h
@@ -88,10 +88,6 @@
SetRemoteDescription,
SetSessionDescriptionObserver*,
SessionDescriptionInterface*)
- PROXY_METHOD2(void,
- SetRemoteDescription,
- std::unique_ptr<SessionDescriptionInterface>,
- rtc::scoped_refptr<SetRemoteDescriptionObserverInterface>);
PROXY_METHOD0(PeerConnectionInterface::RTCConfiguration, GetConfiguration);
PROXY_METHOD2(bool,
SetConfiguration,
diff --git a/api/setremotedescriptionobserverinterface.cc b/api/setremotedescriptionobserverinterface.cc
deleted file mode 100644
index 67006a3..0000000
--- a/api/setremotedescriptionobserverinterface.cc
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * 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 "api/setremotedescriptionobserverinterface.h"
-
-#include <string>
-
-namespace webrtc {
-
-// The message keeps the observer alive through reference counting.
-class SetRemoteDescriptionObserverAdapter::MessageData
- : public rtc::MessageData {
- public:
- static MessageData* Create(
- rtc::scoped_refptr<SetRemoteDescriptionObserverAdapter> observer,
- RTCError error) {
- return new MessageData(std::move(observer), std::move(error));
- }
-
- const RTCError& error() const { return error_; }
-
- private:
- MessageData(rtc::scoped_refptr<SetRemoteDescriptionObserverAdapter> observer,
- RTCError error)
- : observer_(std::move(observer)), error_(std::move(error)) {}
-
- rtc::scoped_refptr<SetRemoteDescriptionObserverAdapter> observer_;
- RTCError error_;
-};
-
-SetRemoteDescriptionObserverAdapter::SetRemoteDescriptionObserverAdapter(
- rtc::scoped_refptr<SetSessionDescriptionObserver> wrapper)
- : wrapper_(std::move(wrapper)) {}
-
-void SetRemoteDescriptionObserverAdapter::OnSetRemoteDescriptionComplete(
- RTCError error) {
- // MessageData keeps a reference to |this|, ensuring it is not deleted until
- // OnMessage().
- rtc::Thread::Current()->Post(RTC_FROM_HERE, this, 0,
- MessageData::Create(this, std::move(error)));
-}
-
-void SetRemoteDescriptionObserverAdapter::OnMessage(rtc::Message* msg) {
- MessageData* message = static_cast<MessageData*>(msg->pdata);
- if (message->error().ok())
- wrapper_->OnSuccess();
- else
- wrapper_->OnFailure(message->error().message());
- // Delete the message data, this may delete |this|. Don't use member variables
- // after this line.
- delete message;
-}
-
-} // namespace webrtc
diff --git a/api/setremotedescriptionobserverinterface.h b/api/setremotedescriptionobserverinterface.h
deleted file mode 100644
index f1b65a1..0000000
--- a/api/setremotedescriptionobserverinterface.h
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * 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.
- */
-
-#ifndef API_SETREMOTEDESCRIPTIONOBSERVERINTERFACE_H_
-#define API_SETREMOTEDESCRIPTIONOBSERVERINTERFACE_H_
-
-#include <utility>
-#include <vector>
-
-#include "api/jsep.h"
-#include "api/mediastreaminterface.h"
-#include "api/rtcerror.h"
-#include "api/rtpreceiverinterface.h"
-#include "rtc_base/bind.h"
-#include "rtc_base/messagehandler.h"
-#include "rtc_base/refcount.h"
-#include "rtc_base/refcountedobject.h"
-#include "rtc_base/scoped_ref_ptr.h"
-#include "rtc_base/thread.h"
-
-namespace webrtc {
-
-// An observer for PeerConnectionInterface::SetRemoteDescription(). The
-// callback is invoked such that the state of the peer connection can be
-// examined to accurately reflect the effects of the SetRemoteDescription
-// operation.
-class SetRemoteDescriptionObserverInterface : public rtc::RefCountInterface {
- public:
- // On success, |error.ok()| is true.
- virtual void OnSetRemoteDescriptionComplete(RTCError error) = 0;
-};
-
-// Upon completion, posts a task to execute the callback of the
-// SetSessionDescriptionObserver asynchronously on the same thread. At this
-// point, the state of the peer connection might no longer reflect the effects
-// of the SetRemoteDescription operation, as the peer connection could have been
-// modified during the post.
-// TODO(hbos): Remove this class once we remove the version of
-// PeerConnectionInterface::SetRemoteDescription() that takes a
-// SetSessionDescriptionObserver as an argument.
-class SetRemoteDescriptionObserverAdapter
- : public rtc::RefCountedObject<SetRemoteDescriptionObserverInterface>,
- public rtc::MessageHandler {
- public:
- SetRemoteDescriptionObserverAdapter(
- rtc::scoped_refptr<SetSessionDescriptionObserver> wrapper);
-
- // SetRemoteDescriptionObserverInterface implementation.
- void OnSetRemoteDescriptionComplete(RTCError error) override;
-
- // rtc::MessageHandler implementation.
- void OnMessage(rtc::Message* msg) override;
-
- private:
- class MessageData;
-
- rtc::scoped_refptr<SetSessionDescriptionObserver> wrapper_;
-};
-
-} // namespace webrtc
-
-#endif // API_SETREMOTEDESCRIPTIONOBSERVERINTERFACE_H_