Handle false return values from visitor_.OnMetadataEndForStream() in OgHttp2Session.

This CL applies the same error-handling from visitor_.OnMetadataForStream() to
visitor_.OnMetadataEndForStream(). Specifically, if the visitor callback fails,
indicating a connection-level error, then OgHttp2Session saves the error and
stops further processing on the connection.

This change further aligns oghttp2 with nghttp2. With this CL, codec_impl_test
BadMetadataVecReceivedTest passes with oghttp2:
http://sponge2/d36f0333-8080-4e97-97fb-21dd3514caa1

This CL also performs some minor test changes.

PiperOrigin-RevId: 417478489
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 15d1e3a..4970f52 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -622,29 +622,26 @@
   EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
 }
 
-TEST(OgHttp2AdapterClientTest, ClientHandlesMetadataWithError) {
+TEST(OgHttp2AdapterClientTest, ClientHandlesMetadataWithPayloadError) {
   DataSavingVisitor visitor;
   OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
   auto adapter = OgHttp2Adapter::Create(visitor, options);
 
   testing::InSequence s;
 
-  const std::vector<const Header> headers1 =
+  const std::vector<const Header> headers =
       ToHeaders({{":method", "GET"},
                  {":scheme", "http"},
                  {":authority", "example.com"},
                  {":path", "/this/is/request/one"}});
 
-  const char* kSentinel1 = "arbitrary pointer 1";
-  const int32_t stream_id1 =
-      adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
-  ASSERT_GT(stream_id1, 0);
-  QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+  const int32_t stream_id = adapter->SubmitRequest(headers, nullptr, nullptr);
+  ASSERT_GT(stream_id, 0);
 
   EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
   EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
-  EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
-  EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+  EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5));
+  EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 0x5, 0));
 
   int result = adapter->Send();
   EXPECT_EQ(0, result);
@@ -654,13 +651,13 @@
       TestFrameSequence()
           .ServerPreface()
           .Metadata(0, "Example connection metadata")
-          .Headers(1,
+          .Headers(stream_id,
                    {{":status", "200"},
                     {"server", "my-fake-server"},
                     {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
                    /*fin=*/false)
-          .Metadata(1, "Example stream metadata")
-          .Data(1, "This is the response body.", true)
+          .Metadata(stream_id, "Example stream metadata")
+          .Data(stream_id, "This is the response body.", true)
           .Serialize();
 
   // Server preface (empty SETTINGS)
@@ -672,16 +669,86 @@
   EXPECT_CALL(visitor, OnBeginMetadataForStream(0, _));
   EXPECT_CALL(visitor, OnMetadataForStream(0, _));
   EXPECT_CALL(visitor, OnMetadataEndForStream(0));
-  EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
-  EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
-  EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
-  EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
-  EXPECT_CALL(visitor,
-              OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
-  EXPECT_CALL(visitor, OnEndHeadersForStream(1));
-  EXPECT_CALL(visitor, OnFrameHeader(1, _, kMetadataFrameType, 4));
-  EXPECT_CALL(visitor, OnBeginMetadataForStream(1, _));
-  EXPECT_CALL(visitor, OnMetadataForStream(1, _))
+  EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, HEADERS, 4));
+  EXPECT_CALL(visitor, OnBeginHeadersForStream(stream_id));
+  EXPECT_CALL(visitor, OnHeaderForStream(stream_id, _, _)).Times(3);
+  EXPECT_CALL(visitor, OnEndHeadersForStream(stream_id));
+  EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, kMetadataFrameType, 4));
+  EXPECT_CALL(visitor, OnBeginMetadataForStream(stream_id, _));
+  EXPECT_CALL(visitor, OnMetadataForStream(stream_id, _))
+      .WillOnce(testing::Return(false));
+  // Remaining frames are not processed due to the error.
+  EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kParseError));
+
+  const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+  // Negative integer returned to indicate an error.
+  EXPECT_LT(stream_result, 0);
+
+  EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+  EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+
+  EXPECT_FALSE(adapter->want_read());
+  EXPECT_TRUE(adapter->want_write());
+  result = adapter->Send();
+  EXPECT_EQ(0, result);
+  EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
+TEST(OgHttp2AdapterClientTest, ClientHandlesMetadataWithCompletionError) {
+  DataSavingVisitor visitor;
+  OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+  auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+  testing::InSequence s;
+
+  const std::vector<const Header> headers =
+      ToHeaders({{":method", "GET"},
+                 {":scheme", "http"},
+                 {":authority", "example.com"},
+                 {":path", "/this/is/request/one"}});
+
+  const int32_t stream_id = adapter->SubmitRequest(headers, nullptr, nullptr);
+  ASSERT_GT(stream_id, 0);
+
+  EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+  EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+  EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5));
+  EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 0x5, 0));
+
+  int result = adapter->Send();
+  EXPECT_EQ(0, result);
+  visitor.Clear();
+
+  const std::string stream_frames =
+      TestFrameSequence()
+          .ServerPreface()
+          .Metadata(0, "Example connection metadata")
+          .Headers(stream_id,
+                   {{":status", "200"},
+                    {"server", "my-fake-server"},
+                    {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+                   /*fin=*/false)
+          .Metadata(stream_id, "Example stream metadata")
+          .Data(stream_id, "This is the response body.", true)
+          .Serialize();
+
+  // Server preface (empty SETTINGS)
+  EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+  EXPECT_CALL(visitor, OnSettingsStart());
+  EXPECT_CALL(visitor, OnSettingsEnd());
+
+  EXPECT_CALL(visitor, OnFrameHeader(0, _, kMetadataFrameType, 4));
+  EXPECT_CALL(visitor, OnBeginMetadataForStream(0, _));
+  EXPECT_CALL(visitor, OnMetadataForStream(0, _));
+  EXPECT_CALL(visitor, OnMetadataEndForStream(0));
+  EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, HEADERS, 4));
+  EXPECT_CALL(visitor, OnBeginHeadersForStream(stream_id));
+  EXPECT_CALL(visitor, OnHeaderForStream(stream_id, _, _)).Times(3);
+  EXPECT_CALL(visitor, OnEndHeadersForStream(stream_id));
+  EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, kMetadataFrameType, 4));
+  EXPECT_CALL(visitor, OnBeginMetadataForStream(stream_id, _));
+  EXPECT_CALL(visitor, OnMetadataForStream(stream_id, _));
+  EXPECT_CALL(visitor, OnMetadataEndForStream(stream_id))
       .WillOnce(testing::Return(false));
   // Remaining frames are not processed due to the error.
   EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kParseError));
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index af843e8..0357f7b 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -1300,12 +1300,17 @@
 void OgHttp2Session::OnFramePayload(const char* data, size_t len) {
   if (metadata_length_ > 0) {
     QUICHE_DCHECK_LE(len, metadata_length_);
-    const bool success = visitor_.OnMetadataForStream(
+    const bool payload_success = visitor_.OnMetadataForStream(
         metadata_stream_id_, absl::string_view(data, len));
-    if (success) {
+    if (payload_success) {
       metadata_length_ -= len;
       if (metadata_length_ == 0 && end_metadata_) {
-        visitor_.OnMetadataEndForStream(metadata_stream_id_);
+        const bool completion_success =
+            visitor_.OnMetadataEndForStream(metadata_stream_id_);
+        if (!completion_success) {
+          fatal_visitor_callback_failure_ = true;
+          decoder_.StopProcessing();
+        }
         metadata_stream_id_ = 0;
         end_metadata_ = false;
       }