Fix multi-layer virtual function calls. (#643)

proxygen/lib/http/codec/HQControlCodec.h

@@ -67,8 +67,9 @@ class HQControlCodec
   // HTTPCodec API
   bool isWaitingToDrain() const override;
 
-  CodecProtocol getProtocol() const override {
-    return CodecProtocol::HQ;
+  HTTPCodecTraits getTraits() const override {
+    return HTTPCodecTraits{.protocol = CodecProtocol::HQ,
+                           .direction = transportDirection_};
   }
 
   size_t onIngress(const folly::IOBuf& /*buf*/) override {

proxygen/lib/http/codec/HQFramedCodec.h

@@ -36,22 +36,12 @@ class HQFramedCodec : public HTTPCodec {
 
   // HTTPCodec API
 
-  // Only implemented in the Stream Codec
-  CodecProtocol getProtocol() const override {
-    LOG(FATAL) << __func__ << " not supported on this codec";
-    folly::assume_unreachable();
-  }
-
   // Only implemented in the Stream Codec
   const std::string& getUserAgent() const override {
     LOG(FATAL) << __func__ << " not supported on this codec";
     folly::assume_unreachable();
   }
 
-  TransportDirection getTransportDirection() const override {
-    return transportDirection_;
-  }
-
   // Stream multiplexing handled at the transport
   HTTPCodec::StreamID createStream() override {
     LOG(FATAL) << __func__ << " not supported on this codec";
@@ -93,10 +83,6 @@ class HQFramedCodec : public HTTPCodec {
     return false;
   }
 
-  bool supportsParallelRequests() const override {
-    return false;
-  }
-
   // no connection preface for HQ
   size_t generateConnectionPreface(folly::IOBufQueue& /*writeBuf*/) override {
     LOG(FATAL) << __func__ << " not supported on this codec";

proxygen/lib/http/codec/HQMultiCodec.h

@@ -164,8 +164,10 @@ class HQMultiCodec : public HQControlCodec {
     return res;
   }
 
-  bool supportsParallelRequests() const override {
-    return true;
+  HTTPCodecTraits getTraits() const override {
+    return HTTPCodecTraits{.protocol = CodecProtocol::HQ,
+                           .direction = transportDirection_,
+                           .supportsParallelRequests = true};
   }
 
   size_t generateConnectionPreface(folly::IOBufQueue& /*writeBuf*/) override {

proxygen/lib/http/codec/HQStreamCodec.h

@@ -58,8 +58,9 @@ class HQStreamCodec
     return streamId_;
   }
 
-  CodecProtocol getProtocol() const override {
-    return CodecProtocol::HQ;
+  HTTPCodecTraits getTraits() const override {
+    return HTTPCodecTraits{.protocol = CodecProtocol::HQ,
+                           .direction = transportDirection_};
   }
 
   const std::string& getUserAgent() const override {

proxygen/lib/http/codec/HTTP1xCodec.h

@@ -37,17 +37,15 @@ class HTTP1xCodec : public HTTPCodec {
   static HTTP1xCodec makeResponseCodec(bool mayChunkEgress);
 
   // HTTPCodec API
-  CodecProtocol getProtocol() const override {
-    return CodecProtocol::HTTP_1_1;
+  HTTPCodecTraits getTraits() const override {
+    return HTTPCodecTraits{.protocol = CodecProtocol::HTTP_1_1,
+                           .direction = transportDirection_};
   }
 
   const std::string& getUserAgent() const override {
     return userAgent_;
   }
 
-  TransportDirection getTransportDirection() const override {
-    return transportDirection_;
-  }
   StreamID createStream() override;
   void setCallback(Callback* callback) override {
     callback_ = callback;
@@ -80,9 +78,6 @@ class HTTP1xCodec : public HTTPCodec {
   bool closeOnEgressComplete() const override {
     return !isEgressBusy() && !isReusable();
   }
-  bool supportsParallelRequests() const override {
-    return false;
-  }
   bool supportsPushTransactions() const override {
     return false;
   }

proxygen/lib/http/codec/HTTP2Codec.h

@@ -35,8 +35,12 @@ class HTTP2Codec
   ~HTTP2Codec() override;
 
   // HTTPCodec API
-  CodecProtocol getProtocol() const override {
-    return CodecProtocol::HTTP_2;
+  HTTPCodecTraits getTraits() const override {
+    return HTTPCodecTraits{.protocol = CodecProtocol::HTTP_2,
+                           .direction = transportDirection_,
+                           .supportsParallelRequests = true,
+                           .supportsSessionFlowControl = true,
+                           .supportsStreamFlowControl = true};
   }
 
   const std::string& getUserAgent() const override {

proxygen/lib/http/codec/HTTPBinaryCodec.h

@@ -64,17 +64,15 @@ class HTTPBinaryCodec : public HTTPCodec {
   HTTPBinaryCodec(HTTPBinaryCodec&&) = default;
 
   // HTTPCodec API
-  CodecProtocol getProtocol() const override {
-    return CodecProtocol::HTTP_BINARY;
+  HTTPCodecTraits getTraits() const override {
+    return HTTPCodecTraits{.protocol = CodecProtocol::HTTP_BINARY,
+                           .direction = transportDirection_};
   }
 
   const std::string& getUserAgent() const override {
     return userAgent_;
   }
 
-  TransportDirection getTransportDirection() const override {
-    return transportDirection_;
-  }
   StreamID createStream() override {
     return 0;
   }
@@ -111,9 +109,6 @@ class HTTPBinaryCodec : public HTTPCodec {
   bool closeOnEgressComplete() const override {
     return !isEgressBusy() && !isReusable();
   }
-  bool supportsParallelRequests() const override {
-    return false;
-  }
   bool supportsPushTransactions() const override {
     return false;
   }

proxygen/lib/http/codec/HTTPCodec.h

@@ -28,6 +28,19 @@ class HTTPMessage;
 class HTTPTransactionHandler;
 class HTTPErrorPage;
 
+/**
+ * Codec properties that never change after construction.
+ */
+struct HTTPCodecTraits {
+  CodecProtocol protocol{};
+  TransportDirection direction{};
+  bool supportsParallelRequests{false};
+  bool supportsSessionFlowControl{false};
+  bool supportsStreamFlowControl{false};
+
+  bool operator==(const HTTPCodecTraits&) const = default;
+};
+
 /**
  * Interface for a parser&generator that can translate between an internal
  * representation of an HTTP request and a wire format.  The details of the
@@ -334,11 +347,15 @@ class HTTPCodec {
     return defaultCompressionInfo;
   }
 
+  [[nodiscard]] virtual HTTPCodecTraits getTraits() const = 0;
+
   /**
    * Gets the session protocol currently used by the codec. This can be
    * mapped to a string for logging and diagnostic use.
    */
-  [[nodiscard]] virtual CodecProtocol getProtocol() const = 0;
+  [[nodiscard]] CodecProtocol getProtocol() const {
+    return getTraits().protocol;
+  }
 
   /**
    * Gets the user agent string of the client. Thus, it is only meaningful for a
@@ -353,20 +370,22 @@ class HTTPCodec {
    * DOWNSTREAM if the codec receives requests from clients or
    * UPSTREAM if the codec sends requests to servers.
    */
-  [[nodiscard]] virtual TransportDirection getTransportDirection() const = 0;
+  [[nodiscard]] TransportDirection getTransportDirection() const {
+    return getTraits().direction;
+  }
 
   /**
    * Returns true iff this codec supports per stream flow control
    */
-  [[nodiscard]] virtual bool supportsStreamFlowControl() const {
-    return false;
+  [[nodiscard]] bool supportsStreamFlowControl() const {
+    return getTraits().supportsStreamFlowControl;
   }
 
   /**
    * Returns true iff this codec supports session level flow control
    */
-  [[nodiscard]] virtual bool supportsSessionFlowControl() const {
-    return false;
+  [[nodiscard]] bool supportsSessionFlowControl() const {
+    return getTraits().supportsSessionFlowControl;
   }
 
   /**
@@ -442,7 +461,9 @@ class HTTPCodec {
    * Check whether the codec supports the processing of multiple
    * requests in parallel.
    */
-  [[nodiscard]] virtual bool supportsParallelRequests() const = 0;
+  [[nodiscard]] bool supportsParallelRequests() const {
+    return getTraits().supportsParallelRequests;
+  }
 
   /**
    * Check whether the codec supports pushing resources from server to

proxygen/lib/http/codec/HTTPCodecFilter.cpp

@@ -127,26 +127,14 @@ CompressionInfo PassThroughHTTPCodecFilter::getCompressionInfo() const {
   return call_->getCompressionInfo();
 }
 
-CodecProtocol PassThroughHTTPCodecFilter::getProtocol() const {
-  return call_->getProtocol();
+HTTPCodecTraits PassThroughHTTPCodecFilter::getTraits() const {
+  return call_->getTraits();
 }
 
 const std::string& PassThroughHTTPCodecFilter::getUserAgent() const {
   return call_->getUserAgent();
 }
 
-TransportDirection PassThroughHTTPCodecFilter::getTransportDirection() const {
-  return call_->getTransportDirection();
-}
-
-bool PassThroughHTTPCodecFilter::supportsStreamFlowControl() const {
-  return call_->supportsStreamFlowControl();
-}
-
-bool PassThroughHTTPCodecFilter::supportsSessionFlowControl() const {
-  return call_->supportsSessionFlowControl();
-}
-
 HTTPCodec::StreamID PassThroughHTTPCodecFilter::createStream() {
   return call_->createStream();
 }
@@ -187,10 +175,6 @@ bool PassThroughHTTPCodecFilter::closeOnEgressComplete() const {
   return call_->closeOnEgressComplete();
 }
 
-bool PassThroughHTTPCodecFilter::supportsParallelRequests() const {
-  return call_->supportsParallelRequests();
-}
-
 bool PassThroughHTTPCodecFilter::supportsPushTransactions() const {
   return call_->supportsPushTransactions();
 }

proxygen/lib/http/codec/HTTPCodecFilter.h

@@ -97,16 +97,10 @@ class PassThroughHTTPCodecFilter : public HTTPCodecFilter {
   // HTTPCodec methods
   [[nodiscard]] CompressionInfo getCompressionInfo() const override;
 
-  [[nodiscard]] CodecProtocol getProtocol() const override;
+  [[nodiscard]] HTTPCodecTraits getTraits() const override;
 
   [[nodiscard]] const std::string& getUserAgent() const override;
 
-  [[nodiscard]] TransportDirection getTransportDirection() const override;
-
-  [[nodiscard]] bool supportsStreamFlowControl() const override;
-
-  [[nodiscard]] bool supportsSessionFlowControl() const override;
-
   StreamID createStream() override;
 
   void setCallback(HTTPCodec::Callback* callback) override;
@@ -127,8 +121,6 @@ class PassThroughHTTPCodecFilter : public HTTPCodecFilter {
 
   [[nodiscard]] bool closeOnEgressComplete() const override;
 
-  [[nodiscard]] bool supportsParallelRequests() const override;
-
   [[nodiscard]] bool supportsPushTransactions() const override;
 
   size_t generateConnectionPreface(folly::IOBufQueue& writeBuf) override;
@@ -218,10 +210,97 @@ class PassThroughHTTPCodecFilter : public HTTPCodecFilter {
   [[nodiscard]] uint32_t getDefaultWindowSize() const override;
 };
 
-using HTTPCodecFilterChain = FilterChain<HTTPCodec,
-                                         HTTPCodec::Callback,
-                                         PassThroughHTTPCodecFilter,
-                                         &HTTPCodec::setCallback,
-                                         true>;
+class HTTPCodecFilterChain {
+  using Chain = FilterChain<HTTPCodec,
+                            HTTPCodec::Callback,
+                            PassThroughHTTPCodecFilter,
+                            &HTTPCodec::setCallback,
+                            true>;
+
+ public:
+  explicit HTTPCodecFilterChain(std::unique_ptr<HTTPCodec> codec)
+      : chain_(std::move(codec)), traits_(chain_.getChainEnd().getTraits()) {
+  }
+
+  HTTPCodecFilterChain(const HTTPCodecFilterChain&) = delete;
+  HTTPCodecFilterChain& operator=(const HTTPCodecFilterChain&) = delete;
+  HTTPCodecFilterChain(HTTPCodecFilterChain&&) = delete;
+  HTTPCodecFilterChain& operator=(HTTPCodecFilterChain&&) = delete;
+  ~HTTPCodecFilterChain() = default;
+
+  [[nodiscard]] CodecProtocol getProtocol() const {
+    return traits().protocol;
+  }
+
+  [[nodiscard]] TransportDirection getTransportDirection() const {
+    return traits().direction;
+  }
+
+  [[nodiscard]] bool supportsParallelRequests() const {
+    return traits().supportsParallelRequests;
+  }
+
+  [[nodiscard]] bool supportsSessionFlowControl() const {
+    return traits().supportsSessionFlowControl;
+  }
+
+  [[nodiscard]] bool supportsStreamFlowControl() const {
+    return traits().supportsStreamFlowControl;
+  }
+
+  HTTPCodec* operator->() {
+    return chain_.operator->();
+  }
+  const HTTPCodec* operator->() const {
+    return chain_.operator->();
+  }
+
+  HTTPCodec* call() {
+    return chain_.call();
+  }
+
+  [[nodiscard]] const HTTPCodec& getChainEnd() const {
+    return chain_.getChainEnd();
+  }
+
+  HTTPCodec* getChainEndPtr() {
+    return chain_.getChainEndPtr();
+  }
+
+  void setCallback(HTTPCodec::Callback* callback) {
+    chain_.setCallback(callback);
+  }
+
+  template <typename Filter, typename... Args>
+  void add(Args&&... args) {
+    chain_.add<Filter>(std::forward<Args>(args)...);
+  }
+
+  template <typename... Filters>
+  void addFilters(Filters&&... filters) {
+    chain_.addFilters(std::forward<Filters>(filters)...);
+  }
+
+  template <typename Fn>
+  void foreach (Fn&& fn) {
+    chain_.foreach (std::forward<Fn>(fn));
+  }
+
+  std::unique_ptr<HTTPCodec> setDestination(std::unique_ptr<HTTPCodec> dest) {
+    auto old = chain_.setDestination(std::move(dest));
+    traits_ = chain_.getChainEnd().getTraits();
+    return old;
+  }
+
+ private:
+  [[nodiscard]] const HTTPCodecTraits& traits() const {
+    DCHECK(chain_.getChainEnd().getTraits() == traits_)
+        << "codec traits changed after the chain cached them";
+    return traits_;
+  }
+
+  Chain chain_;
+  HTTPCodecTraits traits_;
+};
 
 } // namespace proxygen