Bug 1383404 - Part 2. When SourceBuffer::ExpectLength creates the initial buffer, it should not round up. r=tnikkel

Currently SourceBuffer::ExpectLength will allocate a buffer which is a
multiple of MIN_CHUNK_CAPACITY (4096) bytes, no matter what the expected
size is. While it is true that HTTP servers can lie, and that we need to
handle that for legacy purposes, it is more likely the HTTP servers are
telling the truth when it comes to the content length. Additionally
images sourced from other locations, such as the file system or data
URIs, are always going to have the correct size information (barring a
bug elsewhere in the file system or our code). We should be able to
trust the size given as a good first guess.

While overallocating in general is a waste of memory,
SourceBuffer::Compact causes a far worse problem. After we have written
all of the data, and there are no active readers, we attempt to shrink
the allocated buffer(s) into a single contiguous chunk of the exact
length that we need (e.g. N allocations to 1, or 1 oversized allocation
to 1 perfect). Since we almost always overallocate, that means we almost
always trigger the logic in SourceBuffer::Compact to reallocate the data
into a properly sized buffer. If we had simply trusted the expected size
in the first place, we could have avoided this situation for the
majority of images.

In the case that we really do get the wrong size, then we will allocate
additional chunks which are multiples of MIN_CHUNK_CAPACITY bytes to fit
the data. At most, this will increase the number of discrete allocations
by 1, and trigger SourceBuffer::Compact to consolidate at the end. Since
we are almost always doing that before, and now we rarely do, this is a
significant win.

image/SourceBuffer.cpp

@@ -334,7 +334,7 @@ SourceBuffer::ExpectLength(size_t aExpectedLength)
     return NS_OK;
   }
 
-  if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(CreateChunk(aExpectedLength))))) {
+  if (MOZ_UNLIKELY(NS_FAILED(AppendChunk(CreateChunk(aExpectedLength, /* aRoundUp */ false))))) {
     return HandleError(NS_ERROR_OUT_OF_MEMORY);
   }
 

image/test/gtest/TestSourceBuffer.cpp

@@ -373,22 +373,25 @@ TEST_F(ImageSourceBuffer, MinChunkCapacity)
   CheckIteratorIsComplete(iterator, 2, SourceBuffer::MIN_CHUNK_CAPACITY + 1);
 }
 
-TEST_F(ImageSourceBuffer, ExpectLengthDoesNotShrinkBelowMinCapacity)
+TEST_F(ImageSourceBuffer, ExpectLengthAllocatesRequestedCapacity)
 {
   SourceBufferIterator iterator = mSourceBuffer->Iterator();
 
   // Write SourceBuffer::MIN_CHUNK_CAPACITY bytes of test data to the buffer,
   // but call ExpectLength() first to make SourceBuffer expect only a single
-  // byte. We expect this to still result in only one chunk, because
-  // regardless of ExpectLength() we won't allocate a chunk smaller than
-  // MIN_CHUNK_CAPACITY bytes.
+  // byte. We expect this to still result in two chunks, because we trust the
+  // initial guess of ExpectLength() but after that it will only allocate chunks
+  // of at least MIN_CHUNK_CAPACITY bytes.
   EXPECT_TRUE(NS_SUCCEEDED(mSourceBuffer->ExpectLength(1)));
   CheckedAppendToBufferInChunks(10, SourceBuffer::MIN_CHUNK_CAPACITY);
   CheckedCompleteBuffer(iterator, SourceBuffer::MIN_CHUNK_CAPACITY);
 
-  // Verify that the iterator sees a single chunk.
-  CheckedAdvanceIterator(iterator, SourceBuffer::MIN_CHUNK_CAPACITY);
-  CheckIteratorIsComplete(iterator, 1, SourceBuffer::MIN_CHUNK_CAPACITY);
+  // Verify that the iterator sees a first chunk with 1 byte, and a second chunk
+  // with the remaining data.
+  CheckedAdvanceIterator(iterator, 1, 1, 1);
+  CheckedAdvanceIterator(iterator, SourceBuffer::MIN_CHUNK_CAPACITY - 1, 2,
+		                   SourceBuffer::MIN_CHUNK_CAPACITY);
+  CheckIteratorIsComplete(iterator, 2, SourceBuffer::MIN_CHUNK_CAPACITY);
 }
 
 TEST_F(ImageSourceBuffer, ExpectLengthGrowsAboveMinCapacity)