Merge pull request #9067 from markdroth/cpp_filter_api_cleanup

C++ filter API cleanup

src/core/lib/channel/channel_stack.h

@@ -34,6 +34,13 @@
 #ifndef GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_H
 #define GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_H
 
+//////////////////////////////////////////////////////////////////////////////
+// IMPORTANT NOTE:
+//
+// When you update this API, please make the corresponding changes to
+// the C++ API in src/cpp/common/channel_filter.{h,cc}
+//////////////////////////////////////////////////////////////////////////////
+
 /* A channel filter defines how operations on a channel are implemented.
    Channel filters are chained together to create full channels, and if those
    chains are linear, then channel stacks provide a mechanism to minimize

src/cpp/common/channel_filter.h

@@ -216,15 +216,13 @@ class TransportStreamOp {
 /// Represents channel data.
 class ChannelData {
  public:
-  virtual ~ChannelData() {
-    if (peer_) gpr_free((void *)peer_);
-  }
+  virtual ~ChannelData() {}
 
   /// Initializes the call data.
-  virtual grpc_error *Init() { return GRPC_ERROR_NONE; }
-
-  /// Caller does NOT take ownership of result.
-  const char *peer() const { return peer_; }
+  virtual grpc_error *Init(grpc_exec_ctx *exec_ctx,
+                           grpc_channel_element_args *args) {
+    return GRPC_ERROR_NONE;
+  }
 
   // TODO(roth): Find a way to avoid passing elem into these methods.
 
@@ -235,11 +233,7 @@ class ChannelData {
                        const grpc_channel_info *channel_info);
 
  protected:
-  /// Takes ownership of \a peer.
-  ChannelData(const grpc_channel_args &args, const char *peer) : peer_(peer) {}
-
- private:
-  const char *peer_;
+  ChannelData() {}
 };
 
 /// Represents call data.
@@ -248,7 +242,10 @@ class CallData {
   virtual ~CallData() {}
 
   /// Initializes the call data.
-  virtual grpc_error *Init() { return GRPC_ERROR_NONE; }
+  virtual grpc_error *Init(grpc_exec_ctx *exec_ctx, ChannelData *channel_data,
+                           grpc_call_element_args *args) {
+    return GRPC_ERROR_NONE;
+  }
 
   // TODO(roth): Find a way to avoid passing elem into these methods.
 
@@ -266,7 +263,7 @@ class CallData {
   virtual char *GetPeer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem);
 
  protected:
-  explicit CallData(const ChannelData &) {}
+  CallData() {}
 };
 
 namespace internal {
@@ -282,14 +279,8 @@ class ChannelFilter final {
   static grpc_error *InitChannelElement(grpc_exec_ctx *exec_ctx,
                                         grpc_channel_element *elem,
                                         grpc_channel_element_args *args) {
-    const char *peer =
-        args->optional_transport
-            ? grpc_transport_get_peer(exec_ctx, args->optional_transport)
-            : nullptr;
-    // Construct the object in the already-allocated memory.
-    ChannelDataType *channel_data =
-        new (elem->channel_data) ChannelDataType(*args->channel_args, peer);
-    return channel_data->Init();
+    ChannelDataType *channel_data = new (elem->channel_data) ChannelDataType();
+    return channel_data->Init(exec_ctx, args);
   }
 
   static void DestroyChannelElement(grpc_exec_ctx *exec_ctx,
@@ -317,11 +308,10 @@ class ChannelFilter final {
   static grpc_error *InitCallElement(grpc_exec_ctx *exec_ctx,
                                      grpc_call_element *elem,
                                      grpc_call_element_args *args) {
-    const ChannelDataType &channel_data =
-        *(ChannelDataType *)elem->channel_data;
+    ChannelDataType *channel_data = (ChannelDataType *)elem->channel_data;
     // Construct the object in the already-allocated memory.
-    CallDataType *call_data = new (elem->call_data) CallDataType(channel_data);
-    return call_data->Init();
+    CallDataType *call_data = new (elem->call_data) CallDataType();
+    return call_data->Init(exec_ctx, channel_data, args);
   }
 
   static void DestroyCallElement(grpc_exec_ctx *exec_ctx,

test/cpp/common/channel_filter_test.cc

@@ -41,14 +41,24 @@ namespace testing {
 
 class MyChannelData : public ChannelData {
  public:
-  MyChannelData(const grpc_channel_args& args, const char* peer)
-      : ChannelData(args, peer) {}
+  MyChannelData() {}
+
+  grpc_error* Init(grpc_exec_ctx* exec_ctx,
+                   grpc_channel_element_args* args) override {
+    (void)args->channel_args;  // Make sure field is available.
+    return GRPC_ERROR_NONE;
+  }
 };
 
 class MyCallData : public CallData {
  public:
-  explicit MyCallData(const ChannelData& channel_data)
-      : CallData(channel_data) {}
+  MyCallData() {}
+
+  grpc_error* Init(grpc_exec_ctx* exec_ctx, ChannelData* channel_data,
+                   grpc_call_element_args* args) override {
+    (void)args->path;  // Make sure field is available.
+    return GRPC_ERROR_NONE;
+  }
 };
 
 // This test ensures that when we make changes to the filter API in

test/cpp/end2end/filter_end2end_test.cc

@@ -114,20 +114,17 @@ int GetCallCounterValue() {
 
 class ChannelDataImpl : public ChannelData {
  public:
-  ChannelDataImpl(const grpc_channel_args& args, const char* peer)
-      : ChannelData(args, peer) {
+  grpc_error* Init(grpc_exec_ctx* exec_ctx, grpc_channel_element_args* args) {
     IncrementConnectionCounter();
+    return GRPC_ERROR_NONE;
   }
 };
 
 class CallDataImpl : public CallData {
  public:
-  explicit CallDataImpl(const ChannelDataImpl& channel_data)
-      : CallData(channel_data) {}
-
   void StartTransportStreamOp(grpc_exec_ctx* exec_ctx, grpc_call_element* elem,
                               TransportStreamOp* op) override {
-    // Incrementing the counter could be done from the ctor, but we want
+    // Incrementing the counter could be done from Init(), but we want
     // to test that the individual methods are actually called correctly.
     if (op->recv_initial_metadata() != nullptr) IncrementCallCounter();
     grpc_call_next_op(exec_ctx, elem, op->op());