[widget] Rewrite data-read loop in OnDataAvailable.

The read-loop in OnDataAvailable was needlessly baroque and used a very
strange dialect of Hungarian notation. Factored out the zero-element
case for simplicity, and added justification in comments as-appropriate.

1 files changed, 40 insertions, 17 deletions

diff --git a/widget/windows/nsDataObj.cpp b/widget/windows/nsDataObj.cpp
index f665e63105..6dc9eb7ecd 100644
--- a/widget/windows/nsDataObj.cpp
+++ b/widget/windows/nsDataObj.cpp
@@ -121,25 +121,48 @@ nsDataObj::CStream::OnDataAvailable(nsIRequest *aRequest,
                                     uint64_t aOffset, // offset within the stream
                                     uint32_t aCount) // bytes available on this call
 {
-    // Extend the write buffer for the incoming data.
-    uint8_t* buffer = mChannelData.AppendElements(aCount, fallible);
-    if (!buffer) {
-      return NS_ERROR_OUT_OF_MEMORY;
+  // If we've been asked to read zero bytes, call `Read` once, just to ensure
+  // any side-effects take place, and return immediately.
+  if (aCount == 0) {
+    char buffer[1] = {0};
+    uint32_t bytesReadByCall = 0;
+    nsresult rv = aInputStream->Read(buffer, 0, &bytesReadByCall);
+    MOZ_ASSERT(bytesReadByCall == 0);
+    return rv;
+  }
+
+  // Extend the write buffer for the incoming data.
+  size_t oldLength = mChannelData.Length();
+  char* buffer = reinterpret_cast<char*>(mChannelData.AppendElements(aCount, fallible));
+  if (!buffer) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  MOZ_ASSERT(mChannelData.Length() == (aOffset + aCount),
+             "stream length mismatch w/write buffer");
+
+  // Read() may not return aCount on a single call, so loop until we've
+  // accumulated all the data OnDataAvailable has promised.
+  uint32_t bytesRead = 0;
+  while (bytesRead < aCount) {
+    uint32_t bytesReadByCall = 0;
+    nsresult rv = aInputStream->Read(buffer + bytesRead, aCount - bytesRead,
+                                     &bytesReadByCall);
+    bytesRead += bytesReadByCall;
+
+    if (bytesReadByCall == 0) {
+      // A `bytesReadByCall` of zero indicates EOF without failure... but we
+      // were promised `aCount` elements and haven't gotten them. Return a
+      // generic failure.
+      rv = NS_ERROR_FAILURE;
     }
-    NS_ASSERTION((mChannelData.Length() == (aOffset + aCount)),
-      "stream length mismatch w/write buffer");
 
-    // Read() may not return aCount on a single call, so loop until we've
-    // accumulated all the data OnDataAvailable has promised.
-    nsresult rv;
-    uint32_t odaBytesReadTotal = 0;
-    do {
-      uint32_t bytesReadByCall = 0;
-      rv = aInputStream->Read((char*)(buffer + odaBytesReadTotal),
-                              aCount, &bytesReadByCall);
-      odaBytesReadTotal += bytesReadByCall;
-    } while (aCount < odaBytesReadTotal && NS_SUCCEEDED(rv));
-    return rv;
+    if (NS_FAILED(rv)) {
+      // Drop any trailing uninitialized elements before erroring out.
+      mChannelData.RemoveElementsAt(oldLength + bytesRead, aCount - bytesRead);
+      return rv;
+    }
+  }
+  return NS_OK;
 }
 
 NS_IMETHODIMP nsDataObj::CStream::OnStartRequest(nsIRequest *aRequest,