spanification: Add RWBuffer::ROIter::operator*
Similar to the one on ROBuffer::Iter. Convert callers. Fold the separate
data() and size() accessors into the dereference operator. Do the same
on ROBuffer::Iter.
Bug: 351564777
Change-Id: I23b7b0efcf0e54ec055ccb72ef6c794be649569e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6074672
Commit-Queue: Kent Tamura <tkent@chromium.org>
Auto-Submit: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1392575}
diff --git a/third_party/blink/renderer/platform/graphics/parkable_image.cc b/third_party/blink/renderer/platform/graphics/parkable_image.cc
index ed20b3712..ae8cefd 100644
--- a/third_party/blink/renderer/platform/graphics/parkable_image.cc
+++ b/third_party/blink/renderer/platform/graphics/parkable_image.cc
@@ -74,7 +74,8 @@
auto ro_buffer = rw_buffer->MakeROBufferSnapshot();
ROBuffer::Iter iter(ro_buffer);
do {
- ASAN_POISON_MEMORY_REGION(iter.data(), iter.size());
+ auto data = *iter;
+ ASAN_POISON_MEMORY_REGION(data.data(), data.size());
} while (iter.Next());
#endif
}
@@ -87,7 +88,8 @@
auto ro_buffer = rw_buffer->MakeROBufferSnapshot();
ROBuffer::Iter iter(ro_buffer);
do {
- ASAN_UNPOISON_MEMORY_REGION(iter.data(), iter.size());
+ auto data = *iter;
+ ASAN_UNPOISON_MEMORY_REGION(data.data(), data.size());
} while (iter.Next());
#endif
}
@@ -157,8 +159,9 @@
// longer limetime than the SkData.
parkable_image_->AddRef();
parkable_image_->LockData();
+ auto data = *iter;
return SkData::MakeWithProc(
- iter.data(), available_,
+ data.data(), data.size(),
[](const void* ptr, void* context) -> void {
auto* parkable_image = static_cast<ParkableImage*>(context);
{
@@ -221,7 +224,7 @@
scoped_refptr<SharedBuffer> shared_buffer = SharedBuffer::Create();
ROBuffer::Iter it(ro_buffer.get());
do {
- shared_buffer->Append(it.data(), it.size());
+ shared_buffer->Append(*it);
} while (it.Next());
return shared_buffer;
diff --git a/third_party/blink/renderer/platform/graphics/parkable_image_test.cc b/third_party/blink/renderer/platform/graphics/parkable_image_test.cc
index 54839ae..7d3c45d 100644
--- a/third_party/blink/renderer/platform/graphics/parkable_image_test.cc
+++ b/third_party/blink/renderer/platform/graphics/parkable_image_test.cc
@@ -112,10 +112,11 @@
return pi->impl_->is_frozen();
}
- scoped_refptr<ParkableImage> MakeParkableImageForTesting(base::span<const char> buffer) {
+ scoped_refptr<ParkableImage> MakeParkableImageForTesting(
+ base::span<const uint8_t> buffer) {
auto pi = ParkableImage::Create();
- pi->Append(WTF::SharedBuffer::Create(buffer.data(), buffer.size()).get(), 0);
+ pi->Append(WTF::SharedBuffer::Create(buffer).get(), 0);
return pi;
}
@@ -123,7 +124,7 @@
// Checks content matches the ParkableImage returned from
// |MakeParkableImageForTesting|.
static bool IsSameContent(scoped_refptr<ParkableImage> pi,
- base::span<const char> buffer) {
+ base::span<const uint8_t> buffer) {
if (pi->size() != buffer.size()) {
return false;
}
@@ -133,13 +134,14 @@
auto ro_buffer = pi->impl_->rw_buffer_->MakeROBufferSnapshot();
ROBuffer::Iter iter(ro_buffer.get());
- const char* cur = buffer.data();
do {
- if (memcmp(iter.data(), cur, iter.size()) != 0) {
+ auto iter_data = *iter;
+ auto [buffer_slice, rest] = buffer.split_at(iter_data.size());
+ if (iter_data != buffer_slice) {
pi->UnlockData();
return false;
}
- cur += iter.size();
+ buffer = rest;
} while (iter.Next());
pi->UnlockData();
@@ -287,13 +289,13 @@
// Tests that |Append|ing to a ParkableImage correctly adds data to it.
TEST_F(ParkableImageTest, Append) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = ParkableImage::Create();
ASSERT_EQ(pi->size(), 0u); // Should be empty when created.
- pi->Append(WTF::SharedBuffer::Create(data.data(), data.size()).get(), 0);
+ pi->Append(WTF::SharedBuffer::Create(data).get(), 0);
EXPECT_TRUE(IsSameContent(pi, data));
}
@@ -301,13 +303,13 @@
// Tests that multiple |Append|s correctly add data to the end of ParkableImage.
TEST_F(ParkableImageTest, AppendMultiple) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = ParkableImage::Create();
ASSERT_EQ(pi->size(), 0u); // Should be empty when created.
- auto sb = WTF::SharedBuffer::Create(data.data(), data.size());
+ auto sb = WTF::SharedBuffer::Create(data);
ASSERT_EQ(sb->size(), kDataSize);
pi->Append(sb.get(), 0);
@@ -325,7 +327,7 @@
// Tests that we can read/write to disk correctly, preserving the data.
TEST_F(ParkableImageTest, ParkAndUnpark) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
// We have no images currently.
@@ -369,7 +371,7 @@
// Tests that trying to park multiple times doesn't add any extra tasks.
TEST_F(ParkableImageTest, ParkTwiceAndUnpark) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
// We have no images currently.
@@ -409,7 +411,7 @@
// disk the first time.
TEST_F(ParkableImageTest, ParkAndUnparkSync) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
// We have no images currently.
@@ -480,7 +482,7 @@
// discarding the data.
TEST_F(ParkableImageTest, ParkAndUnparkAborted) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
// We have no images currently.
@@ -554,7 +556,7 @@
// Tests that a frozen image will be written to disk by the manager.
TEST_F(ParkableImageTest, ManagerSimple) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -591,7 +593,7 @@
// Tests that a small image is not kept in the manager.
TEST_F(ParkableImageTest, ManagerSmall) {
const size_t kDataSize = ParkableImageImpl::kMinSizeToPark - 10;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -620,7 +622,7 @@
// created at once.
TEST_F(ParkableImageTest, ManagerTwo) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -657,7 +659,7 @@
// Test that a non-frozen image will not be written to disk.
TEST_F(ParkableImageTest, ManagerNonFrozen) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -687,7 +689,7 @@
// effect.
TEST_F(ParkableImageNoParkingTest, Unpark) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = MakeParkableImageForTesting(data);
@@ -710,7 +712,7 @@
// minutes.
TEST_F(ParkableImageTest, ManagerStatistics5min) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = MakeParkableImageForTesting(data);
@@ -738,7 +740,7 @@
// recorded in this case, since no reads/writes will happen.
TEST_F(ParkableImageNoParkingTest, ManagerStatistics5min) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = MakeParkableImageForTesting(data);
@@ -763,7 +765,7 @@
// disabled.
TEST_F(ParkableImageNoParkingTest, ManagerSimple) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = MakeParkableImageForTesting(data);
@@ -794,7 +796,7 @@
// Test a locked image will not be written to disk.
TEST_F(ParkableImageTest, ManagerNotUnlocked) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -821,7 +823,7 @@
// unfrozen ParkableImages.
TEST_F(ParkableImageTest, ManagerRescheduleUnfrozen) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -863,7 +865,7 @@
// inefficient, but the safest way to do it.
TEST_F(ParkableImageTest, DestroyOnSeparateThread) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -911,7 +913,7 @@
set_may_write(false);
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
EXPECT_EQ(0u, manager.Size());
@@ -933,7 +935,7 @@
// Test that we park only after 30 seconds, not immediately after freezing.
TEST_F(ParkableImageDelayedTest, Simple) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -967,7 +969,7 @@
// immediately after freezing.
TEST_F(ParkableImageDelayedTest, Read) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto& manager = ParkableImageManager::Instance();
@@ -1003,7 +1005,7 @@
// parking/unparking.
TEST_F(ParkableImageDelayedTest, ParkAndUnpark) {
const size_t kDataSize = 3.5 * 4096;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
// We have no images currently.
@@ -1042,7 +1044,7 @@
TEST_F(ParkableImageWithLimitedDiskCapacityTest, ParkWithLimitedDiskCapacity) {
constexpr size_t kMB = 1024 * 1024;
constexpr size_t kDataSize = kMB;
- auto data = base::HeapArray<char>::Uninit(kDataSize);
+ auto data = base::HeapArray<uint8_t>::Uninit(kDataSize);
PrepareReferenceData(data);
auto pi = MakeParkableImageForTesting(data);
diff --git a/third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.h b/third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.h
index aa094c4..9a831a5 100644
--- a/third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.h
+++ b/third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.h
@@ -33,6 +33,10 @@
}
}
+inline void PrepareReferenceData(base::span<uint8_t> buffer) {
+ PrepareReferenceData(base::as_writable_chars(buffer));
+}
+
Vector<char> ReadFile(StringView file_name);
Vector<char> ReadFile(const char* dir, const char* file_name);
diff --git a/third_party/blink/renderer/platform/image-decoders/rw_buffer.cc b/third_party/blink/renderer/platform/image-decoders/rw_buffer.cc
index 73f6634..bd80def 100644
--- a/third_party/blink/renderer/platform/image-decoders/rw_buffer.cc
+++ b/third_party/blink/renderer/platform/image-decoders/rw_buffer.cc
@@ -112,8 +112,7 @@
// `buffer_` has a `raw_ptr` that needs to be destroyed to
// properly lower the refcount.
block_.~BufferBlock();
- WTF::Partitions::BufferFree(
- reinterpret_cast<void*>(const_cast<RWBuffer::BufferHead*>(this)));
+ WTF::Partitions::BufferFree(const_cast<RWBuffer::BufferHead*>(this));
while (block) {
RWBuffer::BufferBlock* next = block->next_;
block->~BufferBlock();
@@ -144,27 +143,24 @@
}
};
-size_t RWBuffer::ROIter::size() const {
- if (!block_) {
- return 0;
- }
-
- return std::min(block_->capacity_, remaining_);
-}
-
RWBuffer::ROIter::ROIter(RWBuffer* rw_buffer, size_t available)
: rw_buffer_(rw_buffer), remaining_(available) {
DCHECK(rw_buffer_);
block_ = &rw_buffer_->head_->block_;
}
-const uint8_t* RWBuffer::ROIter::data() const {
- return remaining_ ? block_->startData() : nullptr;
+base::span<const uint8_t> RWBuffer::ROIter::operator*() const {
+ if (!remaining_) {
+ return {};
+ }
+ DCHECK(block_);
+ return base::span(block_->startData(),
+ std::min(block_->capacity_, remaining_));
}
bool RWBuffer::ROIter::Next() {
if (remaining_) {
- size_t current_size = size();
+ const size_t current_size = std::min(block_->capacity_, remaining_);
DCHECK_LE(current_size, remaining_);
remaining_ -= current_size;
if (remaining_ == 0) {
@@ -225,24 +221,19 @@
}
}
-const uint8_t* ROBuffer::Iter::data() const {
- return remaining_ ? block_->startData() : nullptr;
-}
-
-size_t ROBuffer::Iter::size() const {
- if (!block_) {
- return 0;
- }
- return std::min(block_->capacity_, remaining_);
-}
-
base::span<const uint8_t> ROBuffer::Iter::operator*() const {
- return base::span(data(), size());
+ if (!remaining_) {
+ return {};
+ }
+ DCHECK(block_);
+ return base::span(block_->startData(),
+ std::min(block_->capacity_, remaining_));
}
bool ROBuffer::Iter::Next() {
if (remaining_) {
- remaining_ -= size();
+ const size_t current_size = std::min(block_->capacity_, remaining_);
+ remaining_ -= current_size;
if (buffer_->tail_ == block_) {
// There are more blocks, but buffer_ does not know about them.
DCHECK_EQ(0u, remaining_);
diff --git a/third_party/blink/renderer/platform/image-decoders/rw_buffer.h b/third_party/blink/renderer/platform/image-decoders/rw_buffer.h
index 41ad4c9..c6e3c56 100644
--- a/third_party/blink/renderer/platform/image-decoders/rw_buffer.h
+++ b/third_party/blink/renderer/platform/image-decoders/rw_buffer.h
@@ -28,8 +28,10 @@
class PLATFORM_EXPORT ROIter {
public:
explicit ROIter(RWBuffer*, size_t);
- size_t size() const;
- const uint8_t* data() const;
+
+ // Returns a span of the current continuous block of memory. The span will
+ // be empty if the iterator is exhausted.
+ base::span<const uint8_t> operator*() const;
// Checks whether there is another block available and advances the iterator
// if there is.
bool Next();
@@ -39,7 +41,7 @@
private:
raw_ptr<const RWBuffer> rw_buffer_;
- raw_ptr<RWBuffer::BufferBlock> block_;
+ raw_ptr<const RWBuffer::BufferBlock> block_;
size_t remaining_;
};
@@ -104,18 +106,9 @@
void Reset(const ROBuffer*);
/**
- * Return the current continuous block of memory, or nullptr if the
- * iterator is exhausted
+ * Returns a span of the current continuous block of memory. The span will
+ * be empty if the iterator is exhausted.
*/
- const uint8_t* data() const;
-
- /**
- * Returns the number of bytes in the current contiguous block of memory,
- * or 0 if the iterator is exhausted.
- */
- size_t size() const;
-
- // Returns a span with data() and size().
base::span<const uint8_t> operator*() const;
/**
diff --git a/third_party/blink/renderer/platform/image-decoders/rw_buffer_test.cc b/third_party/blink/renderer/platform/image-decoders/rw_buffer_test.cc
index cced9fbf..d0f368c3 100644
--- a/third_party/blink/renderer/platform/image-decoders/rw_buffer_test.cc
+++ b/third_party/blink/renderer/platform/image-decoders/rw_buffer_test.cc
@@ -29,18 +29,20 @@
// reader should contains an integral number of copies of gABC.
void check_alphabet_buffer(const ROBuffer* reader) {
- size_t size = reader->size();
+ const size_t size = reader->size();
ASSERT_EQ(size % 26, 0u);
std::vector<char> storage(size);
+ auto dest = base::as_writable_byte_span(storage);
+
ROBuffer::Iter iter(reader);
- size_t offset = 0;
do {
- ASSERT_LE(offset + iter.size(), size);
- memcpy(storage.data() + offset, iter.data(), iter.size());
- offset += iter.size();
+ auto src = *iter;
+ ASSERT_LE(src.size(), dest.size());
+ dest.copy_prefix_from(src);
+ dest = dest.subspan(src.size());
} while (iter.Next());
- ASSERT_EQ(offset, size);
+ ASSERT_TRUE(dest.empty());
check_abcs(storage.data(), size);
}
@@ -128,12 +130,12 @@
scoped_refptr<ROBuffer> roBuffer(buffer.MakeROBufferSnapshot());
ROBuffer::Iter iter(roBuffer.get());
- EXPECT_TRUE(iter.data());
- EXPECT_EQ(iter.size(), 26u);
+ EXPECT_TRUE((*iter).data());
+ EXPECT_EQ((*iter).size(), 26u);
// There is only one block in this buffer.
EXPECT_TRUE(!iter.Next());
- EXPECT_EQ(0u, iter.size());
+ EXPECT_TRUE((*iter).empty());
}
// Tests that operations (including the destructor) are safe on an RWBuffer
@@ -147,8 +149,8 @@
if (roBuffer) {
EXPECT_EQ(roBuffer->size(), 0u);
ROBuffer::Iter iter(roBuffer.get());
- EXPECT_EQ(iter.size(), 0u);
- EXPECT_TRUE(!iter.data());
+ EXPECT_TRUE((*iter).empty());
+ EXPECT_TRUE(!(*iter).data());
EXPECT_TRUE(!iter.Next());
}
}
@@ -204,7 +206,7 @@
scoped_refptr<ROBuffer> roBuffer = buffer.MakeROBufferSnapshot();
ROBuffer::Iter iter(roBuffer.get());
- EXPECT_EQ(0, memcmp(iter.data(), gABC, 20U));
+ EXPECT_EQ(*iter, base::span_from_cstring(gABC).first(20U));
}
TEST(RWBufferTest, FunctionConstructorLarge) {
diff --git a/third_party/blink/renderer/platform/image-decoders/segment_reader.cc b/third_party/blink/renderer/platform/image-decoders/segment_reader.cc
index 70f0aa3..669e2cd9 100644
--- a/third_party/blink/renderer/platform/image-decoders/segment_reader.cc
+++ b/third_party/blink/renderer/platform/image-decoders/segment_reader.cc
@@ -26,20 +26,23 @@
base::span<const uint8_t> BufferGetSomeData(Iter& iter,
size_t& position_of_block,
size_t position) {
- for (size_t size_of_block = iter.size(); size_of_block != 0;
- position_of_block += size_of_block, size_of_block = iter.size()) {
+ auto current_span = *iter;
+ for (size_t size_of_block = current_span.size(); size_of_block != 0;
+ position_of_block += size_of_block,
+ size_of_block = current_span.size()) {
DCHECK_LE(position_of_block, position);
if (position_of_block + size_of_block > position) {
// |position| is in this block.
const size_t position_in_block = position - position_of_block;
- return base::span(iter.data(), iter.size()).subspan(position_in_block);
+ return current_span.subspan(position_in_block);
}
// Move to next block.
if (!iter.Next()) {
break;
}
+ current_span = *iter;
}
return {};
}
@@ -47,11 +50,12 @@
template <class Iter>
sk_sp<SkData> BufferCopyAsSkData(Iter iter, size_t available) {
sk_sp<SkData> data = SkData::MakeUninitialized(available);
- char* dst = static_cast<char*>(data->writable_data());
+ auto dst =
+ base::span(static_cast<uint8_t*>(data->writable_data()), available);
do {
- size_t size = iter.size();
- memcpy(dst, iter.data(), size);
- dst += size;
+ auto src = *iter;
+ dst.copy_prefix_from(src);
+ dst = dst.subspan(src.size());
} while (iter.Next());
return data;
}
@@ -188,7 +192,7 @@
auto data = BufferGetSomeData(iter_, position_of_block_, position);
- if (!iter_.data()) {
+ if ((*iter_).empty()) {
// Reset to the beginning, so future calls can succeed.
iter_.Reset(ro_buffer_.get());
position_of_block_ = 0;
@@ -214,7 +218,8 @@
if (!multiple_blocks) {
// Contiguous data. No need to copy.
ro_buffer_->AddRef();
- return SkData::MakeWithProc(iter.data(), iter.size(), &UnrefROBuffer,
+ auto data = *iter;
+ return SkData::MakeWithProc(data.data(), data.size(), &UnrefROBuffer,
ro_buffer_.get());
}
