SPDY - chunked upload - speech recognition doesn't work with SPDY/3

net/spdy/spdy_http_stream_spdy3_unittest.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/spdy/spdy_http_stream.h"
 
 #include "base/threading/sequenced_worker_pool.h"
 #include "crypto/ec_private_key.h"
 #include "crypto/ec_signature_creator.h"
 #include "crypto/signature_creator.h"
@@ skipping 113 matching lines @@
   EXPECT_TRUE(data()->at_write_eof());
 }
 
 TEST_F(SpdyHttpStreamSpdy3Test, SendChunkedPost) {
   UploadDataStream::set_merge_chunks(false);
 
   scoped_ptr<SpdyFrame> req(ConstructChunkedSpdyPost(NULL, 0));
   scoped_ptr<SpdyFrame> chunk1(ConstructSpdyBodyFrame(1, false));
   scoped_ptr<SpdyFrame> chunk2(ConstructSpdyBodyFrame(1, true));
   MockWrite writes[] = {
-    CreateMockWrite(*req.get(), 1),
-    CreateMockWrite(*chunk1, 2),  // POST upload frames
-    CreateMockWrite(*chunk2, 3),
+    CreateMockWrite(*req.get(), 0),
+    CreateMockWrite(*chunk1, 1),  // POST upload frames
+    CreateMockWrite(*chunk2, 2),
   };
   scoped_ptr<SpdyFrame> resp(ConstructSpdyPostSynReply(NULL, 0));
   MockRead reads[] = {
-    CreateMockRead(*resp, 4),
-    CreateMockRead(*chunk1, 5),
+    CreateMockRead(*resp, 3),
+    CreateMockRead(*chunk1, 4),
     CreateMockRead(*chunk2, 5),
     MockRead(SYNCHRONOUS, 0, 6)  // EOF
   };
 
   HostPortPair host_port_pair("www.google.com", 80);
   HostPortProxyPair pair(host_port_pair, ProxyServer::Direct());
   EXPECT_EQ(OK, InitSession(reads, arraysize(reads), writes, arraysize(writes),
                             host_port_pair));
 
   HttpRequestInfo request;
@@ skipping 26 matching lines @@
   data()->CompleteRead();
   MessageLoop::current()->RunAllPending();
 
   // Because we abandoned the stream, we don't expect to find a session in the
   // pool anymore.
   EXPECT_FALSE(http_session_->spdy_session_pool()->HasSession(pair));
   EXPECT_TRUE(data()->at_read_eof());
   EXPECT_TRUE(data()->at_write_eof());
 }
 
+TEST_F(SpdyHttpStreamSpdy3Test, DelayedSendChunkedPost) {
+  UploadDataStream::set_merge_chunks(false);
+
+  scoped_ptr<SpdyFrame> req(ConstructChunkedSpdyPost(NULL, 0));
+  scoped_ptr<SpdyFrame> chunk1(ConstructSpdyBodyFrame(1, false));
+  scoped_ptr<SpdyFrame> chunk2(ConstructSpdyBodyFrame(1, true));
+  MockWrite writes[] = {
+    CreateMockWrite(*req.get(), 0),
+    CreateMockWrite(*chunk1, 1),  // POST upload frames
+    CreateMockWrite(*chunk2, 2),
+  };
+  scoped_ptr<SpdyFrame> resp(ConstructSpdyPostSynReply(NULL, 0));
+  MockRead reads[] = {
+    CreateMockRead(*resp, 3),
+    CreateMockRead(*chunk1, 4),
+    CreateMockRead(*chunk2, 5),
+    MockRead(ASYNC, 0, 6)  // EOF
+  };
+
+  HostPortPair host_port_pair("www.google.com", 80);
+  HostPortProxyPair pair(host_port_pair, ProxyServer::Direct());
+
+  scoped_refptr<DeterministicSocketData> data(
+      new DeterministicSocketData(reads, arraysize(reads),
+                                  writes, arraysize(writes)));
+
+  DeterministicMockClientSocketFactory* socket_factory =
+      session_deps_.deterministic_socket_factory.get();
+  socket_factory->AddSocketDataProvider(data.get());
+
+  http_session_ = SpdySessionDependencies::SpdyCreateSessionDeterministic(
+      &session_deps_);
+  session_ = http_session_->spdy_session_pool()->Get(pair, BoundNetLog());
+  transport_params_ = new TransportSocketParams(host_port_pair,
+                                                MEDIUM, false, false,
+                                                OnHostResolutionCallback());
+
+  TestCompletionCallback callback0;

[Ryan Sleevi, 2012/07/02 21:52:47]

nit: You can re-use existing TestCompletionCallbacks, which may make more sense
here, rather than requiring callback0.

[ramant (doing other things), 2012/07/04 21:04:33]

Done.

+  scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
+
+  EXPECT_EQ(ERR_IO_PENDING,
+            connection->Init(host_port_pair.ToString(),
+                             transport_params_,
+                             MEDIUM,
+                             callback0.callback(),
+                             http_session_->GetTransportSocketPool(
+                                 HttpNetworkSession::NORMAL_SOCKET_POOL),
+                             BoundNetLog()));
+
+  callback0.WaitForResult();
+  EXPECT_EQ(OK,
+            session_->InitializeWithSocket(connection.release(), false, OK));
+
+  HttpRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.google.com/");
+  request.upload_data = new UploadData();
+  request.upload_data->set_is_chunked(true);
+
+  BoundNetLog net_log;
+  scoped_ptr<SpdyHttpStream> http_stream(
+      new SpdyHttpStream(session_.get(), true));
+  ASSERT_EQ(
+      OK,
+      http_stream->InitializeStream(&request, net_log, CompletionCallback()));

[Ryan Sleevi, 2012/07/02 21:52:47]

nit: Would this be more readable as

ASSERT_EQ(OK,
          http_stream->InitializeStream(&request, net_log,
                                        CompletionCallback()));

Same number of lines (3), but a little more readable I think. It also parallels
lines 238/239 and 228/229

[ramant (doing other things), 2012/07/04 21:04:33]

Done.

+
+  scoped_ptr<UploadDataStream> upload_stream(
+      new UploadDataStream(request.upload_data));
+  ASSERT_EQ(OK, upload_stream->Init());
+
+  request.upload_data->AppendChunk(kUploadData, kUploadDataSize, false);
+
+  TestCompletionCallback callback;
+  HttpRequestHeaders headers;
+  HttpResponseInfo response;
+  // This will attempt to Write() the initial request and headers, which will
+  // complete asynchronously.
+  EXPECT_EQ(ERR_IO_PENDING, http_stream->SendRequest(

[Ryan Sleevi, 2012/07/02 21:52:47]

whitespace nit:

EXPECT_EQ(ERR_IO_PENDING,
          http_stream->SendRequest(...) )?

Not critical at all.

[ramant (doing other things), 2012/07/04 21:04:33]

Done.

+      headers, upload_stream.Pass(), &response, callback.callback()));
+  EXPECT_TRUE(http_session_->spdy_session_pool()->HasSession(pair));
+
+  // Complete the initial request write. Additionally, this should enqueue the
+  // first chunk.
+  data->RunFor(1);
+
+  // Now append final chunk. This will enqueue another write.
+  request.upload_data->AppendChunk(kUploadData, kUploadDataSize, true);
+
+  // Finalize writing the last chunk, which will enqueue the trailer.

[Ryan Sleevi, 2012/07/02 21:52:47]

Does |trailer| make sense here? In the case of HTTP chunked uploads, the meaning
of trailer is defined as part of the Transfer-Encoding. Does SPDY also have a
concept of "trailer" when doing chunked uploads? Would this be better with more
context?

[ramant (doing other things), 2012/07/04 21:04:33]

You are right. "trailer" doesn't make sense.
Done.

+  data->RunFor(3);
+  int rv = callback.WaitForResult();
+  EXPECT_GT(rv, 0);
+
+  data->RunFor(2);

[Ryan Sleevi, 2012/07/02 21:52:47]

nit: Add a comment explaining what the two events are

[ramant (doing other things), 2012/07/04 21:04:33]

Done.

+  EXPECT_EQ(OK, http_stream->ReadResponseHeaders(callback.callback()));
+
+  data->RunFor(1);
+  ASSERT_TRUE(response.headers.get() != NULL);

[Ryan Sleevi, 2012/07/02 21:52:47]

I thought the preference was instead

ASSERT_TRUE(response.headers.get())

eg: no explicit NULL check

(This is the same as using "if (foo)" in conditionals). I'm not sure if the !=
NULL pattern is more prevalent in SPDY code, but I see the "if (foo)" as more
prevalent in general Chromium code)

[ramant (doing other things), 2012/07/04 21:04:33]

Done.

+  ASSERT_EQ(200, response.headers->response_code());
+  EXPECT_TRUE(data->at_read_eof());
+  EXPECT_TRUE(data->at_write_eof());
+}
+
 // Test case for bug: http://code.google.com/p/chromium/issues/detail?id=50058
 TEST_F(SpdyHttpStreamSpdy3Test, SpdyURLTest) {
   const char * const full_url = "http://www.google.com/foo?query=what#anchor";
   const char * const base_url = "http://www.google.com/foo?query=what";
   scoped_ptr<SpdyFrame> req(ConstructSpdyGet(base_url, false, 1, LOWEST));
   MockWrite writes[] = {
     CreateMockWrite(*req.get(), 1),
   };
   scoped_ptr<SpdyFrame> resp(ConstructSpdyGetSynReply(NULL, 0, 1));
   MockRead reads[] = {
@@ skipping 328 matching lines @@
 
   sequenced_worker_pool->Shutdown();
 }
 
 #endif  // !defined(USE_OPENSSL)
 
 // TODO(willchan): Write a longer test for SpdyStream that exercises all
 // methods.
 
 }  // namespace net