Added and implemented HttpStream::GetTotalSentBytes for basic streams.

net/http/http_basic_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/http/http_basic_stream.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_body_drainer.h"
 #include "net/http/http_stream_parser.h"
@@ skipping 69 matching lines @@
 bool HttpBasicStream::IsConnectionReusable() const {
   return parser()->IsConnectionReusable();
 }
 
 int64 HttpBasicStream::GetTotalReceivedBytes() const {
   if (parser())
     return parser()->received_bytes();
   return 0;
 }
 
+int64_t HttpBasicStream::GetTotalSentBytes() const {
+  if (parser())

[mmenke, 2015/08/31 22:23:13]

This check isn't necessary (And while you're here, could you remove the
corresponding one from HttpBasicStream::GetTotalReceivedBytes?)

[sclittle, 2015/09/01 01:21:47]

This check is needed for GetTotalSent/ReceivedBytes to be called before
InitializeStream is called. I don't know if anything actually calls
GetTotalReceivedBytes before the stream is initialized, but the
HttpStream::GetTotalSent/ReceivedBytes interface doesn't forbid it and 0 is the
correct value to return in that case, so I think we should support that here.

WDYT?

[mmenke, 2015/09/01 17:39:43]

The same can be said of pretty much all these methods.  I don't see a path where
it is possible to call this with a NULL parser.

My feeling is that we shouldn't check for things that don't happen.  If we see
crashes here without the check, we should figure out why, and make sure we have
a test for that case.

[sclittle, 2015/09/01 20:03:00]

Removing the check from GetTotalReceivedBytes causes a bunch of tests to crash,
so it looks like it does get called before stream initialization sometimes. I
don't want to re architect that, especially not in this CL, so do you mind if I
keep the check in here?

For example, there's a DCHECK in HttpNetworkTransaction to ensure that the
stream's GetTotalReceivedBytes is not carried over when a stream is renewed.
This would crash since GetTotalReceivedBytes is called before the renewed stream
is initialized:
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_netw...

[mmenke, 2015/09/02 17:36:54]

Do you know if it's needed for anything but the DCHECK?  (Fine as-is for now,
but I plan on significantly reworking that path, so I may change things here, at
some point).

[sclittle, 2015/09/02 18:32:37]

I don't know of anything else but that DCHECK, but there's nothing stopping code
from calling GetTotalReceivedBytes for a not-yet-initialized stream, especially
since InitializeStream() is allowed to return ERR_IO_PENDING.

+    return parser()->sent_bytes();
+  return 0;
+}
+
 bool HttpBasicStream::GetLoadTimingInfo(
     LoadTimingInfo* load_timing_info) const {
   return state_.connection()->GetLoadTimingInfo(IsConnectionReused(),
                                                 load_timing_info);
 }
 
 void HttpBasicStream::GetSSLInfo(SSLInfo* ssl_info) {
   parser()->GetSSLInfo(ssl_info);
 }
 
 void HttpBasicStream::GetSSLCertRequestInfo(
     SSLCertRequestInfo* cert_request_info) {
   parser()->GetSSLCertRequestInfo(cert_request_info);
 }
 
 bool HttpBasicStream::IsSpdyHttpStream() const { return false; }
 
 void HttpBasicStream::Drain(HttpNetworkSession* session) {
   HttpResponseBodyDrainer* drainer = new HttpResponseBodyDrainer(this);
   drainer->Start(session);
   // |drainer| will delete itself.
 }
 
 void HttpBasicStream::SetPriority(RequestPriority priority) {
   // TODO(akalin): Plumb this through to |connection_|.
 }
 
 }  // namespace net