Fix JingleSession class to avoid bogus incompatible protocol error

remoting/protocol/jingle_session.cc

 // Copyright (c) 2012 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 "remoting/protocol/jingle_session.h"
 
 #include "base/bind.h"
 #include "base/rand_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
@@ skipping 325 matching lines @@
   } else {
     LOG(ERROR) << "Failed to send a "
                << JingleMessage::GetActionName(message.action) << " message";
   }
 }
 
 void JingleSession::OnMessageResponse(
     JingleMessage::ActionType request_type,
     IqRequest* request,
     const buzz::XmlElement* response) {
-  std::string type_str = JingleMessage::GetActionName(request_type);
-
   // Delete the request from the list of pending requests.
   pending_requests_.erase(request);
   delete request;
 
+  if (state_ == CLOSED || state_ == FAILED) {
+    // Ignore all responses after session was closed.

[Wez, 2014/08/20 01:45:23]

nit: Suggest moving this comment outside the if()

[Sergey Ulanov, 2014/08/25 17:28:08]

Done.

+    return;
+  }
+
+  std::string type_str = JingleMessage::GetActionName(request_type);
+
   // |response| will be NULL if the request timed out.
   if (!response) {
     LOG(ERROR) << type_str << " request timed out.";
     CloseInternal(SIGNALING_TIMEOUT);
     return;
   } else {
     const std::string& type =
         response->Attr(buzz::QName(std::string(), "type"));
     if (type != "result") {
       LOG(ERROR) << "Received error in response to " << type_str
                  << " message: \"" << response->Str()
                  << "\". Terminating the session.";
 
-      switch (request_type) {
-        case JingleMessage::SESSION_INFO:
-          // session-info is used for the new authentication protocol,
-          // and wasn't previously supported.
-          CloseInternal(INCOMPATIBLE_PROTOCOL);

[Wez, 2014/08/20 01:45:23]

Does this mean we can't ever return INCOMPATIBLE_PROTOCOL now?

[Sergey Ulanov, 2014/08/25 17:28:07]

No, there are some placed where INCOMPATIBLE_PROTOCOL is still used.

-          break;
-
-        default:
-          // TODO(sergeyu): There may be different reasons for error
-          // here. Parse the response stanza to find failure reason.
-          CloseInternal(PEER_IS_OFFLINE);
-      }
+      // TODO(sergeyu): There may be different reasons for error
+      // here. Parse the response stanza to find failure reason.

[Wez, 2014/08/20 01:45:23]

nit: Please file a bug for that work and add a link here. :)

[Sergey Ulanov, 2014/08/25 17:28:08]

This is not a new TODO. I don't think we need a bug for it as it's not something
that will get fixed unless there is a specific error case that we need to handle

+      CloseInternal(PEER_IS_OFFLINE);
     }
   }
 }
 
 void JingleSession::SendTransportInfo() {
   JingleMessage message(peer_jid_, JingleMessage::TRANSPORT_INFO, session_id_);
   message.candidates.swap(pending_candidates_);
 
   scoped_ptr<IqRequest> request = session_manager_->iq_sender()->SendIq(
       message.ToXml(),
@@ skipping 293 matching lines @@
   }
 }
 
 bool JingleSession::is_session_active() {
   return state_ == CONNECTING || state_ == ACCEPTING || state_ == CONNECTED ||
         state_ == AUTHENTICATING || state_ == AUTHENTICATED;
 }
 
 }  // namespace protocol
 }  // namespace remoting