Allow PeerConnection to be garbage collected after close().

Source/modules/mediastream/RTCPeerConnection.h

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 86 matching lines @@
     void removeStream(PassRefPtrWillBeRawPtr<MediaStream>, ExceptionState&);
 
     void getStats(PassOwnPtr<RTCStatsCallback> successCallback, PassRefPtr<MediaStreamTrack> selector);
 
     PassRefPtrWillBeRawPtr<RTCDataChannel> createDataChannel(String label, const Dictionary& dataChannelDict, ExceptionState&);
 
     PassRefPtrWillBeRawPtr<RTCDTMFSender> createDTMFSender(PassRefPtrWillBeRawPtr<MediaStreamTrack>, ExceptionState&);
 
     void close(ExceptionState&);
 
+    // We allow getStats after close, but not other calls or callbacks.
+    bool requestMayFireNonStatsCallback() { return !m_closed && !m_stopped; }
+    bool requestMayFireStatsCallback() { return !m_stopped; }

[haraken, 2014/06/17 11:43:45]

Nit: I'd rename these methods to shouldFireDefaultCallbacks &
shouldFireGetStatsCallback.

[Henrik Grunell, 2014/06/17 12:12:21]

Done.

+
     DEFINE_ATTRIBUTE_EVENT_LISTENER(negotiationneeded);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(icecandidate);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(signalingstatechange);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(addstream);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(removestream);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(iceconnectionstatechange);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(datachannel);
 
     // blink::WebRTCPeerConnectionHandlerClient
     virtual void negotiationNeeded() OVERRIDE;
     virtual void didGenerateICECandidate(const blink::WebRTCICECandidate&) OVERRIDE;
     virtual void didChangeSignalingState(SignalingState) OVERRIDE;
     virtual void didChangeICEGatheringState(ICEGatheringState) OVERRIDE;
     virtual void didChangeICEConnectionState(ICEConnectionState) OVERRIDE;
     virtual void didAddRemoteStream(const blink::WebMediaStream&) OVERRIDE;
     virtual void didRemoveRemoteStream(const blink::WebMediaStream&) OVERRIDE;
     virtual void didAddRemoteDataChannel(blink::WebRTCDataChannelHandler*) OVERRIDE;
     virtual void releasePeerConnectionHandler() OVERRIDE;
 
     // EventTarget
     virtual const AtomicString& interfaceName() const OVERRIDE;
     virtual ExecutionContext* executionContext() const OVERRIDE;
 
     // ActiveDOMObject
     virtual void suspend() OVERRIDE;
     virtual void resume() OVERRIDE;
     virtual void stop() OVERRIDE;
-    virtual bool hasPendingActivity() const OVERRIDE { return !m_stopped; }
+    // We require a call to close() or stop() 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). In stop(),
+    // |m_peer_handler| is deleted. 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 need to keep track of if there are still
+    // outstandning requests. The request is given a pointer to this object and
+    // checks if it should fire the callback before doing so.

[haraken, 2014/06/17 11:43:45]

Nit: This comment sounds a bit redundant. Can we just mention that the
RTCPeerConnection object is kept alive as long as it is not closed nor stopped?

[Henrik Grunell, 2014/06/17 12:12:21]

Done.

+    virtual bool hasPendingActivity() const OVERRIDE
+    {
+        return !m_closed && !m_stopped;
+    }
 
     virtual void trace(Visitor*) OVERRIDE;
 
 private:
     RTCPeerConnection(ExecutionContext*, PassRefPtr<RTCConfiguration>, blink::WebMediaConstraints, ExceptionState&);
 
     static PassRefPtr<RTCConfiguration> parseConfiguration(const Dictionary& configuration, ExceptionState&);
     void scheduleDispatchEvent(PassRefPtrWillBeRawPtr<Event>);
     void dispatchScheduledEvent();
     bool hasLocalStreamWithTrackId(const String& trackId);
@@ skipping 10 matching lines @@
     MediaStreamVector m_remoteStreams;
 
     WillBeHeapVector<RefPtrWillBeMember<RTCDataChannel> > m_dataChannels;
 
     OwnPtr<blink::WebRTCPeerConnectionHandler> m_peerHandler;
 
     AsyncMethodRunner<RTCPeerConnection> m_dispatchScheduledEventRunner;
     WillBeHeapVector<RefPtrWillBeMember<Event> > m_scheduledEvents;
 
     bool m_stopped;
+    bool m_closed;
 };
 
 } // namespace WebCore
 
 #endif // RTCPeerConnection_h