Bug 1484990 - Use BulkWrite instead of write past length via BeginWriting() in XHR. r=baku
☠☠ backed out by 618546b5d441 ☠ ☠
authorHenri Sivonen <hsivonen@hsivonen.fi>
Wed, 29 Aug 2018 07:43:24 +0000
changeset 491513 510decfb443d585f9913e12227764fecaad5ea8d
parent 491512 4e42a6a201f067a107d0bc0e28508dc2585be87f
child 491514 75b8ac536f30108bcefb6ffa139a1b57bc43f878
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1484990
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1484990 - Use BulkWrite instead of write past length via BeginWriting() in XHR. r=baku The old code assumes that it's OK to use nsAString::BeginWriting() to write past the string's logical length if the string has enough capacity. This is bogus, because the string doesn't know of data written past its logical length. The BulkWrite API has been created precisely for this purpose and allows orderly capacity-aware low-level writes to the string. MozReview-Commit-ID: BYQHl8Z9Fbd Differential Revision: https://phabricator.services.mozilla.com/D3886
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
dom/xhr/XMLHttpRequestString.cpp
dom/xhr/XMLHttpRequestString.h
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -488,56 +488,56 @@ XMLHttpRequestMainThread::DetectCharset(
   } else {
     mDecoder = encoding->NewDecoder();
   }
 
   return NS_OK;
 }
 
 nsresult
-XMLHttpRequestMainThread::AppendToResponseText(const char * aSrcBuffer,
-                                               uint32_t aSrcBufferLen,
+XMLHttpRequestMainThread::AppendToResponseText(Span<const uint8_t> aBuffer,
                                                bool aLast)
 {
   // Call this with an empty buffer to send the decoder the signal
   // that we have hit the end of the stream.
 
   NS_ENSURE_STATE(mDecoder);
 
+  uint32_t len = mResponseText.Length();
+
   CheckedInt<size_t> destBufferLen =
-    mDecoder->MaxUTF16BufferLength(aSrcBufferLen);
-  if (!destBufferLen.isValid()) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  CheckedInt32 size = mResponseText.Length();
-  size += destBufferLen.value();
-  if (!size.isValid()) {
+    mDecoder->MaxUTF16BufferLength(aBuffer.Length());
+  destBufferLen += len;    
+  if (!destBufferLen.isValid() || destBufferLen.value() > UINT32_MAX) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   XMLHttpRequestStringWriterHelper helper(mResponseText);
 
-  if (!helper.AddCapacity(destBufferLen.value())) {
-    return NS_ERROR_OUT_OF_MEMORY;
+  nsresult rv;
+  BulkWriteHandle<char16_t> handle =
+    helper.BulkWrite(destBufferLen.value(), rv);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
 
   uint32_t result;
   size_t read;
   size_t written;
   bool hadErrors;
   Tie(result, read, written, hadErrors) = mDecoder->DecodeToUTF16(
-    AsBytes(MakeSpan(aSrcBuffer, aSrcBufferLen)),
-    MakeSpan(helper.EndOfExistingData(), destBufferLen.value()),
+    aBuffer,
+    handle,
     aLast);
   MOZ_ASSERT(result == kInputEmpty);
-  MOZ_ASSERT(read == aSrcBufferLen);
-  MOZ_ASSERT(written <= destBufferLen.value());
+  MOZ_ASSERT(read == aBuffer.Length());
+  len += written;
+  MOZ_ASSERT(len <= destBufferLen.value());
   Unused << hadErrors;
-  helper.AddLength(written);
+  handle.Finish(len, false);
   if (aLast) {
     // Drop the finished decoder to avoid calling into a decoder
     // that has finished.
     mDecoder = nullptr;
     mEofDecoded = true;
   }
   return NS_OK;
 }
@@ -592,18 +592,18 @@ XMLHttpRequestMainThread::GetResponseTex
     return;
   }
 
   MatchCharsetAndDecoderToResponseDocument();
 
   MOZ_ASSERT(mResponseBodyDecodedPos < mResponseBody.Length() ||
              mState == XMLHttpRequest_Binding::DONE,
              "Unexpected mResponseBodyDecodedPos");
-  aRv = AppendToResponseText(mResponseBody.get() + mResponseBodyDecodedPos,
-                             mResponseBody.Length() - mResponseBodyDecodedPos,
+  Span<const uint8_t> span = mResponseBody;
+  aRv = AppendToResponseText(span.From(mResponseBodyDecodedPos),
                              mState == XMLHttpRequest_Binding::DONE);
   if (aRv.Failed()) {
     return;
   }
 
   mResponseBodyDecodedPos = mResponseBody.Length();
 
   if (mEofDecoded) {
@@ -1603,17 +1603,17 @@ XMLHttpRequestMainThread::StreamReaderFu
     if (!xmlHttpRequest->mResponseBody.Append(fromRawSegment, count, fallible)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
   } else if (xmlHttpRequest->mResponseType == XMLHttpRequestResponseType::_empty ||
              xmlHttpRequest->mResponseType == XMLHttpRequestResponseType::Text ||
              xmlHttpRequest->mResponseType == XMLHttpRequestResponseType::Json) {
     MOZ_ASSERT(!xmlHttpRequest->mResponseXML,
                "We shouldn't be parsing a doc here");
-    rv = xmlHttpRequest->AppendToResponseText(fromRawSegment, count);
+    rv = xmlHttpRequest->AppendToResponseText(AsBytes(MakeSpan(fromRawSegment, count)));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   if (xmlHttpRequest->mFlagParseBody) {
     // Give the same data to the parser.
 
@@ -2094,17 +2094,17 @@ XMLHttpRequestMainThread::OnStopRequest(
   }
 
   // Send the decoder the signal that we've hit the end of the stream,
   // but only when decoding text eagerly.
   if (mDecoder &&
       ((mResponseType == XMLHttpRequestResponseType::Text) ||
         (mResponseType == XMLHttpRequestResponseType::Json) ||
         (mResponseType == XMLHttpRequestResponseType::_empty && !mResponseXML))) {
-    AppendToResponseText(nullptr, 0, true);
+    AppendToResponseText(Span<const uint8_t>(), true);
   }
 
   mWaitingForOnStopRequest = false;
 
   if (mRequestObserver) {
     NS_ASSERTION(mFirstStartRequestSeen, "Inconsistent state!");
     mFirstStartRequestSeen = false;
     mRequestObserver->OnStopRequest(request, ctxt, status);
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -476,17 +476,17 @@ public:
                           Blob* aBlob,
                           nsresult aResult) override;
 
   void
   LocalFileToBlobCompleted(Blob* aBlob);
 
 protected:
   nsresult DetectCharset();
-  nsresult AppendToResponseText(const char * aBuffer, uint32_t aBufferLen,
+  nsresult AppendToResponseText(Span<const uint8_t> aBuffer,
                                 bool aLast = false);
   static nsresult StreamReaderFunc(nsIInputStream* in,
                                    void* closure,
                                    const char* fromRawSegment,
                                    uint32_t toOffset,
                                    uint32_t count,
                                    uint32_t *writeCount);
   nsresult CreateResponseParsedJSON(JSContext* aCx);
--- a/dom/xhr/XMLHttpRequestString.cpp
+++ b/dom/xhr/XMLHttpRequestString.cpp
@@ -33,16 +33,22 @@ public:
   }
 
   uint32_t
   UnsafeLength() const
   {
     return mData.Length();
   }
 
+  mozilla::BulkWriteHandle<char16_t>
+  UnsafeBulkWrite(uint32_t aCapacity, nsresult& aRv)
+  {
+    return mData.BulkWrite(aCapacity, UnsafeLength(), false, aRv);
+  }
+
   void
   Append(const nsAString& aString)
   {
     NS_ASSERT_OWNINGTHREAD(XMLHttpRequestStringBuffer);
 
     MutexAutoLock lock(mMutex);
     mData.Append(aString);
   }
@@ -221,32 +227,26 @@ XMLHttpRequestStringSnapshot::GetAsStrin
 // XMLHttpRequestStringWriterHelper
 
 XMLHttpRequestStringWriterHelper::XMLHttpRequestStringWriterHelper(XMLHttpRequestString& aString)
   : mBuffer(aString.mBuffer)
   , mLock(aString.mBuffer->mMutex)
 {
 }
 
-bool
-XMLHttpRequestStringWriterHelper::AddCapacity(int32_t aCapacity)
+uint32_t
+XMLHttpRequestStringWriterHelper::Length() const
 {
-  return mBuffer->UnsafeData().SetCapacity(mBuffer->UnsafeLength() + aCapacity, fallible);
+  return mBuffer->UnsafeLength();
 }
 
-char16_t*
-XMLHttpRequestStringWriterHelper::EndOfExistingData()
+mozilla::BulkWriteHandle<char16_t>
+XMLHttpRequestStringWriterHelper::BulkWrite(uint32_t aCapacity, nsresult& aRv)
 {
-  return mBuffer->UnsafeData().BeginWriting() + mBuffer->UnsafeLength();
-}
-
-void
-XMLHttpRequestStringWriterHelper::AddLength(int32_t aLength)
-{
-  mBuffer->UnsafeData().SetLength(mBuffer->UnsafeLength() + aLength);
+  return mBuffer->UnsafeBulkWrite(aCapacity, aRv);
 }
 
 // ---------------------------------------------------------------------------
 // XMLHttpRequestStringReaderHelper
 
 XMLHttpRequestStringSnapshotReaderHelper::XMLHttpRequestStringSnapshotReaderHelper(XMLHttpRequestStringSnapshot& aSnapshot)
   : mBuffer(aSnapshot.mBuffer)
   , mLock(aSnapshot.mBuffer->mMutex)
--- a/dom/xhr/XMLHttpRequestString.h
+++ b/dom/xhr/XMLHttpRequestString.h
@@ -59,24 +59,24 @@ private:
 };
 
 // This class locks the buffer and allows the callee to write data into it.
 class MOZ_STACK_CLASS XMLHttpRequestStringWriterHelper final
 {
 public:
   explicit XMLHttpRequestStringWriterHelper(XMLHttpRequestString& aString);
 
-  bool
-  AddCapacity(int32_t aCapacity);
+  /**
+   * The existing length of the string. Do not call during BulkWrite().
+   */
+  uint32_t
+  Length() const;
 
-  char16_t*
-  EndOfExistingData();
-
-  void
-  AddLength(int32_t aLength);
+  mozilla::BulkWriteHandle<char16_t>
+  BulkWrite(uint32_t aCapacity, nsresult& aRv);
 
 private:
   XMLHttpRequestStringWriterHelper(const XMLHttpRequestStringWriterHelper&) = delete;
   XMLHttpRequestStringWriterHelper& operator=(const XMLHttpRequestStringWriterHelper&) = delete;
   XMLHttpRequestStringWriterHelper& operator=(const XMLHttpRequestStringWriterHelper&&) = delete;
 
   RefPtr<XMLHttpRequestStringBuffer> mBuffer;
   MutexAutoLock mLock;