From 1354ead075dd9b41f4a415d8b2fd9820416d26b1 Mon Sep 17 00:00:00 2001
From: tommi <tommi@chromium.org>
Date: Sun, 23 Nov 2014 09:38:49 -0800
Subject: [PATCH] Change RtcDataChannel to release the channel and free the
 observer when WebKit no longer needs the datachannel.

Based on http://crrev.com/748783002

BUG=434972

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

Cr-Commit-Position: refs/heads/master@{#305390}
---
 content/renderer/media/rtc_data_channel_handler.cc | 22 +++++++++++++++++++---
 content/renderer/media/rtc_data_channel_handler.h  |  7 +++++--
 .../media/rtc_peer_connection_handler_unittest.cc  |  1 +
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/content/renderer/media/rtc_data_channel_handler.cc b/content/renderer/media/rtc_data_channel_handler.cc
index 084a02018cb9..e37adf35c160 100644
--- a/content/renderer/media/rtc_data_channel_handler.cc
+++ b/content/renderer/media/rtc_data_channel_handler.cc
@@ -50,7 +50,10 @@ RtcDataChannelHandler::Observer::Observer(
   channel_->RegisterObserver(this);
 }
 
-RtcDataChannelHandler::Observer::~Observer() {}
+RtcDataChannelHandler::Observer::~Observer() {
+  DVLOG(3) << "dtor";
+  DCHECK(!channel_.get()) << "Unregister hasn't been called.";
+}
 
 const scoped_refptr<base::SingleThreadTaskRunner>&
 RtcDataChannelHandler::Observer::main_thread() const {
@@ -59,12 +62,19 @@ RtcDataChannelHandler::Observer::main_thread() const {
 
 const scoped_refptr<webrtc::DataChannelInterface>&
 RtcDataChannelHandler::Observer::channel() const {
+  DCHECK(main_thread_->BelongsToCurrentThread());
   return channel_;
 }
 
-void RtcDataChannelHandler::Observer::ClearHandler() {
+void RtcDataChannelHandler::Observer::Unregister() {
   DCHECK(main_thread_->BelongsToCurrentThread());
   handler_ = nullptr;
+  if (channel_.get()) {
+    channel_->UnregisterObserver();
+    // Now that we're guaranteed to not get further OnStateChange callbacks,
+    // it's safe to release our reference to the channel.
+    channel_ = nullptr;
+  }
 }
 
 void RtcDataChannelHandler::Observer::OnStateChange() {
@@ -127,13 +137,18 @@ RtcDataChannelHandler::RtcDataChannelHandler(
 RtcDataChannelHandler::~RtcDataChannelHandler() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "::dtor";
-  observer_->ClearHandler();
+  DCHECK(!observer_.get());
 }
 
 void RtcDataChannelHandler::setClient(
     blink::WebRTCDataChannelHandlerClient* client) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(3) << "setClient " << client;
   webkit_client_ = client;
+  if (!client) {
+    observer_->Unregister();
+    observer_ = nullptr;
+  }
 }
 
 blink::WebString RtcDataChannelHandler::label() {
@@ -273,6 +288,7 @@ void RtcDataChannelHandler::OnMessage(scoped_ptr<webrtc::DataBuffer> buffer) {
 }
 
 void RtcDataChannelHandler::RecordMessageSent(size_t num_bytes) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Currently, messages are capped at some fairly low limit (16 Kb?)
   // but we may allow unlimited-size messages at some point, so making
   // the histogram maximum quite large (100 Mb) to have some
diff --git a/content/renderer/media/rtc_data_channel_handler.h b/content/renderer/media/rtc_data_channel_handler.h
index 79addde17283..efcec2d3b189 100644
--- a/content/renderer/media/rtc_data_channel_handler.h
+++ b/content/renderer/media/rtc_data_channel_handler.h
@@ -74,7 +74,10 @@ class CONTENT_EXPORT RtcDataChannelHandler
     const scoped_refptr<base::SingleThreadTaskRunner>& main_thread() const;
     const scoped_refptr<webrtc::DataChannelInterface>& channel() const;
 
-    void ClearHandler();
+    // Clears the internal |handler_| pointer so that no further callbacks
+    // will be attempted, disassociates this observer from the channel and
+    // releases the channel pointer. Must be called on the main thread.
+    void Unregister();
 
    private:
     friend class base::RefCountedThreadSafe<RtcDataChannelHandler::Observer>;
@@ -89,7 +92,7 @@ class CONTENT_EXPORT RtcDataChannelHandler
 
     RtcDataChannelHandler* handler_;
     const scoped_refptr<base::SingleThreadTaskRunner> main_thread_;
-    const scoped_refptr<webrtc::DataChannelInterface> channel_;
+    scoped_refptr<webrtc::DataChannelInterface> channel_;
   };
 
   scoped_refptr<Observer> observer_;
diff --git a/content/renderer/media/rtc_peer_connection_handler_unittest.cc b/content/renderer/media/rtc_peer_connection_handler_unittest.cc
index 27d2d4e800a5..5d2f6483a60e 100644
--- a/content/renderer/media/rtc_peer_connection_handler_unittest.cc
+++ b/content/renderer/media/rtc_peer_connection_handler_unittest.cc
@@ -967,6 +967,7 @@ TEST_F(RTCPeerConnectionHandlerTest, CreateDataChannel) {
       pc_handler_->createDataChannel("d1", blink::WebRTCDataChannelInit()));
   EXPECT_TRUE(channel.get() != NULL);
   EXPECT_EQ(label, channel->label());
+  channel->setClient(nullptr);
 }
 
 TEST_F(RTCPeerConnectionHandlerTest, CreateDtmfSender) {
-- 
2.11.4.GIT