Description for changes from the original CL:
Calling legacy SRD, implemented using
SetRemoteDescriptionObserverAdapter wrapping the old observer, was
meant to have the exact same behavior as the legacy SRD implementation
which invokes the callbacks in a Post.
However, in the CL that landed and got reverted (PS1), the Adapter had
its own message handler, and callbacks would be invoked even if the PC
was destroyed.
In PS2 I've changed the Adapter to use the PeerConnection's message
handler. If the PC is destroyed, the callback will not be invoked.
This gives identical behavior to before this CL, and the legacy
behavior is covered by a new unittest.
I changed the adapter to be an implementation detail of
peerconnection.cc, therefor some stuff was moved, and the only tests
covering this is now in peerconnection_rtp_unittest.cc.
This is a reland of 6c7ec32bd63ab2b45d4d83ae1de817ee946b4d72
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=pthatcher@webrtc.org
Bug: webrtc:8473
Change-Id: If2b1a1929663b0e77fcc9c2ebeef043e6f73adf5
Reviewed-on: https://webrtc-review.googlesource.com/25640
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20854}
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}
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}
Enable cpplint check in the PRESUBMIT for pc/ and fix all existing
warnings.
Bug: webrtc:5583
Change-Id: If39994692ab6f6f3c83c74f23850f02fdfe810e8
Reviewed-on: https://webrtc-review.googlesource.com/16540
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20482}
This is a reland of b49b66109ea8a0a33a3192ebccf91366af2e49ae.
Original change's description:
> Rewrite WebRtcSession BUNDLE tests as PeerConnection tests
>
> Bug: webrtc:8222
> Change-Id: Id47e4544dc073564ad7e63d02865ca80dd5a85ff
> Reviewed-on: https://webrtc-review.googlesource.com/8280
> Commit-Queue: Steve Anton <steveanton@webrtc.org>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20365}
Bug: webrtc:8222
Change-Id: If3dcd8090875c641881e2b9e92fc1db387ba1de5
Reviewed-on: https://webrtc-review.googlesource.com/14400
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20397}
This is a reland of 3df5dcac9b339ba4d3f4969602f094c2c8035b51
Original change's description:
> Rewrite WebRtcSession media tests as PeerConnection tests
>
> Bug: webrtc:8222
> Change-Id: I782a3227e30de70eb8f6c26a48723cb3510a84ad
> Reviewed-on: https://webrtc-review.googlesource.com/6640
> Commit-Queue: Steve Anton <steveanton@webrtc.org>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20364}
Bug: webrtc:8222
Change-Id: I0a5398170d469eb9223bc781bfb417a85a72a2d2
Reviewed-on: https://webrtc-review.googlesource.com/14380
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20377}
This lays the groundwork for splitting up the
PeerConnectionInterface unit tests into multiple files so that
the tests can be organized better. The intent is for each unit
test file to declare a test fixture which subclasses
PeerConnectionUnitTestFixture and creates PeerConnectionWrappers
to write assertions against.
Bug: webrtc:8222
Change-Id: I21175b1e1828a6cd5012305a8a27faaf4eecf81c
Reviewed-on: https://webrtc-review.googlesource.com/1120
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20004}
