[Draft] Fix canceling pin prompt causes host overload

remoting/protocol/jingle_session.cc

Index: remoting/protocol/jingle_session.cc
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index 2aecd4c25d6856d86a4f8f0ab1645219edad48a1..f590b6308fbd2c2a815f4f39f51d9c8a63a894ec 100644
--- a/remoting/protocol/jingle_session.cc
+++ b/remoting/protocol/jingle_session.cc
@@ -181,6 +181,9 @@ void JingleSession::ContinueAcceptIncomingConnection() {
     SetState(AUTHENTICATED);
   } else {
     DCHECK_EQ(authenticator_->state(), Authenticator::WAITING_MESSAGE);
+    if (authenticator_->started()) {

[rmsousa, 2014/03/26 21:37:35]

Do we want to ensure that we pass through the AUTHENTICATING state between
CONNECTED and AUTHENTICATED? if so, this should be before the outer if block.

It might be useful to keep the backoff behavior consistent between
authenticators that start immediately or not, but I don't really see a practical
consequence if we don't do that.

[kelvinp, 2014/03/27 03:23:21]

After talking to Sergey, I think we can allow switching directly from connected
to authenticated.

+      SetState(AUTHENTICATING);
+    }
   }
 
   return;

[Sergey Ulanov, 2014/03/26 01:49:16]

while you are here, please remove this line

[kelvinp, 2014/03/27 03:23:21]

Done.

@@ -485,7 +488,7 @@ void JingleSession::OnSessionInfo(const JingleMessage& message,
     return;
   }
 
-  if (state_ != CONNECTED ||
+  if ((state_ != CONNECTED && state_ != AUTHENTICATING) ||
       authenticator_->state() != Authenticator::WAITING_MESSAGE) {
     LOG(WARNING) << "Received unexpected authenticator message "
                  << message.info->Str();
@@ -518,7 +521,7 @@ void JingleSession::ProcessTransportInfo(const JingleMessage& message) {
 void JingleSession::OnTerminate(const JingleMessage& message,
                                 const ReplyCallback& reply_callback) {
   if (state_ != CONNECTING && state_ != ACCEPTING && state_ != CONNECTED &&
-      state_ != AUTHENTICATED) {
+      state_ != AUTHENTICATING && state_ != AUTHENTICATED) {
     LOG(WARNING) << "Received unexpected session-terminate message.";
     reply_callback.Run(JingleMessageReply::UNEXPECTED_REQUEST);
     return;
@@ -577,13 +580,17 @@ void JingleSession::ProcessAuthenticationStep() {
   DCHECK(CalledOnValidThread());
   DCHECK_NE(authenticator_->state(), Authenticator::PROCESSING_MESSAGE);
 
-  if (state_ != CONNECTED) {
+  if (state_ != CONNECTED && state_ != AUTHENTICATING) {
     DCHECK(state_ == FAILED || state_ == CLOSED);
     // The remote host closed the connection while the authentication was being
     // processed asynchronously, nothing to do.
     return;
   }
 
+  if (authenticator_->started()) {

[rmsousa, 2014/03/26 21:37:35]

authenticator may set the started bit in GetNextMessage, so this should be after
the if block below.

[Sergey Ulanov, 2014/03/27 00:00:03]

+1 I think it should be an else-if case in the end of this method.

[kelvinp, 2014/03/27 03:23:21]

Done.

[kelvinp, 2014/03/27 03:23:21]

Done.

+      SetState(AUTHENTICATING);

[Sergey Ulanov, 2014/03/26 01:49:16]

nit: 2-space indent relative to if

[kelvinp, 2014/03/27 03:23:21]

My bad. Fixed

Done.

+  }
+
   if (authenticator_->state() == Authenticator::MESSAGE_READY) {
     JingleMessage message(peer_jid_, JingleMessage::SESSION_INFO, session_id_);
     message.info = authenticator_->GetNextMessage();
@@ -604,7 +611,7 @@ void JingleSession::CloseInternal(ErrorCode error) {
   DCHECK(CalledOnValidThread());
 
   if (state_ == CONNECTING || state_ == ACCEPTING || state_ == CONNECTED ||
-      state_ == AUTHENTICATED) {
+      state_ == AUTHENTICATING || state_ == AUTHENTICATED) {

[Sergey Ulanov, 2014/03/26 01:49:16]

this is duplicated in OnTerminate(). Maybe add is_session_active() to avoid
duplication.

[kelvinp, 2014/03/27 03:23:21]

Good Idea. Done.

     // Send session-terminate message with the appropriate error code.
     JingleMessage::Reason reason;
     switch (error) {