The WebRTCPeerConnectionHandler is the main glue between blink and webrtc.
https://cs.chromium.org/chromium/src/third_party/WebKit/public/platform/WebRTCPeerConnectionHandler.h
It has the real implementation: RTCPeerConnectionHandler.
https://cs.chromium.org/chromium/src/content/renderer/media/rtc_peer_connection_handler.h
And the mock implementation: MockRTCPeerConnectionHandler.
https://cs.chromium.org/chromium/src/content/shell/test_runner/mock_webrtc_peer_connection_handler.h
The majority of the .js tests are the third_party/WebKit/LayoutTests/ tests which uses the mock handler. Problems:
- This does not test actual webrtc yet the majority of our tests rely on this mock.
- Many of our tests are dummy tests, just making sure the wrong argument throw exceptions and the right arguments doesn't (assuming the right thing happens if you do). Ideally though, arguments should be accepted or rejected by the webrtc implementation. With this we either or both have to 1) verify arguments in the blink layer and have to make sure it matches (duplicates) how the webrtc layer verifies arguments, and 2) verify arguments by the mock handler, again duplicating argument verification that must match the webrtc impl.
- Many things cannot be tested with the mock. For example, we can't cannot set up a real call or rely on any complex behavior/sequence of method calls.
- Sometimes the handler need to perform logic that gets duplicated in the real and mock handlers.
- The mock requires mock implementation of other classes, increasing the size of our code base. For good test coverage we have to write multiple tests for the same thing; LayoutTests and browser_tests: e.g. https://cs.chromium.org/chromium/src/chrome/browser/media/webrtc/webrtc_browsertest.cc. While the size mostly have to do with the amount of layers this is a component of the size and complexity, a blink CL surfacing trivial webrtc functionality can easily reach over 1000 lines of code and touching over 35 files.
With the recent auto-up- and downstream of LayoutTests/external/wpt, https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md#Writing-tests, it would seem even more important that LayoutTests test actual webrtc code. These tests should run by all browsers and they should verify that THE REAL THING works. We can't have LayoutTests relying on behavior that is work-around for using a mock (such as making remote streams match local streams when we can't set up a real call). We are currently limited in what we can test with LayoutTests/external/wpt.
Ideally all of our tests would be LayoutTests/external/wpt tests so that we don't have to have the same tests twice (or three times, browser_tests)?
I propose we one of the following:
1. Remove MockRTCPeerConnectionHandler, always use real WebRTC.
2. Use real by default, but have tests that want to use mock explicitly ask for the mock to be used before constructing the PC.
3. Make LayoutTests/external/wpt use the real handler and other tests use the mock.
Thoughts, people?
Comment 1 by foolip@chromium.org
, Apr 24 2017