Revert "[arraybuffer] Switch backing store from DataHandle to v8::BackingStore"

This reverts commit b0985684.

Reason for revert: Speculative revert. Suspect this is causing renderer crashes for

webaudio/AudioBuffer/huge-buffer.html
webaudio/dom-exceptions.html

https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Win10/58083

Original change's description:
> [arraybuffer] Switch backing store from DataHandle to v8::BackingStore
> 
> With this CL we finally use v8::BackingStore as the backing store
> implementation of blink::ArrayBuffer.
> 
> Changes:
> * Use v8::BackingStore as the backing store for ArrayBufferContents.
> * Remove ArraybufferContents::DataHolder and
>   ArrayBufferContents::DataHandle.
> * Replace all uses of DataHolder in ArraybufferContents with uses of
>   v8::BackingStore.
> * Replace the use of DataHandle in the V8 bindings.
> * Add an exception to the presubmit check to allow std::shared_ptr in
>   ArrayBufferContents. It is contained there and does not escape.
> 
> R=​haraken@chromium.org, ulan@chromium.org
> 
> Bug: chromium:1008840
> Change-Id: Iffa31e889daf4ae54de96f55b4612833d096a884
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1886858
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#712571}

TBR=ulan@chromium.org,haraken@chromium.org,agrieve@chromium.org,ahaas@chromium.org

Change-Id: I88668a9d2d582ef38ccb82f1c87139da4c74db62
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1008840
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899413Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712620}

PRESUBMIT.py

@@ -837,9 +837,7 @@ _BANNED_CPP_FUNCTIONS = (
         'std::shared_ptr should not be used. Use scoped_refptr instead.',
       ),
       True,
-      [_THIRD_PARTY_EXCEPT_BLINK,
-       '^third_party/blink/renderer/core/typed_arrays/array_buffer/' +
-         'array_buffer_contents\.(cc|h)'],
+      [_THIRD_PARTY_EXCEPT_BLINK],  # Not an error in third_party folders.
     ),
     (
       r'/\bstd::weak_ptr\b',

third_party/blink/renderer/bindings/templates/interface.cc.tmpl

@@ -1100,9 +1100,6 @@ v8::Local<v8::Object> {{v8_class}}::FindInstanceInPrototypeChain(
 {{cpp_class}}* V8{{interface_name}}::ToImpl(v8::Local<v8::Object> object) {
   DCHECK(object->Is{{interface_name}}());
   v8::Local<v8::{{interface_name}}> v8buffer = object.As<v8::{{interface_name}}>();
-  // TODO(ahaas): The use of IsExternal is wrong here. Instead we should call
-  // ToScriptWrappable(object)->ToImpl<ArrayBuffer>() and check for nullptr.
-  // We can then also avoid the call to Externalize below.
   if (v8buffer->IsExternal()) {
     const WrapperTypeInfo* wrapper_type = ToWrapperTypeInfo(object);
     CHECK(wrapper_type);
@@ -1112,9 +1109,12 @@ v8::Local<v8::Object> {{v8_class}}::FindInstanceInPrototypeChain(
 
   // Transfer the ownership of the allocated memory to an {{interface_name}} without
   // copying.
-  auto backing_store = v8buffer->GetBackingStore();
-  v8buffer->Externalize(backing_store);
-  ArrayBufferContents contents(std::move(backing_store));
+  v8::{{interface_name}}::Contents v8_contents = v8buffer->Externalize();
+  ArrayBufferContents::DataHandle data(v8_contents.Data(),
+                                       v8_contents.ByteLength(),
+                                       v8_contents.Deleter(),
+                                       v8_contents.DeleterData());
+  ArrayBufferContents contents(std::move(data), ArrayBufferContents::k{% if interface_name == 'ArrayBuffer' %}Not{% endif %}Shared);
   {{cpp_class}}* buffer = {{cpp_class}}::Create(contents);
   v8::Local<v8::Object> associatedWrapper = buffer->AssociateWithWrapper(v8::Isolate::GetCurrent(), buffer->GetWrapperTypeInfo(), object);
   DCHECK(associatedWrapper == object);

third_party/blink/renderer/bindings/tests/results/core/v8_array_buffer.cc

@@ -69,9 +69,6 @@ static_assert(
 TestArrayBuffer* V8ArrayBuffer::ToImpl(v8::Local<v8::Object> object) {
   DCHECK(object->IsArrayBuffer());
   v8::Local<v8::ArrayBuffer> v8buffer = object.As<v8::ArrayBuffer>();
-  // TODO(ahaas): The use of IsExternal is wrong here. Instead we should call
-  // ToScriptWrappable(object)->ToImpl<ArrayBuffer>() and check for nullptr.
-  // We can then also avoid the call to Externalize below.
   if (v8buffer->IsExternal()) {
     const WrapperTypeInfo* wrapper_type = ToWrapperTypeInfo(object);
     CHECK(wrapper_type);
@@ -81,9 +78,12 @@ TestArrayBuffer* V8ArrayBuffer::ToImpl(v8::Local<v8::Object> object) {
 
   // Transfer the ownership of the allocated memory to an ArrayBuffer without
   // copying.
-  auto backing_store = v8buffer->GetBackingStore();
-  v8buffer->Externalize(backing_store);
-  ArrayBufferContents contents(std::move(backing_store));
+  v8::ArrayBuffer::Contents v8_contents = v8buffer->Externalize();
+  ArrayBufferContents::DataHandle data(v8_contents.Data(),
+                                       v8_contents.ByteLength(),
+                                       v8_contents.Deleter(),
+                                       v8_contents.DeleterData());
+  ArrayBufferContents contents(std::move(data), ArrayBufferContents::kNotShared);
   TestArrayBuffer* buffer = TestArrayBuffer::Create(contents);
   v8::Local<v8::Object> associatedWrapper = buffer->AssociateWithWrapper(v8::Isolate::GetCurrent(), buffer->GetWrapperTypeInfo(), object);
   DCHECK(associatedWrapper == object);

third_party/blink/renderer/core/typed_arrays/array_buffer/array_buffer_contents.cc

@@ -34,93 +34,79 @@
 
 namespace blink {
 
-ArrayBufferContents::ArrayBufferContents(void* data,
-                                         size_t length,
-                                         DataDeleter deleter,
-                                         SharingType is_shared) {
-  if (!data) {
-    return;
-  }
-  if (is_shared == kNotShared) {
-    backing_store_ =
-        v8::ArrayBuffer::NewBackingStore(data, length, deleter, nullptr);
-  } else {
-    backing_store_ =
-        v8::SharedArrayBuffer::NewBackingStore(data, length, deleter, nullptr);
-  }
-}
+ArrayBufferContents::ArrayBufferContents()
+    : holder_(base::AdoptRef(new DataHolder())) {}
 
 ArrayBufferContents::ArrayBufferContents(
     size_t num_elements,
-    size_t element_byte_size,
+    unsigned element_byte_size,
     SharingType is_shared,
     ArrayBufferContents::InitializationPolicy policy)
-    : ArrayBufferContents(
-          AllocateMemoryOrNull(num_elements * element_byte_size, policy),
-          num_elements * element_byte_size,
-          [](void* data, size_t, void*) { FreeMemory(data); },
-          is_shared) {}
-
-ArrayBufferContents::ArrayBufferContents(
-    std::shared_ptr<v8::BackingStore> backing_store) {
-  if (!backing_store || backing_store->Data()) {
-    backing_store_ = std::move(backing_store);
-    return;
+    : holder_(base::AdoptRef(new DataHolder())) {
+  // Do not allow 32-bit overflow of the total size.
+  size_t total_size = num_elements * element_byte_size;
+  if (num_elements) {
+    if (total_size / num_elements != element_byte_size) {
+      return;
+    }
   }
-  // ArrayBufferContents has to guarantee that Data() provides a valid pointer,
-  // even when DataSize() is '0'. That's why we create a new BackingStore here.
-
-  // TODO(ahaas): Remove this code here once nullptr is a valid result for
-  // Data().
-  CHECK_EQ(backing_store->ByteLength(), 0u);
-  void* data = AllocateMemoryOrNull(0, kDontInitialize);
-  CHECK_NE(data, nullptr);
-  DataDeleter deleter = [](void* data, size_t, void*) { FreeMemory(data); };
-  if (!backing_store->IsShared()) {
-    backing_store_ =
-        v8::ArrayBuffer::NewBackingStore(data, 0, deleter, nullptr);
+
+  holder_->AllocateNew(total_size, is_shared, policy);
+}
+
+ArrayBufferContents::ArrayBufferContents(DataHandle data, SharingType is_shared)
+    : holder_(base::AdoptRef(new DataHolder())) {
+  if (data) {
+    holder_->Adopt(std::move(data), is_shared);
   } else {
-    backing_store_ =
-        v8::SharedArrayBuffer::NewBackingStore(data, 0, deleter, nullptr);
+    // Allow null data if size is 0 bytes, make sure data is valid pointer.
+    // (PartitionAlloc guarantees valid pointer for size 0)
+    holder_->AllocateNew(0, is_shared, kZeroInitialize);
   }
 }
 
+ArrayBufferContents::ArrayBufferContents(void* data,
+                                         size_t length,
+                                         DataDeleter deleter,
+                                         SharingType is_shared)
+    : holder_(base::AdoptRef(new DataHolder())) {
+  holder_->Adopt(DataHandle(data, length, deleter, nullptr), is_shared);
+}
+
 ArrayBufferContents::~ArrayBufferContents() = default;
 
 void ArrayBufferContents::Detach() {
-  backing_store_.reset();
+  holder_ = nullptr;
 }
 
 void ArrayBufferContents::Reset() {
-  backing_store_.reset();
+  holder_ = base::MakeRefCounted<DataHolder>();
 }
 
 void ArrayBufferContents::Transfer(ArrayBufferContents& other) {
   DCHECK(!IsShared());
-  DCHECK(!other.Data());
-  other.backing_store_ = std::move(backing_store_);
+  DCHECK(!other.holder_->Data());
+  other.holder_ = holder_;
+  Detach();
 }
 
 void ArrayBufferContents::ShareWith(ArrayBufferContents& other) {
   DCHECK(IsShared());
-  DCHECK(!other.Data());
-  other.backing_store_ = backing_store_;
+  DCHECK(!other.holder_->Data());
+  other.holder_ = holder_;
 }
 
 void ArrayBufferContents::ShareNonSharedForInternalUse(
     ArrayBufferContents& other) {
   DCHECK(!IsShared());
-  DCHECK(!other.Data());
-  DCHECK(Data());
-  other.backing_store_ = backing_store_;
+  DCHECK(!other.holder_->Data());
+  DCHECK(holder_->Data());
+  other.holder_ = holder_;
 }
 
 void ArrayBufferContents::CopyTo(ArrayBufferContents& other) {
-  other = ArrayBufferContents(
-      DataLength(), 1, IsShared() ? kShared : kNotShared, kDontInitialize);
-  if (!IsValid() || !other.IsValid())
-    return;
-  memcpy(other.Data(), Data(), DataLength());
+  DCHECK(!holder_->IsShared() && !other.holder_->IsShared());
+  other.holder_->CopyMemoryFrom(*holder_);
 }
 
 void* ArrayBufferContents::AllocateMemoryWithFlags(size_t size,
@@ -144,4 +130,58 @@ void ArrayBufferContents::FreeMemory(void* data) {
   WTF::Partitions::ArrayBufferPartition()->Free(data);
 }
 
+ArrayBufferContents::DataHandle ArrayBufferContents::CreateDataHandle(
+    size_t size,
+    InitializationPolicy policy) {
+  return DataHandle(
+      ArrayBufferContents::AllocateMemoryOrNull(size, policy), size,
+      [](void* buffer, size_t, void*) { FreeMemory(buffer); }, nullptr);
+}
+
+ArrayBufferContents::DataHolder::DataHolder()
+    : data_(
+          nullptr,
+          0,
+          [](void*, size_t, void*) {},
+          nullptr),
+      is_shared_(kNotShared),
+      has_registered_external_allocation_(false) {}
+
+ArrayBufferContents::DataHolder::~DataHolder() {
+  is_shared_ = kNotShared;
+}
+
+void ArrayBufferContents::DataHolder::AllocateNew(size_t length,
+                                                  SharingType is_shared,
+                                                  InitializationPolicy policy) {
+  DCHECK(!data_);
+  DCHECK(!has_registered_external_allocation_);
+
+  data_ = CreateDataHandle(length, policy);
+  if (!data_)
+    return;
+
+  is_shared_ = is_shared;
+}
+
+void ArrayBufferContents::DataHolder::Adopt(DataHandle data,
+                                            SharingType is_shared) {
+  DCHECK(!data_);
+  DCHECK(!has_registered_external_allocation_);
+
+  data_ = std::move(data);
+  is_shared_ = is_shared;
+}
+
+void ArrayBufferContents::DataHolder::CopyMemoryFrom(const DataHolder& source) {
+  DCHECK(!data_);
+  DCHECK(!has_registered_external_allocation_);
+
+  data_ = CreateDataHandle(source.DataLength(), kDontInitialize);
+  if (!data_)
+    return;
+
+  memcpy(data_.Data(), source.Data(), source.DataLength());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/typed_arrays/array_buffer/array_buffer_contents.h

@@ -36,7 +36,6 @@
 #include "third_party/blink/renderer/platform/wtf/cross_thread_copier.h"
 #include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
 #include "third_party/blink/renderer/platform/wtf/wtf.h"
-#include "v8/include/v8.h"
 
 namespace blink {
 
@@ -44,13 +43,65 @@ class CORE_EXPORT ArrayBufferContents {
   DISALLOW_NEW();
 
  public:
+  using AdjustAmountOfExternalAllocatedMemoryFunction = void (*)(int64_t diff);
   // Types that need to be used when injecting external memory.
-  // v8::BackingStore allows specifying a deleter which will be invoked when
-  // v8::BackingStore instance goes out of scope. If the data memory is
-  // allocated using ArrayBufferContents::AllocateMemoryOrNull, it is necessary
-  // to specify ArrayBufferContents::FreeMemory as the DataDeleter.
+  // DataHandle allows specifying a deleter which will be invoked when
+  // DataHandle instance goes out of scope. If the data memory is allocated
+  // using ArrayBufferContents::AllocateMemoryOrNull, it is necessary to specify
+  // ArrayBufferContents::FreeMemory as the DataDeleter. Most clients would want
+  // to use ArrayBufferContents::CreateDataHandle, which allocates memory and
+  // specifies the correct deleter.
   using DataDeleter = void (*)(void* data, size_t length, void* info);
 
+  class DataHandle {
+    DISALLOW_COPY_AND_ASSIGN(DataHandle);
+
+   public:
+    DataHandle() {}
+
+    DataHandle(void* data,
+               size_t length,
+               DataDeleter deleter,
+               void* deleter_info)
+        : data_(data),
+          data_length_(data ? length : 0),
+          deleter_(deleter),
+          deleter_info_(deleter_info) {}
+    // Move constructor
+    DataHandle(DataHandle&& other) { *this = std::move(other); }
+    ~DataHandle() {
+      if (!data_)
+        return;
+      deleter_(data_, data_length_, deleter_info_);
+    }
+
+    // Move operator
+    DataHandle& operator=(DataHandle&& other) {
+      if (data_)
+        deleter_(data_, data_length_, deleter_info_);
+      data_ = other.data_;
+      data_length_ = other.data_length_;
+      deleter_ = other.deleter_;
+      deleter_info_ = other.deleter_info_;
+      other.data_ = nullptr;
+      return *this;
+    }
+
+    void reset() { *this = DataHandle(); }
+
+    void* Data() const { return data_; }
+    size_t DataLength() const { return data_length_; }
+
+    operator bool() const { return data_; }
+
+   private:
+    void* data_ = nullptr;
+    size_t data_length_ = 0;
+
+    DataDeleter deleter_ = nullptr;
+    void* deleter_info_ = nullptr;
+  };
+
   enum InitializationPolicy { kZeroInitialize, kDontInitialize };
 
   enum SharingType {
@@ -58,17 +109,17 @@ class CORE_EXPORT ArrayBufferContents {
     kShared,
   };
 
-  ArrayBufferContents() = default;
+  ArrayBufferContents();
   ArrayBufferContents(size_t num_elements,
-                      size_t element_byte_size,
+                      unsigned element_byte_size,
                       SharingType is_shared,
                       InitializationPolicy);
   ArrayBufferContents(void* data,
                       size_t length,
                       DataDeleter deleter,
                       SharingType is_shared);
+  ArrayBufferContents(DataHandle, SharingType is_shared);
   ArrayBufferContents(ArrayBufferContents&&) = default;
-  explicit ArrayBufferContents(std::shared_ptr<v8::BackingStore> backing_store);
 
   ~ArrayBufferContents();
 
@@ -87,16 +138,10 @@ class CORE_EXPORT ArrayBufferContents {
     DCHECK(IsShared());
     return DataMaybeShared();
   }
-  void* DataMaybeShared() const {
-    return backing_store_ ? backing_store_->Data() : nullptr;
-  }
-  size_t DataLength() const {
-    return backing_store_ ? backing_store_->ByteLength() : 0;
-  }
-  bool IsShared() const {
-    return backing_store_ ? backing_store_->IsShared() : false;
-  }
-  bool IsValid() const { return backing_store_ && backing_store_->Data(); }
+  void* DataMaybeShared() const { return holder_ ? holder_->Data() : nullptr; }
+  size_t DataLength() const { return holder_ ? holder_->DataLength() : 0; }
+  bool IsShared() const { return holder_ ? holder_->IsShared() : false; }
+  bool IsValid() const { return holder_->Data(); }
 
   void Transfer(ArrayBufferContents& other);
   void ShareWith(ArrayBufferContents& other);
@@ -105,6 +150,7 @@ class CORE_EXPORT ArrayBufferContents {
 
   static void* AllocateMemoryOrNull(size_t, InitializationPolicy);
   static void FreeMemory(void*);
+  static DataHandle CreateDataHandle(size_t, InitializationPolicy);
 
  private:
   static void* AllocateMemoryWithFlags(size_t, InitializationPolicy, int);
@@ -112,7 +158,31 @@ class CORE_EXPORT ArrayBufferContents {
   static void DefaultAdjustAmountOfExternalAllocatedMemoryFunction(
       int64_t diff);
 
-  std::shared_ptr<v8::BackingStore> backing_store_;
+  class CORE_EXPORT DataHolder : public ThreadSafeRefCounted<DataHolder> {
+    DISALLOW_COPY_AND_ASSIGN(DataHolder);
+
+   public:
+    DataHolder();
+    ~DataHolder();
+
+    void AllocateNew(size_t length,
+                     SharingType is_shared,
+                     InitializationPolicy);
+    void Adopt(DataHandle, SharingType is_shared);
+    void CopyMemoryFrom(const DataHolder& source);
+
+    const void* Data() const { return data_.Data(); }
+    void* Data() { return data_.Data(); }
+    size_t DataLength() const { return data_.DataLength(); }
+    bool IsShared() const { return is_shared_ == kShared; }
+
+   private:
+    DataHandle data_;
+    SharingType is_shared_;
+    bool has_registered_external_allocation_;
+  };
+
+  scoped_refptr<DataHolder> holder_;
 
   DISALLOW_COPY_AND_ASSIGN(ArrayBufferContents);
 };