commit 13074419cdd890bfe67fbba70e70857375a1eb9f
author QUICHE team <quiche-dev@google.com> Thu Dec 16 12:40:23 2021 -0800
committer Copybara-Service <copybara-worker@google.com> Thu Dec 16 12:41:49 2021 -0800
tree 7c9cb3a509a3dafe26f153df58df1b83ded64f4a
parent ed970f7c38381309e9306982efa8dcfac1201269

Refactor OgHttp2Session::ProcessBytes().

This CL adds internal OgHttp2Session::ProcessBytesImpl() that returns an
absl::variant that contains either the number of bytes processed on success or
an error code on failure. Then ProcessBytes() calls ProcessBytesImpl() and
returns an overall int based on the absl::variant.

This CL is a pure refactoring CL with only one error code at the moment, but
the error codes will grow in a future CL to include values compatible with
nghttp2. This compatibility will be useful for some codec_impl_test tests.

PiperOrigin-RevId: 416872938

http2/adapter/oghttp2_session.cc

diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 0519cee..84d6473 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -1,5 +1,6 @@
 #include "http2/adapter/oghttp2_session.h"
 
+#include <cstdint>
 #include <tuple>
 #include <utility>
 
@@ -263,6 +264,19 @@
   frame_contains_fin_ = false;
 }
 
+// A visitor that extracts an int64_t from each type of a ProcessBytesResult.
+struct OgHttp2Session::ProcessBytesResultVisitor {
+  int64_t operator()(const int64_t bytes) const { return bytes; }
+
+  int64_t operator()(const ProcessBytesError error) const {
+    switch (error) {
+      case ProcessBytesError::kUnspecified:
+        return -1;
+    }
+    return -1;
+  }
+};
+
 OgHttp2Session::OgHttp2Session(Http2VisitorInterface& visitor, Options options)
     : visitor_(visitor),
       event_forwarder_([this]() { return !latched_error_; }, *this),
@@ -377,6 +391,11 @@
 int64_t OgHttp2Session::ProcessBytes(absl::string_view bytes) {
   QUICHE_VLOG(2) << TracePerspectiveAsString(options_.perspective)
                  << " processing [" << absl::CEscape(bytes) << "]";
+  return absl::visit(ProcessBytesResultVisitor(), ProcessBytesImpl(bytes));
+}
+
+absl::variant<int64_t, OgHttp2Session::ProcessBytesError>
+OgHttp2Session::ProcessBytesImpl(absl::string_view bytes) {
   if (processing_bytes_) {
     QUICHE_VLOG(1) << "Returning early; already processing bytes.";
     return 0;
@@ -396,7 +415,7 @@
                         << absl::CEscape(bytes) << "]";
       LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
                           ConnectionError::kInvalidConnectionPreface);
-      return -1;
+      return ProcessBytesError::kUnspecified;
     }
     remaining_preface_.remove_prefix(min_size);
     bytes.remove_prefix(min_size);
@@ -408,13 +427,12 @@
     preface_consumed = min_size;
   }
   int64_t result = decoder_.ProcessInput(bytes.data(), bytes.size());
-  if (latched_error_) {
+  QUICHE_VLOG(2) << "ProcessBytes result: " << result;
+  if (latched_error_ || result < 0) {
     QUICHE_VLOG(2) << "ProcessBytes encountered an error.";
-    return -1;
+    return ProcessBytesError::kUnspecified;
   }
-  const int64_t ret = result < 0 ? result : result + preface_consumed;
-  QUICHE_VLOG(2) << "ProcessBytes returning: " << ret;
-  return ret;
+  return result + preface_consumed;
 }
 
 int OgHttp2Session::Consume(Http2StreamId stream_id, size_t num_bytes) {

http2/adapter/oghttp2_session.h

diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index aea6749..c3fbab2 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -9,6 +9,7 @@
 
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
+#include "absl/types/variant.h"
 #include "http2/adapter/data_source.h"
 #include "http2/adapter/event_forwarder.h"
 #include "http2/adapter/header_validator.h"
@@ -265,6 +266,8 @@
     bool frame_contains_fin_ = false;
   };
 
+  struct QUICHE_EXPORT_PRIVATE ProcessBytesResultVisitor;
+
   // Queues the connection preface, if not already done.
   void MaybeSetupPreface();
 
@@ -287,6 +290,16 @@
     SEND_ERROR,
   };
 
+  enum class ProcessBytesError {
+    // A general, unspecified error.
+    kUnspecified,
+  };
+  using ProcessBytesResult = absl::variant<int64_t, ProcessBytesError>;
+
+  // Attempts to process `bytes` and returns the number of bytes proccessed on
+  // success or the processing error on failure.
+  ProcessBytesResult ProcessBytesImpl(absl::string_view bytes);
+
   // Returns true if at least one stream has data or control frames to write.
   bool HasReadyStream() const;