Verify garbage collection of RTCPeerConnection objects

Closed objects with no references are garbage collected. Non-closed objects are not garbage collected. NOTE: This uses a very primitive mechanism (static variable) for collecting the count. It should use a more reasonable one, and the accessor should be marked "only visible in testing". Advice on how to best do that appreciated. Bug: 782099 Change-Id: If44f9e6ecaabc8f0dfebe3ee959c5179308f2b0b Reviewed-on: https://chromium-review.googlesource.com/806195Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Harald Alvestrand <hta@chromium.org> Cr-Commit-Position: refs/heads/master@{#521679}

Verify garbage collection of RTCPeerConnection objects
Closed objects with no references are garbage collected. Non-closed objects are not garbage collected. NOTE: This uses a very primitive mechanism (static variable) for collecting the count. It should use a more reasonable one, and the accessor should be marked "only visible in testing". Advice on how to best do that appreciated. Bug: 782099 Change-Id: If44f9e6ecaabc8f0dfebe3ee959c5179308f2b0b Reviewed-on: https://chromium-review.googlesource.com/806195Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Harald Alvestrand <hta@chromium.org> Cr-Commit-Position: refs/heads/master@{#521679}
cd5a4dfc · Harald Alvestrand · Commit Bot · 9db32735 · cd5a4dfc · cd5a4dfc
Commit cd5a4dfc authored Dec 05, 2017 by Harald Alvestrand Committed by Commit Bot Dec 05, 2017
8 changed files
--- a/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-garbagecollected.html
+++ b/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-garbagecollected.html
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+
+const limit = 10;
+
+function waitAndGc() {
+  return new Promise(function(resolve, reject) {
+    setTimeout(function() {
+      gc();
+      resolve();
+    }, 0);
+  });
+}
+
+promise_test(function(t) {
+  const baseline = internals.peerConnectionCount();
+  let pc;
+  for (let count = 0; count < limit; count++) {
+    pc = new RTCPeerConnection();
+    pc.close();
+  }
+  return waitAndGc().then(function() {
+    // All the non-referenced PCs should now be collected.
+    assert_equals(internals.peerConnectionCount(), baseline + 1);
+    pc = null;
+    return waitAndGc();
+  }).then(function() {
+    gc();
+    assert_equals(internals.peerConnectionCount(), baseline);
+  });
+}, 'Closed Peerconnections are garbage collected');
+
+
+promise_test(function(t) {
+  const baseline = internals.peerConnectionCount();
+  for (let count = 0; count < limit; count++) {
+    pc = new RTCPeerConnection();
+  }
+  pc = null;
+  return waitAndGc().then(function() {
+    assert_equals(baseline + limit, internals.peerConnectionCount());
+  });
+}, 'Non-closed Peerconnections are NOT garbage collected');
+
+</script>
+</body>
+</html>
--- a/third_party/WebKit/Source/modules/BUILD.gn
+++ b/third_party/WebKit/Source/modules/BUILD.gn
@@ -190,6 +190,8 @@ jumbo_source_set("modules_testing") {
    "navigatorcontentutils/testing/NavigatorContentUtilsClientMock.h",
    "peerconnection/testing/InternalsRTCCertificate.cpp",
    "peerconnection/testing/InternalsRTCCertificate.h",
+    "peerconnection/testing/InternalsRTCPeerConnection.cpp",
+    "peerconnection/testing/InternalsRTCPeerConnection.h",
    "serviceworkers/testing/InternalsServiceWorker.cpp",
    "serviceworkers/testing/InternalsServiceWorker.h",
    "speech/testing/InternalsSpeechSynthesis.cpp",

--- a/third_party/WebKit/Source/modules/modules_idl_files.gni
+++ b/third_party/WebKit/Source/modules/modules_idl_files.gni
@@ -741,6 +741,7 @@ modules_testing_dependency_idl_files =
          "mediastream/testing/InternalsMediaStream.idl",
          "navigatorcontentutils/testing/InternalsNavigatorContentUtils.idl",
          "peerconnection/testing/InternalsRTCCertificate.idl",
+          "peerconnection/testing/InternalsRTCPeerConnection.idl",
          "serviceworkers/testing/InternalsServiceWorker.idl",
          "speech/testing/InternalsSpeechSynthesis.idl",
          "vibration/testing/InternalsVibration.idl",

--- a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
+++ b/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
@@ -81,6 +81,7 @@
 #include "modules/peerconnection/RTCTrackEvent.h"
 #include "modules/peerconnection/RTCVoidRequestImpl.h"
 #include "modules/peerconnection/RTCVoidRequestPromiseImpl.h"
+#include "modules/peerconnection/testing/InternalsRTCPeerConnection.h"
 #include "platform/bindings/Microtask.h"
 #include "platform/bindings/ScriptState.h"
 #include "platform/bindings/V8ThrowException.h"
@@ -114,6 +115,9 @@ namespace {
 const char kSignalingStateClosedMessage[] =
    "The RTCPeerConnection's signalingState is 'closed'.";

+// For testing: Keep track of number of existing PeerConnections.
+int g_peer_connection_counter = 0;
+
 bool ThrowExceptionIfSignalingStateClosed(
    RTCPeerConnection::SignalingState state,
    ExceptionState& exception_state) {
@@ -500,6 +504,7 @@ RTCPeerConnection::RTCPeerConnection(ExecutionContext* context,
  // If we fail, set |m_closed| and |m_stopped| to true, to avoid hitting the
  // assert in the destructor.

+  g_peer_connection_counter++;
  if (!document->GetFrame()) {
    closed_ = true;
    stopped_ = true;
@@ -539,6 +544,7 @@ RTCPeerConnection::~RTCPeerConnection() {
  // We are assuming that a wrapper is always created when RTCPeerConnection is
  // created.
  DCHECK(closed_ || stopped_);
+  g_peer_connection_counter--;
 }

 void RTCPeerConnection::Dispose() {
@@ -1769,4 +1775,8 @@ void RTCPeerConnection::Trace(blink::Visitor* visitor) {
  MediaStreamObserver::Trace(visitor);
 }

+int RTCPeerConnection::PeerConnectionCount() {
+  return g_peer_connection_counter;
+}
+
 }  // namespace blink
--- a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
+++ b/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
@@ -214,6 +214,9 @@ class MODULES_EXPORT RTCPeerConnection final
  // We keep the this object alive until either stopped or closed.
  bool HasPendingActivity() const final { return !closed_ && !stopped_; }

+  // For testing; exported to testing/InternalWebRTCPeerConnection
+  static int PeerConnectionCount();
+
  virtual void Trace(blink::Visitor*);

 private:

--- a/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.cpp
+++ b/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.cpp
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "modules/peerconnection/testing/InternalsRTCPeerConnection.h"
+
+namespace blink {
+
+int InternalsRTCPeerConnection::peerConnectionCount(Internals& internals) {
+  return RTCPeerConnection::PeerConnectionCount();
+}
+
+}  // namespace blink
--- a/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.h
+++ b/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.h
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef InternalsRTCPeerConnection_h
+#define InternalsRTCPeerConnection_h
+
+#include "modules/peerconnection/RTCPeerConnection.h"
+#include "platform/wtf/Allocator.h"
+
+namespace blink {
+
+class Internals;
+
+class InternalsRTCPeerConnection {
+  STATIC_ONLY(InternalsRTCPeerConnection);
+
+ public:
+  static int peerConnectionCount(Internals&);
+};
+
+}  // namespace blink
+
+#endif  // InternalsRTCPeerConnection_h
--- a/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.idl
+++ b/third_party/WebKit/Source/modules/peerconnection/testing/InternalsRTCPeerConnection.idl
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+[
+    ImplementedAs=InternalsRTCPeerConnection
+] partial interface Internals {
+    long peerConnectionCount();
+};