commit 6ab8c79659aa4682ec67f0027ed6184eb5c164ba
author bnc <bnc@google.com> Fri Mar 27 16:15:03 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Mar 27 16:15:40 2020 -0700
tree f2eb823c190baf82095d9bc59d0350b1247f71bd
parent 32f36f9f3f2a26b12cfa22a6aebbafe532e3b4b2

Simplify Http3DebugVisitor methods.

Pass frame arguments by const ref instead of value.
Combine multiple methods into one for DATA and unknown frames.
Add compressed header block size argument to OnPushPromiseFrameReceived().
Call OnMaxPushIdFrameReceived() and OnGoAwayFrameReceived() even if they cause
the connection to be closed to make debugging easier.

This is all motivated by renjietang's comments at https://crrev.com/c/2117316.

Bug is https://crbug.com/1062700.

gfe-relnote: n/a, Http3DebugVisitor not used in production.
PiperOrigin-RevId: 303427808
Change-Id: I91ee190aea66f565f91575379537fa50c62c9c58

quic/core/http/quic_spdy_stream.cc

diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index 573ab90..c376d01 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -867,8 +867,13 @@
 }
 
 bool QuicSpdyStream::OnDataFrameStart(QuicByteCount header_length,
-                                      QuicByteCount /*payload_length*/) {
+                                      QuicByteCount payload_length) {
   DCHECK(VersionUsesHttp3(transport_version()));
+
+  if (spdy_session_->debug_visitor()) {
+    spdy_session_->debug_visitor()->OnDataFrameReceived(id(), payload_length);
+  }
+
   if (!headers_decompressed_ || trailers_decompressed_) {
     stream_delegate()->OnStreamError(
         QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
@@ -876,10 +881,6 @@
     return false;
   }
 
-  if (spdy_session_->debug_visitor()) {
-    spdy_session_->debug_visitor()->OnDataFrameStart(id());
-  }
-
   sequencer()->MarkConsumed(body_manager_.OnNonBody(header_length));
 
   return true;
@@ -888,10 +889,6 @@
 bool QuicSpdyStream::OnDataFramePayload(quiche::QuicheStringPiece payload) {
   DCHECK(VersionUsesHttp3(transport_version()));
 
-  if (spdy_session_->debug_visitor()) {
-    spdy_session_->debug_visitor()->OnDataFramePayload(id(), payload.length());
-  }
-
   body_manager_.OnBody(payload);
 
   return true;
@@ -900,10 +897,6 @@
 bool QuicSpdyStream::OnDataFrameEnd() {
   DCHECK(VersionUsesHttp3(transport_version()));
 
-  if (spdy_session_->debug_visitor()) {
-    spdy_session_->debug_visitor()->OnDataFrameEnd(id());
-  }
-
   QUIC_DVLOG(1) << ENDPOINT
                 << "Reaches the end of a data frame. Total bytes received are "
                 << body_manager_.total_body_bytes_received();
@@ -960,10 +953,15 @@
 }
 
 bool QuicSpdyStream::OnHeadersFrameStart(QuicByteCount header_length,
-                                         QuicByteCount /*payload_length*/) {
+                                         QuicByteCount payload_length) {
   DCHECK(VersionUsesHttp3(transport_version()));
   DCHECK(!qpack_decoded_headers_accumulator_);
 
+  if (spdy_session_->debug_visitor()) {
+    spdy_session_->debug_visitor()->OnHeadersFrameReceived(id(),
+                                                           payload_length);
+  }
+
   if (trailers_decompressed_) {
     stream_delegate()->OnStreamError(
         QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
@@ -1009,18 +1007,6 @@
   DCHECK(VersionUsesHttp3(transport_version()));
   DCHECK(qpack_decoded_headers_accumulator_);
 
-  if (spdy_session_->debug_visitor()) {
-    if (spdy_session_->promised_stream_id() ==
-        QuicUtils::GetInvalidStreamId(transport_version())) {
-      spdy_session_->debug_visitor()->OnHeadersFrameReceived(
-          id(), headers_decompressed_ ? trailers_payload_length_
-                                      : headers_payload_length_);
-    } else {
-      spdy_session_->debug_visitor()->OnPushPromiseFrameReceived(
-          id(), spdy_session_->promised_stream_id());
-    }
-  }
-
   qpack_decoded_headers_accumulator_->EndHeaderBlock();
 
   // If decoding is complete or an error is detected, then
@@ -1045,10 +1031,15 @@
 bool QuicSpdyStream::OnPushPromiseFramePushId(
     PushId push_id,
     QuicByteCount push_id_length,
-    QuicByteCount /*header_block_length*/) {
+    QuicByteCount header_block_length) {
   DCHECK(VersionUsesHttp3(transport_version()));
   DCHECK(!qpack_decoded_headers_accumulator_);
 
+  if (spdy_session_->debug_visitor()) {
+    spdy_session_->debug_visitor()->OnPushPromiseFrameReceived(
+        id(), push_id, header_block_length);
+  }
+
   // TODO(renjietang): Check max push id and handle errors.
   spdy_session_->OnPushPromise(id(), push_id);
   sequencer()->MarkConsumed(body_manager_.OnNonBody(push_id_length));
@@ -1075,9 +1066,10 @@
 
 bool QuicSpdyStream::OnUnknownFrameStart(uint64_t frame_type,
                                          QuicByteCount header_length,
-                                         QuicByteCount /*payload_length*/) {
+                                         QuicByteCount payload_length) {
   if (spdy_session_->debug_visitor()) {
-    spdy_session_->debug_visitor()->OnUnknownFrameStart(id(), frame_type);
+    spdy_session_->debug_visitor()->OnUnknownFrameReceived(id(), frame_type,
+                                                           payload_length);
   }
 
   // Ignore unknown frames, but consume frame header.
@@ -1089,10 +1081,6 @@
 }
 
 bool QuicSpdyStream::OnUnknownFramePayload(quiche::QuicheStringPiece payload) {
-  if (spdy_session_->debug_visitor()) {
-    spdy_session_->debug_visitor()->OnUnknownFramePayload(id(), payload.size());
-  }
-
   // Ignore unknown frames, but consume frame payload.
   QUIC_DVLOG(1) << ENDPOINT << "Discarding " << payload.size()
                 << " bytes of payload of frame of unknown type.";
@@ -1101,10 +1089,6 @@
 }
 
 bool QuicSpdyStream::OnUnknownFrameEnd() {
-  if (spdy_session_->debug_visitor()) {
-    spdy_session_->debug_visitor()->OnUnknownFrameEnd(id());
-  }
-
   return true;
 }