src,stream: improve DoWrite() and Write()

Clarify StreamResource::DoWrite() interface definition.
Fix error-prone Http2Stream::DoWrite().
Change StreamBase::Write() to allow caller to avoid
inefficient write behavior.

PR-URL: #44434
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

src/node_http2.cc

@@ -2375,8 +2375,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
   CHECK_NULL(send_handle);
   Http2Scope h2scope(this);
   if (!is_writable() || is_destroyed()) {
-    req_wrap->Done(UV_EOF);
-    return 0;
+    return UV_EOF;
   }
   Debug(this, "queuing %d buffers to send", nbufs);
   for (size_t i = 0; i < nbufs; ++i) {

src/stream_base-inl.h

@@ -160,11 +160,11 @@ int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
   return err;
 }
 
-StreamWriteResult StreamBase::Write(
-    uv_buf_t* bufs,
-    size_t count,
-    uv_stream_t* send_handle,
-    v8::Local<v8::Object> req_wrap_obj) {
+StreamWriteResult StreamBase::Write(uv_buf_t* bufs,
+                                    size_t count,
+                                    uv_stream_t* send_handle,
+                                    v8::Local<v8::Object> req_wrap_obj,
+                                    bool skip_try_write) {
   Environment* env = stream_env();
   int err;
 
@@ -173,7 +173,7 @@ StreamWriteResult StreamBase::Write(
     total_bytes += bufs[i].len;
   bytes_written_ += total_bytes;
 
-  if (send_handle == nullptr) {
+  if (send_handle == nullptr && !skip_try_write) {
     err = DoTryWrite(&bufs, &count);
     if (err != 0 || count == 0) {
       return StreamWriteResult { false, err, nullptr, total_bytes, {} };

src/stream_base.cc

@@ -335,7 +335,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
     }
   }
 
-  StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj);
+  StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj, try_write);
   res.bytes += synchronously_written;
 
   SetWriteResult(res);

src/stream_base.h

@@ -244,14 +244,19 @@ class StreamResource {
   // `*bufs` and `*count` accordingly. This is a no-op by default.
   // Return 0 for success and a libuv error code for failures.
   virtual int DoTryWrite(uv_buf_t** bufs, size_t* count);
-  // Initiate a write of data. If the write completes synchronously, return 0 on
-  // success (with bufs modified to indicate how much data was consumed) or a
-  // libuv error code on failure. If the write will complete asynchronously,
-  // return 0. When the write completes asynchronously, call req_wrap->Done()
-  // with 0 on success (with bufs modified to indicate how much data was
-  // consumed) or a libuv error code on failure. Do not call req_wrap->Done() if
-  // the write completes synchronously, that is, it should never be called
-  // before DoWrite() has returned.
+  // Initiate a write of data.
+  // Upon an immediate failure, a libuv error code is returned,
+  // w->Done() will never be called and caller should free `bufs`.
+  // Otherwise, 0 is returned and w->Done(status) will be called
+  // with status set to either
+  //  (1) 0 after all data are written, or
+  //  (2) a libuv error code when an error occurs
+  // in either case, w->Done() will never be called before DoWrite() returns.
+  // When 0 is returned:
+  //  (1) memory specified by `bufs` and `count` must remain valid until
+  //      w->Done() gets called.
+  //  (2) `bufs` might or might not be changed, caller should not rely on this.
+  //  (3) `bufs` should be freed after w->Done() gets called.
   virtual int DoWrite(WriteWrap* w,
                       uv_buf_t* bufs,
                       size_t count,
@@ -343,13 +348,17 @@ class StreamBase : public StreamResource {
   // WriteWrap object (that was created in JS), or a new one will be created.
   // This will first try to write synchronously using `DoTryWrite()`, then
   // asynchronously using `DoWrite()`.
+  // Caller can pass `skip_try_write` as true if it has already called
+  // `DoTryWrite()` and ends up with a partial write, or it knows that the
+  // write is too large to finish synchronously.
   // If the return value indicates a synchronous completion, no callback will
   // be invoked.
   inline StreamWriteResult Write(
       uv_buf_t* bufs,
       size_t count,
       uv_stream_t* send_handle = nullptr,
-      v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>());
+      v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>(),
+      bool skip_try_write = false);
 
   // These can be overridden by subclasses to get more specific wrap instances.
   // For example, a subclass Foo could create a FooWriteWrap or FooShutdownWrap