Eliminate copies of encoded image data

third_party/WebKit/Source/platform/image-decoders/FastSharedBufferReaderTest.cpp

 /*
  * Copyright (C) 2015 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 11 matching lines @@
  * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
  * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "platform/image-decoders/FastSharedBufferReader.h"
+#include "platform/image-decoders/SharedBufferSegmentReader.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace blink {
 
 namespace {
 
 const unsigned kDefaultTestSize = 4 * SharedBuffer::kSegmentSize;
 
 void prepareReferenceData(char* buffer, size_t size)
 {
     for (size_t i = 0; i < size; ++i)
         buffer[i] = static_cast<char>(i);
 }
 
 } // namespace
 
 TEST(FastSharedBufferReaderTest, nonSequentialReads)
 {
     char referenceData[kDefaultTestSize];
     prepareReferenceData(referenceData, sizeof(referenceData));
     RefPtr<SharedBuffer> data = SharedBuffer::create();
     data->append(referenceData, sizeof(referenceData));
 
-    FastSharedBufferReader reader(data);
+    RefPtr<SharedBufferSegmentReader> segmentReader = adoptRef(new SharedBufferSegmentReader(data));
+    FastSharedBufferReader reader(segmentReader);

[Peter Kasting, 2016/03/23 02:42:58]

This pattern happening repeatedly makes me wonder if FastSharedBufferReader
should take |data| directly.  But maybe that would screw up other callers that
aren't creating a temporary SharedBufferSegmentReader just for this.

[f(malita), 2016/03/23 16:41:58]

nit: segmentReader.release()

(here and below)

[scroggo_chromium, 2016/03/24 13:59:45]

The typical caller (outside of tests) already has a SegmentReader. We could add
a second constructor that takes a SharedBuffer, though (similar to your proposal
for ImageDecoder). I'm not sure that's appropriate for a call that's only used
in tests, though.

[scroggo_chromium, 2016/03/24 13:59:45]

Done.

[Peter Kasting, 2016/03/24 22:05:43]

Up to you.  I'm not opposed to another constructor with a "// Used by tests."
sort of comment on it, especially if we minimize added code by having one
constructor delegate to the other (yay C++11).  Seems like a reasonably small
cost to simplify these calls, but we could live without it as well.

[scroggo_chromium, 2016/03/25 01:05:10]

Acknowledged. Left out the constructor for now.

 
     // Read size is prime such there will be a segment-spanning
     // read eventually.
     char tempBuffer[17];
     for (size_t dataPosition = 0; dataPosition + sizeof(tempBuffer) < sizeof(referenceData); dataPosition += sizeof(tempBuffer)) {
         const char* block = reader.getConsecutiveData(
             dataPosition, sizeof(tempBuffer), tempBuffer);
         ASSERT_FALSE(memcmp(block, referenceData + dataPosition, sizeof(tempBuffer)));
     }
 }
 
 TEST(FastSharedBufferReaderTest, readBackwards)
 {
     char referenceData[kDefaultTestSize];
     prepareReferenceData(referenceData, sizeof(referenceData));
     RefPtr<SharedBuffer> data = SharedBuffer::create();
     data->append(referenceData, sizeof(referenceData));
 
-    FastSharedBufferReader reader(data);
+    RefPtr<SharedBufferSegmentReader> segmentReader = adoptRef(new SharedBufferSegmentReader(data));
+    FastSharedBufferReader reader(segmentReader);
 
     // Read size is prime such there will be a segment-spanning
     // read eventually.
     char tempBuffer[17];
     for (size_t dataOffset = sizeof(tempBuffer); dataOffset < sizeof(referenceData); dataOffset += sizeof(tempBuffer)) {
         const char* block = reader.getConsecutiveData(
             sizeof(referenceData) - dataOffset, sizeof(tempBuffer), tempBuffer);
         ASSERT_FALSE(memcmp(block, referenceData + sizeof(referenceData) - dataOffset, sizeof(tempBuffer)));
     }
 }
 
 TEST(FastSharedBufferReaderTest, byteByByte)
 {
     char referenceData[kDefaultTestSize];
     prepareReferenceData(referenceData, sizeof(referenceData));
     RefPtr<SharedBuffer> data = SharedBuffer::create();
     data->append(referenceData, sizeof(referenceData));
 
-    FastSharedBufferReader reader(data);
+    RefPtr<SharedBufferSegmentReader> segmentReader = adoptRef(new SharedBufferSegmentReader(data));
+    FastSharedBufferReader reader(segmentReader);
     for (size_t i = 0; i < sizeof(referenceData); ++i) {
         ASSERT_EQ(referenceData[i], reader.getOneByte(i));
     }
 }
 
 // Tests that a read from inside the penultimate segment to the very end of the
 // buffer doesn't try to read off the end of the buffer.
 TEST(FastSharedBufferReaderTest, readAllOverlappingLastSegmentBoundary)
 {
     const unsigned dataSize = 2 * SharedBuffer::kSegmentSize;
     char referenceData[dataSize];
     prepareReferenceData(referenceData, dataSize);
     RefPtr<SharedBuffer> data = SharedBuffer::create();
     data->append(referenceData, dataSize);
 
+    RefPtr<SharedBufferSegmentReader> segmentReader = adoptRef(new SharedBufferSegmentReader(data));
     char buffer[dataSize];
-    FastSharedBufferReader reader(data);
+    FastSharedBufferReader reader(segmentReader);
     reader.getConsecutiveData(0, dataSize, buffer);
 
     ASSERT_FALSE(memcmp(buffer, referenceData, dataSize));
 }
 
 } // namespace blink