Improve connection backoff logic in xmpp client

Do not reset the connection attempts counter immediately on reaching
Established state. Do it only after 3 keepalives have been received.

This ensures that the client backs off it's connection attempts even
for cases where the state machine goes to Established, but then goes
down right away.  For example, when the server closes the connection
because some higher level requirement is not satisfied.

Change-Id: Ie6d68c1bef910faf3b7b6adc7a236127ffc9a0ea
Closes-Bug: 1488636

src/xmpp/test/xmpp_client_sm_test.cc

@@ -135,7 +135,7 @@ class XmppStateMachineTest : public ::testing::Test {
             EXPECT_TRUE(HoldTimerRunning());
             EXPECT_TRUE(sm_->session() != NULL);
             EXPECT_TRUE(connection_->session() != NULL);
-            EXPECT_TRUE(sm_->GetConnectTime() == 0);
+            EXPECT_TRUE(sm_->get_connect_attempts() != 0);
             break;
         default:
             ASSERT_TRUE(false);
@@ -635,6 +635,40 @@ TEST_F(XmppStateMachineTest, Connect_EvTcpConnectFailed_EvTcpConnected) {
     VerifyState(xmsm::OPENSENT);
 }
 
+TEST_F(XmppStateMachineTest, ConnectionBackoff) {
+    sm_->connect_attempts_inc(); // set attempts as we do not
+                                 // want connection timer to expire
+    sm_->connect_attempts_inc(); // set attempts as we do not
+                                 // want connection timer to expire
+    VerifyState(xmsm::IDLE);
+    EvStart();
+    VerifyState(xmsm::ACTIVE);
+
+    for (int idx = 0; idx <= 6; ++idx) {
+        EvConnectTimerExpired();
+        VerifyState(xmsm::CONNECT);
+
+        EvTcpConnected();
+        VerifyState(xmsm::OPENSENT);
+
+        EvXmppOpen();
+        VerifyState(xmsm::ESTABLISHED);
+
+        if (idx == 6)
+            break;
+
+        EvTcpClose();
+        VerifyState(xmsm::ACTIVE);
+    }
+
+    TASK_UTIL_EXPECT_TRUE(sm_->get_connect_attempts() >= 6);
+    EvXmppKeepalive();
+    EvXmppKeepalive();
+    EvXmppKeepalive();
+    TASK_UTIL_EXPECT_EQ(3, sm_->get_keepalive_count());
+    TASK_UTIL_EXPECT_EQ(0, sm_->get_connect_attempts());
+}
+
 }
 
 static void SetUp() {

src/xmpp/xmpp_state_machine.cc

@@ -210,6 +210,7 @@ struct Active : public sc::state<Active, XmppStateMachine> {
     Active(my_context ctx) : my_base(ctx) {
         XmppStateMachine *state_machine = &context<XmppStateMachine>();
         SM_LOG(state_machine, "(Xmpp Active State)");
+        state_machine->keepalive_count_clear();
         bool flap = (state_machine->get_state() == ESTABLISHED);
         state_machine->set_state(ACTIVE);
         if (flap) {
@@ -701,7 +702,6 @@ struct XmppStreamEstablished :
     XmppStreamEstablished(my_context ctx) : my_base(ctx) {
         XmppStateMachine *state_machine = &context<XmppStateMachine>();
         SM_LOG(state_machine, "(XMPP Established)");
-        state_machine->connect_attempts_clear();
         XmppConnection *connection = state_machine->connection();
         state_machine->StartHoldTimer();
         state_machine->set_state(ESTABLISHED);
@@ -734,6 +734,10 @@ struct XmppStreamEstablished :
         if (event.session != state_machine->session()) {
             return discard_event();
         }
+        state_machine->keepalive_count_inc();
+        if (state_machine->get_keepalive_count() == 3) {
+            state_machine->connect_attempts_clear();
+        }
         state_machine->StartHoldTimer();
         return discard_event();
     }
@@ -867,6 +871,7 @@ XmppStateMachine::XmppStateMachine(XmppConnection *connection, bool active)
           TaskScheduler::GetInstance()->GetTaskId("xmpp::StateMachine"), 0)),
       hold_time_(GetConfiguredHoldTime()),
       attempts_(0),
+      keepalive_count_(0),
       deleted_(false),
       in_dequeue_(false),
       is_active_(active),
@@ -1165,7 +1170,7 @@ void XmppStateMachine::AssignSession() {
 const int XmppStateMachine::kConnectInterval;
 
 int XmppStateMachine::GetConnectTime() const {
-    int backoff = min(attempts_, 6);
+    int backoff = attempts_ > 6 ? 6 : attempts_;
     return std::min(backoff ? 1 << (backoff - 1) : 0, kConnectInterval);
 }
 

src/xmpp/xmpp_state_machine.h

@@ -112,7 +112,11 @@ class XmppStateMachine :
 
     void connect_attempts_inc() { attempts_++; }
     void connect_attempts_clear() { attempts_ = 0; }
-    int get_connect_attempts() { return attempts_; }
+    int get_connect_attempts() const { return attempts_; }
+
+    void keepalive_count_inc() { keepalive_count_++; }
+    void keepalive_count_clear() { keepalive_count_ = 0; }
+    int get_keepalive_count() const { return keepalive_count_; }
 
     int hold_time() const { return hold_time_; }
     virtual int hold_time_msecs() const { return hold_time_ * 1000; }
@@ -151,7 +155,8 @@ class XmppStateMachine :
     Timer *open_timer_;
     Timer *hold_timer_;
     int hold_time_;
-    int attempts_;
+    uint32_t attempts_;
+    uint32_t keepalive_count_;
     bool deleted_;
     bool in_dequeue_;
     bool is_active_;