Fixed the following issues with SPDY server push.

  * A NULL stream was added to pushed_streams_
  * FlipStream had a NULL HttpResponseInfo pointer. In non-server-push paths, this pointer references a member of the associated FlipNetworkTransaction. In this case, I just leak memory - needs a better solution.
  * io_state_ was set to NONE on a server push, which DCHECK'ed in the first state machine loop. Changed it to jump to READ_HEADERS state.

BUG=NONE
TEST=Manual

Review URL: http://codereview.chromium.org/551006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38241 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 48 insertions, 15 deletions

diff --git a/net/flip/flip_network_transaction_unittest.cc b/net/flip/flip_network_transaction_unittest.cc
index af6ca0e..3dc1b90 100644
--- a/net/flip/flip_network_transaction_unittest.cc
+++ b/net/flip/flip_network_transaction_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -738,7 +738,7 @@ TEST_F(FlipNetworkTransactionTest, CorruptFrameSessionError) {
   }
 }
 
-TEST_F(FlipNetworkTransactionTest, DISABLED_ServerPush) {
+TEST_F(FlipNetworkTransactionTest, ServerPush) {
   // Reply with the X-Associated-Content header.
   static const unsigned char syn_reply[] = {
     0x80, 0x01, 0x00, 0x02,
diff --git a/net/flip/flip_session.cc b/net/flip/flip_session.cc
index d91c37c..47528fa 100644
--- a/net/flip/flip_session.cc
+++ b/net/flip/flip_session.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -896,10 +896,6 @@ void FlipSession::OnSyn(const flip::FlipSynStreamControlFrame* frame,
   if (it != pending_streams_.end()) {
     stream = it->second;
     pending_streams_.erase(it);
-    if (stream)
-      pushed_streams_.push_back(stream);
-  } else {
-    pushed_streams_.push_back(stream);
   }
 
   if (stream) {
@@ -909,8 +905,20 @@ void FlipSession::OnSyn(const flip::FlipSynStreamControlFrame* frame,
   } else {
     // TODO(mbelshe): can we figure out how to use a LoadLog here?
     stream = new FlipStream(this, stream_id, true, NULL);
+
+    // A new HttpResponseInfo object needs to be generated so the call to
+    // OnResponseReceived below has something to fill in.
+    // When a FlipNetworkTransaction is created for this resource, the
+    // response_info is copied over and this version is destroyed.
+    //
+    // TODO(cbentzel): Minimize allocations and copies of HttpResponseInfo
+    // object. Should it just be part of FlipStream?
+    HttpResponseInfo* response_info = new HttpResponseInfo();
+    stream->set_response_info_pointer(response_info);
   }
 
+  pushed_streams_.push_back(stream);
+
   // Activate a stream and parse the headers.
   ActivateStream(stream);
 
diff --git a/net/flip/flip_stream.cc b/net/flip/flip_stream.cc
index 15555cc..08106f0 100755..100644
--- a/net/flip/flip_stream.cc
+++ b/net/flip/flip_stream.cc
@@ -125,6 +125,10 @@ int FlipStream::SendRequest(UploadDataStream* upload_data,
   CHECK(!cancelled_);
   CHECK(response);
 
+  if (response_) {
+    *response = *response_;
+    delete response_;
+  }
   response_ = response;
 
   if (upload_data) {
@@ -139,8 +143,13 @@ int FlipStream::SendRequest(UploadDataStream* upload_data,
   DCHECK_EQ(io_state_, STATE_NONE);
   if (!pushed_)
     io_state_ = STATE_SEND_HEADERS;
-  else
-    io_state_ = STATE_READ_HEADERS;
+  else {
+    if (response_->headers) {
+      io_state_ = STATE_READ_BODY;
+    } else {
+      io_state_ = STATE_READ_HEADERS;
+    }
+  }
   int result = DoLoop(OK);
   if (result == ERR_IO_PENDING) {
     CHECK(!user_callback_);
@@ -168,8 +177,15 @@ void FlipStream::OnResponseReceived(const HttpResponseInfo& response) {
 
   if (io_state_ == STATE_NONE) {
     CHECK(pushed_);
+    io_state_ = STATE_READ_HEADERS;
   } else if (io_state_ == STATE_READ_HEADERS_COMPLETE) {
-    CHECK(!pushed_);
+    // This FlipStream could be in this state in both true and false pushed_
+    // conditions.
+    // The false pushed_ condition (client request) will always go through
+    // this state.
+    // The true pushed_condition (server push) can be in this state when the
+    // client requests an X-Associated-Content piece of content prior
+    // to when the server push happens.
   } else {
     NOTREACHED();
   }
@@ -224,10 +240,15 @@ bool FlipStream::OnDataReceived(const char* data, int length) {
   // ReadResponseBody(), therefore user_callback_ may be NULL.  This may often
   // happen for server initiated streams.
   if (user_callback_) {
-    int rv = ReadResponseBody(user_buffer_, user_buffer_len_, user_callback_);
-    CHECK(rv != ERR_IO_PENDING);
-    user_buffer_ = NULL;
-    user_buffer_len_ = 0;
+    int rv;
+    if (user_buffer_) {
+      rv = ReadResponseBody(user_buffer_, user_buffer_len_, user_callback_);
+      CHECK(rv != ERR_IO_PENDING);
+      user_buffer_ = NULL;
+      user_buffer_len_ = 0;
+    } else {
+      rv = OK;
+    }
     DoCallback(rv);
   }
 
diff --git a/net/flip/flip_stream.h b/net/flip/flip_stream.h
index f38e33f..083235b 100755..100644
--- a/net/flip/flip_stream.h
+++ b/net/flip/flip_stream.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -122,6 +122,10 @@ class FlipStream : public base::RefCounted<FlipStream> {
 
   bool cancelled() const { return cancelled_; }
 
+  void set_response_info_pointer(HttpResponseInfo* response_info) {
+    response_ = response_info;
+  }
+
  private:
   friend class base::RefCounted<FlipStream>;