Fixes for re-enabling more MSVC level 4 warnings: net/quic/ edition

net/quic/quic_reliable_client_stream.cc

 // Copyright (c) 2012 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 "net/quic/quic_reliable_client_stream.h"
 
 #include "base/callback_helpers.h"
 #include "net/base/net_errors.h"
 #include "net/quic/quic_session.h"
 #include "net/quic/quic_write_blocked_list.h"
 
 namespace net {
 
 QuicReliableClientStream::QuicReliableClientStream(QuicStreamId id,
                                                    QuicSession* session,
                                                    const BoundNetLog& net_log)
     : QuicDataStream(id, session),
       net_log_(net_log),
       delegate_(NULL) {
 }
 
 QuicReliableClientStream::~QuicReliableClientStream() {
   if (delegate_)
     delegate_->OnClose(connection_error());
 }
 
 uint32 QuicReliableClientStream::ProcessData(const char* data,
                                              uint32 data_len) {
   // TODO(rch): buffer data if we don't have a delegate.
+  // !!! Callers seem to assume this will return 0 (or some value less than
+  // |data_len|) on error!  Returning a very large positive value may be wrong?
   if (!delegate_)
-    return ERR_ABORTED;
+    return static_cast<uint32>(ERR_ABORTED);

[Ryan Hamilton, 2014/07/08 03:37:08]

Yes, this seems problematic. :> It should probably do something very similar to
the rv != OK block below. I'll take care of fixing this in a different CL.

[Peter Kasting, 2014/07/08 06:26:46]

So do you want me to just remove the comment and leave the static cast here for
now?

[Ryan Hamilton, 2014/07/08 18:00:57]

Either way. The comment is a good reminder in any case :>

[Peter Kasting, 2014/07/08 18:16:46]

I think what I'll do, then, is change to something like this, assuming you don't
mind:

  // TODO(rch): buffer data if we don't have a delegate.
  // Also, if we're going to return 0 here, we should probably do something
  // like the "rv != OK" code below.
  if (!delegate_)
    return 0;

[Ryan Hamilton, 2014/07/08 18:24:01]

Sure, that's fine. Or you can simply do:

  if (!delegate_) {
    DLOG(ERROR) << "Missing delegate";
    Reset(QUIC_STREAM_CANCELLED);
    return 0;
  }

Which I was planning to land in https://codereview.chromium.org/372343002/ But
if you want to do it here, that's totally good too.

[Peter Kasting, 2014/07/08 18:44:45]

Ah!  I'd rather let you make the functional change through the normal review
process if you've already got it in the pipeline.  I'll just revert my change to
this file and leave it in your hands.

 
   int rv = delegate_->OnDataReceived(data, data_len);
   if (rv != OK) {
     DLOG(ERROR) << "Delegate refused data, rv: " << rv;
     Reset(QUIC_BAD_APPLICATION_PAYLOAD);
     return 0;
   }
   return data_len;
 }
 
@@ skipping 54 matching lines @@
   bool can_write =  session()->connection()->CanWrite(HAS_RETRANSMITTABLE_DATA);
   if (!can_write) {
     session()->MarkWriteBlocked(id(), EffectivePriority());
     DCHECK(callback_.is_null());
     callback_ = callback;
   }
   return can_write;
 }
 
 }  // namespace net