From a1ce083005fa664f7a80da4eca1700b534c4185c Mon Sep 17 00:00:00 2001 From: "zea@chromium.org" Date: Thu, 17 Jul 2014 09:48:08 +0000 Subject: [PATCH] [GCM] Add a SelectiveAck test for midstream acks SelectiveAck was over confirming due to unsigned in misuse. This adds a test that exercises the confirm logic used by SelectiveAck. BUG=392664 Review URL: https://codereview.chromium.org/383993006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283729 0039d316-1c4b-4281-b951-d872f2087c98 --- google_apis/gcm/engine/mcs_client.cc | 6 +- google_apis/gcm/engine/mcs_client_unittest.cc | 77 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/google_apis/gcm/engine/mcs_client.cc b/google_apis/gcm/engine/mcs_client.cc index a99a333a37e0..fbabda16f12c 100644 --- a/google_apis/gcm/engine/mcs_client.cc +++ b/google_apis/gcm/engine/mcs_client.cc @@ -770,7 +770,7 @@ void MCSClient::HandleStreamAck(StreamId last_stream_id_received) { void MCSClient::HandleSelectiveAck(const PersistentIdList& id_list) { std::set remaining_ids(id_list.begin(), id_list.end()); - StreamId last_stream_id_received = -1; + StreamId last_stream_id_received = 0; // First check the to_resend_ queue. Acknowledgments are always contiguous, // so if there's a pending message that hasn't been acked, all newer messages @@ -839,6 +839,10 @@ void MCSClient::HandleSelectiveAck(const PersistentIdList& id_list) { to_send_.push_front(to_resend_.back()); to_resend_.pop_back(); } + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&MCSClient::MaybeSendMessage, + weak_ptr_factory_.GetWeakPtr())); } void MCSClient::HandleServerConfirmedReceipt(StreamId device_stream_id) { diff --git a/google_apis/gcm/engine/mcs_client_unittest.cc b/google_apis/gcm/engine/mcs_client_unittest.cc index 915b25d18db1..6b23084bb9cb 100644 --- a/google_apis/gcm/engine/mcs_client_unittest.cc +++ b/google_apis/gcm/engine/mcs_client_unittest.cc @@ -548,6 +548,83 @@ TEST_F(MCSClientTest, SendMessageRMQPartialAckOnReconnect) { EXPECT_TRUE(GetFakeHandler()->AllOutgoingMessagesReceived()); } +// Handle a selective ack that only acks some messages. The remaining unacked +// messages should be resent. On restart, those same unacked messages should be +// resent, and any pending acks for incoming messages should also be resent. +TEST_F(MCSClientTest, SelectiveAckMidStream) { + BuildMCSClient(); + InitializeClient(); + LoginClient(std::vector()); + + // Server stream id 2 ("s1"). + // Acks client stream id 0 (login). + MCSMessage sMessage1(BuildDataMessage( + "from", "category", "X", 0, "s1", kTTLValue, 1, 0, "", 0)); + GetFakeHandler()->ReceiveMessage(sMessage1); + WaitForMCSEvent(); + PumpLoop(); + + // Client stream id 1 ("1"). + // Acks server stream id 2 ("s1"). + MCSMessage cMessage1(BuildDataMessage( + "from", "category", "Y", 2, "1", kTTLValue, 1, 0, "", 0)); + GetFakeHandler()->ExpectOutgoingMessage(cMessage1); + mcs_client()->SendMessage(cMessage1); + PumpLoop(); + EXPECT_TRUE(GetFakeHandler()->AllOutgoingMessagesReceived()); + + // Server stream id 3 ("s2"). + // Acks client stream id 1 ("1"). + // Confirms ack of server stream id 2 ("s1"). + MCSMessage sMessage2(BuildDataMessage( + "from", "category", "X", 1, "s2", kTTLValue, 1, 0, "", 0)); + GetFakeHandler()->ReceiveMessage(sMessage2); + WaitForMCSEvent(); + PumpLoop(); + + // Client Stream id 2 ("2"). + // Acks server stream id 3 ("s2"). + MCSMessage cMessage2(BuildDataMessage( + "from", "category", "Y", 3, "2", kTTLValue, 1, 0, "", 0)); + GetFakeHandler()->ExpectOutgoingMessage(cMessage2); + mcs_client()->SendMessage(cMessage2); + PumpLoop(); + EXPECT_TRUE(GetFakeHandler()->AllOutgoingMessagesReceived()); + + // Simulate the last message being dropped by having the server selectively + // ack client message "1". + // Client message "2" should be resent, acking server stream id 4 (selective + // ack). + MCSMessage cMessage3(BuildDataMessage( + "from", "category", "Y", 4, "2", kTTLValue, 1, 0, "", 0)); + GetFakeHandler()->ExpectOutgoingMessage(cMessage3); + std::vector acked_ids(1, "1"); + scoped_ptr ack(BuildSelectiveAck(acked_ids)); + GetFakeHandler()->ReceiveMessage( + MCSMessage(kIqStanzaTag, + ack.PassAs())); + WaitForMCSEvent(); + PumpLoop(); + EXPECT_TRUE(GetFakeHandler()->AllOutgoingMessagesReceived()); + + // Rebuild the client without any further acks from server. Note that this + // resets the stream ids. + // Sever message "s2" should be acked as part of login. + // Client message "2" should be resent. + StoreCredentials(); + BuildMCSClient(); + InitializeClient(); + + acked_ids[0] = "s2"; + LoginClient(acked_ids); + + MCSMessage cMessage4(BuildDataMessage( + "from", "category", "Y", 1, "2", kTTLValue, 1, 0, "", 0)); + GetFakeHandler()->ExpectOutgoingMessage(cMessage4); + PumpLoop(); + EXPECT_TRUE(GetFakeHandler()->AllOutgoingMessagesReceived()); +} + // Receive some messages. On restart, the login request should contain the // appropriate acknowledged ids. TEST_F(MCSClientTest, AckOnLogin) {