Fixes for re-enabling more MSVC level 4 warnings: misc edition #2

google_apis/gcm/engine/mcs_client.cc

 // Copyright 2013 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 "google_apis/gcm/engine/mcs_client.h"
 
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 752 matching lines @@
       acked_outgoing_persistent_ids,
       base::Bind(&MCSClient::OnGCMUpdateFinished,
                  weak_ptr_factory_.GetWeakPtr()));
 
   HandleServerConfirmedReceipt(last_stream_id_received);
 }
 
 void MCSClient::HandleSelectiveAck(const PersistentIdList& id_list) {
   std::set<PersistentId> remaining_ids(id_list.begin(), id_list.end());
 
-  StreamId last_stream_id_received = -1;
+  // !!! This looks really wrong, shouldn't this be zero?
+  StreamId last_stream_id_received = 0xFFFFFFFF;

[Peter Kasting, 2014/07/09 22:19:54]

This needs review.  See how code below tries to check whether other values are
greater than this.

[Nicolas Zea, 2014/07/09 22:52:35]

You're right, this is broken. The lines at 815 are effectively dead code at the
moment. I suspect the only effect this has though is one of inefficiency.

And yes, the right solution here is to set the value to 0. I've filed
crbug.com/392664 to better test this code (looks like it wasn't verifying the
internal ack state properly).

[Peter Kasting, 2014/07/09 23:34:25]

Done.

 
   // 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
   // must also be unacked.
   while(!to_resend_.empty() && !remaining_ids.empty()) {
     const MCSPacketInternal& outgoing_packet = to_resend_.front();
     if (remaining_ids.count(outgoing_packet->persistent_id) == 0)
       break;  // Newer message must be unacked too.
     remaining_ids.erase(outgoing_packet->persistent_id);
     NotifyMessageSendStatus(*outgoing_packet->protobuf, SENT);
@@ skipping 117 matching lines @@
         reinterpret_cast<mcs_proto::DataMessageStanza*>(packet->protobuf.get());
     CollapseKey collapse_key(*data_message);
     if (collapse_key.IsValid())
       collapse_key_map_.erase(collapse_key);
   }
 
   return packet;
 }
 
 } // namespace gcm