Allow PeerConnection to be garbage collected after close().

Source/modules/mediastream/RTCPeerConnection.cpp

Index: Source/modules/mediastream/RTCPeerConnection.cpp
diff --git a/Source/modules/mediastream/RTCPeerConnection.cpp b/Source/modules/mediastream/RTCPeerConnection.cpp
index 5c0d5df96329f78a1d431fb78abaf5ea11cbc42f..405d13081425826e6099d0d094a572399fa372bf 100644
--- a/Source/modules/mediastream/RTCPeerConnection.cpp
+++ b/Source/modules/mediastream/RTCPeerConnection.cpp
@@ -182,6 +182,7 @@ RTCPeerConnection::RTCPeerConnection(ExecutionContext* context, PassRefPtr<RTCCo
     , m_iceConnectionState(ICEConnectionStateNew)
     , m_dispatchScheduledEventRunner(this, &RTCPeerConnection::dispatchScheduledEvent)
     , m_stopped(false)
+    , m_closed(false)
 {
     ScriptWrappable::init(this);
     Document* document = toDocument(executionContext());
@@ -191,6 +192,16 @@ RTCPeerConnection::RTCPeerConnection(ExecutionContext* context, PassRefPtr<RTCCo
         return;
     }
 
+    // We require a call to close() before the object can be garbage collected.
+    // In close(), stop() is called on |m_peer_handler|, after which no events
+    // are fired on this object (blink::WebRTCPeerConnectionHandlerClient
+    // interface). So we increase the ref count right away, and decrease it
+    // correspondingly in close(). Note that async operations, such as
+    // createOffer or getStats doesn't respond through this object, but through
+    // a request object containing the callback. The request object is handed to
+    // the client. Hence, we don't ref count these operations.
+    setPendingActivity(this);

[haraken, 2014/06/13 01:23:00]

setPendingActivity/unsetPendingActivity are deprecated, so please keep using
hasPendingActivity. I guess you can return something like '!m_closed ||
!m_stopped' from hasPendingActivity?

[Henrik Grunell, 2014/06/13 07:50:46]

Oh, I didn't know that. Maybe a comment should be added in ActiveDOMObject.h and
http://www.chromium.org/blink/activedomobject be updated?

Changed it to use m_closed and m_stopped.

+
     m_peerHandler = adoptPtr(blink::Platform::current()->createRTCPeerConnectionHandler(this));
     if (!m_peerHandler) {
         exceptionState.throwDOMException(NotSupportedError, "No PeerConnection handler can be created, perhaps WebRTC is disabled?");
@@ -208,7 +219,9 @@ RTCPeerConnection::RTCPeerConnection(ExecutionContext* context, PassRefPtr<RTCCo
 RTCPeerConnection::~RTCPeerConnection()
 {
     // This checks that stop() is called if necessary before the destructor.
-    // We are assuming that a wrapper is always created when RTCPeerConnection is created
+    // We are assuming that a wrapper is always created when RTCPeerConnection is created.
+    // TODO - BEFORE COMMIT: We should call stop here.
+    ASSERT(m_closed);
     ASSERT(m_dataChannels.isEmpty());
 }
 
@@ -223,7 +236,8 @@ void RTCPeerConnection::createOffer(PassOwnPtr<RTCSessionDescriptionCallback> su
     if (exceptionState.hadException())
         return;
 
-    RefPtr<RTCSessionDescriptionRequest> request = RTCSessionDescriptionRequestImpl::create(executionContext(), successCallback, errorCallback);
+    setPendingActivity(this);
+    RefPtr<RTCSessionDescriptionRequest> request = RTCSessionDescriptionRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->createOffer(request.release(), constraints);
 }
 
@@ -238,7 +252,8 @@ void RTCPeerConnection::createAnswer(PassOwnPtr<RTCSessionDescriptionCallback> s
     if (exceptionState.hadException())
         return;
 
-    RefPtr<RTCSessionDescriptionRequest> request = RTCSessionDescriptionRequestImpl::create(executionContext(), successCallback, errorCallback);
+    setPendingActivity(this);
+    RefPtr<RTCSessionDescriptionRequest> request = RTCSessionDescriptionRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->createAnswer(request.release(), constraints);
 }
 
@@ -253,7 +268,8 @@ void RTCPeerConnection::setLocalDescription(PassRefPtrWillBeRawPtr<RTCSessionDes
         return;
     }
 
-    RefPtr<RTCVoidRequest> request = RTCVoidRequestImpl::create(executionContext(), successCallback, errorCallback);
+    setPendingActivity(this);
+    RefPtr<RTCVoidRequest> request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->setLocalDescription(request.release(), sessionDescription->webSessionDescription());
 }
 
@@ -277,7 +293,8 @@ void RTCPeerConnection::setRemoteDescription(PassRefPtrWillBeRawPtr<RTCSessionDe
         return;
     }
 
-    RefPtr<RTCVoidRequest> request = RTCVoidRequestImpl::create(executionContext(), successCallback, errorCallback);
+    setPendingActivity(this);
+    RefPtr<RTCVoidRequest> request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
     m_peerHandler->setRemoteDescription(request.release(), sessionDescription->webSessionDescription());
 }
 
@@ -335,11 +352,14 @@ void RTCPeerConnection::addIceCandidate(RTCIceCandidate* iceCandidate, PassOwnPt
     ASSERT(successCallback);
     ASSERT(errorCallback);
 
-    RefPtr<RTCVoidRequest> request = RTCVoidRequestImpl::create(executionContext(), successCallback, errorCallback);
+    setPendingActivity(this);
+    RefPtr<RTCVoidRequest> request = RTCVoidRequestImpl::create(executionContext(), this, successCallback, errorCallback);
 
     bool implemented = m_peerHandler->addICECandidate(request.release(), iceCandidate->webCandidate());
-    if (!implemented)
+    if (!implemented) {
+        unsetPendingActivity(this);
         exceptionState.throwDOMException(NotSupportedError, "This method is not yet implemented.");
+    }
 }
 
 String RTCPeerConnection::signalingState() const
@@ -474,7 +494,8 @@ MediaStream* RTCPeerConnection::getStreamById(const String& streamId)
 
 void RTCPeerConnection::getStats(PassOwnPtr<RTCStatsCallback> successCallback, PassRefPtr<MediaStreamTrack> selector)
 {
-    RefPtr<RTCStatsRequest> statsRequest = RTCStatsRequestImpl::create(executionContext(), successCallback, selector);
+    setPendingActivity(this);
+    RefPtr<RTCStatsRequest> statsRequest = RTCStatsRequestImpl::create(executionContext(), this, successCallback, selector);
     // FIXME: Add passing selector as part of the statsRequest.
     m_peerHandler->getStats(statsRequest.release());
 }
@@ -546,18 +567,34 @@ void RTCPeerConnection::close(ExceptionState& exceptionState)
 
     m_peerHandler->stop();
 
+    m_closed = true;
+
     changeIceConnectionState(ICEConnectionStateClosed);
     changeIceGatheringState(ICEGatheringStateComplete);
     changeSignalingState(SignalingStateClosed);
+
+    // TODO - BEFORE COMMIT: Stop data channels here?
+
+    m_dispatchScheduledEventRunner.stop();
+
+    unsetPendingActivity(this);
+}
+
+bool RTCPeerConnection::requestCompletedCheckIfCallbackShouldFire()
+{
+    unsetPendingActivity(this);
+    return !m_closed;
 }
 
 void RTCPeerConnection::negotiationNeeded()
 {
+    ASSERT(!m_closed);
     scheduleDispatchEvent(Event::create(EventTypeNames::negotiationneeded));
 }
 
 void RTCPeerConnection::didGenerateICECandidate(const blink::WebRTCICECandidate& webCandidate)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
     if (webCandidate.isNull())
         scheduleDispatchEvent(RTCIceCandidateEvent::create(false, false, nullptr));
@@ -569,24 +606,28 @@ void RTCPeerConnection::didGenerateICECandidate(const blink::WebRTCICECandidate&
 
 void RTCPeerConnection::didChangeSignalingState(SignalingState newState)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
     changeSignalingState(newState);
 }
 
 void RTCPeerConnection::didChangeICEGatheringState(ICEGatheringState newState)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
     changeIceGatheringState(newState);
 }
 
 void RTCPeerConnection::didChangeICEConnectionState(ICEConnectionState newState)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
     changeIceConnectionState(newState);
 }
 
 void RTCPeerConnection::didAddRemoteStream(const blink::WebMediaStream& remoteStream)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
 
     if (m_signalingState == SignalingStateClosed)
@@ -600,6 +641,7 @@ void RTCPeerConnection::didAddRemoteStream(const blink::WebMediaStream& remoteSt
 
 void RTCPeerConnection::didRemoveRemoteStream(const blink::WebMediaStream& remoteStream)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
 
     MediaStreamDescriptor* streamDescriptor = remoteStream;
@@ -620,6 +662,7 @@ void RTCPeerConnection::didRemoveRemoteStream(const blink::WebMediaStream& remot
 
 void RTCPeerConnection::didAddRemoteDataChannel(blink::WebRTCDataChannelHandler* handler)
 {
+    ASSERT(!m_closed);
     ASSERT(executionContext()->isContextThread());
 
     if (m_signalingState == SignalingStateClosed)
@@ -705,6 +748,7 @@ void RTCPeerConnection::scheduleDispatchEvent(PassRefPtrWillBeRawPtr<Event> even
 
 void RTCPeerConnection::dispatchScheduledEvent()
 {
+    ASSERT(!m_closed);
     if (m_stopped)
         return;