RTCPeerConnectionHandlerTest: Don't use mock peer connection |
|
Issue descriptionIn WebRTC, mocking has proven to be a case of where we end up not testing the real thing and it's a significant maintenance burden, where you often have to implement complex behavior to get the mock to act like the real thing would. It rarely adds value. It often adds burden. When assumptions change, the tests relying on mocks fail even though the real thing works, and updating the mocks requires lots of code. Recently, SetRemoteDescription-related tests failed because how callbacks are handled changed. In order to get the mock to pass the tests, they would have to implement updating all the states that a real peer connection would update as a result of the call. This is getting closer and closer to implementing the real thing. We should just use the real thing. If every layer tests itself using real implementations of layers below, we'll find out where something breaks when it does. If the layer below breaks it will get rolled back anyway, there is no value to having fake versions of the lower layers. TODO: In rtc_peer_connection_handler_unittest.cc - Update to use a real PeerConnectionInterface implementation. - Remove mock_peer_connection_impl.h. - Remove tests that don't add value. - Enable or remove disable tests.
,
Jul 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505 commit ae070c77aae47dabe7bd1cf2ee14e68f53f5f505 Author: Henrik Boström <hbos@chromium.org> Date: Thu Jul 05 08:39:39 2018 RtpReceiverState and Chrome to stop relying on WebRTC MediaStreams. RtpReceiverState allows surfacing states between third_party/webrtc on the signaling thread and conten/blink on the main thread in a separate helper class that will enable doing this more easily in different code paths, both synchronous and asynchronous ones. It unblocks transceiver work and will make onion souping away the content receiver layer easy. See RtpSenderState for more information, this is the same thing but for receivers: https://chromium-review.googlesource.com/c/chromium/src/+/1118558 Additionally, this CL removes our dependency on third_party/webrtc's webrtc::MediaStreamInterface, isolating stream creation and management to the blink layer. This simplifies things greatly, and will allow us to remove the WebRtcMediaStreamAdapter[Map] in follow-up CLs and makes the updating of receiver's streams much easier and straight-forward. No more logic spread out over different callbacks with regards to streams. MediaStream::[Add/Remove]RemoteTrack() added, allowing adding/removing tracks using blink-layer tracks as argument rather than web components, allowing us to move away from this old web wiring. When a track is added/removed as "remote" it means it was not added by the addTrack()/removeTrack() JavaScript APIs, so associated events (e.g. "onaddtrack", "onremovetrack") should fire on the stream. This CL also does misc drive-by changes, like renaming Did[Add/Remove]RemoteTrack to the more accurately named Did[Add/Remove]Receiver, and removing some already-disabled RTCPeerConnectionHandler unittests that we don't want to maintain (https://crbug.com/788659). Bug: 777617 , 810708, 788659 Change-Id: I5c187c6346472aaecf6bc6fe4710e415b2a44902 Reviewed-on: https://chromium-review.googlesource.com/1122216 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#572738} [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/mock_web_rtc_peer_connection_handler_client.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/mock_web_rtc_peer_connection_handler_client.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/peer_connection_tracker.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/peer_connection_tracker.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_peer_connection_handler.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_peer_connection_handler.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_peer_connection_handler_unittest.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_rtp_receiver.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_rtp_receiver.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/rtc_rtp_receiver_unittest.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/webrtc_set_remote_description_observer.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/webrtc_set_remote_description_observer.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/content/renderer/media/webrtc/webrtc_set_remote_description_observer_unittest.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/public/platform/web_rtc_peer_connection_handler_client.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/public/platform/web_rtc_rtp_receiver.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/mediastream/media_stream.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/mediastream/media_stream.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.h [modify] https://crrev.com/ae070c77aae47dabe7bd1cf2ee14e68f53f5f505/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc |
|
►
Sign in to add a comment |
|
Comment 1 by hta@chromium.org
, Feb 9 2018