Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3249)

Unified Diff: content/renderer/media/rtc_peer_connection_handler.cc

Issue 1439973006: Reland Fix leak of RTCPeerConnectionHandler if PeerConnection.close() is called from js.… (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix bug and add expectation. Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/renderer/media/rtc_peer_connection_handler.cc
diff --git a/content/renderer/media/rtc_peer_connection_handler.cc b/content/renderer/media/rtc_peer_connection_handler.cc
index 6dfe0d9f2925f989f47eef6e5a7b6c04881d6ed5..aa2a2ec9a373ff88221fb57a9a70dfa98c2176dc 100644
--- a/content/renderer/media/rtc_peer_connection_handler.cc
+++ b/content/renderer/media/rtc_peer_connection_handler.cc
@@ -796,8 +796,10 @@ RTCPeerConnectionHandler::RTCPeerConnectionHandler(
blink::WebRTCPeerConnectionHandlerClient* client,
PeerConnectionDependencyFactory* dependency_factory)
: client_(client),
+ is_closed_(false),
dependency_factory_(dependency_factory),
weak_factory_(this) {
+ CHECK(client_),
g_peer_connection_handlers.Get().insert(this);
}
@@ -817,13 +819,13 @@ RTCPeerConnectionHandler::~RTCPeerConnectionHandler() {
// static
void RTCPeerConnectionHandler::DestructAllHandlers() {
+ // Copy g_peer_connection_handlers since releasePeerConnectionHandler will
+ // remove an item.
std::set<RTCPeerConnectionHandler*> handlers(
g_peer_connection_handlers.Get().begin(),
g_peer_connection_handlers.Get().end());
- for (auto handler : handlers) {
- if (handler->client_)
- handler->client_->releasePeerConnectionHandler();
- }
+ for (auto* handler : handlers)
+ handler->client_->releasePeerConnectionHandler();
}
// static
@@ -1309,7 +1311,7 @@ void RTCPeerConnectionHandler::GetStats(
void RTCPeerConnectionHandler::CloseClientPeerConnection() {
DCHECK(thread_checker_.CalledOnValidThread());
- if (client_)
+ if (!is_closed_)
client_->closePeerConnection();
}
@@ -1380,7 +1382,7 @@ void RTCPeerConnectionHandler::stop() {
DCHECK(thread_checker_.CalledOnValidThread());
DVLOG(1) << "RTCPeerConnectionHandler::stop";
- if (!client_ || !native_peer_connection_.get())
+ if (is_closed_ || !native_peer_connection_.get())
return; // Already stopped.
if (peer_connection_tracker_)
@@ -1388,9 +1390,8 @@ void RTCPeerConnectionHandler::stop() {
native_peer_connection_->Close();
- // The client_ pointer is not considered valid after this point and no further
- // callbacks must be made.
- client_ = nullptr;
+ // This object may no longer forward call backs to blink.
+ is_closed_ = true;
}
void RTCPeerConnectionHandler::OnSignalingChange(
@@ -1402,7 +1403,7 @@ void RTCPeerConnectionHandler::OnSignalingChange(
GetWebKitSignalingState(new_state);
if (peer_connection_tracker_)
peer_connection_tracker_->TrackSignalingStateChange(this, state);
- if (client_)
+ if (!is_closed_)
client_->didChangeSignalingState(state);
}
@@ -1438,7 +1439,7 @@ void RTCPeerConnectionHandler::OnIceConnectionChange(
GetWebKitIceConnectionState(new_state);
if (peer_connection_tracker_)
peer_connection_tracker_->TrackIceConnectionStateChange(this, state);
- if(client_)
+ if (!is_closed_)
client_->didChangeICEConnectionState(state);
}
@@ -1451,7 +1452,7 @@ void RTCPeerConnectionHandler::OnIceGatheringChange(
if (new_state == webrtc::PeerConnectionInterface::kIceGatheringComplete) {
// If ICE gathering is completed, generate a NULL ICE candidate,
// to signal end of candidates.
- if (client_) {
+ if (!is_closed_) {
blink::WebRTCICECandidate null_candidate;
client_->didGenerateICECandidate(null_candidate);
}
@@ -1508,7 +1509,7 @@ void RTCPeerConnectionHandler::OnAddStream(
track_metrics_.AddStream(MediaStreamTrackMetrics::RECEIVED_STREAM,
s->webrtc_stream().get());
- if (client_)
+ if (!is_closed_)
client_->didAddRemoteStream(s->webkit_stream());
}
@@ -1536,7 +1537,7 @@ void RTCPeerConnectionHandler::OnRemoveStream(
this, webkit_stream, PeerConnectionTracker::SOURCE_REMOTE);
}
- if (client_)
+ if (!is_closed_)
client_->didRemoveRemoteStream(webkit_stream);
}
@@ -1550,7 +1551,7 @@ void RTCPeerConnectionHandler::OnDataChannel(
this, handler->channel().get(), PeerConnectionTracker::SOURCE_REMOTE);
}
- if (client_)
+ if (!is_closed_)
client_->didAddRemoteDataChannel(handler.release());
}
@@ -1579,7 +1580,7 @@ void RTCPeerConnectionHandler::OnIceCandidate(
NOTREACHED();
}
}
- if (client_)
+ if (!is_closed_)
client_->didGenerateICECandidate(web_candidate);
}
« no previous file with comments | « content/renderer/media/rtc_peer_connection_handler.h ('k') | content/renderer/media/rtc_peer_connection_handler_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698