Revert of Remove references between RTCPeerConnection and RTCDataChannel....

Revert of Remove references between RTCPeerConnection and RTCDataChannel. (patchset #3 id:40001 of https://codereview.chromium.org/1634073003/ )

Reason for revert:
It turns out that the underlying WebRTC DataChannel  objects does not maintain a reference to the WebRTC PeerConnection, so we do need to keep the reference in the blink layer.

PS2 was the correct patch.

Original issue's description:
> The underlying WebRTC objects maintain all the needed references.
> No references are actually needed in the blink layer.
>
> BUG=564965
>
> Committed: https://crrev.com/0004ce15cdf065966508c61a5265ae8662e363ed
> Cr-Commit-Position: refs/heads/master@{#371667}

TBR=tommi@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=564965

Review URL: https://codereview.chromium.org/1641113002

Cr-Commit-Position: refs/heads/master@{#372072}

third_party/WebKit/Source/modules/mediastream/RTCDataChannel.cpp

@@ -51,28 +51,30 @@ static void throwNoBlobSupportException(ExceptionState& exceptionState)
     exceptionState.throwDOMException(NotSupportedError, "Blob support not implemented yet");
 }
 
-RTCDataChannel* RTCDataChannel::create(ExecutionContext* context, PassOwnPtr<WebRTCDataChannelHandler> handler)
+RTCDataChannel* RTCDataChannel::create(ExecutionContext* context, RTCPeerConnection* connection, PassOwnPtr<WebRTCDataChannelHandler> handler)
 {
     ASSERT(handler);
-    return new RTCDataChannel(context, handler);
+    return new RTCDataChannel(context, connection, handler);
 }
 
-RTCDataChannel* RTCDataChannel::create(ExecutionContext* context, WebRTCPeerConnectionHandler* peerConnectionHandler, const String& label, const WebRTCDataChannelInit& init, ExceptionState& exceptionState)
+RTCDataChannel* RTCDataChannel::create(ExecutionContext* context, RTCPeerConnection* connection, WebRTCPeerConnectionHandler* peerConnectionHandler, const String& label, const WebRTCDataChannelInit& init, ExceptionState& exceptionState)
 {
     OwnPtr<WebRTCDataChannelHandler> handler = adoptPtr(peerConnectionHandler->createDataChannel(label, init));
     if (!handler) {
         exceptionState.throwDOMException(NotSupportedError, "RTCDataChannel is not supported");
         return nullptr;
     }
-    return new RTCDataChannel(context, handler.release());
+    return new RTCDataChannel(context, connection, handler.release());
 }
 
-RTCDataChannel::RTCDataChannel(ExecutionContext* context, PassOwnPtr<WebRTCDataChannelHandler> handler)
+RTCDataChannel::RTCDataChannel(ExecutionContext* context, RTCPeerConnection* connection, PassOwnPtr<WebRTCDataChannelHandler> handler)
     : m_executionContext(context)
     , m_handler(handler)
+    , m_stopped(false)
     , m_readyState(ReadyStateConnecting)
     , m_binaryType(BinaryTypeArrayBuffer)
     , m_scheduledEventTimer(this, &RTCDataChannel::scheduledEventTimerFired)
+    , m_connection(connection)
     , m_bufferedAmountLowThreshold(0U)
 {
     m_handler->setClient(this);
@@ -80,8 +82,11 @@ RTCDataChannel::RTCDataChannel(ExecutionContext* context, PassOwnPtr<WebRTCDataC
 
 RTCDataChannel::~RTCDataChannel()
 {
-    // Notify the client that the channel is gone.
-    m_handler->setClient(0);
+    // If the peer connection and the data channel die in the same
+    // GC cycle stop has not been called and we need to notify the
+    // client that the channel is gone.
+    if (!m_stopped)
+        m_handler->setClient(0);
 }
 
 RTCDataChannel::ReadyState RTCDataChannel::getHandlerState() const
@@ -230,12 +235,15 @@ void RTCDataChannel::send(Blob* data, ExceptionState& exceptionState)
 
 void RTCDataChannel::close()
 {
+    if (m_stopped)
+        return;
+
     m_handler->close();
 }
 
 void RTCDataChannel::didChangeReadyState(WebRTCDataChannelHandlerClient::ReadyState newState)
 {
-    if (m_readyState == ReadyStateClosed)
+    if (m_stopped || m_readyState == ReadyStateClosed)
         return;
 
     m_readyState = newState;
@@ -262,11 +270,17 @@ void RTCDataChannel::didDecreaseBufferedAmount(unsigned previousAmount)
 
 void RTCDataChannel::didReceiveStringData(const WebString& text)
 {
+    if (m_stopped)
+        return;
+
     scheduleDispatchEvent(MessageEvent::create(text));
 }
 
 void RTCDataChannel::didReceiveRawData(const char* data, size_t dataLength)
 {
+    if (m_stopped)
+        return;
+
     if (m_binaryType == BinaryTypeBlob) {
         // FIXME: Implement.
         return;
@@ -281,6 +295,9 @@ void RTCDataChannel::didReceiveRawData(const char* data, size_t dataLength)
 
 void RTCDataChannel::didDetectError()
 {
+    if (m_stopped)
+        return;
+
     scheduleDispatchEvent(Event::create(EventTypeNames::error));
 }
 
@@ -294,6 +311,14 @@ ExecutionContext* RTCDataChannel::executionContext() const
     return m_executionContext;
 }
 
+void RTCDataChannel::stop()
+{
+    m_stopped = true;
+    m_readyState = ReadyStateClosed;
+    m_handler->setClient(0);
+    m_executionContext = 0;
+}
+
 void RTCDataChannel::scheduleDispatchEvent(PassRefPtrWillBeRawPtr<Event> event)
 {
     m_scheduledEvents.append(event);
@@ -304,6 +329,9 @@ void RTCDataChannel::scheduleDispatchEvent(PassRefPtrWillBeRawPtr<Event> event)
 
 void RTCDataChannel::scheduledEventTimerFired(Timer<RTCDataChannel>*)
 {
+    if (m_stopped)
+        return;
+
     WillBeHeapVector<RefPtrWillBeMember<Event>> events;
     events.swap(m_scheduledEvents);
 
@@ -314,10 +342,19 @@ void RTCDataChannel::scheduledEventTimerFired(Timer<RTCDataChannel>*)
     events.clear();
 }
 
+void RTCDataChannel::clearWeakMembers(Visitor* visitor)
+{
+    if (Heap::isHeapObjectAlive(m_connection))
+        return;
+    stop();
+    m_connection = nullptr;
+}
+
 DEFINE_TRACE(RTCDataChannel)
 {
     visitor->trace(m_executionContext);
     visitor->trace(m_scheduledEvents);
+    visitor->template registerWeakMembers<RTCDataChannel, &RTCDataChannel::clearWeakMembers>(this);
     RefCountedGarbageCollectedEventTargetWithInlineData<RTCDataChannel>::trace(visitor);
 }
 

third_party/WebKit/Source/modules/mediastream/RTCDataChannel.h

@@ -49,8 +49,8 @@ class MODULES_EXPORT RTCDataChannel final
     REFCOUNTED_GARBAGE_COLLECTED_EVENT_TARGET(RTCDataChannel);
     DEFINE_WRAPPERTYPEINFO();
 public:
-    static RTCDataChannel* create(ExecutionContext*, PassOwnPtr<WebRTCDataChannelHandler>);
-    static RTCDataChannel* create(ExecutionContext*, WebRTCPeerConnectionHandler*, const String& label, const WebRTCDataChannelInit&, ExceptionState&);
+    static RTCDataChannel* create(ExecutionContext*, RTCPeerConnection*, PassOwnPtr<WebRTCDataChannelHandler>);
+    static RTCDataChannel* create(ExecutionContext*, RTCPeerConnection*, WebRTCPeerConnectionHandler*, const String& label, const WebRTCDataChannelInit&, ExceptionState&);
     ~RTCDataChannel() override;
 
     ReadyState getHandlerState() const;
@@ -88,10 +88,14 @@ public:
     DEFINE_ATTRIBUTE_EVENT_LISTENER(close);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(message);
 
+    void stop();
+
     // EventTarget
     const AtomicString& interfaceName() const override;
     ExecutionContext* executionContext() const override;
 
+    void clearWeakMembers(Visitor*);
+
     // Oilpan: need to eagerly finalize m_handler
     EAGERLY_FINALIZE();
     DECLARE_VIRTUAL_TRACE();
@@ -104,7 +108,7 @@ public:
     void didDetectError() override;
 
 private:
-    RTCDataChannel(ExecutionContext*, PassOwnPtr<WebRTCDataChannelHandler>);
+    RTCDataChannel(ExecutionContext*, RTCPeerConnection*, PassOwnPtr<WebRTCDataChannelHandler>);
 
     void scheduleDispatchEvent(PassRefPtrWillBeRawPtr<Event>);
     void scheduledEventTimerFired(Timer<RTCDataChannel>*);
@@ -113,6 +117,8 @@ private:
 
     OwnPtr<WebRTCDataChannelHandler> m_handler;
 
+    bool m_stopped;
+
     WebRTCDataChannelHandlerClient::ReadyState m_readyState;
 
     enum BinaryType {
@@ -124,6 +130,8 @@ private:
     Timer<RTCDataChannel> m_scheduledEventTimer;
     WillBeHeapVector<RefPtrWillBeMember<Event>> m_scheduledEvents;
 
+    WeakMember<RTCPeerConnection> m_connection;
+
     unsigned m_bufferedAmountLowThreshold;
 
     FRIEND_TEST_ALL_PREFIXES(RTCDataChannelTest, BufferedAmountLow);

third_party/WebKit/Source/modules/mediastream/RTCDataChannelTest.cpp

@@ -79,7 +79,7 @@ private:
 TEST(RTCDataChannelTest, BufferedAmount)
 {
     MockHandler* handler = new MockHandler();
-    RTCDataChannel* channel = RTCDataChannel::create(0, adoptPtr(handler));
+    RTCDataChannel* channel = RTCDataChannel::create(0, 0, adoptPtr(handler));
 
     handler->changeState(WebRTCDataChannelHandlerClient::ReadyStateOpen);
     String message(std::string(100, 'A').c_str());
@@ -90,7 +90,7 @@ TEST(RTCDataChannelTest, BufferedAmount)
 TEST(RTCDataChannelTest, BufferedAmountLow)
 {
     MockHandler* handler = new MockHandler();
-    RTCDataChannel* channel = RTCDataChannel::create(0, adoptPtr(handler));
+    RTCDataChannel* channel = RTCDataChannel::create(0, 0, adoptPtr(handler));
 
     // Add and drain 100 bytes
     handler->changeState(WebRTCDataChannelHandlerClient::ReadyStateOpen);

third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.cpp

@@ -796,9 +796,10 @@ RTCDataChannel* RTCPeerConnection::createDataChannel(String label, const Diction
     DictionaryHelper::get(options, "protocol", protocolString);
     init.protocol = protocolString;
 
-    RTCDataChannel* channel = RTCDataChannel::create(executionContext(), m_peerHandler.get(), label, init, exceptionState);
+    RTCDataChannel* channel = RTCDataChannel::create(executionContext(), this, m_peerHandler.get(), label, init, exceptionState);
     if (exceptionState.hadException())
         return nullptr;
+    m_dataChannels.append(channel);
     RTCDataChannel::ReadyState handlerState = channel->getHandlerState();
     if (handlerState != RTCDataChannel::ReadyStateConnecting) {
         // There was an early state transition.  Don't miss it!
@@ -924,7 +925,9 @@ void RTCPeerConnection::didAddRemoteDataChannel(WebRTCDataChannelHandler* handle
     if (m_signalingState == SignalingStateClosed)
         return;
 
-    RTCDataChannel* channel = RTCDataChannel::create(executionContext(), adoptPtr(handler));
+    RTCDataChannel* channel = RTCDataChannel::create(executionContext(), this, adoptPtr(handler));
+    m_dataChannels.append(channel);
+
     scheduleDispatchEvent(RTCDataChannelEvent::create(EventTypeNames::datachannel, false, false, channel));
 }
 
@@ -968,6 +971,11 @@ void RTCPeerConnection::stop()
     m_iceConnectionState = ICEConnectionStateClosed;
     m_signalingState = SignalingStateClosed;
 
+    HeapVector<Member<RTCDataChannel>>::iterator i = m_dataChannels.begin();
+    for (; i != m_dataChannels.end(); ++i)
+        (*i)->stop();
+    m_dataChannels.clear();
+
     m_dispatchScheduledEventRunner->stop();
 
     m_peerHandler.clear();
@@ -1049,6 +1057,7 @@ DEFINE_TRACE(RTCPeerConnection)
 {
     visitor->trace(m_localStreams);
     visitor->trace(m_remoteStreams);
+    visitor->trace(m_dataChannels);
     visitor->trace(m_dispatchScheduledEventRunner);
     visitor->trace(m_scheduledEvents);
     RefCountedGarbageCollectedEventTargetWithInlineData<RTCPeerConnection>::trace(visitor);

third_party/WebKit/Source/modules/mediastream/RTCPeerConnection.h

@@ -199,6 +199,8 @@ private:
     MediaStreamVector m_localStreams;
     MediaStreamVector m_remoteStreams;
 
+    HeapVector<Member<RTCDataChannel>> m_dataChannels;
+
     OwnPtr<WebRTCPeerConnectionHandler> m_peerHandler;
 
     Member<AsyncMethodRunner<RTCPeerConnection>> m_dispatchScheduledEventRunner;