Make sure we process all pending offer/answer requests before terminating.

This fixes a bug in the WebRtcSessionDescriptionFactory where messages would be dropped or worse yet processed after the factory was deleted.

BUG=chromium:507307

Review URL: https://codereview.webrtc.org/1231823002

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

talk/app/webrtc/dtlsidentitystore.cc

@@ -31,7 +31,6 @@
 #include "webrtc/base/logging.h"
 
 using webrtc::DTLSIdentityRequestObserver;
-using webrtc::WebRtcSessionDescriptionFactory;
 
 namespace webrtc {
 

talk/app/webrtc/dtlsidentitystore_unittest.cc

@@ -33,7 +33,6 @@
 #include "webrtc/base/ssladapter.h"
 
 using webrtc::DtlsIdentityStore;
-using webrtc::WebRtcSessionDescriptionFactory;
 
 static const int kTimeoutMs = 10000;
 

talk/app/webrtc/peerconnection.cc

@@ -338,6 +338,7 @@ PeerConnection::PeerConnection(PeerConnectionFactory* factory)
 }
 
 PeerConnection::~PeerConnection() {
+  ASSERT(signaling_thread()->IsCurrent());
   if (mediastream_signaling_)
     mediastream_signaling_->TearDown();
   if (stream_handler_container_)

talk/app/webrtc/webrtcsession.cc

@@ -499,6 +499,7 @@ WebRtcSession::WebRtcSession(
 }
 
 WebRtcSession::~WebRtcSession() {
+  ASSERT(signaling_thread()->IsCurrent());
   // Destroy video_channel_ first since it may have a pointer to the
   // voice_channel_.
   if (video_channel_) {

talk/app/webrtc/webrtcsession_unittest.cc

@@ -3852,6 +3852,38 @@ TEST_F(WebRtcSessionTest, TestSetSocketOptionBeforeBundle) {
   EXPECT_EQ(8000, option_val);
 }
 
+// Test creating a session, request multiple offers, destroy the session
+// and make sure we got success/failure callbacks for all of the requests.
+// Background: crbug.com/507307
+TEST_F(WebRtcSessionTest, CreateOffersAndShutdown) {
+  Init();
+
+  rtc::scoped_refptr<WebRtcSessionCreateSDPObserverForTest> observers[100];
+  PeerConnectionInterface::RTCOfferAnswerOptions options;
+  options.offer_to_receive_audio =
+      RTCOfferAnswerOptions::kOfferToReceiveMediaTrue;
+
+  for (auto& o : observers) {
+    o = new WebRtcSessionCreateSDPObserverForTest();
+    session_->CreateOffer(o, options);
+  }
+
+  session_.reset();
+
+  // Make sure we process pending messages on the current (signaling) thread
+  // before checking we we got our callbacks.  Quit() will do this and then
+  // immediately exit.  We won't need the queue after this point anyway.
+  rtc::Thread::Current()->Quit();
+
+  for (auto& o : observers) {
+    // We expect to have received a notification now even if the session was
+    // terminated.  The offer creation may or may not have succeeded, but we
+    // must have received a notification which, so the only invalid state
+    // is kInit.
+    EXPECT_NE(WebRtcSessionCreateSDPObserverForTest::kInit, o->state());
+  }
+}
+
 // TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled.  That test
 // currently fails because upon disconnection and reconnection OnIceComplete is
 // called more than once without returning to IceGatheringGathering.

talk/app/webrtc/webrtcsessiondescriptionfactory.cc

@@ -40,6 +40,8 @@ namespace webrtc {
 namespace {
 static const char kFailedDueToIdentityFailed[] =
     " failed because DTLS identity request failed";
+static const char kFailedDueToSessionShutdown[] =
+    " failed because the session was shut down";
 
 static const uint64 kInitSessionVersion = 2;
 
@@ -183,6 +185,18 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
 }
 
 WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() {
+  ASSERT(signaling_thread_->IsCurrent());
+
+  // Fail any requests that were asked for before identity generation completed.
+  FailPendingRequests(kFailedDueToSessionShutdown);
+
+  // Process all pending notifications in the message queue.  If we don't do
+  // this, requests will linger and not know they succeeded or failed.
+  rtc::MessageList list;
+  signaling_thread_->Clear(this, rtc::MQID_ANY, &list);
+  for (auto& msg : list)
+    OnMessage(&msg);
+
   transport_desc_factory_.set_identity(NULL);
 }
 
@@ -400,6 +414,19 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
   PostCreateSessionDescriptionSucceeded(request.observer, answer);
 }
 
+void WebRtcSessionDescriptionFactory::FailPendingRequests(
+    const std::string& reason) {
+  ASSERT(signaling_thread_->IsCurrent());
+  while (!create_session_description_requests_.empty()) {
+    const CreateSessionDescriptionRequest& request =
+        create_session_description_requests_.front();
+    PostCreateSessionDescriptionFailed(request.observer,
+        ((request.type == CreateSessionDescriptionRequest::kOffer) ?
+            "CreateOffer" : "CreateAnswer") + reason);
+    create_session_description_requests_.pop();
+  }
+}
+
 void WebRtcSessionDescriptionFactory::PostCreateSessionDescriptionFailed(
     CreateSessionDescriptionObserver* observer, const std::string& error) {
   CreateSessionDescriptionMsg* msg = new CreateSessionDescriptionMsg(observer);
@@ -422,16 +449,7 @@ void WebRtcSessionDescriptionFactory::OnIdentityRequestFailed(int error) {
   LOG(LS_ERROR) << "Async identity request failed: error = " << error;
   identity_request_state_ = IDENTITY_FAILED;
 
-  std::string msg = kFailedDueToIdentityFailed;
-  while (!create_session_description_requests_.empty()) {
-    const CreateSessionDescriptionRequest& request =
-        create_session_description_requests_.front();
-    PostCreateSessionDescriptionFailed(
-        request.observer,
-        ((request.type == CreateSessionDescriptionRequest::kOffer) ?
-            "CreateOffer" : "CreateAnswer") + msg);
-    create_session_description_requests_.pop();
-  }
+  FailPendingRequests(kFailedDueToIdentityFailed);
 }
 
 void WebRtcSessionDescriptionFactory::SetIdentity(

talk/app/webrtc/webrtcsessiondescriptionfactory.h

@@ -133,6 +133,8 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
 
   void InternalCreateOffer(CreateSessionDescriptionRequest request);
   void InternalCreateAnswer(CreateSessionDescriptionRequest request);
+  // Posts failure notifications for all pending session description requests.
+  void FailPendingRequests(const std::string& reason);
   void PostCreateSessionDescriptionFailed(
       CreateSessionDescriptionObserver* observer,
       const std::string& error);
@@ -145,17 +147,16 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
 
   std::queue<CreateSessionDescriptionRequest>
       create_session_description_requests_;
-  rtc::Thread* signaling_thread_;
-  MediaStreamSignaling* mediastream_signaling_;
+  rtc::Thread* const signaling_thread_;
+  MediaStreamSignaling* const mediastream_signaling_;
   cricket::TransportDescriptionFactory transport_desc_factory_;
   cricket::MediaSessionDescriptionFactory session_desc_factory_;
   uint64 session_version_;
   rtc::scoped_ptr<DTLSIdentityServiceInterface> identity_service_;
-  rtc::scoped_refptr<WebRtcIdentityRequestObserver>
-      identity_request_observer_;
-  WebRtcSession* session_;
-  std::string session_id_;
-  cricket::DataChannelType data_channel_type_;
+  rtc::scoped_refptr<WebRtcIdentityRequestObserver> identity_request_observer_;
+  WebRtcSession* const session_;
+  const std::string session_id_;
+  const cricket::DataChannelType data_channel_type_;
   IdentityRequestState identity_request_state_;
 
   DISALLOW_COPY_AND_ASSIGN(WebRtcSessionDescriptionFactory);