0

Don't use BigBuffer::byte_span to access a general BigBuffer

BigBuffer::byte_span is misleading. It's the accessor for the kBytes
variant of the sum type, not a way to view a general BigBuffer as a
span.

See discussion in
https://chromium-review.googlesource.com/c/chromium/src/+/5420741/comment/f7e3c07a_341f8171/

Now that BigBuffer is span- and range-compatible, fix all the callers to
avoid that. To help avoid mistakes in the future, I've upgraded the
DCHECKs to CHECKs.

Change-Id: I47c1fdae18d45e1896674894a874609ef682f612
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5419810
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1282967}
This commit is contained in:
David Benjamin
2024-04-05 05:00:03 +00:00
committed by Chromium LUCI CQ
parent 9456b23459
commit 37b313c7b2
8 changed files with 23 additions and 18 deletions
content/browser
mojo/public/cpp/base
services/webnn
storage/browser/blob
third_party/blink/renderer/bindings/core/v8

@ -4,6 +4,7 @@
#include "content/browser/code_cache/simple_lru_cache.h"
#include "base/containers/to_vector.h"
#include "base/feature_list.h"
#include "base/test/scoped_feature_list.h"
#include "content/common/features.h"
@ -26,11 +27,6 @@ class SimpleLruCacheTest : public testing::TestWithParam<bool> {
base::test::ScopedFeatureList scoped_feature_list_;
};
std::vector<uint8_t> ToVector(const mojo_base::BigBuffer& buffer) {
return std::vector<uint8_t>(buffer.byte_span().begin(),
buffer.byte_span().end());
}
TEST_P(SimpleLruCacheTest, Empty) {
const std::string kKey = "hello";
SimpleLruCache cache(/*capacity=*/100 * 1024);
@ -101,9 +97,9 @@ TEST_P(SimpleLruCacheTest, PutAndGet) {
EXPECT_EQ(result4->response_time, kResponseTime4);
if (base::FeatureList::IsEnabled(features::kInMemoryCodeCache)) {
EXPECT_EQ(ToVector(result1->data), kData2);
EXPECT_EQ(ToVector(result2->data), kData3);
EXPECT_EQ(ToVector(result4->data), kData4);
EXPECT_EQ(base::ToVector(result1->data), kData2);
EXPECT_EQ(base::ToVector(result2->data), kData3);
EXPECT_EQ(base::ToVector(result4->data), kData4);
} else {
EXPECT_EQ(result1->data.size(), 0u);
EXPECT_EQ(result2->data.size(), 0u);

@ -2595,7 +2595,7 @@ bool InterestGroupAuction::HandleServerResponseImpl(
AdAuctionPageData& ad_auction_page_data) {
// Check that response was witnessed by seller origin.
std::array<uint8_t, crypto::kSHA256Length> hash =
crypto::SHA256Hash(response.byte_span());
crypto::SHA256Hash(response);
if (!ad_auction_page_data.WitnessedAuctionResultForOrigin(
config_->seller,
std::string(reinterpret_cast<char*>(hash.data()), hash.size()))) {
@ -4967,9 +4967,8 @@ void InterestGroupAuction::OnDecompressedServerResponse(
}
return;
}
base::span<const uint8_t> result_span = result.value().byte_span();
data_decoder->ParseCbor(
result_span,
result.value(),
base::BindOnce(
&InterestGroupAuction::OnParsedServerResponse,
weak_ptr_factory_.GetWeakPtr(),

@ -107,6 +107,10 @@ BigBuffer& BigBuffer::operator=(BigBuffer&& other) {
return *this;
}
BigBuffer BigBuffer::Clone() const {
return BigBuffer(base::span(*this));
}
uint8_t* BigBuffer::data() {
return const_cast<uint8_t*>(const_cast<const BigBuffer*>(this)->data());
}

@ -73,6 +73,7 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBufferSharedMemoryRegion {
// The |size()| of the data cannot be manipulated.
class COMPONENT_EXPORT(MOJO_BASE) BigBuffer {
public:
using value_type = uint8_t;
using iterator = base::span<uint8_t>::iterator;
using const_iterator = base::span<const uint8_t>::iterator;
@ -112,6 +113,11 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBuffer {
BigBuffer& operator=(BigBuffer&& other);
// Returns a new BigBuffer containing a copy of this BigBuffer's contents.
// Note that the new BigBuffer may not necessarily have the same backing
// storage type as the original one, only the same contents.
BigBuffer Clone() const;
// Returns a pointer to the data stored by this BigBuffer, regardless of
// backing storage type. Prefer to use `base::span(big_buffer)` instead, or
// the implicit conversion to `base::span`.
@ -128,12 +134,12 @@ class COMPONENT_EXPORT(MOJO_BASE) BigBuffer {
// get a span independent of the storage type, write `base::span(big_buffer)`,
// or rely on the implicit conversion.
base::span<const uint8_t> byte_span() const {
DCHECK_EQ(storage_type_, StorageType::kBytes);
CHECK_EQ(storage_type_, StorageType::kBytes);
return bytes_.as_span();
}
internal::BigBufferSharedMemoryRegion& shared_memory() {
DCHECK_EQ(storage_type_, StorageType::kSharedMemory);
CHECK_EQ(storage_type_, StorageType::kSharedMemory);
return shared_memory_.value();
}

@ -75,7 +75,7 @@ class COMPONENT_EXPORT(MOJO_BASE_PROTOBUF_SUPPORT) ProtoWrapper {
if (!is_valid()) {
return std::nullopt;
}
return bytes_->byte_span();
return base::span(*bytes_);
}
private:

@ -423,7 +423,7 @@ mojom::GraphInfoPtr GraphInfoBuilder::CloneGraphInfo() const {
for (auto& [constant_id, constant_buffer] :
graph_info_->constant_id_to_buffer_map) {
cloned_graph_info->constant_id_to_buffer_map[constant_id] =
mojo_base::BigBuffer(constant_buffer.byte_span());
constant_buffer.Clone();
}
return cloned_graph_info;
}

@ -699,7 +699,7 @@ void BlobStorageContext::RegisterFromMemory(
std::unique_ptr<BlobDataBuilder> builder =
std::make_unique<BlobDataBuilder>(uuid);
builder->AppendData(data.byte_span());
builder->AppendData(data);
std::unique_ptr<BlobDataHandle> handle = AddFinishedBlob(std::move(builder));
BlobImpl::Create(std::move(handle), std::move(blob));
}

@ -868,7 +868,7 @@ class DummyBackgroundResponseProcessorClient
}
ASSERT_EQ(expected_cached_metadata, cached_metadata_);
if (expected_cached_metadata) {
EXPECT_THAT(cached_metadata_->byte_span(),
EXPECT_THAT(*cached_metadata_,
testing::ElementsAreArray(*expected_cached_metadata));
}
}
@ -1108,7 +1108,7 @@ TEST_F(BackgroundResourceScriptStreamerTest, HasCodeCache) {
EXPECT_TRUE(head);
EXPECT_TRUE(consumer_handle_);
ASSERT_TRUE(cached_metadata);
EXPECT_THAT(cached_metadata->byte_span(),
EXPECT_THAT(*cached_metadata,
testing::ElementsAreArray(code_cache_data_copy));
}));
Finish();