Skip to content

Commit

Permalink
Switch blink::PeerConnectionTracker away from std::string
Browse files Browse the repository at this point in the history
This CL converts PeerConnectionTracker away from string,
to use WTF::String.

Note that there is still left over std::string when it is
needed to interface with libwebrtc.

Other classes got minimally changed to adapt to the changes
needed for PeerConnectionTracker, but will have their own conversion
round next.

BUG=787254
R=guidou@chromium.org, haraken@chromium.org

Change-Id: I94c05b7d7830bba16e05a3fb7b2bfb8c063ab61b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1880961
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711892}
  • Loading branch information
tonikitoo authored and Commit Bot committed Nov 1, 2019
1 parent 1ce60a4 commit 1672223
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -822,15 +822,14 @@ void PeerConnectionTracker::TrackCreateAnswer(

void PeerConnectionTracker::TrackSetSessionDescription(
RTCPeerConnectionHandler* pc_handler,
const std::string& sdp,
const std::string& type,
const String& sdp,
const String& type,
Source source) {
DCHECK_CALLED_ON_VALID_THREAD(main_thread_);
int id = GetLocalIDForHandler(pc_handler);
if (id == -1)
return;
String value =
"type: " + String::FromUTF8(type) + ", sdp: " + String::FromUTF8(sdp);
String value = "type: " + type + ", sdp: " + sdp;
SendPeerConnectionUpdate(
id,
source == SOURCE_LOCAL ? "setLocalDescription" : "setRemoteDescription",
Expand Down Expand Up @@ -1036,8 +1035,8 @@ void PeerConnectionTracker::TrackIceGatheringStateChange(
void PeerConnectionTracker::TrackSessionDescriptionCallback(
RTCPeerConnectionHandler* pc_handler,
Action action,
const std::string& callback_type,
const std::string& value) {
const String& callback_type,
const String& value) {
DCHECK_CALLED_ON_VALID_THREAD(main_thread_);
int id = GetLocalIDForHandler(pc_handler);
if (id == -1)
Expand All @@ -1063,27 +1062,25 @@ void PeerConnectionTracker::TrackSessionDescriptionCallback(
NOTREACHED();
break;
}
update_type = update_type + String::FromUTF8(callback_type);
update_type = update_type + callback_type;

SendPeerConnectionUpdate(id, update_type, String::FromUTF8(value));
SendPeerConnectionUpdate(id, update_type, value);
}

void PeerConnectionTracker::TrackSessionId(RTCPeerConnectionHandler* pc_handler,
const std::string& session_id) {
const String& session_id) {
DCHECK_CALLED_ON_VALID_THREAD(main_thread_);
DCHECK(pc_handler);
DCHECK(!session_id.empty());
DCHECK(!session_id.IsEmpty());
const int local_id = GetLocalIDForHandler(pc_handler);
if (local_id == -1) {
return;
}

String wtf_session_id = String::FromUTF8(session_id);
if (wtf_session_id.IsNull())
wtf_session_id = String("");

peer_connection_tracker_host_->OnPeerConnectionSessionIdSet(local_id,
wtf_session_id);
String non_null_session_id =
session_id.IsNull() ? WTF::g_empty_string : session_id;
peer_connection_tracker_host_->OnPeerConnectionSessionIdSet(
local_id, non_null_session_id);
}

void PeerConnectionTracker::TrackOnRenegotiationNeeded(
Expand All @@ -1108,17 +1105,14 @@ void PeerConnectionTracker::TrackGetUserMedia(

void PeerConnectionTracker::TrackRtcEventLogWrite(
RTCPeerConnectionHandler* pc_handler,
const std::string& output) {
const String& output) {
DCHECK_CALLED_ON_VALID_THREAD(main_thread_);
int id = GetLocalIDForHandler(pc_handler);
if (id == -1)
return;

String wtf_output = String::FromUTF8(output);
if (wtf_output.IsNull())
wtf_output = String("");

peer_connection_tracker_host_->WebRtcEventLogWrite(id, wtf_output);
String non_null_output = output.IsNull() ? WTF::g_empty_string : output;
peer_connection_tracker_host_->WebRtcEventLogWrite(id, non_null_output);
}

int PeerConnectionTracker::GetNextLocalID() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ class MODULES_EXPORT PeerConnectionTracker

// Sends an update when setLocalDescription or setRemoteDescription is called.
virtual void TrackSetSessionDescription(RTCPeerConnectionHandler* pc_handler,
const std::string& sdp,
const std::string& type,
const String& sdp,
const String& type,
Source source);
virtual void TrackSetSessionDescriptionImplicit(
RTCPeerConnectionHandler* pc_handler);
Expand Down Expand Up @@ -203,12 +203,12 @@ class MODULES_EXPORT PeerConnectionTracker
virtual void TrackSessionDescriptionCallback(
RTCPeerConnectionHandler* pc_handler,
Action action,
const std::string& type,
const std::string& value);
const String& type,
const String& value);

// Sends an update when the session description's ID is set.
virtual void TrackSessionId(RTCPeerConnectionHandler* pc_handler,
const std::string& session_id);
const String& session_id);

// Sends an update when onRenegotiationNeeded is called.
virtual void TrackOnRenegotiationNeeded(RTCPeerConnectionHandler* pc_handler);
Expand All @@ -219,7 +219,7 @@ class MODULES_EXPORT PeerConnectionTracker

// Sends a new fragment on an RtcEventLog.
virtual void TrackRtcEventLogWrite(RTCPeerConnectionHandler* pc_handler,
const std::string& output);
const String& output);

private:
FRIEND_TEST_ALL_PREFIXES(PeerConnectionTrackerTest, OnSuspend);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#include "third_party/blink/renderer/modules/peerconnection/webrtc_set_description_observer.h"
#include "third_party/blink/renderer/platform/peerconnection/rtc_event_log_output_sink.h"
#include "third_party/blink/renderer/platform/peerconnection/rtc_event_log_output_sink_proxy.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
#include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
#include "third_party/webrtc/api/rtc_event_log_output.h"
#include "third_party/webrtc/pc/media_session.h"
Expand Down Expand Up @@ -245,9 +246,10 @@ class CreateSessionDescriptionRequest
desc->ToString(&value);
value = "type: " + desc->type() + ", sdp: " + value;
}
tracker_->TrackSessionDescriptionCallback(handler_.get(), action_,
"OnSuccess", value);
tracker_->TrackSessionId(handler_.get(), desc->session_id());
tracker_->TrackSessionDescriptionCallback(
handler_.get(), action_, "OnSuccess", String::FromUTF8(value));
tracker_->TrackSessionId(handler_.get(),
String::FromUTF8(desc->session_id()));
}
webkit_request_.RequestSucceeded(CreateWebKitSessionDescription(desc));
webkit_request_.Reset();
Expand All @@ -265,8 +267,9 @@ class CreateSessionDescriptionRequest
}

if (handler_ && tracker_) {
tracker_->TrackSessionDescriptionCallback(handler_.get(), action_,
"OnFailure", error.message());
tracker_->TrackSessionDescriptionCallback(
handler_.get(), action_, "OnFailure",
String::FromUTF8(error.message()));
}
// TODO(hta): Convert CreateSessionDescriptionRequest.OnFailure
webkit_request_.RequestFailed(error);
Expand Down Expand Up @@ -667,8 +670,9 @@ class RTCPeerConnectionHandler::WebRtcSetDescriptionObserverImpl
WebRtcSetDescriptionObserver::States states) override {
if (!error.ok()) {
if (tracker_ && handler_) {
tracker_->TrackSessionDescriptionCallback(handler_.get(), action_,
"OnFailure", error.message());
tracker_->TrackSessionDescriptionCallback(
handler_.get(), action_, "OnFailure",
String::FromUTF8(error.message()));
}
web_request_.RequestFailed(error);
web_request_.Reset();
Expand Down Expand Up @@ -698,7 +702,7 @@ class RTCPeerConnectionHandler::WebRtcSetDescriptionObserverImpl
handler_->OnSignalingChange(signaling_state);

if (tracker_ && handler_) {
std::string value = "";
StringBuilder value;
if (action_ ==
PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION_IMPLICIT) {
webrtc::SessionDescriptionInterface* created_session_description =
Expand Down Expand Up @@ -726,14 +730,15 @@ class RTCPeerConnectionHandler::WebRtcSetDescriptionObserverImpl
if (created_session_description) {
std::string sdp;
created_session_description->ToString(&sdp);
value = "type: " +
std::string(webrtc::SdpTypeToString(
created_session_description->GetType())) +
", sdp: " + sdp;
value.Append("type: ");
value.Append(webrtc::SdpTypeToString(
created_session_description->GetType()));
value.Append(", sdp: ");
value.Append(sdp.c_str());
}
}
tracker_->TrackSessionDescriptionCallback(handler_.get(), action_,
"OnSuccess", value);
tracker_->TrackSessionDescriptionCallback(
handler_.get(), action_, "OnSuccess", value.ToString());
}
}
if (action_ == PeerConnectionTracker::ACTION_SET_REMOTE_DESCRIPTION) {
Expand Down Expand Up @@ -1289,8 +1294,8 @@ void RTCPeerConnectionHandler::SetLocalDescription(
DCHECK(task_runner_->RunsTasksInCurrentSequence());
TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::setLocalDescription");

std::string sdp = description.Sdp().Utf8();
std::string type = description.GetType().Utf8();
String sdp = String(description.Sdp());
String type = String(description.GetType());

if (peer_connection_tracker_) {
peer_connection_tracker_->TrackSetSessionDescription(
Expand All @@ -1311,7 +1316,7 @@ void RTCPeerConnectionHandler::SetLocalDescription(
if (peer_connection_tracker_) {
peer_connection_tracker_->TrackSessionDescriptionCallback(
this, PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION,
"OnFailure", reason_str);
"OnFailure", String::FromUTF8(reason_str));
}
// Warning: this line triggers the error callback to be executed, causing
// arbitrary JavaScript to be executed synchronously. As a result, it is
Expand Down Expand Up @@ -1365,8 +1370,9 @@ void RTCPeerConnectionHandler::SetRemoteDescription(
const blink::WebRTCSessionDescription& description) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
TRACE_EVENT0("webrtc", "RTCPeerConnectionHandler::setRemoteDescription");
std::string sdp = description.Sdp().Utf8();
std::string type = description.GetType().Utf8();

String sdp = String(description.Sdp());
String type = String(description.GetType());

if (peer_connection_tracker_) {
peer_connection_tracker_->TrackSetSessionDescription(
Expand All @@ -1387,7 +1393,7 @@ void RTCPeerConnectionHandler::SetRemoteDescription(
if (peer_connection_tracker_) {
peer_connection_tracker_->TrackSessionDescriptionCallback(
this, PeerConnectionTracker::ACTION_SET_REMOTE_DESCRIPTION,
"OnFailure", reason_str);
"OnFailure", String::FromUTF8(reason_str));
}
// Warning: this line triggers the error callback to be executed, causing
// arbitrary JavaScript to be executed synchronously. As a result, it is
Expand Down Expand Up @@ -2035,7 +2041,8 @@ void RTCPeerConnectionHandler::OnWebRtcEventLogWrite(
const std::string& output) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (peer_connection_tracker_) {
peer_connection_tracker_->TrackRtcEventLogWrite(this, output);
peer_connection_tracker_->TrackRtcEventLogWrite(this,
String::FromUTF8(output));
}
}

Expand Down Expand Up @@ -2433,11 +2440,12 @@ void RTCPeerConnectionHandler::OnInterestingUsage(int usage_pattern) {

webrtc::SessionDescriptionInterface*
RTCPeerConnectionHandler::CreateNativeSessionDescription(
const std::string& sdp,
const std::string& type,
const String& sdp,
const String& type,
webrtc::SdpParseError* error) {
webrtc::SessionDescriptionInterface* native_desc =
dependency_factory_->CreateSessionDescription(type, sdp, error);
dependency_factory_->CreateSessionDescription(type.Utf8(), sdp.Utf8(),
error);

LOG_IF(ERROR, !native_desc) << "Failed to create native session description."
<< " Type: " << type << " SDP: " << sdp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ class MODULES_EXPORT RTCPeerConnectionHandler
};

webrtc::SessionDescriptionInterface* CreateNativeSessionDescription(
const std::string& sdp,
const std::string& type,
const String& sdp,
const String& type,
webrtc::SdpParseError* error);

blink::WebRTCSessionDescription GetWebRTCSessionDescriptionOnSignalingThread(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ class MockPeerConnectionTracker : public PeerConnectionTracker {
const blink::WebMediaConstraints& constraints));
MOCK_METHOD4(TrackSetSessionDescription,
void(RTCPeerConnectionHandler* pc_handler,
const std::string& sdp,
const std::string& type,
const String& sdp,
const String& type,
Source source));
MOCK_METHOD1(TrackSetSessionDescriptionImplicit,
void(RTCPeerConnectionHandler* pc_handler));
Expand Down Expand Up @@ -206,8 +206,8 @@ class MockPeerConnectionTracker : public PeerConnectionTracker {
MOCK_METHOD4(TrackSessionDescriptionCallback,
void(RTCPeerConnectionHandler* pc_handler,
Action action,
const std::string& type,
const std::string& value));
const String& type,
const String& value));
MOCK_METHOD1(TrackOnRenegotiationNeeded,
void(RTCPeerConnectionHandler* pc_handler));
MOCK_METHOD2(TrackCreateDTMFSender,
Expand Down Expand Up @@ -648,10 +648,10 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescription) {
// PeerConnectionTracker::TrackSetSessionDescription is expected to be called
// before |mock_peer_connection| is called.
testing::InSequence sequence;
EXPECT_CALL(
*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), kDummySdp, kDummySdpType,
PeerConnectionTracker::SOURCE_LOCAL));
EXPECT_CALL(*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), String(kDummySdp),
String(kDummySdpType),
PeerConnectionTracker::SOURCE_LOCAL));
EXPECT_CALL(*mock_peer_connection_, SetLocalDescription(_, _));

pc_handler_->SetLocalDescription(request, description);
Expand All @@ -678,15 +678,15 @@ TEST_F(RTCPeerConnectionHandlerTest, setLocalDescriptionParseError) {
testing::InSequence sequence;
// Expect two "Track" calls, one for the start of the attempt and one for the
// failure.
EXPECT_CALL(
*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), kDummySdp, kDummySdpType,
PeerConnectionTracker::SOURCE_LOCAL));
EXPECT_CALL(
*mock_tracker_.get(),
TrackSessionDescriptionCallback(
pc_handler_.get(),
PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION, "OnFailure", _));
EXPECT_CALL(*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), String(kDummySdp),
String(kDummySdpType),
PeerConnectionTracker::SOURCE_LOCAL));
EXPECT_CALL(*mock_tracker_.get(),
TrackSessionDescriptionCallback(
pc_handler_.get(),
PeerConnectionTracker::ACTION_SET_LOCAL_DESCRIPTION,
String("OnFailure"), _));

// Used to simulate a parse failure.
mock_dependency_factory_->SetFailToCreateSessionDescription(true);
Expand All @@ -704,10 +704,10 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescription) {
// PeerConnectionTracker::TrackSetSessionDescription is expected to be called
// before |mock_peer_connection| is called.
testing::InSequence sequence;
EXPECT_CALL(
*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), kDummySdp, kDummySdpType,
PeerConnectionTracker::SOURCE_REMOTE));
EXPECT_CALL(*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), String(kDummySdp),
String(kDummySdpType),
PeerConnectionTracker::SOURCE_REMOTE));
EXPECT_CALL(*mock_peer_connection_, SetRemoteDescriptionForMock(_, _));

pc_handler_->SetRemoteDescription(request, description);
Expand All @@ -734,15 +734,15 @@ TEST_F(RTCPeerConnectionHandlerTest, setRemoteDescriptionParseError) {
testing::InSequence sequence;
// Expect two "Track" calls, one for the start of the attempt and one for the
// failure.
EXPECT_CALL(
*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), kDummySdp, kDummySdpType,
PeerConnectionTracker::SOURCE_REMOTE));
EXPECT_CALL(*mock_tracker_.get(),
TrackSetSessionDescription(pc_handler_.get(), String(kDummySdp),
String(kDummySdpType),
PeerConnectionTracker::SOURCE_REMOTE));
EXPECT_CALL(*mock_tracker_.get(),
TrackSessionDescriptionCallback(
pc_handler_.get(),
PeerConnectionTracker::ACTION_SET_REMOTE_DESCRIPTION,
"OnFailure", _));
String("OnFailure"), _));

// Used to simulate a parse failure.
mock_dependency_factory_->SetFailToCreateSessionDescription(true);
Expand Down

0 comments on commit 1672223

Please sign in to comment.