From 29540b18795a14c3eaa3bcdbd74b46239a1d2055 Mon Sep 17 00:00:00 2001
From: "fischman@webrtc.org"
 <fischman@webrtc.org@4adac7df-926f-26a2-2b94-8c16560cd09d>
Date: Thu, 17 Apr 2014 22:54:30 +0000
Subject: [PATCH] Revert "PeerConnectionFactory: delay deletion of owned
 threads."

This reverts r5933 because it broke
http://build.chromium.org/p/client.webrtc/builders/Win64%20Release/builds/1598

BUG=3100

Review URL: https://webrtc-codereview.appspot.com/12159004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5935 4adac7df-926f-26a2-2b94-8c16560cd09d
---
 talk/app/webrtc/peerconnectionfactory.cc      | 25 ++-----------------
 .../webrtc/peerconnectionfactory_unittest.cc  | 21 ----------------
 2 files changed, 2 insertions(+), 44 deletions(-)

diff --git a/talk/app/webrtc/peerconnectionfactory.cc b/talk/app/webrtc/peerconnectionfactory.cc
index 89f62a6a8..dc14bfb56 100644
--- a/talk/app/webrtc/peerconnectionfactory.cc
+++ b/talk/app/webrtc/peerconnectionfactory.cc
@@ -183,33 +183,12 @@ PeerConnectionFactory::PeerConnectionFactory(
   // ASSERT(default_adm != NULL);
 }
 
-// Deletes |thread| if it is not the current thread, else causes it to
-// exit and delete itself after completing the currently processing
-// message.
-//
-// NOTE: this is required because:
-// 1) PeerConnection holds a ref on PeerConnectionFactory; and
-// 2) PeerConnectionFactory may own the signaling & worker threads; and
-// 3) PeerConnection is always destroyed on the signaling thread.
-// As a result, if the last ref on PeerConnectionFactory is held by
-// PeerConnection, a naive "delete signaling_thread_;" in
-// ~PeerConnectionFactory() would result in deadlock.  See
-// https://code.google.com/p/webrtc/issues/detail?id=3100 for history.
-static void DeleteOrRelease(talk_base::Thread* thread) {
-  if (thread->IsCurrent()) {
-    thread->Release();  // Causes thread to delete itself after Quit().
-    thread->Quit();
-  } else {
-    delete thread;  // Calls thread->Stop() implicitly.
-  }
-}
-
 PeerConnectionFactory::~PeerConnectionFactory() {
   signaling_thread_->Clear(this);
   signaling_thread_->Send(this, MSG_TERMINATE_FACTORY);
   if (owns_ptrs_) {
-    DeleteOrRelease(signaling_thread_);
-    DeleteOrRelease(worker_thread_);
+    delete signaling_thread_;
+    delete worker_thread_;
   }
 }
 
diff --git a/talk/app/webrtc/peerconnectionfactory_unittest.cc b/talk/app/webrtc/peerconnectionfactory_unittest.cc
index 4d21e1b26..9f9d7881a 100644
--- a/talk/app/webrtc/peerconnectionfactory_unittest.cc
+++ b/talk/app/webrtc/peerconnectionfactory_unittest.cc
@@ -340,24 +340,3 @@ TEST_F(PeerConnectionFactoryTest, LocalRendering) {
   EXPECT_TRUE(capturer->CaptureFrame());
   EXPECT_EQ(2, local_renderer.num_rendered_frames());
 }
-
-// Test that no deadlock or ASSERT triggers on releasing the last
-// reference to a PeerConnectionFactory (regression test for
-// https://code.google.com/p/webrtc/issues/detail?id=3100).
-TEST(PeerConnectionFactory2Test, ThreadTeardown) {
-  talk_base::scoped_refptr<PeerConnectionFactoryInterface> factory(
-      webrtc::CreatePeerConnectionFactory());
-  NullPeerConnectionObserver observer;
-  talk_base::scoped_refptr<PeerConnectionInterface> pc(
-      factory->CreatePeerConnection(
-          webrtc::PeerConnectionInterface::IceServers(),
-          NULL,
-          NULL,
-          &observer));
-  factory = NULL;
-  // Now |pc| holds the last ref to the factory (and thus its
-  // threads).  If the next line, which causes |pc| to be freed,
-  // doesn't ASSERT (in Debug) or deadlock (in Release) then the test
-  // is successful.
-  pc = NULL;
-}