Stop RTCDataChannel when closing page
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Mar 2017 00:06:11 +0000 (00:06 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Mar 2017 00:06:11 +0000 (00:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170166

Patch by Youenn Fablet <youenn@apple.com> on 2017-03-28
Reviewed by Eric Carlson.

Source/WebCore:

Test: webrtc/datachannel/datachannel-gc.html

Making RTCDataChannel an ActiveDOMObject.
Closing the data channel backend and freeing upon close and stop.

* Modules/mediastream/RTCDataChannel.cpp:
(WebCore::RTCDataChannel::create):
(WebCore::RTCDataChannel::RTCDataChannel):
(WebCore::RTCDataChannel::close):
(WebCore::RTCDataChannel::stop):
* Modules/mediastream/RTCDataChannel.h:
* Modules/mediastream/RTCDataChannel.idl:
* Modules/mediastream/RTCPeerConnection.h:

LayoutTests:

* webrtc/datachannel/datachannel-gc-expected.txt: Added.
* webrtc/datachannel/datachannel-gc.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@214508 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webrtc/datachannel/datachannel-gc-expected.txt [new file with mode: 0644]
LayoutTests/webrtc/datachannel/datachannel-gc.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/RTCDataChannel.cpp
Source/WebCore/Modules/mediastream/RTCDataChannel.h
Source/WebCore/Modules/mediastream/RTCDataChannel.idl
Source/WebCore/Modules/mediastream/RTCPeerConnection.h

index 031f103..8581671 100644 (file)
@@ -1,3 +1,13 @@
+2017-03-28  Youenn Fablet  <youenn@apple.com>
+
+        Stop RTCDataChannel when closing page
+        https://bugs.webkit.org/show_bug.cgi?id=170166
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/datachannel/datachannel-gc-expected.txt: Added.
+        * webrtc/datachannel/datachannel-gc.html: Added.
+
 2017-03-28  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Ranges for variation font properties are not enforced
diff --git a/LayoutTests/webrtc/datachannel/datachannel-gc-expected.txt b/LayoutTests/webrtc/datachannel/datachannel-gc-expected.txt
new file mode 100644 (file)
index 0000000..62720b7
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASS Losing local channel reference and closing it on the other side should still allow emitting the close event 
+
diff --git a/LayoutTests/webrtc/datachannel/datachannel-gc.html b/LayoutTests/webrtc/datachannel/datachannel-gc.html
new file mode 100644 (file)
index 0000000..8a4053a
--- /dev/null
@@ -0,0 +1,35 @@
+<!doctype html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Testing basic data channel and garbage collection</title>
+    <script src="../../resources/testharness.js"></script>
+    <script src="../../resources/testharnessreport.js"></script>
+  </head>
+  <body>
+    <script src ="../routines.js"></script>
+    <script>
+var finishTest;
+promise_test((test) => {
+    counter = 0;
+    return new Promise((resolve, reject) => {
+        if (window.internals)
+            internals.useMockRTCPeerConnectionFactory("TwoRealPeerConnections");
+
+        createConnections((localConnection) => {
+            localConnection.createDataChannel('sendDataChannel').onclose = resolve;
+        }, (remoteConnection) => {
+            remoteConnection.ondatachannel = (event) => {
+                var remoteChannel = event.channel;
+                remoteChannel.close();
+            };
+        });
+        if (window.GCController)
+            return GCController.collect();
+    }).then(() => {
+        closeConnections();
+    });
+}, "Losing local channel reference and closing it on the other side should still allow emitting the close event");
+    </script>
+  </body>
+</html>
index 5efeec6..0d81edc 100644 (file)
@@ -1,3 +1,24 @@
+2017-03-28  Youenn Fablet  <youenn@apple.com>
+
+        Stop RTCDataChannel when closing page
+        https://bugs.webkit.org/show_bug.cgi?id=170166
+
+        Reviewed by Eric Carlson.
+
+        Test: webrtc/datachannel/datachannel-gc.html
+
+        Making RTCDataChannel an ActiveDOMObject.
+        Closing the data channel backend and freeing upon close and stop.
+
+        * Modules/mediastream/RTCDataChannel.cpp:
+        (WebCore::RTCDataChannel::create):
+        (WebCore::RTCDataChannel::RTCDataChannel):
+        (WebCore::RTCDataChannel::close):
+        (WebCore::RTCDataChannel::stop):
+        * Modules/mediastream/RTCDataChannel.h:
+        * Modules/mediastream/RTCDataChannel.idl:
+        * Modules/mediastream/RTCPeerConnection.h:
+
 2017-03-28  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Ranges for variation font properties are not enforced
index 76723e4..9d2cf6c 100644 (file)
@@ -56,12 +56,14 @@ Ref<RTCDataChannel> RTCDataChannel::create(ScriptExecutionContext& context, std:
 {
     ASSERT(handler);
     auto channel = adoptRef(*new RTCDataChannel(context, WTFMove(handler), WTFMove(label), WTFMove(options)));
-    channel->m_handler->setClient(&channel.get());
+    channel->suspendIfNeeded();
+    channel->m_handler->setClient(channel.ptr());
+    channel->setPendingActivity(channel.ptr());
     return channel;
 }
 
 RTCDataChannel::RTCDataChannel(ScriptExecutionContext& context, std::unique_ptr<RTCDataChannelHandler>&& handler, String&& label, RTCDataChannelInit&& options)
-    : m_scriptExecutionContext(&context)
+    : ActiveDOMObject(&context)
     , m_handler(WTFMove(handler))
     , m_scheduledEventTimer(*this, &RTCDataChannel::scheduledEventTimerFired)
     , m_label(WTFMove(label))
@@ -168,7 +170,13 @@ void RTCDataChannel::close()
     if (m_stopped)
         return;
 
+    m_stopped = true;
+    m_readyState = ReadyStateClosed;
+
     m_handler->close();
+    m_handler->setClient(nullptr);
+    m_handler = nullptr;
+    unsetPendingActivity(this);
 }
 
 void RTCDataChannel::didChangeReadyState(ReadyState newState)
@@ -234,10 +242,7 @@ void RTCDataChannel::bufferedAmountIsDecreasing()
 
 void RTCDataChannel::stop()
 {
-    m_stopped = true;
-    m_readyState = ReadyStateClosed;
-    m_handler->setClient(nullptr);
-    m_scriptExecutionContext = nullptr;
+    close();
 }
 
 void RTCDataChannel::scheduleDispatchEvent(Ref<Event>&& event)
index c94f6b9..c35512b 100644 (file)
@@ -26,6 +26,7 @@
 
 #if ENABLE(WEB_RTC)
 
+#include "ActiveDOMObject.h"
 #include "Event.h"
 #include "EventTarget.h"
 #include "ExceptionOr.h"
@@ -44,7 +45,7 @@ namespace WebCore {
 class Blob;
 class RTCPeerConnectionHandler;
 
-class RTCDataChannel final : public RTCDataChannelHandlerClient, public EventTargetWithInlineData {
+class RTCDataChannel final : public ActiveDOMObject, public RTCDataChannelHandlerClient, public EventTargetWithInlineData {
 public:
     static Ref<RTCDataChannel> create(ScriptExecutionContext&, std::unique_ptr<RTCDataChannelHandler>&&, String&&, RTCDataChannelInit&&);
 
@@ -71,8 +72,6 @@ public:
 
     void close();
 
-    void stop();
-
     using RTCDataChannelHandlerClient::ref;
     using RTCDataChannelHandlerClient::deref;
 
@@ -88,7 +87,10 @@ private:
     void refEventTarget() final { ref(); }
     void derefEventTarget() final { deref(); }
 
-    ScriptExecutionContext* m_scriptExecutionContext;
+    // ActiveDOMObject API
+    void stop() final;
+    const char* activeDOMObjectName() const final { return "RTCDataChannel"; }
+    bool canSuspendForDocumentSuspension() const final { return m_readyState == ReadyStateClosed; }
 
     // RTCDataChannelHandlerClient API
     void didChangeReadyState(ReadyState) final;
index 78321ea..5a8d3ed 100644 (file)
@@ -23,6 +23,7 @@
  */
 
 [
+    ActiveDOMObject,
     Conditional=WEB_RTC,
     NoInterfaceObject,
 ] interface RTCDataChannel : EventTarget {
index 7ce871c..8f6df4d 100644 (file)
@@ -178,8 +178,6 @@ private:
 
     std::unique_ptr<RtpTransceiverSet> m_transceiverSet { std::unique_ptr<RtpTransceiverSet>(new RtpTransceiverSet()) };
 
-    Vector<RefPtr<RTCDataChannel>> m_dataChannels;
-
     std::unique_ptr<PeerConnectionBackend> m_backend;
 
     RTCConfiguration m_configuration;