Modify Http2PriorityDependencies::OnStreamCreation to take a weight

outparam and compute the HTTP/2 weight in addition to the parent stream
ID, and exclusive bool so that callers do not need to.

Change-Id: I13ebd842f86b5aed9e517a0dc94612c8007af33f
Reviewed-on: https://chromium-review.googlesource.com/915108
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537494}

net/quic/chromium/quic_chromium_client_session.cc

@@ -878,14 +878,16 @@ size_t QuicChromiumClientSession::WriteHeaders(
     QuicReferenceCountedPointer<QuicAckListenerInterface>
         ack_notifier_delegate) {
   SpdyStreamId parent_stream_id = 0;
+  int weight = 0;
   bool exclusive = false;
   if (headers_include_h2_stream_dependency_) {
     priority_dependency_state_.OnStreamCreation(id, priority, &parent_stream_id,
-                                                &exclusive);
+                                                &weight, &exclusive);
+  } else {
+    weight = Spdy3PriorityToHttp2Weight(priority);
   }
-  return WriteHeadersImpl(
-      id, std::move(headers), fin, Spdy3PriorityToHttp2Weight(priority),
-      parent_stream_id, exclusive, std::move(ack_notifier_delegate));
+  return WriteHeadersImpl(id, std::move(headers), fin, weight, parent_stream_id,
+                          exclusive, std::move(ack_notifier_delegate));
 }
 
 void QuicChromiumClientSession::OnHeadersHeadOfLineBlocking(
@@ -2647,11 +2649,11 @@ bool QuicChromiumClientSession::HandlePromised(QuicStreamId id,
       // initial SpdyPriority of the push promise stream when created.
       const SpdyPriority priority = kDefaultPriority;
       SpdyStreamId parent_stream_id = 0;
+      int weight = 0;
       bool exclusive = false;
       priority_dependency_state_.OnStreamCreation(
-          promised_id, priority, &parent_stream_id, &exclusive);
-      WritePriority(promised_id, parent_stream_id,
-                    Spdy3PriorityToHttp2Weight(priority), exclusive);
+          promised_id, priority, &parent_stream_id, &weight, &exclusive);
+      WritePriority(promised_id, parent_stream_id, weight, exclusive);
     }
   }
   net_log_.AddEvent(NetLogEventType::QUIC_SESSION_PUSH_PROMISE_RECEIVED,

net/spdy/chromium/http2_priority_dependencies.cc

@@ -15,12 +15,20 @@ void Http2PriorityDependencies::OnStreamCreation(
     SpdyStreamId id,
     SpdyPriority priority,
     SpdyStreamId* dependent_stream_id,
+    int* weight,
     bool* exclusive) {
   if (entry_by_stream_id_.find(id) != entry_by_stream_id_.end())
     return;
 
-  *dependent_stream_id = 0ul;
+  *dependent_stream_id = 0;
   *exclusive = true;
+  // Since the generated dependency graph is a single linked list, the value
+  // of weight should not actually matter, and perhaps the default weight of 16
+  // from the HTTP/2 spec would be reasonable. However, there are some servers
+  // which currently interpret the weight field like an old SPDY priority value.
+  // As long as those servers need to be supported, weight should be set to
+  // a value those servers will interpret correctly.
+  *weight = Spdy3PriorityToHttp2Weight(priority);
 
   // Dependent on the lowest-priority stream that has a priority >= |priority|.
   IdList::iterator parent;

net/spdy/chromium/http2_priority_dependencies.h

@@ -15,9 +15,13 @@
 
 namespace net {
 
-// A helper class encapsulating the state and logic to set dependencies of
-// HTTP2 streams based on their SpdyPriority and the ordering
-// of creation and deletion of the streams.
+// A helper class encapsulating the state and logic to set the priority fields
+// for HTTP/2 streams based on their SpdyPriority and the ordering of creation
+// and deletion of the streams. This implentation includes a gross hack in which
+// the HTTP/2 weight is set to a transformation of the SpdyPriority value
+// in order to support servers which do not honor HTTP/2 stream dependencies
+// and instead treat the weight value like a SPDY/3 priority.
+// TODO(rch): Eliminate this gross hack when servers no longer act like this.
 class NET_EXPORT_PRIVATE Http2PriorityDependencies {
  public:
   Http2PriorityDependencies();
@@ -26,11 +30,13 @@ class NET_EXPORT_PRIVATE Http2PriorityDependencies {
   // Called when a stream is created. This is used for both client-initiated
   // and server-initiated (pushed) streams.
   // On return, |*dependent_stream_id| is set to the stream id that
-  // this stream should be made dependent on, and |*exclusive| set to
-  // whether that dependency should be exclusive.
+  // this stream should be made dependent on, |*exclusive| is set to
+  // whether that dependency should be exclusive, and |*weight| is set to
+  // the relative weight for the created stream given this priority.
   void OnStreamCreation(SpdyStreamId id,
                         SpdyPriority priority,
                         SpdyStreamId* dependent_stream_id,
+                        int* weight,
                         bool* exclusive);
 
   // Called when a stream is destroyed.

net/spdy/chromium/http2_priority_dependencies_unittest.cc

@@ -46,15 +46,21 @@ class HttpPriorityDependencyTest : public PlatformTest {
   void TestStreamCreation(SpdyStreamId new_id,
                           SpdyPriority priority,
                           SpdyStreamId expected_dependent_id) {
+    int expected_weight = Spdy3PriorityToHttp2Weight(priority);
+
     SpdyStreamId dependent_id = 999u;
+    int weight = -1;
     bool exclusive = false;
-    dependency_state_.OnStreamCreation(new_id, priority, &dependent_id,
+    dependency_state_.OnStreamCreation(new_id, priority, &dependent_id, &weight,
                                        &exclusive);
-    if (expected_dependent_id != dependent_id || !exclusive) {
+    if (expected_dependent_id != dependent_id || !exclusive ||
+        expected_weight != weight) {
       ADD_FAILURE() << "OnStreamCreation(" << new_id << ", " << int(priority)
                     << ")\n"
-                    << "  Got:  (" << dependent_id << ", " << exclusive << ")\n"
-                    << "  Want: (" << expected_dependent_id << ", true)\n";
+                    << "  Got:  (" << dependent_id << ", " << weight << ", "
+                    << exclusive << ")\n"
+                    << "  Want: (" << expected_dependent_id << ", "
+                    << expected_weight << ", true)\n";
     }
   }
 

net/spdy/chromium/spdy_session.cc

@@ -966,12 +966,12 @@ std::unique_ptr<SpdySerializedFrame> SpdySession::CreateHeaders(
   SpdyPriority spdy_priority = ConvertRequestPriorityToSpdyPriority(priority);
 
   bool has_priority = true;
-  int weight = Spdy3PriorityToHttp2Weight(spdy_priority);
+  int weight = 0;
   SpdyStreamId dependent_stream_id = 0;
   bool exclusive = false;
 
-  priority_dependency_state_.OnStreamCreation(stream_id, spdy_priority,
-                                              &dependent_stream_id, &exclusive);
+  priority_dependency_state_.OnStreamCreation(
+      stream_id, spdy_priority, &dependent_stream_id, &weight, &exclusive);
 
   if (net_log().IsCapturing()) {
     net_log().AddEvent(
@@ -1698,11 +1698,11 @@ void SpdySession::TryCreatePushStream(SpdyStreamId stream_id,
   SpdyPriority spdy_priority =
       ConvertRequestPriorityToSpdyPriority(request_priority);
   SpdyStreamId dependency_id = 0;
+  int weight = 0;
   bool exclusive = false;
-  priority_dependency_state_.OnStreamCreation(stream_id, spdy_priority,
-                                              &dependency_id, &exclusive);
-  EnqueuePriorityFrame(stream_id, dependency_id,
-                       Spdy3PriorityToHttp2Weight(spdy_priority), exclusive);
+  priority_dependency_state_.OnStreamCreation(
+      stream_id, spdy_priority, &dependency_id, &weight, &exclusive);
+  EnqueuePriorityFrame(stream_id, dependency_id, weight, exclusive);
 
   // PUSH_PROMISE arrives on associated stream.
   associated_it->second->AddRawReceivedBytes(last_compressed_frame_len_);