Commit 24a3cb05 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

[arraybuffer] Remove list of views

This CL removes the list of ArrayBufferViews held by the ArrayBuffer.
This list was originally introduced to handle a bug use-after-free issue
in audio buffers, but this issue has been dealt with otherwise in the
meantime, see https://docs.google.com/document/d/1h0bfIeGIqNCFi7iKyKKYDbQKpflSvA1kMNh74MCgHZM/edit?usp=sharing.

Additionally I removed dead code:
* ArrayBufferViews are never detached anymore.
* ArrayBufferViews cannot be marked as detachable.

R=ulan@chromium.org, haraken@chromium.org
CC=hongchan@chromium.org, rtoy@chromium.org

Bug: chromium:1008840
Change-Id: I5ee1093a94882e7e2260157cbb88c618bf052a7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102530Reviewed-by: default avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750500}
parent 30e0fabc
...@@ -45,32 +45,9 @@ bool ArrayBuffer::Transfer(ArrayBufferContents& result) { ...@@ -45,32 +45,9 @@ bool ArrayBuffer::Transfer(ArrayBufferContents& result) {
return true; return true;
} }
bool all_views_are_detachable = true; contents_.Transfer(result);
for (ArrayBufferView* i = first_view_; i; i = i->next_view_) {
if (!i->IsDetachable())
all_views_are_detachable = false;
}
if (all_views_are_detachable) {
contents_.Transfer(result);
while (first_view_) { is_detached_ = true;
ArrayBufferView* current = first_view_;
RemoveView(current);
current->Detach();
}
is_detached_ = true;
} else {
// TODO(https://crbug.com/763038): See original bug at
// https://crbug.com/254728. Copying the buffer instead of transferring is
// not spec compliant but was added for a WebAudio bug fix. The only time
// this branch is taken is when attempting to transfer an AudioBuffer's
// channel data ArrayBuffer.
contents_.CopyTo(result);
if (!result.Data())
return false;
}
return true; return true;
} }
...@@ -100,25 +77,4 @@ bool ArrayBuffer::ShareNonSharedForInternalUse(ArrayBufferContents& result) { ...@@ -100,25 +77,4 @@ bool ArrayBuffer::ShareNonSharedForInternalUse(ArrayBufferContents& result) {
contents_.ShareNonSharedForInternalUse(result); contents_.ShareNonSharedForInternalUse(result);
return true; return true;
} }
void ArrayBuffer::AddView(ArrayBufferView* view) {
view->buffer_ = this;
view->prev_view_ = nullptr;
view->next_view_ = first_view_;
if (first_view_)
first_view_->prev_view_ = view;
first_view_ = view;
}
void ArrayBuffer::RemoveView(ArrayBufferView* view) {
DCHECK_EQ(this, view->buffer_.get());
if (view->next_view_)
view->next_view_->prev_view_ = view->prev_view_;
if (view->prev_view_)
view->prev_view_->next_view_ = view->next_view_;
if (first_view_ == view)
first_view_ = view->next_view_;
view->prev_view_ = view->next_view_ = nullptr;
}
} // namespace blink } // namespace blink
...@@ -104,7 +104,6 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> { ...@@ -104,7 +104,6 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> {
ArrayBufferContents::InitializationPolicy); ArrayBufferContents::InitializationPolicy);
ArrayBufferContents contents_; ArrayBufferContents contents_;
ArrayBufferView* first_view_;
bool is_detached_; bool is_detached_;
}; };
...@@ -198,8 +197,7 @@ scoped_refptr<ArrayBuffer> ArrayBuffer::CreateShared( ...@@ -198,8 +197,7 @@ scoped_refptr<ArrayBuffer> ArrayBuffer::CreateShared(
return base::AdoptRef(new ArrayBuffer(contents)); return base::AdoptRef(new ArrayBuffer(contents));
} }
ArrayBuffer::ArrayBuffer(ArrayBufferContents& contents) ArrayBuffer::ArrayBuffer(ArrayBufferContents& contents) : is_detached_(false) {
: first_view_(nullptr), is_detached_(false) {
if (contents.IsShared()) if (contents.IsShared())
contents.ShareWith(contents_); contents.ShareWith(contents_);
else else
......
...@@ -31,27 +31,10 @@ namespace blink { ...@@ -31,27 +31,10 @@ namespace blink {
ArrayBufferView::ArrayBufferView(scoped_refptr<ArrayBuffer> buffer, ArrayBufferView::ArrayBufferView(scoped_refptr<ArrayBuffer> buffer,
size_t byte_offset) size_t byte_offset)
: raw_byte_offset_(byte_offset), : raw_byte_offset_(byte_offset), buffer_(std::move(buffer)) {
is_detachable_(true),
buffer_(std::move(buffer)),
prev_view_(nullptr),
next_view_(nullptr) {
raw_base_address_ = raw_base_address_ =
buffer_ ? (static_cast<char*>(buffer_->DataMaybeShared()) + byte_offset) buffer_ ? (static_cast<char*>(buffer_->DataMaybeShared()) + byte_offset)
: nullptr; : nullptr;
if (buffer_)
buffer_->AddView(this);
}
ArrayBufferView::~ArrayBufferView() {
if (buffer_)
buffer_->RemoveView(this);
}
void ArrayBufferView::Detach() {
buffer_ = nullptr;
raw_base_address_ = nullptr;
raw_byte_offset_ = 0;
} }
const char* ArrayBufferView::TypeName() { const char* ArrayBufferView::TypeName() {
......
...@@ -71,16 +71,13 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> { ...@@ -71,16 +71,13 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> {
virtual size_t ByteLengthAsSizeT() const = 0; virtual size_t ByteLengthAsSizeT() const = 0;
virtual unsigned TypeSize() const = 0; virtual unsigned TypeSize() const = 0;
void SetDetachable(bool flag) { is_detachable_ = flag; }
bool IsDetachable() const { return is_detachable_; }
bool IsShared() const { return buffer_ ? buffer_->IsShared() : false; } bool IsShared() const { return buffer_ ? buffer_->IsShared() : false; }
virtual ~ArrayBufferView(); virtual ~ArrayBufferView() = default;
protected: protected:
ArrayBufferView(scoped_refptr<ArrayBuffer>, size_t byte_offset); ArrayBufferView(scoped_refptr<ArrayBuffer>, size_t byte_offset);
virtual void Detach();
bool IsDetached() const { return !buffer_ || buffer_->IsDetached(); } bool IsDetached() const { return !buffer_ || buffer_->IsDetached(); }
private: private:
...@@ -88,12 +85,8 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> { ...@@ -88,12 +85,8 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> {
// This is the address of the ArrayBuffer's storage, plus the byte offset. // This is the address of the ArrayBuffer's storage, plus the byte offset.
void* raw_base_address_; void* raw_base_address_;
size_t raw_byte_offset_; size_t raw_byte_offset_;
bool is_detachable_;
friend class ArrayBuffer;
scoped_refptr<ArrayBuffer> buffer_; scoped_refptr<ArrayBuffer> buffer_;
ArrayBufferView* prev_view_;
ArrayBufferView* next_view_;
}; };
} // namespace blink } // namespace blink
......
...@@ -76,10 +76,6 @@ class TypedArray : public ArrayBufferView { ...@@ -76,10 +76,6 @@ class TypedArray : public ArrayBufferView {
} }
private: private:
void Detach() final {
ArrayBufferView::Detach();
raw_length_ = 0;
}
// It may be stale after Detach. Use length() instead. // It may be stale after Detach. Use length() instead.
size_t raw_length_; size_t raw_length_;
}; };
......
...@@ -69,7 +69,6 @@ class CORE_EXPORT DOMArrayBufferView : public ScriptWrappable { ...@@ -69,7 +69,6 @@ class CORE_EXPORT DOMArrayBufferView : public ScriptWrappable {
return base::checked_cast<unsigned>(View()->ByteLengthAsSizeT()); return base::checked_cast<unsigned>(View()->ByteLengthAsSizeT());
} }
unsigned TypeSize() const { return View()->TypeSize(); } unsigned TypeSize() const { return View()->TypeSize(); }
void SetDetachable(bool flag) { return View()->SetDetachable(flag); }
bool IsShared() const { return View()->IsShared(); } bool IsShared() const { return View()->IsShared(); }
void* BaseAddressMaybeShared() const { void* BaseAddressMaybeShared() const {
......
...@@ -31,11 +31,6 @@ class DataView final : public ArrayBufferView { ...@@ -31,11 +31,6 @@ class DataView final : public ArrayBufferView {
unsigned TypeSize() const override { return 1; } unsigned TypeSize() const override { return 1; }
protected: protected:
void Detach() override {
ArrayBufferView::Detach();
raw_byte_length_ = 0;
}
private: private:
DataView(ArrayBuffer* buffer, size_t byte_offset, size_t byte_length) DataView(ArrayBuffer* buffer, size_t byte_offset, size_t byte_length)
: ArrayBufferView(buffer, byte_offset), raw_byte_length_(byte_length) {} : ArrayBufferView(buffer, byte_offset), raw_byte_length_(byte_length) {}
......
...@@ -164,7 +164,6 @@ AudioBuffer::AudioBuffer(unsigned number_of_channels, ...@@ -164,7 +164,6 @@ AudioBuffer::AudioBuffer(unsigned number_of_channels,
if (!channel_data_array) if (!channel_data_array)
return; return;
channel_data_array->SetDetachable(false);
channels_.push_back(channel_data_array); channels_.push_back(channel_data_array);
} }
} }
...@@ -182,7 +181,6 @@ AudioBuffer::AudioBuffer(AudioBus* bus) ...@@ -182,7 +181,6 @@ AudioBuffer::AudioBuffer(AudioBus* bus)
if (!channel_data_array) if (!channel_data_array)
return; return;
channel_data_array->SetDetachable(false);
const float* src = bus->Channel(i)->Data(); const float* src = bus->Channel(i)->Data();
float* dst = channel_data_array->Data(); float* dst = channel_data_array->Data();
memmove(dst, src, length_ * sizeof(*dst)); memmove(dst, src, length_ * sizeof(*dst));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment