[DevTools] Always report encodedDataLength in Network.ResponseReceived.

net/url_request/url_request_http_job_unittest.cc

Index: net/url_request/url_request_http_job_unittest.cc
diff --git a/net/url_request/url_request_http_job_unittest.cc b/net/url_request/url_request_http_job_unittest.cc
index aa1599aa6cce1b56b906753b84491447423c7fff..3d27b4888c82a3ee4ec6b90ba6bfa2f4131589f3 100644
--- a/net/url_request/url_request_http_job_unittest.cc
+++ b/net/url_request/url_request_http_job_unittest.cc
@@ -267,6 +267,113 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
 }
 
 TEST_F(URLRequestHttpJobWithMockSocketsTest,
+       TestRawHeaderSizeSuccessfullRequest) {
+  MockWrite writes[] = {MockWrite(kSimpleGetMockWrite)};
+  MockRead reads[] = {MockRead("HTTP/1.1 200 OK\r\n"
+                               "Content-Length: 12\r\n\r\n"),
+                      MockRead("Test Content"),
+                      MockRead(net::SYNCHRONOUS, net::OK)};
+
+  StaticSocketDataProvider socket_data(reads, arraysize(reads), writes,
+                                       arraysize(writes));
+  socket_factory_.AddSocketDataProvider(&socket_data);
+
+  TestDelegate delegate;
+  std::unique_ptr<URLRequest> request = context_->CreateRequest(
+      GURL("http://www.example.com"), DEFAULT_PRIORITY, &delegate);
+
+  request->Start();
+  ASSERT_TRUE(request->is_pending());
+  base::RunLoop().Run();
+
+  EXPECT_TRUE(request->status().is_success());
+  EXPECT_EQ(12, request->received_response_content_length());
+  EXPECT_EQ(39, request->raw_header_size());
+  EXPECT_EQ(CountReadBytes(reads, arraysize(reads)),
+            request->GetTotalReceivedBytes());
+}
+
+TEST_F(URLRequestHttpJobWithMockSocketsTest,
+       TestRawHeaderSizeSuccessfull100ContinueRequest) {
+  MockWrite writes[] = {MockWrite(kSimpleGetMockWrite)};
+  MockRead reads[] = {MockRead("HTTP/1.1 100 Continue\r\n\r\n"),
+                      MockRead("HTTP/1.1 200 OK\r\n"
+                               "Content-Length: 12\r\n\r\n"),
+                      MockRead("Test Content"),
+                      MockRead(net::SYNCHRONOUS, net::OK)};
+
+  StaticSocketDataProvider socket_data(reads, arraysize(reads), writes,
+                                       arraysize(writes));
+  socket_factory_.AddSocketDataProvider(&socket_data);
+
+  TestDelegate delegate;
+  std::unique_ptr<URLRequest> request = context_->CreateRequest(
+      GURL("http://www.example.com"), DEFAULT_PRIORITY, &delegate);
+
+  request->Start();
+  ASSERT_TRUE(request->is_pending());
+  base::RunLoop().Run();
+
+  EXPECT_TRUE(request->status().is_success());
+  EXPECT_EQ(12, request->received_response_content_length());
+  EXPECT_EQ(64, request->raw_header_size());
+  EXPECT_EQ(CountReadBytes(reads, arraysize(reads)),
+            request->GetTotalReceivedBytes());
+}
+
+TEST_F(URLRequestHttpJobWithMockSocketsTest,
+       TestRawHeaderSizeFailureTruncatedHeaders) {
+  MockWrite writes[] = {MockWrite(kSimpleGetMockWrite)};
+  MockRead reads[] = {MockRead("HTTP/1.0 200 OK\r\n"
+                               "Content-Len"),
+                      MockRead(net::SYNCHRONOUS, net::OK)};
+
+  StaticSocketDataProvider socket_data(reads, arraysize(reads), writes,
+                                       arraysize(writes));
+  socket_factory_.AddSocketDataProvider(&socket_data);
+
+  TestDelegate delegate;
+  std::unique_ptr<URLRequest> request = context_->CreateRequest(
+      GURL("http://www.example.com"), DEFAULT_PRIORITY, &delegate);
+
+  delegate.set_cancel_in_response_started(true);
+  request->Start();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(request->status().is_success());

[mmenke, 2016/09/06 18:55:00]

Should expect on the error here, too (We're working on getting rid of
URLRequest::status(), so also checking the exact error code will make it easier
to switch to the new way of doing things)

[allada, 2016/09/07 06:01:12]

Done.

+  EXPECT_EQ(0, request->received_response_content_length());
+  EXPECT_EQ(28, request->raw_header_size());
+  EXPECT_EQ(CountReadBytes(reads, arraysize(reads)),
+            request->GetTotalReceivedBytes());
+}
+
+TEST_F(URLRequestHttpJobWithMockSocketsTest,
+       TestRawHeaderSizeSuccessfullContinuiousRead) {
+  MockWrite writes[] = {MockWrite(kSimpleGetMockWrite)};
+  MockRead reads[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Content-Length: 12\r\n\r\n"
+               "Test Content")};
+
+  StaticSocketDataProvider socket_data(reads, arraysize(reads), writes,
+                                       arraysize(writes));
+  socket_factory_.AddSocketDataProvider(&socket_data);
+
+  TestDelegate delegate;
+  std::unique_ptr<URLRequest> request = context_->CreateRequest(
+      GURL("http://www.example.com"), DEFAULT_PRIORITY, &delegate);
+
+  request->Start();
+  base::RunLoop().Run();
+
+  EXPECT_TRUE(request->status().is_success());
+  EXPECT_EQ(12, request->received_response_content_length());
+  EXPECT_EQ(39, request->raw_header_size());
+  EXPECT_EQ(CountReadBytes(reads, arraysize(reads)),
+            request->GetTotalReceivedBytes());
+}
+

[mmenke, 2016/09/06 18:55:00]

Also, should we make sure the header size is readable during the
URLRequest::Delegate::OnResponseStarted / OnReceivedRedirect invocations, as
opposed to only when the request is complete?  I assume that's where DevTools
will be grabbing it.  The way we generally test such things is either stuffing
them in TestDelegate (in url_request_test_util), or by subclassing it and
overriding just the needed methods.

[mmenke, 2016/09/06 18:55:00]

Should we have a test for the cached path?  That will just return 0 (Which I
assume is expected)

[mmenke, 2016/09/06 18:55:00]

Suggest an HTTP/0.9 test.  (i.e., just a response of "Test Content", and make
sure the header size is 0)

[allada, 2016/09/07 06:01:12]

We have this kind of test in our tests:
third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network-data-length.html

[allada, 2016/09/07 06:01:12]

I can do a cached path, however, in our case it's not important. The data about
the resource is very subjective based on many criteria like where the cached
object was returned (eg: memory, disk, etc...). Many/most developers turn cache
off when devtools is open anyway to get more accurate readings.

[allada, 2016/09/07 06:01:12]

I was opting out of doing this kind of test since we are trying to phase 0.9
out. I would rather not encourage any developers use 0.9.

[mmenke, 2016/09/07 15:18:21]

My concern is just that we don't return nonsense.

[mmenke, 2016/09/07 15:18:21]

Unfortunately, I don't think we'll be able to completely remove it.  The
firmware of some home routers use it (Probably due to bugs, rather than an
intentional behavior), and I'm very reluctant to break them, though I suppose if
we had buy-in from other browsers...

[allada, 2016/09/07 18:14:56]

What if we refuse to set the header size if the version < 1.0? I feel strongly
that if any developer trying to get reliable information about 0.9 (or is using
any 0.9) we should simply stand by the argument of "we cannot maintain reliable
information on 0.9".

In devtool's case we do not need to worry about this since we are subtracting it
from the next chunk's data length, as long as it returns 0.

+TEST_F(URLRequestHttpJobWithMockSocketsTest,
        TestNetworkBytesRedirectedRequest) {
   MockWrite redirect_writes[] = {
       MockWrite("GET / HTTP/1.1\r\n"