Address reviewer comments.

1 files changed, 38 insertions, 42 deletions

diff --git a/include/grpcpp/impl/codegen/server_callback.h b/include/grpcpp/impl/codegen/server_callback.h
index c8f7510ed5..5d56cbf1df 100644
--- a/include/grpcpp/impl/codegen/server_callback.h
+++ b/include/grpcpp/impl/codegen/server_callback.h
@@ -48,7 +48,6 @@ class ServerCallbackRpcController {
   // The method handler must call this function when it is done so that
   // the library knows to free its resources
   virtual void Finish(Status s) = 0;
-  virtual void FinishWithError(Status s) = 0;
 
   // Allow the method handler to push out the initial metadata before
   // the response and status are ready
@@ -75,7 +74,8 @@ class CallbackUnaryHandler : public MethodHandler {
         param.call->call(), sizeof(ServerCallbackRpcControllerImpl)))
         ServerCallbackRpcControllerImpl(
             param.server_context, param.call,
-            static_cast<RequestType*>(param.request), std::move(param.renewer));
+            static_cast<RequestType*>(param.request),
+            std::move(param.call_requester));
     Status status = param.status;
 
     if (status.ok()) {
@@ -112,52 +112,19 @@ class CallbackUnaryHandler : public MethodHandler {
   // The implementation class of ServerCallbackRpcController is a private member
   // of CallbackUnaryHandler since it is never exposed anywhere, and this allows
   // it to take advantage of CallbackUnaryHandler's friendships.
-
   class ServerCallbackRpcControllerImpl
       : public experimental::ServerCallbackRpcController {
    public:
-    void Finish(Status s) override { FinishInternal(std::move(s), false); }
-
-    void FinishWithError(Status s) override {
-      FinishInternal(std::move(s), true);
-    }
-
-    void SendInitialMetadata(std::function<void(bool)> f) override {
-      GPR_CODEGEN_ASSERT(!ctx_->sent_initial_metadata_);
-
-      meta_tag_ =
-          CallbackWithSuccessTag(call_.call(), std::move(f), &meta_buf_);
-      meta_buf_.SendInitialMetadata(&ctx_->initial_metadata_,
-                                    ctx_->initial_metadata_flags());
-      if (ctx_->compression_level_set()) {
-        meta_buf_.set_compression_level(ctx_->compression_level());
-      }
-      ctx_->sent_initial_metadata_ = true;
-      meta_buf_.set_cq_tag(&meta_tag_);
-      call_.PerformOps(&meta_buf_);
-    }
-
-   private:
-    template <class SrvType, class ReqType, class RespType>
-    friend class CallbackUnaryHandler;
-
-    ServerCallbackRpcControllerImpl(ServerContext* ctx, Call* call,
-                                    RequestType* req,
-                                    std::function<void()> renewer)
-        : ctx_(ctx), call_(*call), req_(req), renewer_(std::move(renewer)) {}
-
-    ~ServerCallbackRpcControllerImpl() { req_->~RequestType(); }
-
-    void FinishInternal(Status s, bool allow_error) {
+    void Finish(Status s) override {
       finish_tag_ = CallbackWithSuccessTag(
           call_.call(),
           [this](bool) {
             grpc_call* call = call_.call();
-            auto renewer = std::move(renewer_);
+            auto call_requester = std::move(call_requester_);
             this->~ServerCallbackRpcControllerImpl();  // explicitly call
                                                        // destructor
             g_core_codegen_interface->grpc_call_unref(call);
-            renewer();
+            call_requester();
           },
           &finish_buf_);
       if (!ctx_->sent_initial_metadata_) {
@@ -168,17 +135,46 @@ class CallbackUnaryHandler : public MethodHandler {
         }
         ctx_->sent_initial_metadata_ = true;
       }
-      // The response may be dropped if the status is not OK.
-      if (allow_error || s.ok()) {
+      // The response is dropped if the status is not OK.
+      if (s.ok()) {
         finish_buf_.ServerSendStatus(&ctx_->trailing_metadata_,
                                      finish_buf_.SendMessage(resp_));
       } else {
         finish_buf_.ServerSendStatus(&ctx_->trailing_metadata_, s);
       }
-      finish_buf_.set_cq_tag(&finish_tag_);
+      finish_buf_.set_core_cq_tag(&finish_tag_);
       call_.PerformOps(&finish_buf_);
     }
 
+    void SendInitialMetadata(std::function<void(bool)> f) override {
+      GPR_CODEGEN_ASSERT(!ctx_->sent_initial_metadata_);
+
+      meta_tag_ =
+          CallbackWithSuccessTag(call_.call(), std::move(f), &meta_buf_);
+      meta_buf_.SendInitialMetadata(&ctx_->initial_metadata_,
+                                    ctx_->initial_metadata_flags());
+      if (ctx_->compression_level_set()) {
+        meta_buf_.set_compression_level(ctx_->compression_level());
+      }
+      ctx_->sent_initial_metadata_ = true;
+      meta_buf_.set_core_cq_tag(&meta_tag_);
+      call_.PerformOps(&meta_buf_);
+    }
+
+   private:
+    template <class SrvType, class ReqType, class RespType>
+    friend class CallbackUnaryHandler;
+
+    ServerCallbackRpcControllerImpl(ServerContext* ctx, Call* call,
+                                    RequestType* req,
+                                    std::function<void()> call_requester)
+        : ctx_(ctx),
+          call_(*call),
+          req_(req),
+          call_requester_(std::move(call_requester)) {}
+
+    ~ServerCallbackRpcControllerImpl() { req_->~RequestType(); }
+
     RequestType* request() { return req_; }
     ResponseType* response() { return &resp_; }
 
@@ -193,7 +189,7 @@ class CallbackUnaryHandler : public MethodHandler {
     Call call_;
     RequestType* req_;
     ResponseType resp_;
-    std::function<void()> renewer_;
+    std::function<void()> call_requester_;
   };
 };