Revert "Second attempt: move recv_trailing_metadata into its own callback, don't use on_complete for recv_ops"

3 files changed, 26 insertions, 32 deletions

diff --git a/src/core/lib/transport/transport.cc b/src/core/lib/transport/transport.cc
index cbdb77c844..039d603394 100644
--- a/src/core/lib/transport/transport.cc
+++ b/src/core/lib/transport/transport.cc
@@ -212,32 +212,21 @@ void grpc_transport_stream_op_batch_finish_with_failure(
   if (batch->send_message) {
     batch->payload->send_message.send_message.reset();
   }
-  if (batch->cancel_stream) {
-    GRPC_ERROR_UNREF(batch->payload->cancel_stream.cancel_error);
+  if (batch->recv_message) {
+    GRPC_CALL_COMBINER_START(
+        call_combiner, batch->payload->recv_message.recv_message_ready,
+        GRPC_ERROR_REF(error), "failing recv_message_ready");
   }
-  // Construct a list of closures to execute.
-  grpc_core::CallCombinerClosureList closures;
   if (batch->recv_initial_metadata) {
-    closures.Add(
+    GRPC_CALL_COMBINER_START(
+        call_combiner,
         batch->payload->recv_initial_metadata.recv_initial_metadata_ready,
         GRPC_ERROR_REF(error), "failing recv_initial_metadata_ready");
   }
-  if (batch->recv_message) {
-    closures.Add(batch->payload->recv_message.recv_message_ready,
-                 GRPC_ERROR_REF(error), "failing recv_message_ready");
-  }
-  if (batch->recv_trailing_metadata) {
-    closures.Add(
-        batch->payload->recv_trailing_metadata.recv_trailing_metadata_ready,
-        GRPC_ERROR_REF(error), "failing recv_trailing_metadata_ready");
-  }
-  if (batch->on_complete != nullptr) {
-    closures.Add(batch->on_complete, GRPC_ERROR_REF(error),
-                 "failing on_complete");
+  GRPC_CLOSURE_SCHED(batch->on_complete, error);
+  if (batch->cancel_stream) {
+    GRPC_ERROR_UNREF(batch->payload->cancel_stream.cancel_error);
   }
-  // Execute closures.
-  closures.RunClosures(call_combiner);
-  GRPC_ERROR_UNREF(error);
 }
 
 typedef struct {
diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h
index 585b9dfae9..b2e252d939 100644
--- a/src/core/lib/transport/transport.h
+++ b/src/core/lib/transport/transport.h
@@ -122,15 +122,9 @@ typedef struct grpc_transport_stream_op_batch_payload
 /* Transport stream op: a set of operations to perform on a transport
    against a single stream */
 typedef struct grpc_transport_stream_op_batch {
-  /** Should be scheduled when all of the non-recv operations in the batch
-      are complete.
-
-      The recv ops (recv_initial_metadata, recv_message, and
-      recv_trailing_metadata) each have their own callbacks.  If a batch
-      contains both recv ops and non-recv ops, on_complete should be
-      scheduled as soon as the non-recv ops are complete, regardless of
-      whether or not the recv ops are complete.  If a batch contains
-      only recv ops, on_complete can be null. */
+  /** Should be enqueued when all requested operations (excluding recv_message
+      and recv_initial_metadata which have their own closures) in a given batch
+      have been completed. */
   grpc_closure* on_complete;
 
   /** Values for the stream op (fields set are determined by flags above) */
@@ -155,6 +149,9 @@ typedef struct grpc_transport_stream_op_batch {
    */
   bool recv_trailing_metadata : 1;
 
+  /** Collect any stats into provided buffer, zero internal stat counters */
+  bool collect_stats : 1;
+
   /** Cancel this stream with the provided error */
   bool cancel_stream : 1;
 
@@ -222,11 +219,12 @@ struct grpc_transport_stream_op_batch_payload {
 
   struct {
     grpc_metadata_batch* recv_trailing_metadata;
-    grpc_transport_stream_stats* collect_stats;
-    /** Should be enqueued when initial metadata is ready to be processed. */
-    grpc_closure* recv_trailing_metadata_ready;
   } recv_trailing_metadata;
 
+  struct {
+    grpc_transport_stream_stats* collect_stats;
+  } collect_stats;
+
   /** Forcefully close this stream.
       The HTTP2 semantics should be:
       - server side: if cancel_error has GRPC_ERROR_INT_GRPC_STATUS, and
diff --git a/src/core/lib/transport/transport_op_string.cc b/src/core/lib/transport/transport_op_string.cc
index 8c7db642a5..25ab492f3a 100644
--- a/src/core/lib/transport/transport_op_string.cc
+++ b/src/core/lib/transport/transport_op_string.cc
@@ -120,6 +120,13 @@ char* grpc_transport_stream_op_batch_string(
     gpr_strvec_add(&b, tmp);
   }
 
+  if (op->collect_stats) {
+    gpr_strvec_add(&b, gpr_strdup(" "));
+    gpr_asprintf(&tmp, "COLLECT_STATS:%p",
+                 op->payload->collect_stats.collect_stats);
+    gpr_strvec_add(&b, tmp);
+  }
+
   out = gpr_strvec_flatten(&b, nullptr);
   gpr_strvec_destroy(&b);