[GCM] Investigatory CHECKs for crash in parsing stream

The various places where size_t and uint64 were being used have been converted
to int, so that we can better check to make sure they're non-negative (they're
actually consumed as ints by the protobuf parsing code). Various CHECKS have
therefore been added to verify assumptions.

BUG=409985

Review URL: https://codereview.chromium.org/600223003

Cr-Commit-Position: refs/heads/master@{#296741}

4 files changed, 30 insertions, 25 deletions

diff --git a/google_apis/gcm/base/socket_stream.cc b/google_apis/gcm/base/socket_stream.cc
index 14889e1..edf723c 100644
--- a/google_apis/gcm/base/socket_stream.cc
+++ b/google_apis/gcm/base/socket_stream.cc
@@ -56,8 +56,9 @@ bool SocketInputStream::Next(const void** data, int* size) {
 
 void SocketInputStream::BackUp(int count) {
   DCHECK(GetState() == READY || GetState() == EMPTY);
-  DCHECK_GT(count, 0);
-  DCHECK_LE(count, next_pos_);
+  // TODO(zea): investigating crbug.com/409985
+  CHECK_GT(count, 0);
+  CHECK_LE(count, next_pos_);
 
   next_pos_ -= count;
   DVLOG(1) << "Backing up " << count << " bytes in input buffer. "
@@ -76,7 +77,7 @@ int64 SocketInputStream::ByteCount() const {
   return next_pos_;
 }
 
-size_t SocketInputStream::UnreadByteCount() const {
+int SocketInputStream::UnreadByteCount() const {
   DCHECK_NE(GetState(), CLOSED);
   DCHECK_NE(GetState(), READING);
   return read_buffer_->BytesConsumed() - next_pos_;
@@ -137,6 +138,8 @@ void SocketInputStream::RebuildBuffer() {
     DVLOG(1) << "Have " << unread_data_size << " unread bytes remaining.";
   }
   read_buffer_->DidConsume(unread_data_size);
+  // TODO(zea): investigating crbug.com/409985
+  CHECK_GE(UnreadByteCount(), 0);
 }
 
 net::Error SocketInputStream::last_error() const {
@@ -179,6 +182,8 @@ void SocketInputStream::RefreshCompletionCallback(
   DCHECK_GT(result, 0);
   last_error_ = net::OK;
   read_buffer_->DidConsume(result);
+  // TODO(zea): investigating crbug.com/409985
+  CHECK_GT(UnreadByteCount(), 0);
 
   DVLOG(1) << "Refresh complete with " << result << " new bytes. "
            << "Current position " << next_pos_
diff --git a/google_apis/gcm/base/socket_stream.h b/google_apis/gcm/base/socket_stream.h
index a458420..61aa1fe 100644
--- a/google_apis/gcm/base/socket_stream.h
+++ b/google_apis/gcm/base/socket_stream.h
@@ -72,7 +72,7 @@ class GCM_EXPORT SocketInputStream
   virtual int64 ByteCount() const OVERRIDE;
 
   // The remaining amount of valid data available to be read.
-  size_t UnreadByteCount() const;
+  int UnreadByteCount() const;
 
   // Reads from the socket, appending a max of |byte_limit| bytes onto the read
   // buffer. net::ERR_IO_PENDING is returned if the refresh can't complete
diff --git a/google_apis/gcm/base/socket_stream_unittest.cc b/google_apis/gcm/base/socket_stream_unittest.cc
index d7ba679..10c1f8c 100644
--- a/google_apis/gcm/base/socket_stream_unittest.cc
+++ b/google_apis/gcm/base/socket_stream_unittest.cc
@@ -20,11 +20,11 @@ typedef std::vector<net::MockRead> ReadList;
 typedef std::vector<net::MockWrite> WriteList;
 
 const char kReadData[] = "read_data";
-const uint64 kReadDataSize = arraysize(kReadData) - 1;
+const int kReadDataSize = arraysize(kReadData) - 1;
 const char kReadData2[] = "read_alternate_data";
-const uint64 kReadData2Size = arraysize(kReadData2) - 1;
+const int kReadData2Size = arraysize(kReadData2) - 1;
 const char kWriteData[] = "write_data";
-const uint64 kWriteDataSize = arraysize(kWriteData) - 1;
+const int kWriteDataSize = arraysize(kWriteData) - 1;
 
 class GCMSocketStreamTest : public testing::Test {
  public:
@@ -38,12 +38,12 @@ class GCMSocketStreamTest : public testing::Test {
   void PumpLoop();
 
   // Simulates a google::protobuf::io::CodedInputStream read.
-  base::StringPiece DoInputStreamRead(uint64 bytes);
+  base::StringPiece DoInputStreamRead(int bytes);
   // Simulates a google::protobuf::io::CodedOutputStream write.
-  uint64 DoOutputStreamWrite(const base::StringPiece& write_src);
+  int DoOutputStreamWrite(const base::StringPiece& write_src);
 
   // Synchronous Refresh wrapper.
-  void WaitForData(size_t msg_size);
+  void WaitForData(int msg_size);
 
   base::MessageLoop* message_loop() { return &message_loop_; };
   net::DelayedSocketData* data_provider() { return data_provider_.get(); }
@@ -101,8 +101,8 @@ void GCMSocketStreamTest::PumpLoop() {
   run_loop.RunUntilIdle();
 }
 
-base::StringPiece GCMSocketStreamTest::DoInputStreamRead(uint64 bytes) {
-  uint64 total_bytes_read = 0;
+base::StringPiece GCMSocketStreamTest::DoInputStreamRead(int bytes) {
+  int total_bytes_read = 0;
   const void* initial_buffer = NULL;
   const void* buffer = NULL;
   int size = 0;
@@ -130,22 +130,22 @@ base::StringPiece GCMSocketStreamTest::DoInputStreamRead(uint64 bytes) {
                            total_bytes_read);
 }
 
-uint64 GCMSocketStreamTest::DoOutputStreamWrite(
+int GCMSocketStreamTest::DoOutputStreamWrite(
     const base::StringPiece& write_src) {
   DCHECK_EQ(socket_output_stream_->GetState(), SocketOutputStream::EMPTY);
-  uint64 total_bytes_written = 0;
+  int total_bytes_written = 0;
   void* buffer = NULL;
   int size = 0;
-  size_t bytes = write_src.size();
+  int bytes = write_src.size();
 
   do {
     if (!socket_output_stream_->Next(&buffer, &size))
       break;
-    uint64 bytes_to_write = (static_cast<uint64>(size) < bytes ? size : bytes);
+    int bytes_to_write = (size < bytes ? size : bytes);
     memcpy(buffer,
            write_src.data() + total_bytes_written,
            bytes_to_write);
-    if (bytes_to_write < static_cast<uint64>(size))
+    if (bytes_to_write < size)
       socket_output_stream_->BackUp(size - bytes_to_write);
     total_bytes_written += bytes_to_write;
   } while (total_bytes_written < bytes);
@@ -159,7 +159,7 @@ uint64 GCMSocketStreamTest::DoOutputStreamWrite(
   return total_bytes_written;
 }
 
-void GCMSocketStreamTest::WaitForData(size_t msg_size) {
+void GCMSocketStreamTest::WaitForData(int msg_size) {
   while (input_stream()->UnreadByteCount() < msg_size) {
     base::RunLoop run_loop;
     if (input_stream()->Refresh(run_loop.QuitClosure(),
@@ -207,8 +207,8 @@ TEST_F(GCMSocketStreamTest, ReadDataSync) {
 
 // A read that comes in two parts.
 TEST_F(GCMSocketStreamTest, ReadPartialDataSync) {
-  size_t first_read_len = kReadDataSize / 2;
-  size_t second_read_len = kReadDataSize - first_read_len;
+  int first_read_len = kReadDataSize / 2;
+  int second_read_len = kReadDataSize - first_read_len;
   ReadList read_list;
   read_list.push_back(
       net::MockRead(net::SYNCHRONOUS,
@@ -227,8 +227,8 @@ TEST_F(GCMSocketStreamTest, ReadPartialDataSync) {
 
 // A read where no data is available at first (IO_PENDING will be returned).
 TEST_F(GCMSocketStreamTest, ReadAsync) {
-  size_t first_read_len = kReadDataSize / 2;
-  size_t second_read_len = kReadDataSize - first_read_len;
+  int first_read_len = kReadDataSize / 2;
+  int second_read_len = kReadDataSize - first_read_len;
   ReadList read_list;
   read_list.push_back(
       net::MockRead(net::SYNCHRONOUS, net::ERR_IO_PENDING));
diff --git a/google_apis/gcm/engine/connection_handler_impl.cc b/google_apis/gcm/engine/connection_handler_impl.cc
index d47e0f2..751b1cd 100644
--- a/google_apis/gcm/engine/connection_handler_impl.cc
+++ b/google_apis/gcm/engine/connection_handler_impl.cc
@@ -184,9 +184,9 @@ void ConnectionHandlerImpl::WaitForData(ProcessingState state) {
   }
 
   // Used to determine whether a Socket::Read is necessary.
-  size_t min_bytes_needed = 0;
+  int min_bytes_needed = 0;
   // Used to limit the size of the Socket::Read.
-  size_t max_bytes_needed = 0;
+  int max_bytes_needed = 0;
 
   switch(state) {
     case MCS_VERSION_TAG_AND_SIZE:
@@ -214,7 +214,7 @@ void ConnectionHandlerImpl::WaitForData(ProcessingState state) {
   }
   DCHECK_GE(max_bytes_needed, min_bytes_needed);
 
-  size_t unread_byte_count = input_stream_->UnreadByteCount();
+  int unread_byte_count = input_stream_->UnreadByteCount();
   if (min_bytes_needed > unread_byte_count &&
       input_stream_->Refresh(
           base::Bind(&ConnectionHandlerImpl::WaitForData,