Improvements to C++ filter API:

- Make sure all C-core parameters are passed into C++ methods.
- Add Destroy() methods for ChannelData and CallData.
- Use C++-style casts.
- Add 'extern "C"' to iomgr/closure.h, which is used in C++ filters.

3 files changed, 42 insertions, 24 deletions

diff --git a/src/core/lib/iomgr/closure.h b/src/core/lib/iomgr/closure.h
index 2560bf4527..cd32a4ba38 100644
--- a/src/core/lib/iomgr/closure.h
+++ b/src/core/lib/iomgr/closure.h
@@ -26,6 +26,10 @@
 #include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/support/mpscq.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 struct grpc_closure;
 typedef struct grpc_closure grpc_closure;
 
@@ -197,4 +201,8 @@ void grpc_closure_list_sched(grpc_exec_ctx *exec_ctx,
   grpc_closure_list_sched(exec_ctx, closure_list)
 #endif
 
+#ifdef __cplusplus
+}
+#endif
+
 #endif /* GRPC_CORE_LIB_IOMGR_CLOSURE_H */
diff --git a/src/cpp/common/channel_filter.h b/src/cpp/common/channel_filter.h
index 1b6ace6b13..5d629f7c14 100644
--- a/src/cpp/common/channel_filter.h
+++ b/src/cpp/common/channel_filter.h
@@ -208,38 +208,45 @@ class TransportStreamOpBatch {
 /// Represents channel data.
 class ChannelData {
  public:
+  ChannelData() {}
   virtual ~ChannelData() {}
 
-  /// Initializes the call data.
-  virtual grpc_error *Init(grpc_exec_ctx *exec_ctx,
+  // TODO(roth): Come up with a more C++-like API for the channel element.
+
+  /// Initializes the channel data.
+  virtual grpc_error *Init(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem,
                            grpc_channel_element_args *args) {
     return GRPC_ERROR_NONE;
   }
 
-  // TODO(roth): Find a way to avoid passing elem into these methods.
+  // Called before destruction.
+  virtual void Destroy(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem) {}
 
   virtual void StartTransportOp(grpc_exec_ctx *exec_ctx,
                                 grpc_channel_element *elem, TransportOp *op);
 
   virtual void GetInfo(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem,
                        const grpc_channel_info *channel_info);
-
- protected:
-  ChannelData() {}
 };
 
 /// Represents call data.
 class CallData {
  public:
+  CallData() {}
   virtual ~CallData() {}
 
+  // TODO(roth): Come up with a more C++-like API for the call element.
+
   /// Initializes the call data.
-  virtual grpc_error *Init(grpc_exec_ctx *exec_ctx, ChannelData *channel_data,
+  virtual grpc_error *Init(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
                            const grpc_call_element_args *args) {
     return GRPC_ERROR_NONE;
   }
 
-  // TODO(roth): Find a way to avoid passing elem into these methods.
+  // Called before destruction.
+  virtual void Destroy(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
+                       const grpc_call_final_info *final_info,
+                       grpc_closure *then_call_closure) {}
 
   /// Starts a new stream operation.
   virtual void StartTransportStreamOpBatch(grpc_exec_ctx *exec_ctx,
@@ -253,9 +260,6 @@ class CallData {
 
   /// Gets the peer name.
   virtual char *GetPeer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem);
-
- protected:
-  CallData() {}
 };
 
 namespace internal {
@@ -271,19 +275,24 @@ class ChannelFilter final {
   static grpc_error *InitChannelElement(grpc_exec_ctx *exec_ctx,
                                         grpc_channel_element *elem,
                                         grpc_channel_element_args *args) {
+    // Construct the object in the already-allocated memory.
     ChannelDataType *channel_data = new (elem->channel_data) ChannelDataType();
-    return channel_data->Init(exec_ctx, args);
+    return channel_data->Init(exec_ctx, elem, args);
   }
 
   static void DestroyChannelElement(grpc_exec_ctx *exec_ctx,
                                     grpc_channel_element *elem) {
-    reinterpret_cast<ChannelDataType *>(elem->channel_data)->~ChannelDataType();
+    ChannelDataType *channel_data =
+        reinterpret_cast<ChannelDataType *>(elem->channel_data);
+    channel_data->Destroy(exec_ctx, elem);
+    channel_data->~ChannelDataType();
   }
 
   static void StartTransportOp(grpc_exec_ctx *exec_ctx,
                                grpc_channel_element *elem,
                                grpc_transport_op *op) {
-    ChannelDataType *channel_data = (ChannelDataType *)elem->channel_data;
+    ChannelDataType *channel_data =
+        reinterpret_cast<ChannelDataType *>(elem->channel_data);
     TransportOp op_wrapper(op);
     channel_data->StartTransportOp(exec_ctx, elem, &op_wrapper);
   }
@@ -291,7 +300,8 @@ class ChannelFilter final {
   static void GetChannelInfo(grpc_exec_ctx *exec_ctx,
                              grpc_channel_element *elem,
                              const grpc_channel_info *channel_info) {
-    ChannelDataType *channel_data = (ChannelDataType *)elem->channel_data;
+    ChannelDataType *channel_data =
+        reinterpret_cast<ChannelDataType *>(elem->channel_data);
     channel_data->GetInfo(exec_ctx, elem, channel_info);
   }
 
@@ -300,24 +310,24 @@ class ChannelFilter final {
   static grpc_error *InitCallElement(grpc_exec_ctx *exec_ctx,
                                      grpc_call_element *elem,
                                      const grpc_call_element_args *args) {
-    ChannelDataType *channel_data = (ChannelDataType *)elem->channel_data;
     // Construct the object in the already-allocated memory.
     CallDataType *call_data = new (elem->call_data) CallDataType();
-    return call_data->Init(exec_ctx, channel_data, args);
+    return call_data->Init(exec_ctx, elem, args);
   }
 
   static void DestroyCallElement(grpc_exec_ctx *exec_ctx,
                                  grpc_call_element *elem,
                                  const grpc_call_final_info *final_info,
                                  grpc_closure *then_call_closure) {
-    GPR_ASSERT(then_call_closure == NULL);
-    reinterpret_cast<CallDataType *>(elem->call_data)->~CallDataType();
+    CallDataType *call_data = reinterpret_cast<CallDataType *>(elem->call_data);
+    call_data->Destroy(exec_ctx, elem, final_info, then_call_closure);
+    call_data->~CallDataType();
   }
 
   static void StartTransportStreamOpBatch(grpc_exec_ctx *exec_ctx,
                                           grpc_call_element *elem,
                                           grpc_transport_stream_op_batch *op) {
-    CallDataType *call_data = (CallDataType *)elem->call_data;
+    CallDataType *call_data = reinterpret_cast<CallDataType *>(elem->call_data);
     TransportStreamOpBatch op_wrapper(op);
     call_data->StartTransportStreamOpBatch(exec_ctx, elem, &op_wrapper);
   }
@@ -325,12 +335,12 @@ class ChannelFilter final {
   static void SetPollsetOrPollsetSet(grpc_exec_ctx *exec_ctx,
                                      grpc_call_element *elem,
                                      grpc_polling_entity *pollent) {
-    CallDataType *call_data = (CallDataType *)elem->call_data;
+    CallDataType *call_data = reinterpret_cast<CallDataType *>(elem->call_data);
     call_data->SetPollsetOrPollsetSet(exec_ctx, elem, pollent);
   }
 
   static char *GetPeer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) {
-    CallDataType *call_data = (CallDataType *)elem->call_data;
+    CallDataType *call_data = reinterpret_cast<CallDataType *>(elem->call_data);
     return call_data->GetPeer(exec_ctx, elem);
   }
 };
diff --git a/test/cpp/common/channel_filter_test.cc b/test/cpp/common/channel_filter_test.cc
index e747e633a0..638518107b 100644
--- a/test/cpp/common/channel_filter_test.cc
+++ b/test/cpp/common/channel_filter_test.cc
@@ -28,7 +28,7 @@ class MyChannelData : public ChannelData {
  public:
   MyChannelData() {}
 
-  grpc_error* Init(grpc_exec_ctx* exec_ctx,
+  grpc_error* Init(grpc_exec_ctx* exec_ctx, grpc_channel_element* elem,
                    grpc_channel_element_args* args) override {
     (void)args->channel_args;  // Make sure field is available.
     return GRPC_ERROR_NONE;
@@ -39,7 +39,7 @@ class MyCallData : public CallData {
  public:
   MyCallData() {}
 
-  grpc_error* Init(grpc_exec_ctx* exec_ctx, ChannelData* channel_data,
+  grpc_error* Init(grpc_exec_ctx* exec_ctx, grpc_call_element* elem,
                    const grpc_call_element_args* args) override {
     (void)args->path;  // Make sure field is available.
     return GRPC_ERROR_NONE;