Implement PUSH_PROMISE handling in spdy_session

net/spdy/spdy_test_util_common.cc

Index: net/spdy/spdy_test_util_common.cc
diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
index e12aff4167df3e88013cfb23a45d1cdb18ce8aee..9e505d28fc49115f11439018378c83a220e92789 100644
--- a/net/spdy/spdy_test_util_common.cc
+++ b/net/spdy/spdy_test_util_common.cc
@@ -996,21 +996,44 @@ SpdyFrame* SpdyTestUtil::ConstructSpdyPush(const char* const extra_headers[],
                                            int stream_id,
                                            int associated_stream_id,
                                            const char* url) {
-  scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock());
-  (*headers)["hello"] = "bye";
-  (*headers)[GetStatusKey()] = "200 OK";
-  if (include_version_header()) {
-    (*headers)[GetVersionKey()] = "HTTP/1.1";
+  if (spdy_version() < SPDY4) {
+    SpdySynStreamIR syn_stream(stream_id);
+    syn_stream.set_associated_to_stream_id(associated_stream_id);
+    syn_stream.SetHeader("hello", "bye");
+    syn_stream.SetHeader(GetStatusKey(), "200 OK");
+    syn_stream.SetHeader(GetVersionKey(), "HTTP/1.1");
+    AddUrlToHeaderBlock(url, syn_stream.mutable_name_value_block());
+    AppendToHeaderBlock(extra_headers,
+                        extra_header_count,
+                        syn_stream.mutable_name_value_block());
+    return CreateFramer(false)->SerializeFrame(syn_stream);
+  } else {
+    SpdyPushPromiseIR push_promise(associated_stream_id, stream_id);
+    AddUrlToHeaderBlock(url, push_promise.mutable_name_value_block());
+    scoped_ptr<SpdyFrame> push_promise_frame(
+        CreateFramer(false)->SerializeFrame(push_promise));
+
+    // Use SynStreamIR to create HEADERS+PRIORITY. Direct creation breaks
+    // framer.
+    SpdySynStreamIR headers(stream_id);

[baranovich, 2014/06/13 11:42:46]

Doing it this way, I check codepath via OnSynStream(). But headers can come
directly from OnHeaders() (no priority). This is untested now. Should I write a
single test which will cover this case?

[Johnny, 2014/06/17 04:01:33]

I don't think I fully follow, but as a rule of thumb for SpdyTestUtil: if it's
not necessary to get tests passing, better to leave as-is and incomplete for
SPDY4 than to add implementation which tries to fix it, but doesn't have tests
exercising it.

[baranovich, 2014/06/17 21:33:43]

Server may send HEADERS with or without priority. If priority flag is set, push
'response' will go via OnSynStream() (which will delegate to OnHeaders()),
otherwise we'll directly get to OnHeaders(). I am worried about the second case
which is not covered by tests (well, covered, but not a direct call=)). But its
minor I think.

+    SetPriority(LOWEST, &headers);
+    headers.SetHeader("hello", "bye");
+    headers.SetHeader(GetStatusKey(), "200 OK");
+    AppendToHeaderBlock(
+        extra_headers, extra_header_count, headers.mutable_name_value_block());
+    scoped_ptr<SpdyFrame> headers_frame(
+        CreateFramer(false)->SerializeFrame(headers));
+
+    int joint_data_size = push_promise_frame->size() + headers_frame->size();
+    scoped_ptr<char[]> data(new char[joint_data_size]);
+    const SpdyFrame* frames[2] = {
+        push_promise_frame.get(), headers_frame.get(),
+    };
+    int combined_size =
+        CombineFrames(frames, arraysize(frames), data.get(), joint_data_size);
+    DCHECK_EQ(combined_size, joint_data_size);
+    return new SpdyFrame(data.release(), joint_data_size, true);
   }
-  AddUrlToHeaderBlock(url, headers.get());
-  AppendToHeaderBlock(extra_headers, extra_header_count, headers.get());
-  return ConstructSpdyControlFrame(headers.Pass(),
-                                   false,
-                                   stream_id,
-                                   LOWEST,
-                                   SYN_STREAM,
-                                   CONTROL_FLAG_NONE,
-                                   associated_stream_id);
 }
 
 SpdyFrame* SpdyTestUtil::ConstructSpdyPush(const char* const extra_headers[],
@@ -1020,22 +1043,63 @@ SpdyFrame* SpdyTestUtil::ConstructSpdyPush(const char* const extra_headers[],
                                            const char* url,
                                            const char* status,
                                            const char* location) {
-  scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock());
-  (*headers)["hello"] = "bye";
-  (*headers)[GetStatusKey()] = status;
-  if (include_version_header()) {
-    (*headers)[GetVersionKey()] = "HTTP/1.1";
+  if (spdy_version() < SPDY4) {
+    SpdySynStreamIR syn_stream(stream_id);
+    syn_stream.set_associated_to_stream_id(associated_stream_id);
+    syn_stream.SetHeader("hello", "bye");
+    syn_stream.SetHeader(GetStatusKey(), status);
+    syn_stream.SetHeader(GetVersionKey(), "HTTP/1.1");
+    syn_stream.SetHeader("location", location);
+    AddUrlToHeaderBlock(url, syn_stream.mutable_name_value_block());
+    AppendToHeaderBlock(extra_headers,
+                        extra_header_count,
+                        syn_stream.mutable_name_value_block());
+    return CreateFramer(false)->SerializeFrame(syn_stream);
+  } else {
+    SpdyPushPromiseIR push_promise(associated_stream_id, stream_id);
+    AddUrlToHeaderBlock(url, push_promise.mutable_name_value_block());
+    scoped_ptr<SpdyFrame> push_promise_frame(
+        CreateFramer(false)->SerializeFrame(push_promise));
+
+    // Use SynStreamIR to create HEADERS+PRIORITY. Direct creation breaks
+    // framer.
+    SpdySynStreamIR headers(stream_id);
+    SetPriority(LOWEST, &headers);
+    headers.SetHeader("hello", "bye");
+    headers.SetHeader(GetStatusKey(), status);
+    headers.SetHeader("location", location);
+    AppendToHeaderBlock(
+        extra_headers, extra_header_count, headers.mutable_name_value_block());
+    scoped_ptr<SpdyFrame> headers_frame(
+        CreateFramer(false)->SerializeFrame(headers));
+
+    int joint_data_size = push_promise_frame->size() + headers_frame->size();
+    scoped_ptr<char[]> data(new char[joint_data_size]);
+    const SpdyFrame* frames[2] = {
+        push_promise_frame.get(), headers_frame.get(),
+    };
+    int combined_size =
+        CombineFrames(frames, arraysize(frames), data.get(), joint_data_size);
+    DCHECK_EQ(combined_size, joint_data_size);
+    return new SpdyFrame(data.release(), joint_data_size, true);
+  }
+}
+
+SpdyFrame* SpdyTestUtil::ConstructInitialSpdyPushFrame(
+    scoped_ptr<SpdyHeaderBlock> headers,
+    int stream_id,
+    int associated_stream_id) {
+  if (spdy_version() < SPDY4) {
+    SpdySynStreamIR syn_stream(stream_id);
+    syn_stream.set_associated_to_stream_id(associated_stream_id);
+    SetPriority(LOWEST, &syn_stream);
+    syn_stream.set_name_value_block(*headers);
+    return CreateFramer(false)->SerializeFrame(syn_stream);
+  } else {
+    SpdyPushPromiseIR push_promise(associated_stream_id, stream_id);
+    push_promise.set_name_value_block(*headers);
+    return CreateFramer(false)->SerializeFrame(push_promise);
   }
-  (*headers)["location"] = location;
-  AddUrlToHeaderBlock(url, headers.get());
-  AppendToHeaderBlock(extra_headers, extra_header_count, headers.get());
-  return ConstructSpdyControlFrame(headers.Pass(),
-                                   false,
-                                   stream_id,
-                                   LOWEST,
-                                   SYN_STREAM,
-                                   CONTROL_FLAG_NONE,
-                                   associated_stream_id);
 }
 
 SpdyFrame* SpdyTestUtil::ConstructSpdyPushHeaders(