Bug 1298773 - Make ArrayBufferInputStream copy its input buffer. r=jonco a=abillings
authorJosh Matthews <josh@joshmatthews.net>
Mon, 31 Oct 2016 17:48:04 -0400
changeset 352841 8a3453d75b8da4f0b112257688ced3f3fc381e8b
parent 352840 46049b21980d67b13d6915c052a3025c273a1788
child 352842 2f034f3773f7758795366ec39a763c483ffaccf0
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco, abillings
bugs1298773
milestone52.0a2
Bug 1298773 - Make ArrayBufferInputStream copy its input buffer. r=jonco a=abillings
netwerk/base/ArrayBufferInputStream.cpp
netwerk/base/ArrayBufferInputStream.h
netwerk/test/mochitests/test_arraybufferinputstream.html
--- a/netwerk/base/ArrayBufferInputStream.cpp
+++ b/netwerk/base/ArrayBufferInputStream.cpp
@@ -8,17 +8,16 @@
 #include "nsStreamUtils.h"
 #include "jsapi.h"
 #include "jsfriendapi.h"
 
 NS_IMPL_ISUPPORTS(ArrayBufferInputStream, nsIArrayBufferInputStream, nsIInputStream);
 
 ArrayBufferInputStream::ArrayBufferInputStream()
 : mBufferLength(0)
-, mOffset(0)
 , mPos(0)
 , mClosed(false)
 {
 }
 
 NS_IMETHODIMP
 ArrayBufferInputStream::SetData(JS::Handle<JS::Value> aBuffer,
                                 uint32_t aByteOffset,
@@ -28,21 +27,26 @@ ArrayBufferInputStream::SetData(JS::Hand
   if (!aBuffer.isObject()) {
     return NS_ERROR_FAILURE;
   }
   JS::RootedObject arrayBuffer(aCx, &aBuffer.toObject());
   if (!JS_IsArrayBufferObject(arrayBuffer)) {
     return NS_ERROR_FAILURE;
   }
 
-  mArrayBuffer.emplace(aCx, arrayBuffer);
+  uint32_t buflen = JS_GetArrayBufferByteLength(arrayBuffer);
+  uint32_t offset = std::min(buflen, aByteOffset);
+  mBufferLength = std::min(buflen - offset, aLength);
 
-  uint32_t buflen = JS_GetArrayBufferByteLength(arrayBuffer);
-  mOffset = std::min(buflen, aByteOffset);
-  mBufferLength = std::min(buflen - mOffset, aLength);
+  mArrayBuffer = mozilla::MakeUnique<char[]>(mBufferLength);
+
+  JS::AutoCheckCannotGC nogc;
+  bool isShared;
+  char* src = (char*) JS_GetArrayBufferData(arrayBuffer, &isShared, nogc) + offset;
+  memcpy(&mArrayBuffer[0], src, mBufferLength);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ArrayBufferInputStream::Close()
 {
   mClosed = true;
   return NS_OK;
@@ -50,18 +54,17 @@ ArrayBufferInputStream::Close()
 
 NS_IMETHODIMP
 ArrayBufferInputStream::Available(uint64_t* aCount)
 {
   if (mClosed) {
     return NS_BASE_STREAM_CLOSED;
   }
   if (mArrayBuffer) {
-    uint32_t buflen = JS_GetArrayBufferByteLength(mArrayBuffer->get());
-    *aCount = buflen ? buflen - mPos : 0;
+    *aCount = mBufferLength ? mBufferLength - mPos : 0;
   } else {
     *aCount = 0;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ArrayBufferInputStream::Read(char* aBuf, uint32_t aCount, uint32_t *aReadCount)
@@ -81,44 +84,24 @@ ArrayBufferInputStream::ReadSegments(nsW
   }
 
   MOZ_ASSERT(mArrayBuffer || (mPos == mBufferLength), "stream inited incorrectly");
 
   *result = 0;
   while (mPos < mBufferLength) {
     uint32_t remaining = mBufferLength - mPos;
     MOZ_ASSERT(mArrayBuffer);
-    uint32_t byteLength = JS_GetArrayBufferByteLength(mArrayBuffer->get());
-    if (byteLength == 0) {
-      mClosed = true;
-      return NS_BASE_STREAM_CLOSED;
-    }
 
-    // If you change the size of this buffer, please also remember to
-    // update test_arraybufferinputstream.html.
-    char buffer[8192];
-    uint32_t count = std::min(std::min(aCount, remaining), uint32_t(mozilla::ArrayLength(buffer)));
+    uint32_t count = std::min(aCount, remaining);
     if (count == 0) {
       break;
     }
 
-    // It is just barely possible that writer() will detach the ArrayBuffer's
-    // data, setting its length to zero. Or move the data to a different memory
-    // area. (This would only happen in a subclass that passed something other
-    // than NS_CopySegmentToBuffer as 'writer'). So copy the data out into a
-    // holding area before passing it to writer().
-    {
-      JS::AutoCheckCannotGC nogc;
-      bool isShared;
-      char* src = (char*) JS_GetArrayBufferData(mArrayBuffer->get(), &isShared, nogc) + mOffset + mPos;
-      MOZ_ASSERT(!isShared);    // Because ArrayBuffer
-      memcpy(buffer, src, count);
-    }
     uint32_t written;
-    nsresult rv = writer(this, closure, buffer, *result, count, &written);
+    nsresult rv = writer(this, closure, &mArrayBuffer[0] + mPos, *result, count, &written);
     if (NS_FAILED(rv)) {
       // InputStreams do not propagate errors to caller.
       return NS_OK;
     }
 
     NS_ASSERTION(written <= count,
                  "writer should not write more than we asked it to write");
     mPos += written;
--- a/netwerk/base/ArrayBufferInputStream.h
+++ b/netwerk/base/ArrayBufferInputStream.h
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef ArrayBufferInputStream_h
 #define ArrayBufferInputStream_h
 
 #include "nsIArrayBufferInputStream.h"
 #include "js/Value.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/UniquePtr.h"
 
 #define NS_ARRAYBUFFERINPUTSTREAM_CONTRACTID "@mozilla.org/io/arraybuffer-input-stream;1"
 #define NS_ARRAYBUFFERINPUTSTREAM_CID                \
 { /* 3014dde6-aa1c-41db-87d0-48764a3710f6 */         \
     0x3014dde6,                                      \
     0xaa1c,                                          \
     0x41db,                                          \
     {0x87, 0xd0, 0x48, 0x76, 0x4a, 0x37, 0x10, 0xf6} \
@@ -23,16 +24,15 @@ class ArrayBufferInputStream : public ns
 public:
   ArrayBufferInputStream();
   NS_DECL_ISUPPORTS
   NS_DECL_NSIARRAYBUFFERINPUTSTREAM
   NS_DECL_NSIINPUTSTREAM
 
 private:
   virtual ~ArrayBufferInputStream() {}
-  mozilla::Maybe<JS::PersistentRooted<JSObject*> > mArrayBuffer;
-  uint32_t mBufferLength; // length of slice
-  uint32_t mOffset; // permanent offset from start of actual buffer
-  uint32_t mPos; // offset from start of slice
+  mozilla::UniquePtr<char[]> mArrayBuffer;
+  uint32_t mBufferLength;
+  uint32_t mPos;
   bool mClosed;
 };
 
 #endif // ArrayBufferInputStream_h
--- a/netwerk/test/mochitests/test_arraybufferinputstream.html
+++ b/netwerk/test/mochitests/test_arraybufferinputstream.html
@@ -14,16 +14,17 @@ function detachArrayBuffer(ab)
   w.postMessage(ab, [ab]);
 }
 
 function test()
 {
   var ab = new ArrayBuffer(4000);
   var ta = new Uint8Array(ab);
   ta[0] = 'a'.charCodeAt(0);
+  ta[1] = 'b'.charCodeAt(0);
 
   const Cc = SpecialPowers.Cc, Ci = SpecialPowers.Ci, Cr = SpecialPowers.Cr;
   var abis = Cc["@mozilla.org/io/arraybuffer-input-stream;1"]
                .createInstance(Ci.nsIArrayBufferInputStream);
 
   var sis = Cc["@mozilla.org/scriptableinputstream;1"]
               .createInstance(Ci.nsIScriptableInputStream);
   sis.init(abis);
@@ -36,23 +37,21 @@ function test()
 
   detachArrayBuffer(ab);
 
   SpecialPowers.forceGC();
   SpecialPowers.forceGC();
 
   try
   {
-    sis.read(1);
-    ok(false, "reading from stream shouldn't have worked");
+    is(sis.read(1), "b", "should read 'b' after detaching buffer");
   }
   catch (e)
   {
-    ok(e.result === Cr.NS_BASE_STREAM_CLOSED,
-       "detaching underneath an input stream should close it");
+    ok(false, "reading from stream should have worked");
   }
 
   // A regression test for bug 1265076.  Previously, overflowing
   // the internal buffer from readSegments would cause incorrect
   // copying.  The constant mirrors the value in
   // ArrayBufferInputStream::readSegments.
   var size = 8192;
   ab = new ArrayBuffer(2 * size);