Stop using ArrayBufferBuilder in FileReaderLoader.

Also clean up ArrayBufferBuilder API by removing functionality that was only
used by FileReaderLoader:
- Calling ToArrayBuffer more than once. Now this method is renamed and passes
  ownership of the internal buffer isntead.
- Building fixed size array buffers.
- Converting the array buffer to a string.

Bug: 936889
Change-Id: Ib0fd1014ad012d6b615fc9f4dc5c7ba1a58839b0
Reviewed-on: https://chromium-review.googlesource.com/c/1495036Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636894}

third_party/blink/renderer/core/fetch/fetch_data_loader.cc

@@ -163,7 +163,7 @@ class FetchDataLoaderAsArrayBuffer final : public FetchDataLoader,
           return;
         case BytesConsumer::Result::kDone:
           client_->DidFetchDataLoadedArrayBuffer(
-              DOMArrayBuffer::Create(raw_data_->ToArrayBuffer()));
+              DOMArrayBuffer::Create(raw_data_->PassArrayBuffer()));
           return;
         case BytesConsumer::Result::kError:
           client_->DidFetchDataLoadFailed();

third_party/blink/renderer/core/fileapi/file_reader_loader.cc

@@ -140,13 +140,12 @@ DOMArrayBuffer* FileReaderLoader::ArrayBufferResult() {
     return nullptr;
 
   if (!finished_loading_) {
-    return DOMArrayBuffer::Create(
-        ArrayBuffer::Create(raw_data_->Data(), raw_data_->ByteLength()));
+    return DOMArrayBuffer::Create(ArrayBuffer::Create(
+        raw_data_->Data(), static_cast<unsigned>(bytes_loaded_)));
   }
 
-  array_buffer_result_ = DOMArrayBuffer::Create(raw_data_->ToArrayBuffer());
-  AdjustReportedMemoryUsageToV8(-1 *
-                                static_cast<int64_t>(raw_data_->ByteLength()));
+  array_buffer_result_ = DOMArrayBuffer::Create(std::move(raw_data_));
+  AdjustReportedMemoryUsageToV8(-1 * static_cast<int64_t>(bytes_loaded_));
   raw_data_.reset();
   return array_buffer_result_;
 }
@@ -165,7 +164,8 @@ String FileReaderLoader::StringResult() {
       // No conversion is needed.
       return string_result_;
     case kReadAsBinaryString:
-      SetStringResult(raw_data_->ToString());
+      SetStringResult(String(static_cast<const char*>(raw_data_->Data()),
+                             static_cast<size_t>(bytes_loaded_)));
       break;
     case kReadAsText:
       SetStringResult(ConvertToText());
@@ -181,8 +181,7 @@ String FileReaderLoader::StringResult() {
 
   if (finished_loading_) {
     DCHECK(is_raw_data_converted_);
-    AdjustReportedMemoryUsageToV8(
-        -1 * static_cast<int64_t>(raw_data_->ByteLength()));
+    AdjustReportedMemoryUsageToV8(-1 * static_cast<int64_t>(bytes_loaded_));
     raw_data_.reset();
   }
   return string_result_;
@@ -236,13 +235,12 @@ void FileReaderLoader::OnStartLoading(uint64_t total_bytes) {
       return;
     }
 
-    raw_data_ = std::make_unique<ArrayBufferBuilder>(total_bytes);
-    if (!raw_data_->IsValid()) {
+    raw_data_ = ArrayBuffer::Create(static_cast<unsigned>(total_bytes), 1);
+    if (!raw_data_) {
       Failed(FileErrorCode::kNotReadableErr,
              FailureType::kArrayBufferBuilderCreation);
       return;
     }
-    raw_data_->SetVariableCapacity(false);
   }
 
   if (client_)
@@ -264,17 +262,23 @@ void FileReaderLoader::OnReceivedData(const char* data, unsigned data_length) {
     return;
   }
 
-  unsigned bytes_appended = raw_data_->Append(data, data_length);
-  if (!bytes_appended) {
+  // Receiving more data than expected would indicate a bug in the
+  // implementation of the mojom Blob interface. However there is no guarantee
+  // that the BlobPtr is actually backed by a "real" blob, so to defend against
+  // compromised renderer processes we still need to carefully validate anything
+  // received. So return an error if we received too much data.
+  if (bytes_loaded_ + data_length > raw_data_->ByteLength()) {
     raw_data_.reset();
     bytes_loaded_ = 0;
     Failed(FileErrorCode::kNotReadableErr,
            FailureType::kArrayBufferBuilderAppend);
     return;
   }
-  bytes_loaded_ += bytes_appended;
+  memcpy(static_cast<char*>(raw_data_->Data()) + bytes_loaded_, data,
+         data_length);
+  bytes_loaded_ += data_length;
   is_raw_data_converted_ = false;
-  AdjustReportedMemoryUsageToV8(bytes_appended);
+  AdjustReportedMemoryUsageToV8(data_length);
 
   if (client_)
     client_->DidReceiveData();
@@ -282,7 +286,7 @@ void FileReaderLoader::OnReceivedData(const char* data, unsigned data_length) {
 
 void FileReaderLoader::OnFinishLoading() {
   if (read_type_ != kReadByClient && raw_data_) {
-    raw_data_->ShrinkToFit();
+    DCHECK_EQ(bytes_loaded_, raw_data_->ByteLength());
     is_raw_data_converted_ = false;
   }
 
@@ -425,7 +429,7 @@ String FileReaderLoader::ConvertToText() {
         encoding_.IsValid() ? encoding_ : UTF8Encoding()));
   }
   builder.Append(decoder_->Decode(static_cast<const char*>(raw_data_->Data()),
-                                  raw_data_->ByteLength()));
+                                  static_cast<size_t>(bytes_loaded_)));
 
   if (finished_loading_)
     builder.Append(decoder_->Flush());
@@ -451,7 +455,7 @@ String FileReaderLoader::ConvertToDataURL() {
 
   Vector<char> out;
   Base64Encode(static_cast<const char*>(raw_data_->Data()),
-               raw_data_->ByteLength(), out);
+               static_cast<unsigned>(bytes_loaded_), out);
   out.push_back('\0');
   builder.Append(out.data());
 

third_party/blink/renderer/core/fileapi/file_reader_loader.h

@@ -43,7 +43,7 @@
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 #include "third_party/blink/renderer/platform/wtf/text/text_encoding.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
-#include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_builder.h"
+#include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer.h"
 
 namespace blink {
 
@@ -154,7 +154,7 @@ class CORE_EXPORT FileReaderLoader : public mojom::blink::BlobReaderClient {
   WTF::TextEncoding encoding_;
   String data_type_;
 
-  std::unique_ptr<ArrayBufferBuilder> raw_data_;
+  scoped_refptr<ArrayBuffer> raw_data_;
   bool is_raw_data_converted_ = false;
 
   Persistent<DOMArrayBuffer> array_buffer_result_;

third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_builder.cc

@@ -31,14 +31,15 @@
 #include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_builder.h"
 
 #include <limits>
+#include <utility>
+
 #include "third_party/blink/renderer/platform/wtf/assertions.h"
 
 namespace WTF {
 
 static const int kDefaultBufferCapacity = 32768;
 
-ArrayBufferBuilder::ArrayBufferBuilder()
-    : bytes_used_(0), variable_capacity_(true) {
+ArrayBufferBuilder::ArrayBufferBuilder() : bytes_used_(0) {
   buffer_ = ArrayBuffer::Create(kDefaultBufferCapacity, 1);
 }
 
@@ -82,34 +83,18 @@ unsigned ArrayBufferBuilder::Append(const char* data, unsigned length) {
 
   size_t remaining_buffer_space = current_buffer_size - bytes_used_;
 
-  unsigned bytes_to_save = length;
-
-  if (length > remaining_buffer_space) {
-    if (variable_capacity_) {
-      if (!ExpandCapacity(length))
-        return 0;
-    } else {
-      bytes_to_save = static_cast<unsigned>(remaining_buffer_space);
-    }
-  }
-
-  memcpy(static_cast<char*>(buffer_->Data()) + bytes_used_, data,
-         bytes_to_save);
-  bytes_used_ += bytes_to_save;
-
-  return bytes_to_save;
-}
+  if (length > remaining_buffer_space && !ExpandCapacity(length))
+    return 0;
 
-scoped_refptr<ArrayBuffer> ArrayBufferBuilder::ToArrayBuffer() {
-  // Fully used. Return m_buffer as-is.
-  if (buffer_->ByteLength() == bytes_used_)
-    return buffer_;
+  memcpy(static_cast<char*>(buffer_->Data()) + bytes_used_, data, length);
+  bytes_used_ += length;
 
-  return buffer_->Slice(0, bytes_used_);
+  return length;
 }
 
-String ArrayBufferBuilder::ToString() {
-  return String(static_cast<const char*>(buffer_->Data()), bytes_used_);
+scoped_refptr<ArrayBuffer> ArrayBufferBuilder::PassArrayBuffer() {
+  ShrinkToFit();
+  return std::move(buffer_);
 }
 
 void ArrayBufferBuilder::ShrinkToFit() {

third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_builder.h

@@ -50,8 +50,7 @@ class WTF_EXPORT ArrayBufferBuilder final {
   // Creates an ArrayBufferBuilder using the default capacity.
   ArrayBufferBuilder();
 
-  ArrayBufferBuilder(unsigned capacity)
-      : bytes_used_(0), variable_capacity_(true) {
+  explicit ArrayBufferBuilder(unsigned capacity) : bytes_used_(0) {
     buffer_ = ArrayBuffer::Create(capacity, 1);
   }
 
@@ -60,14 +59,10 @@ class WTF_EXPORT ArrayBufferBuilder final {
   // Appending empty data is not allowed.
   unsigned Append(const char* data, unsigned length);
 
-  // Returns the accumulated data as an ArrayBuffer instance. If needed,
-  // creates a new ArrayBuffer instance and copies contents from the internal
-  // buffer to it. Otherwise, returns a RefPtr pointing to the internal
-  // buffer.
-  scoped_refptr<ArrayBuffer> ToArrayBuffer();
-
-  // Converts the accumulated data into a String using the default encoding.
-  String ToString();
+  // Returns the accumulated data as an ArrayBuffer instance. This transfers
+  // ownership of the internal buffer, making this ArrayBufferBuilder invalid
+  // for future use.
+  scoped_refptr<ArrayBuffer> PassArrayBuffer();
 
   // Number of bytes currently accumulated.
   unsigned ByteLength() const { return bytes_used_; }
@@ -79,10 +74,6 @@ class WTF_EXPORT ArrayBufferBuilder final {
 
   const void* Data() const { return buffer_->Data(); }
 
-  // If set to false, the capacity won't be expanded and when appended data
-  // overflows, the overflowed part will be dropped.
-  void SetVariableCapacity(bool value) { variable_capacity_ = value; }
-
  private:
   // Expands the size of m_buffer to size + m_bytesUsed bytes. Returns true
   // iff successful. If reallocation is needed, copies only data in
@@ -90,7 +81,6 @@ class WTF_EXPORT ArrayBufferBuilder final {
   bool ExpandCapacity(unsigned size);
 
   unsigned bytes_used_;
-  bool variable_capacity_;
   scoped_refptr<ArrayBuffer> buffer_;
 
   DISALLOW_COPY_AND_ASSIGN(ArrayBufferBuilder);

third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer_builder_test.cc

@@ -96,27 +96,7 @@ TEST(ArrayBufferBuilderTest, DefaultConstructorAndAppendRepeatedly) {
   }
 }
 
-TEST(ArrayBufferBuilderTest, AppendFixedCapacity) {
-  const char kData[] = "HelloWorld";
-  uint32_t data_size = sizeof(kData) - 1;
-
-  ArrayBufferBuilder builder(15);
-  builder.SetVariableCapacity(false);
-
-  EXPECT_EQ(data_size, builder.Append(kData, data_size));
-  EXPECT_EQ(data_size, builder.ByteLength());
-  EXPECT_EQ(15u, builder.Capacity());
-
-  EXPECT_EQ(5u, builder.Append(kData, data_size));
-  EXPECT_EQ(15u, builder.ByteLength());
-  EXPECT_EQ(15u, builder.Capacity());
-
-  EXPECT_EQ(0u, builder.Append(kData, data_size));
-  EXPECT_EQ(15u, builder.ByteLength());
-  EXPECT_EQ(15u, builder.Capacity());
-}
-
-TEST(ArrayBufferBuilderTest, ToArrayBuffer) {
+TEST(ArrayBufferBuilderTest, PassArrayBuffer) {
   const char kData1[] = "HelloWorld";
   uint32_t data1_size = sizeof(kData1) - 1;
 
@@ -130,44 +110,13 @@ TEST(ArrayBufferBuilderTest, ToArrayBuffer) {
   const char kExpected[] = "HelloWorldGoodbyeWorld";
   uint32_t expected_size = sizeof(kExpected) - 1;
 
-  scoped_refptr<ArrayBuffer> result = builder.ToArrayBuffer();
+  scoped_refptr<ArrayBuffer> result = builder.PassArrayBuffer();
+  EXPECT_FALSE(builder.IsValid());
   ASSERT_EQ(data1_size + data2_size, result->ByteLength());
   ASSERT_EQ(expected_size, result->ByteLength());
   EXPECT_EQ(0, memcmp(kExpected, result->Data(), expected_size));
 }
 
-TEST(ArrayBufferBuilderTest, ToArrayBufferSameAddressIfExactCapacity) {
-  const char kData[] = "HelloWorld";
-  uint32_t data_size = sizeof(kData) - 1;
-
-  ArrayBufferBuilder builder(data_size);
-  builder.Append(kData, data_size);
-
-  scoped_refptr<ArrayBuffer> result1 = builder.ToArrayBuffer();
-  scoped_refptr<ArrayBuffer> result2 = builder.ToArrayBuffer();
-  EXPECT_EQ(result1.get(), result2.get());
-}
-
-TEST(ArrayBufferBuilderTest, ToString) {
-  const char kData1[] = "HelloWorld";
-  uint32_t data1_size = sizeof(kData1) - 1;
-
-  const char kData2[] = "GoodbyeWorld";
-  uint32_t data2_size = sizeof(kData2) - 1;
-
-  ArrayBufferBuilder builder(1024);
-  builder.Append(kData1, data1_size);
-  builder.Append(kData2, data2_size);
-
-  const char kExpected[] = "HelloWorldGoodbyeWorld";
-  uint32_t expected_size = sizeof(kExpected) - 1;
-
-  String result = builder.ToString();
-  EXPECT_EQ(expected_size, result.length());
-  for (uint32_t i = 0; i < result.length(); ++i)
-    EXPECT_EQ(kExpected[i], result[i]);
-}
-
 TEST(ArrayBufferBuilderTest, ShrinkToFitNoAppend) {
   ArrayBufferBuilder builder(1024);
   EXPECT_EQ(1024u, builder.Capacity());