Skip to content

Commit

Permalink
Rewrite T& into raw_ref<T> under multiple directories [1]
Browse files Browse the repository at this point in the history
The changes were generated by running
tools/clang/rewrite_raw_ref_fields/rewrite-multiple-platforms.sh with
tool-arg=--enable_raw_ref_rewrite

`raw_ref` is a smart pointer for a pointer which can not be null, and
which provides Use-after-Free protection in the same ways as raw_ptr.
This class acts like a combination of std::reference_wrapper and
raw_ptr.

See raw_ptr and //base/memory/raw_ptr.md for more details on the
Use-after-Free protection.

Bug: 1357022
Change-Id: Ibcd714bd5a8e408aa31f07a04c5fc67f7eff4e3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4001524
Owners-Override: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1069181}
  • Loading branch information
Ali Hijazi authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 0545afa commit 5517919
Show file tree
Hide file tree
Showing 89 changed files with 584 additions and 520 deletions.
9 changes: 5 additions & 4 deletions android_webview/test/shell/src/draw_fn/overlays_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/android/jni_array.h"
#include "base/logging.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"

namespace draw_fn {
namespace {
Expand Down Expand Up @@ -51,21 +52,21 @@ class OverlaysManager::ScopedCurrentFunctorCall {

private:
ASurfaceControl* GetSurfaceControl() {
if (!functor_.overlay_surface) {
if (!functor_->overlay_surface) {
DCHECK(native_window_);
functor_.overlay_surface =
functor_->overlay_surface =
base::MakeRefCounted<gfx::SurfaceControl::Surface>(native_window_,
"webview_root");
}

return functor_.overlay_surface->surface();
return functor_->overlay_surface->surface();
}

void MergeTransaction(ASurfaceTransaction* transaction) {
gfx::SurfaceControl::ApplyTransaction(transaction);
}

FunctorData& functor_;
const raw_ref<FunctorData> functor_;
raw_ptr<ANativeWindow> native_window_;
};

Expand Down
168 changes: 85 additions & 83 deletions cc/input/input_handler.cc

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion cc/input/input_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <memory>

#include "base/memory/raw_ref.h"
#include "base/time/time.h"
#include "cc/cc_export.h"
#include "cc/input/browser_controls_state.h"
Expand Down Expand Up @@ -681,7 +682,7 @@ class CC_EXPORT InputHandler : public InputDelegateForCompositor {

// The input handler is owned by the delegate so their lifetimes are tied
// together.
CompositorDelegateForInput& compositor_delegate_;
const raw_ref<CompositorDelegateForInput> compositor_delegate_;

raw_ptr<InputHandlerClient> input_handler_client_ = nullptr;

Expand Down
36 changes: 18 additions & 18 deletions cc/paint/paint_op_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ void PaintOpReader::ReadSimple(T* val) {
uint8_t* PaintOpReader::CopyScratchSpace(size_t bytes) {
DCHECK(SkIsAlign4(reinterpret_cast<uintptr_t>(memory_)));

if (options_.scratch_buffer->size() < bytes)
options_.scratch_buffer->resize(bytes);
memcpy(options_.scratch_buffer->data(), const_cast<const char*>(memory_),
if (options_->scratch_buffer->size() < bytes)
options_->scratch_buffer->resize(bytes);
memcpy(options_->scratch_buffer->data(), const_cast<const char*>(memory_),
bytes);
return options_.scratch_buffer->data();
return options_->scratch_buffer->data();
}

template <typename T>
Expand Down Expand Up @@ -223,7 +223,7 @@ void PaintOpReader::Read(SkPath* path) {
case PaintCacheEntryState::kEmpty:
return;
case PaintCacheEntryState::kCached:
if (!options_.paint_cache->GetPath(path_id, path))
if (!options_->paint_cache->GetPath(path_id, path))
SetInvalid(DeserializationError::kMissingPaintCachePathEntry);
return;
case PaintCacheEntryState::kInlined:
Expand All @@ -245,7 +245,7 @@ void PaintOpReader::Read(SkPath* path) {
return;
}
if (entry_state == PaintCacheEntryState::kInlined) {
options_.paint_cache->PutPath(path_id, *path);
options_->paint_cache->PutPath(path_id, *path);
} else {
// If we know that this path will only be drawn once, which is
// implied by kInlinedDoNotCache, we signal to skia that it should not
Expand Down Expand Up @@ -352,7 +352,7 @@ void PaintOpReader::Read(PaintImage* image) {
}

if (serialized_type == PaintOp::SerializedImageType::kMailbox) {
if (!options_.shared_image_provider) {
if (!options_->shared_image_provider) {
SetInvalid(DeserializationError::kMissingSharedImageProvider);
return;
}
Expand All @@ -366,7 +366,7 @@ void PaintOpReader::Read(PaintImage* image) {

SharedImageProvider::Error error;
sk_sp<SkImage> sk_image =
options_.shared_image_provider->OpenSharedImageForRead(mailbox, error);
options_->shared_image_provider->OpenSharedImageForRead(mailbox, error);
if (error != SharedImageProvider::Error::kNoError) {
switch (error) {
case SharedImageProvider::Error::kNoAccess:
Expand Down Expand Up @@ -418,7 +418,7 @@ void PaintOpReader::Read(PaintImage* image) {

// The transfer cache entry for an image may not exist if the upload fails.
if (auto* entry =
options_.transfer_cache->GetEntryAs<ServiceImageTransferCacheEntry>(
options_->transfer_cache->GetEntryAs<ServiceImageTransferCacheEntry>(
transfer_cache_entry_id)) {
if (needs_mips)
entry->EnsureMips();
Expand Down Expand Up @@ -485,7 +485,7 @@ void PaintOpReader::Read(sk_sp<GrSlug>* slug) {
}

*slug = GrSlug::Deserialize(const_cast<const char*>(memory_), data_bytes,
options_.strike_client);
options_->strike_client);
DidRead(data_bytes);

if (!*slug) {
Expand Down Expand Up @@ -551,9 +551,9 @@ void PaintOpReader::Read(sk_sp<PaintShader>* shader) {

// Track dependent transfer cache entries to make cached shader size
// more realistic.
size_t pre_size = options_.transfer_cache->GetTotalEntrySizes();
size_t pre_size = options_->transfer_cache->GetTotalEntrySizes();
size_t record_size = Read(&ref.record_);
size_t post_size = options_.transfer_cache->GetTotalEntrySizes();
size_t post_size = options_->transfer_cache->GetTotalEntrySizes();
shader_size = post_size - pre_size + record_size;

ref.id_ = shader_id;
Expand Down Expand Up @@ -613,7 +613,7 @@ void PaintOpReader::Read(sk_sp<PaintShader>* shader) {
// transfer cache entries from needing to depend on other transfer cache
// entries.
auto* entry =
options_.transfer_cache->GetEntryAs<ServiceShaderTransferCacheEntry>(
options_->transfer_cache->GetEntryAs<ServiceShaderTransferCacheEntry>(
shader_id);
// Only consider entries that use the same scale. This limits the service
// side transfer cache to only having one entry per shader but this will hit
Expand All @@ -624,7 +624,7 @@ void PaintOpReader::Read(sk_sp<PaintShader>* shader) {
} else {
ref.ResolveSkObjects();
DCHECK(ref.sk_cached_picture_);
options_.transfer_cache->CreateLocalEntry(
options_->transfer_cache->CreateLocalEntry(
shader_id, std::make_unique<ServiceShaderTransferCacheEntry>(
*shader, shader_size));
}
Expand Down Expand Up @@ -700,7 +700,7 @@ void PaintOpReader::Read(gpu::Mailbox* mailbox) {
}

void PaintOpReader::Read(scoped_refptr<SkottieWrapper>* skottie) {
if (!options_.is_privileged) {
if (!options_->is_privileged) {
valid_ = false;
return;
}
Expand All @@ -710,7 +710,7 @@ void PaintOpReader::Read(scoped_refptr<SkottieWrapper>* skottie) {
if (!valid_)
return;
auto* entry =
options_.transfer_cache->GetEntryAs<ServiceSkottieTransferCacheEntry>(
options_->transfer_cache->GetEntryAs<ServiceSkottieTransferCacheEntry>(
transfer_cache_entry_id);
if (entry) {
*skottie = entry->skottie();
Expand Down Expand Up @@ -744,7 +744,7 @@ NOINLINE void PaintOpReader::SetInvalid(DeserializationError error) {
"PaintOpReader deserialization error");
base::UmaHistogramEnumeration("GPU.PaintOpReader.DeserializationError",
error);
if (valid_ && options_.crash_dump_on_failure && base::RandInt(1, 10) == 1) {
if (valid_ && options_->crash_dump_on_failure && base::RandInt(1, 10) == 1) {
crash_reporter::ScopedCrashKeyString crash_key_scope(
&deserialization_error_crash_key,
base::NumberToString(static_cast<int>(error)));
Expand Down Expand Up @@ -1408,7 +1408,7 @@ size_t PaintOpReader::Read(sk_sp<PaintRecord>* record) {
if (!valid_)
return 0;

*record = PaintOpBuffer::MakeFromMemory(memory_, size_bytes, options_);
*record = PaintOpBuffer::MakeFromMemory(memory_, size_bytes, *options_);
if (!*record) {
SetInvalid(DeserializationError::kPaintOpBufferMakeFromMemoryFailure);
return 0;
Expand Down
3 changes: 2 additions & 1 deletion cc/paint/paint_op_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef CC_PAINT_PAINT_OP_READER_H_
#define CC_PAINT_PAINT_OP_READER_H_

#include "base/memory/raw_ref.h"
#include "base/memory/scoped_refptr.h"
#include "cc/paint/paint_export.h"
#include "cc/paint/paint_filter.h"
Expand Down Expand Up @@ -311,7 +312,7 @@ class CC_PAINT_EXPORT PaintOpReader {
const volatile char* memory_ = nullptr;
size_t remaining_bytes_ = 0u;
bool valid_ = true;
const PaintOp::DeserializeOptions& options_;
const raw_ref<const PaintOp::DeserializeOptions> options_;

// Indicates that the data was serialized with the following constraints:
// 1) PaintRecords and SkDrawLoopers are ignored.
Expand Down
37 changes: 19 additions & 18 deletions cc/paint/paint_op_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,13 @@ void PaintOpWriter::Write(const SkColor4f& color) {

void PaintOpWriter::Write(const SkPath& path, UsePaintCache use_paint_cache) {
auto id = path.getGenerationID();
if (!options_.for_identifiability_study)
if (!options_->for_identifiability_study)
Write(id);

DCHECK(use_paint_cache == UsePaintCache::kEnabled ||
!options_.paint_cache->Get(PaintCacheDataType::kPath, id));
!options_->paint_cache->Get(PaintCacheDataType::kPath, id));
if (use_paint_cache == UsePaintCache::kEnabled &&
options_.paint_cache->Get(PaintCacheDataType::kPath, id)) {
options_->paint_cache->Get(PaintCacheDataType::kPath, id)) {
Write(static_cast<uint32_t>(PaintCacheEntryState::kCached));
return;
}
Expand Down Expand Up @@ -224,7 +224,7 @@ void PaintOpWriter::Write(const SkPath& path, UsePaintCache use_paint_cache) {
size_t bytes_written = path.writeToMemory(memory_);
DCHECK_EQ(bytes_written, bytes_required);
if (use_paint_cache == UsePaintCache::kEnabled) {
options_.paint_cache->Put(PaintCacheDataType::kPath, id, bytes_written);
options_->paint_cache->Put(PaintCacheDataType::kPath, id, bytes_written);
}
*bytes_to_skip = bytes_written;
DidWrite(bytes_written);
Expand Down Expand Up @@ -284,7 +284,7 @@ void PaintOpWriter::Write(const DrawImage& draw_image,
}

// Default mode uses the transfer cache.
auto decoded_image = options_.image_provider->GetRasterContent(draw_image);
auto decoded_image = options_->image_provider->GetRasterContent(draw_image);
DCHECK(!decoded_image.decoded_image().image())
<< "Use transfer cache for image serialization";
const DecodedDrawImage& decoded_draw_image = decoded_image.decoded_image();
Expand All @@ -305,14 +305,15 @@ void PaintOpWriter::Write(scoped_refptr<SkottieWrapper> skottie) {
return;

bool locked =
options_.transfer_cache->LockEntry(TransferCacheEntryType::kSkottie, id);
options_->transfer_cache->LockEntry(TransferCacheEntryType::kSkottie, id);

// Add a cache entry for the skottie animation.
uint64_t bytes_written = 0u;
if (!locked) {
bytes_written = options_.transfer_cache->CreateEntry(
bytes_written = options_->transfer_cache->CreateEntry(
ClientSkottieTransferCacheEntry(skottie), memory_);
options_.transfer_cache->AssertLocked(TransferCacheEntryType::kSkottie, id);
options_->transfer_cache->AssertLocked(TransferCacheEntryType::kSkottie,
id);
}

DCHECK_LE(bytes_written, remaining_bytes_);
Expand Down Expand Up @@ -438,21 +439,21 @@ sk_sp<PaintShader> PaintOpWriter::TransformShaderIfNecessary(

if (type == PaintShader::Type::kImage) {
if (!original->paint_image().IsPaintWorklet()) {
return original->CreateDecodedImage(ctm, quality, options_.image_provider,
paint_image_transfer_cache_entry_id,
&quality, paint_image_needs_mips,
mailbox_out);
return original->CreateDecodedImage(
ctm, quality, options_->image_provider,
paint_image_transfer_cache_entry_id, &quality, paint_image_needs_mips,
mailbox_out);
}
sk_sp<PaintShader> record_shader =
original->CreatePaintWorkletRecord(options_.image_provider);
original->CreatePaintWorkletRecord(options_->image_provider);
if (!record_shader)
return nullptr;
return record_shader->CreateScaledPaintRecord(
ctm, options_.max_texture_size, paint_record_post_scale);
ctm, options_->max_texture_size, paint_record_post_scale);
}

if (type == PaintShader::Type::kPaintRecord) {
return original->CreateScaledPaintRecord(ctm, options_.max_texture_size,
return original->CreateScaledPaintRecord(ctm, options_->max_texture_size,
paint_record_post_scale);
}

Expand Down Expand Up @@ -533,7 +534,7 @@ void PaintOpWriter::Write(const PaintShader* shader,
if (shader->record_) {
Write(true);
DCHECK_NE(shader->id_, PaintShader::kInvalidRecordShaderId);
if (!options_.for_identifiability_study)
if (!options_->for_identifiability_study)
Write(shader->id_);
const gfx::Rect playback_rect(
gfx::ToEnclosingRect(gfx::SkRectToRectF(shader->tile())));
Expand Down Expand Up @@ -810,7 +811,7 @@ void PaintOpWriter::Write(const RecordPaintFilter& filter,
// analysis (e.g. this ensures any contained text blobs will not be missing
// from the cache).
auto scaled_filter = filter.CreateScaledPaintRecord(
current_ctm.asM33(), options_.max_texture_size);
current_ctm.asM33(), options_->max_texture_size);
if (!scaled_filter) {
WriteSimple(false);
return;
Expand Down Expand Up @@ -953,7 +954,7 @@ void PaintOpWriter::Write(const PaintRecord* record,
// converted to a fixed scale mode (hence |post_scale|), which means they are
// first rendered offscreen via SkImage::MakeFromPicture. This inherently does
// not support lcd text, so reflect that in the serialization options.
PaintOp::SerializeOptions lcd_disabled_options = options_;
PaintOp::SerializeOptions lcd_disabled_options = *options_;
lcd_disabled_options.can_use_lcd_text = false;
SimpleBufferSerializer serializer(memory_, remaining_bytes_,
lcd_disabled_options);
Expand Down
3 changes: 2 additions & 1 deletion cc/paint/paint_op_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define CC_PAINT_PAINT_OP_WRITER_H_

#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "cc/paint/paint_canvas.h"
#include "cc/paint/paint_export.h"
#include "cc/paint/paint_filter.h"
Expand Down Expand Up @@ -192,7 +193,7 @@ class CC_PAINT_EXPORT PaintOpWriter {
raw_ptr<char> memory_ = nullptr;
size_t size_ = 0u;
size_t remaining_bytes_ = 0u;
const PaintOp::SerializeOptions& options_;
const raw_ref<const PaintOp::SerializeOptions> options_;
bool valid_ = true;

// Indicates that the following security constraints must be applied during
Expand Down
5 changes: 3 additions & 2 deletions cc/tiles/software_image_decode_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/debug/stack_trace.h"
#include "base/format_macros.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/metrics/histogram_macros.h"
#include "base/numerics/ostream_operators.h"
#include "base/ranges/algorithm.h"
Expand Down Expand Up @@ -48,14 +49,14 @@ class AutoRemoveKeyFromTaskMap {
SoftwareImageDecodeCache::CacheKeyHash>* task_map,
const SoftwareImageDecodeCache::CacheKey& key)
: task_map_(task_map), key_(key) {}
~AutoRemoveKeyFromTaskMap() { task_map_->erase(key_); }
~AutoRemoveKeyFromTaskMap() { task_map_->erase(*key_); }

private:
raw_ptr<std::unordered_map<SoftwareImageDecodeCache::CacheKey,
scoped_refptr<TileTask>,
SoftwareImageDecodeCache::CacheKeyHash>>
task_map_;
const SoftwareImageDecodeCache::CacheKey& key_;
const raw_ref<const SoftwareImageDecodeCache::CacheKey> key_;
};

class SoftwareImageDecodeTaskImpl : public TileTask {
Expand Down
5 changes: 3 additions & 2 deletions cc/trees/property_tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
#include "cc/base/synced_property.h"
#include "cc/cc_export.h"
Expand Down Expand Up @@ -717,7 +718,7 @@ class CC_EXPORT PropertyTrees final {
#endif

const ProtectedSequenceSynchronizer& synchronizer() const {
return synchronizer_;
return *synchronizer_;
}

const ClipTree& clip_tree() const { return clip_tree_; }
Expand Down Expand Up @@ -822,7 +823,7 @@ class CC_EXPORT PropertyTrees final {
bool HasElement(ElementId element_id) const;

private:
const ProtectedSequenceSynchronizer& synchronizer_;
const raw_ref<const ProtectedSequenceSynchronizer> synchronizer_;

TransformTree transform_tree_;
EffectTree effect_tree_;
Expand Down
Loading

0 comments on commit 5517919

Please sign in to comment.