gfe-relnote: Fix Http2PriorityWriteScheduler::ShouldYield. Not used in prod. Not protected.

Current ShouldYield() assumes the passed in stream_id has ready==true, and if ready==false, it will yield to any other stream. This change fixes ShouldYield() so that it returns correct answers even if ready==false for the given stream_id.

PiperOrigin-RevId: 260147911
Change-Id: Ieefb5fc3a341c498bb1f7bf89cae0f92db8691ac

spdy/core/http2_priority_write_scheduler.h

diff --git a/spdy/core/http2_priority_write_scheduler.h b/spdy/core/http2_priority_write_scheduler.h
index d08d125..a35e076 100644
--- a/spdy/core/http2_priority_write_scheduler.h
+++ b/spdy/core/http2_priority_write_scheduler.h
@@ -111,6 +111,17 @@
     // Time of latest write event for stream of this priority, in microseconds.
     int64_t last_event_time_usec = 0;
 
+    // Returns true if this stream is ancestor of |other|.
+    bool IsAncestorOf(const StreamInfo& other) const {
+      for (const StreamInfo* parent = other.parent; parent != nullptr;
+           parent = parent->parent) {
+        if (parent == this) {
+          return true;
+        }
+      }
+      return false;
+    }
+
     // Whether this stream should be scheduled ahead of another stream.
     bool SchedulesBefore(const StreamInfo& other) const {
       return (priority != other.priority) ? priority > other.priority
@@ -500,13 +511,20 @@
     SPDY_BUG << "Stream " << stream_id << " not registered";
     return false;
   }
+  if (HasReadyAncestor(*stream_info)) {
+    return true;
+  }
   for (const StreamInfo& scheduled : scheduling_queue_) {
-    if (stream_info == &scheduled) {
+    if (HasReadyAncestor(scheduled)) {
+      // Skip streams which cannot be scheduled.
+      continue;
+    }
+    if (stream_info->IsAncestorOf(scheduled)) {
+      // Do not yield to descendants.
       return false;
     }
-    if (!HasReadyAncestor(scheduled)) {
-      return true;
-    }
+    // Yield to streams with higher priorities.
+    return scheduled.SchedulesBefore(*stream_info);
   }
   return false;
 }

spdy/core/http2_priority_write_scheduler_test.cc

diff --git a/spdy/core/http2_priority_write_scheduler_test.cc b/spdy/core/http2_priority_write_scheduler_test.cc
index 484e59a..e498c37 100644
--- a/spdy/core/http2_priority_write_scheduler_test.cc
+++ b/spdy/core/http2_priority_write_scheduler_test.cc
@@ -653,6 +653,46 @@
             scheduler_.PopNextReadyStreamAndPrecedence());
 }
 
+TEST_F(Http2PriorityWriteSchedulerTest, ShouldYield) {
+  /*
+         0
+        /|\
+       1 2 3
+      /|\ \
+     4 5 6 7
+       |
+       8
+
+  */
+  scheduler_.RegisterStream(1, SpdyStreamPrecedence(0, 100, false));
+  scheduler_.RegisterStream(2, SpdyStreamPrecedence(0, 100, false));
+  scheduler_.RegisterStream(3, SpdyStreamPrecedence(0, 100, false));
+  scheduler_.RegisterStream(4, SpdyStreamPrecedence(1, 100, false));
+  scheduler_.RegisterStream(5, SpdyStreamPrecedence(1, 200, false));
+  scheduler_.RegisterStream(6, SpdyStreamPrecedence(1, 255, false));
+  scheduler_.RegisterStream(7, SpdyStreamPrecedence(2, 100, false));
+  scheduler_.RegisterStream(8, SpdyStreamPrecedence(5, 100, false));
+
+  scheduler_.MarkStreamReady(5, false);
+
+  for (int i = 1; i <= 8; ++i) {
+    // Verify only 4 and 8 should yield to 5.
+    if (i == 4 || i == 8) {
+      EXPECT_TRUE(scheduler_.ShouldYield(i)) << "stream_id: " << i;
+    } else {
+      EXPECT_FALSE(scheduler_.ShouldYield(i)) << "stream_id: " << i;
+    }
+  }
+
+  // Marks streams 1 and 2 ready.
+  scheduler_.MarkStreamReady(1, false);
+  scheduler_.MarkStreamReady(2, false);
+  // 1 should not yield.
+  EXPECT_FALSE(scheduler_.ShouldYield(1));
+  // Verify 2 should yield to 1.
+  EXPECT_TRUE(scheduler_.ShouldYield(2));
+}
+
 class PopNextReadyStreamTest : public Http2PriorityWriteSchedulerTest {
  protected:
   void SetUp() override {