From 695cf6ac42a5f9252409865e010afd00dbeb4ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Mon, 13 May 2019 12:27:23 +0200 Subject: [PATCH] Delete deprecated StartRtcEventLog override with PlatformFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:6463 Change-Id: I57c2372a232d72b054d8e3e4f423e11b3fb22430 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134460 Reviewed-by: Karl Wiberg Reviewed-by: Björn Terelius Reviewed-by: Elad Alon Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#28131} --- api/peer_connection_interface.cc | 5 ---- api/peer_connection_interface.h | 8 ------- api/peer_connection_proxy.h | 1 - api/test/mock_peerconnectioninterface.h | 1 - pc/peer_connection.cc | 23 ++++-------------- pc/peer_connection.h | 2 -- pc/peer_connection_interface_unittest.cc | 24 ------------------- pc/test/fake_peer_connection_base.h | 5 ---- sdk/BUILD.gn | 1 + sdk/android/BUILD.gn | 1 + sdk/android/src/jni/pc/peer_connection.cc | 10 ++++++-- .../api/peerconnection/RTCPeerConnection.mm | 10 ++++++-- 12 files changed, 23 insertions(+), 68 deletions(-) diff --git a/api/peer_connection_interface.cc b/api/peer_connection_interface.cc index 3a96ccf3c7..588a021dc7 100644 --- a/api/peer_connection_interface.cc +++ b/api/peer_connection_interface.cc @@ -171,11 +171,6 @@ PeerConnectionInterface::peer_connection_state() { return PeerConnectionInterface::PeerConnectionState::kFailed; } -bool PeerConnectionInterface::StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) { - return false; -} - bool PeerConnectionInterface::StartRtcEventLog( std::unique_ptr output, int64_t output_period_ms) { diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index bdada31343..1994505242 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1076,14 +1076,6 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { virtual IceGatheringState ice_gathering_state() = 0; - // Starts RtcEventLog using existing file. Takes ownership of |file| and - // passes it on to Call, which will take the ownership. If the - // operation fails the file will be closed. - // The logging will stop when |max_size_bytes| is reached or when the - // StopRtcEventLog function is called. - // TODO(eladalon): Deprecate and remove this. - virtual bool StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes); - // Start RtcEventLog using an existing output-sink. Takes ownership of // |output| and passes it on to Call, which will take the ownership. If the // operation fails the output will be closed and deallocated. The event log diff --git a/api/peer_connection_proxy.h b/api/peer_connection_proxy.h index 61058b5a47..7db41d4bfc 100644 --- a/api/peer_connection_proxy.h +++ b/api/peer_connection_proxy.h @@ -131,7 +131,6 @@ PROXY_METHOD0(IceConnectionState, ice_connection_state) PROXY_METHOD0(IceConnectionState, standardized_ice_connection_state) PROXY_METHOD0(PeerConnectionState, peer_connection_state) PROXY_METHOD0(IceGatheringState, ice_gathering_state) -PROXY_METHOD2(bool, StartRtcEventLog, rtc::PlatformFile, int64_t) PROXY_METHOD2(bool, StartRtcEventLog, std::unique_ptr, diff --git a/api/test/mock_peerconnectioninterface.h b/api/test/mock_peerconnectioninterface.h index d4450069c5..fa132b4aaa 100644 --- a/api/test/mock_peerconnectioninterface.h +++ b/api/test/mock_peerconnectioninterface.h @@ -124,7 +124,6 @@ class MockPeerConnectionInterface MOCK_METHOD0(signaling_state, SignalingState()); MOCK_METHOD0(ice_connection_state, IceConnectionState()); MOCK_METHOD0(ice_gathering_state, IceGatheringState()); - MOCK_METHOD2(StartRtcEventLog, bool(rtc::PlatformFile, int64_t)); MOCK_METHOD2(StartRtcEventLog, bool(std::unique_ptr, int64_t)); MOCK_METHOD0(StopRtcEventLog, void()); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index d0698d8c65..dcb6c8a67a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -54,7 +54,6 @@ #include "rtc_base/bind.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" -#include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/string_encode.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/trace_event.h" @@ -3756,21 +3755,6 @@ PeerConnection::GetFirstAudioTransceiver() const { return nullptr; } -bool PeerConnection::StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) { - // TODO(eladalon): It would be better to not allow negative values into PC. - const size_t max_size = (max_size_bytes < 0) - ? RtcEventLog::kUnlimitedOutput - : rtc::saturated_cast(max_size_bytes); - int64_t output_period_ms = webrtc::RtcEventLog::kImmediateOutput; - if (field_trial::IsEnabled("WebRTC-RtcEventLogNewFormat")) { - output_period_ms = 5000; - } - return StartRtcEventLog( - absl::make_unique(file, max_size), - output_period_ms); -} - bool PeerConnection::StartRtcEventLog(std::unique_ptr output, int64_t output_period_ms) { // TODO(eladalon): In C++14, this can be done with a lambda. @@ -3788,8 +3772,11 @@ bool PeerConnection::StartRtcEventLog(std::unique_ptr output, bool PeerConnection::StartRtcEventLog( std::unique_ptr output) { - return StartRtcEventLog(std::move(output), - webrtc::RtcEventLog::kImmediateOutput); + int64_t output_period_ms = webrtc::RtcEventLog::kImmediateOutput; + if (field_trial::IsEnabled("WebRTC-RtcEventLogNewFormat")) { + output_period_ms = 5000; + } + return StartRtcEventLog(std::move(output), output_period_ms); } void PeerConnection::StopRtcEventLog() { diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 7287b7c150..d2fb768a33 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -209,8 +209,6 @@ class PeerConnection : public PeerConnectionInternal, rtc::scoped_refptr GetSctpTransport() const override; - RTC_DEPRECATED bool StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) override; bool StartRtcEventLog(std::unique_ptr output, int64_t output_period_ms) override; bool StartRtcEventLog(std::unique_ptr output) override; diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 9bf33066c4..4549912225 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -3443,30 +3443,6 @@ TEST_P(PeerConnectionInterfaceTest, CurrentAndPendingDescriptions) { EXPECT_EQ(local_answer_ptr, pc_->current_local_description()); } -// Tests that it won't crash when calling StartRtcEventLog or StopRtcEventLog -// after the PeerConnection is closed. -// This version tests the StartRtcEventLog version that receives a file. -TEST_P(PeerConnectionInterfaceTest, - StartAndStopLoggingToFileAfterPeerConnectionClosed) { - CreatePeerConnection(); - // The RtcEventLog will be reset when the PeerConnection is closed. - pc_->Close(); - - auto test_info = ::testing::UnitTest::GetInstance()->current_test_info(); - std::string filename = webrtc::test::OutputPath() + - test_info->test_case_name() + test_info->name(); - rtc::PlatformFile file = rtc::CreatePlatformFile(filename); - - constexpr int64_t max_size_bytes = 1024; - - EXPECT_FALSE(pc_->StartRtcEventLog(file, max_size_bytes)); - pc_->StopRtcEventLog(); - - // Cleanup. - rtc::ClosePlatformFile(file); - rtc::RemoveFile(filename); -} - // Tests that it won't crash when calling StartRtcEventLog or StopRtcEventLog // after the PeerConnection is closed. // This version tests the StartRtcEventLog version that receives an object diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index f67b305e89..67890cbcce 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -209,11 +209,6 @@ class FakePeerConnectionBase : public PeerConnectionInternal { return IceGatheringState::kIceGatheringNew; } - bool StartRtcEventLog(rtc::PlatformFile file, - int64_t max_size_bytes) override { - return false; - } - bool StartRtcEventLog(std::unique_ptr output, int64_t output_period_ms) override { return false; diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index 784101eb91..d3892988b9 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -907,6 +907,7 @@ if (is_ios || is_mac) { ":videotoolbox_objc", "../api:create_peerconnection_factory", "../api:libjingle_peerconnection_api", + "../api:rtc_event_log_output_file", "../api:rtc_stats_api", "../api:scoped_refptr", "../api/audio_codecs:audio_codecs_api", diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index 722d0b90a0..69f59d9164 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -777,6 +777,7 @@ if (is_android) { ":native_api_stacktrace", "..:media_constraints", "../../api:libjingle_peerconnection_api", + "../../api:rtc_event_log_output_file", "../../api/video_codecs:video_codecs_api", "../../logging:rtc_event_log_api", "../../logging:rtc_event_log_impl_base", diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc index bd50f5b121..98c3dd01af 100644 --- a/sdk/android/src/jni/pc/peer_connection.cc +++ b/sdk/android/src/jni/pc/peer_connection.cc @@ -34,11 +34,13 @@ #include "absl/memory/memory.h" #include "api/peer_connection_interface.h" +#include "api/rtc_event_log_output_file.h" #include "api/rtp_receiver_interface.h" #include "api/rtp_sender_interface.h" #include "api/rtp_transceiver_interface.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/numerics/safe_conversions.h" #include "sdk/android/generated_peerconnection_jni/jni/PeerConnection_jni.h" #include "sdk/android/native_api/jni/java_types.h" #include "sdk/android/src/jni/jni_helpers.h" @@ -741,8 +743,12 @@ static jboolean JNI_PeerConnection_StartRtcEventLog( const JavaParamRef& j_pc, int file_descriptor, int max_size_bytes) { - return ExtractNativePC(jni, j_pc)->StartRtcEventLog(file_descriptor, - max_size_bytes); + // TODO(eladalon): It would be better to not allow negative values into PC. + const size_t max_size = (max_size_bytes < 0) + ? RtcEventLog::kUnlimitedOutput + : rtc::saturated_cast(max_size_bytes); + return ExtractNativePC(jni, j_pc)->StartRtcEventLog( + absl::make_unique(file_descriptor, max_size)); } static void JNI_PeerConnection_StopRtcEventLog( diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm index 8189aecd34..bed85f9afe 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm +++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm @@ -30,7 +30,9 @@ #include "api/jsep_ice_candidate.h" #include "api/media_transport_interface.h" +#include "api/rtc_event_log_output_file.h" #include "rtc_base/checks.h" +#include "rtc_base/numerics/safe_conversions.h" NSString * const kRTCPeerConnectionErrorDomain = @"org.webrtc.RTCPeerConnection"; @@ -540,8 +542,12 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack( RTCLogError(@"Error opening file: %@. Error: %d", filePath, errno); return NO; } - _hasStartedRtcEventLog = - _peerConnection->StartRtcEventLog(fd, maxSizeInBytes); + // TODO(eladalon): It would be better to not allow negative values into PC. + const size_t max_size = (maxSizeInBytes < 0) ? webrtc::RtcEventLog::kUnlimitedOutput : + rtc::saturated_cast(maxSizeInBytes); + + _hasStartedRtcEventLog = _peerConnection->StartRtcEventLog( + absl::make_unique(fd, max_size)); return _hasStartedRtcEventLog; }