Skip to content

Commit

Permalink
Rewrite T& into raw_ref<T> under chrome/
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: Ie52622518a607201c333d14b04943f0403df38af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3998943
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Ali Hijazi <ahijazi@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1069829}
  • Loading branch information
Ali Hijazi authored and Chromium LUCI CQ committed Nov 10, 2022
1 parent 2c777d8 commit f59bd14
Show file tree
Hide file tree
Showing 112 changed files with 549 additions and 466 deletions.
5 changes: 3 additions & 2 deletions chrome/browser/android/feedback/connectivity_checker.cc
Expand Up @@ -9,6 +9,7 @@
#include "base/android/scoped_java_ref.h"
#include "base/bind.h"
#include "base/location.h"
#include "base/memory/raw_ref.h"
#include "base/task/single_thread_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -83,7 +84,7 @@ class ConnectivityChecker {
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;

// The URL to connect to.
const GURL& url_;
const raw_ref<const GURL> url_;

// How long to wait for a response before giving up.
const base::TimeDelta timeout_;
Expand Down Expand Up @@ -131,7 +132,7 @@ ConnectivityChecker::ConnectivityChecker(

void ConnectivityChecker::StartAsyncCheck() {
auto request = std::make_unique<network::ResourceRequest>();
request->url = url_;
request->url = *url_;
request->credentials_mode = network::mojom::CredentialsMode::kOmit;
request->load_flags = net::LOAD_BYPASS_CACHE | net::LOAD_DISABLE_CACHE;
url_loader_ = network::SimpleURLLoader::Create(std::move(request),
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/background/background_contents.h
Expand Up @@ -11,6 +11,7 @@
#include <string>

#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "content/public/browser/site_instance.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/browser/web_contents_observer.h"
Expand Down Expand Up @@ -124,10 +125,10 @@ struct BackgroundContentsOpenedDetails {
raw_ptr<BackgroundContents> contents;

// The name of the parent frame for these contents.
const std::string& frame_name;
const raw_ref<const std::string> frame_name;

// The ID of the parent application (if any).
const std::string& application_id;
const raw_ref<const std::string> application_id;
};

#endif // CHROME_BROWSER_BACKGROUND_BACKGROUND_CONTENTS_H_
4 changes: 2 additions & 2 deletions chrome/browser/background/background_contents_service.cc
Expand Up @@ -616,8 +616,8 @@ BackgroundContents* BackgroundContentsService::CreateBackgroundContents(

// Register the BackgroundContents internally, then send out a notification
// to external listeners.
BackgroundContentsOpenedDetails details = {contents_ptr, frame_name,
application_id};
BackgroundContentsOpenedDetails details = {contents_ptr, raw_ref(frame_name),
raw_ref(application_id)};
for (auto& observer : observers_)
observer.OnBackgroundContentsOpened(details);

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/browser_switcher/browser_switcher_prefs.cc
Expand Up @@ -69,9 +69,9 @@ void SetCachedRules(PrefService* prefs,
} // namespace

NoCopyUrl::NoCopyUrl(const GURL& original) : original_(original) {
spec_without_port_ = original_.spec();
spec_without_port_ = original_->spec();

int int_port = original_.IntPort();
int int_port = original_->IntPort();
std::string port_suffix;
if (int_port != url::PORT_UNSPECIFIED) {
port_suffix = base::StrCat({":", base::NumberToString(int_port)});
Expand Down
7 changes: 4 additions & 3 deletions chrome/browser/browser_switcher/browser_switcher_prefs.h
Expand Up @@ -13,6 +13,7 @@
#include "base/callback_list.h"
#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
#include "base/strings/string_piece.h"
#include "build/build_config.h"
Expand All @@ -38,13 +39,13 @@ class NoCopyUrl {
explicit NoCopyUrl(const GURL& original);
NoCopyUrl(const NoCopyUrl&) = delete;

const GURL& original() const { return original_; }
const GURL& original() const { return *original_; }
base::StringPiece host_and_port() const { return host_and_port_; }
base::StringPiece spec() const { return original_.spec(); }
base::StringPiece spec() const { return original_->spec(); }
base::StringPiece spec_without_port() const { return spec_without_port_; }

private:
const GURL& original_;
const raw_ref<const GURL> original_;
// If there is a port number, then this is "<host>:<port>". Otherwise, this is
// just the host.
std::string host_and_port_;
Expand Down
Expand Up @@ -19,6 +19,7 @@
#include "base/json/values_util.h"
#include "base/memory/ptr_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
Expand Down Expand Up @@ -813,16 +814,16 @@ class ProbablySameFilterMatcher

bool MatchAndExplain(const base::RepeatingCallback<bool(const GURL&)>& filter,
MatchResultListener* listener) const override {
if (filter.is_null() && to_match_.is_null())
if (filter.is_null() && to_match_->is_null())
return true;
if (filter.is_null() != to_match_.is_null())
if (filter.is_null() != to_match_->is_null())
return false;

const GURL urls_to_test_[] = {
GURL("http://host1.com:1"), GURL("http://host2.com:1"),
GURL("http://host3.com:1"), GURL("invalid spec")};
for (GURL url : urls_to_test_) {
if (filter.Run(url) != to_match_.Run(url)) {
if (filter.Run(url) != to_match_->Run(url)) {
if (listener)
*listener << "The filters differ on the URL " << url;
return false;
Expand All @@ -832,15 +833,15 @@ class ProbablySameFilterMatcher
}

void DescribeTo(::std::ostream* os) const override {
*os << "is probably the same url filter as " << &to_match_;
*os << "is probably the same url filter as " << &*to_match_;
}

void DescribeNegationTo(::std::ostream* os) const override {
*os << "is definitely NOT the same url filter as " << &to_match_;
*os << "is definitely NOT the same url filter as " << &*to_match_;
}

private:
const base::RepeatingCallback<bool(const GURL&)>& to_match_;
const raw_ref<const base::RepeatingCallback<bool(const GURL&)>> to_match_;
};

inline Matcher<const base::RepeatingCallback<bool(const GURL&)>&>
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/cart/cart_service_browsertest.cc
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_timeouts.h"
#include "chrome/browser/cart/cart_service.h"
Expand Down Expand Up @@ -245,11 +246,11 @@ class FakeCartDiscountLinkFetcher : public CartDiscountLinkFetcher {
std::unique_ptr<network::PendingSharedURLLoaderFactory> pending_factory,
cart_db::ChromeCartContentProto cart_content_proto,
CartDiscountLinkFetcherCallback callback) override {
std::move(callback).Run(discount_url_);
std::move(callback).Run(*discount_url_);
}

private:
const GURL& discount_url_;
const raw_ref<const GURL> discount_url_;
};

class CartServiceBrowserDiscountTest : public CartServiceBrowserTest {
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/download/download_query.cc
Expand Up @@ -18,6 +18,7 @@
#include "base/files/file_path.h"
#include "base/i18n/case_conversion.h"
#include "base/i18n/string_search.h"
#include "base/memory/raw_ref.h"
#include "base/notreached.h"
#include "base/strings/string_split.h"
#include "base/strings/utf_string_conversions.h"
Expand Down Expand Up @@ -261,14 +262,14 @@ class DownloadQuery::DownloadComparator {
bool operator()(const DownloadItem* left, const DownloadItem* right);

private:
const DownloadQuery::SorterVector& terms_;
const raw_ref<const DownloadQuery::SorterVector> terms_;

// std::sort requires this class to be copyable.
};

bool DownloadQuery::DownloadComparator::operator()(const DownloadItem* left,
const DownloadItem* right) {
for (auto term = terms_.begin(); term != terms_.end(); ++term) {
for (auto term = terms_->begin(); term != terms_->end(); ++term) {
switch (term->sorter.Run(*left, *right)) {
case LT:
return term->direction == DownloadQuery::ASCENDING;
Expand Down
Expand Up @@ -186,7 +186,7 @@ safe_browsing::FileAnalysisRequest* FilesRequestHandler::PrepareFileRequest(
DCHECK_LT(index, paths_.size());
base::FilePath path = paths_[index];
auto request = std::make_unique<safe_browsing::FileAnalysisRequest>(
analysis_settings_, path, path.BaseName(), /*mime_type*/ "",
*analysis_settings_, path, path.BaseName(), /*mime_type*/ "",
/* delay_opening_file */ true,
base::BindOnce(&FilesRequestHandler::FileRequestCallback,
weak_ptr_factory_.GetWeakPtr(), index),
Expand All @@ -213,7 +213,7 @@ void FilesRequestHandler::OnGotFileInfo(
file_info_[index].size = data.size;
file_info_[index].mime_type = data.mime_type;

bool failed = analysis_settings_.cloud_or_local_settings.is_cloud_analysis()
bool failed = analysis_settings_->cloud_or_local_settings.is_cloud_analysis()
? CloudResultIsFailure(result)
: LocalResultIsFailure(result);
if (failed) {
Expand Down Expand Up @@ -288,12 +288,12 @@ void FilesRequestHandler::FileRequestCallback(
: upload_start_time_;

RecordDeepScanMetrics(
analysis_settings_.cloud_or_local_settings.is_cloud_analysis(),
analysis_settings_->cloud_or_local_settings.is_cloud_analysis(),
access_point_, base::TimeTicks::Now() - start_timestamp,
file_info_[index].size, upload_result, response);

RequestHandlerResult request_handler_result = CalculateRequestHandlerResult(
analysis_settings_, upload_result, response);
*analysis_settings_, upload_result, response);
results_[index] = request_handler_result;
++file_result_count_;

Expand All @@ -308,7 +308,7 @@ void FilesRequestHandler::FileRequestCallback(
file_info_[index].sha256, file_info_[index].mime_type,
AccessPointToTriggerString(access_point_), access_point_,
file_info_[index].size, upload_result, response,
CalculateEventResult(analysis_settings_, request_handler_result.complies,
CalculateEventResult(*analysis_settings_, request_handler_result.complies,
result_is_warning));

safe_browsing::DecrementCrashKey(
Expand Down
Expand Up @@ -48,11 +48,11 @@ void RequestHandlerBase::AppendFinalActionsTo(
void RequestHandlerBase::PrepareRequest(
enterprise_connectors::AnalysisConnector connector,
safe_browsing::BinaryUploadService::Request* request) {
if (analysis_settings_.cloud_or_local_settings.is_cloud_analysis()) {
if (analysis_settings_->cloud_or_local_settings.is_cloud_analysis()) {
request->set_device_token(
analysis_settings_.cloud_or_local_settings.dm_token());
analysis_settings_->cloud_or_local_settings.dm_token());
}
if (analysis_settings_.cloud_or_local_settings.is_local_analysis()) {
if (analysis_settings_->cloud_or_local_settings.is_local_analysis()) {
request->set_user_action_id(user_action_id_);
request->set_user_action_requests_count(user_action_requests_count_);
}
Expand All @@ -63,11 +63,11 @@ void RequestHandlerBase::PrepareRequest(
request->set_source(source_);
request->set_destination(destination_);
request->set_tab_url(url_);
request->set_per_profile_request(analysis_settings_.per_profile);
for (const auto& tag : analysis_settings_.tags)
request->set_per_profile_request(analysis_settings_->per_profile);
for (const auto& tag : analysis_settings_->tags)
request->add_tag(tag.first);
if (analysis_settings_.client_metadata)
request->set_client_metadata(*analysis_settings_.client_metadata);
if (analysis_settings_->client_metadata)
request->set_client_metadata(*analysis_settings_->client_metadata);
}

safe_browsing::BinaryUploadService*
Expand Down
Expand Up @@ -9,6 +9,7 @@
#include <vector>

#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/time/time.h"
#include "chrome/browser/enterprise/connectors/common.h"
#include "chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h"
Expand Down Expand Up @@ -83,7 +84,8 @@ class RequestHandlerBase {

base::raw_ptr<safe_browsing::BinaryUploadService> upload_service_ = nullptr;
base::raw_ptr<Profile> profile_ = nullptr;
const enterprise_connectors::AnalysisSettings& analysis_settings_;
const raw_ref<const enterprise_connectors::AnalysisSettings>
analysis_settings_;
GURL url_;
std::string source_;
std::string destination_;
Expand Down
Expand Up @@ -820,12 +820,12 @@ std::string BoxPartFileUploadApiCallFlow::CreateFileDigest(
net::HttpRequestHeaders BoxPartFileUploadApiCallFlow::CreateApiCallHeaders() {
net::HttpRequestHeaders headers;
headers.SetHeader("content-range", content_range_);
headers.SetHeader("digest", CreateFileDigest(part_content_));
headers.SetHeader("digest", CreateFileDigest(*part_content_));
return headers;
}
std::string BoxPartFileUploadApiCallFlow::CreateApiCallBody() {
return part_content_;
return *part_content_;
}
std::string BoxPartFileUploadApiCallFlow::CreateApiCallBodyContentType() {
Expand Down
Expand Up @@ -7,6 +7,7 @@

#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/memory/raw_ref.h"
#include "base/time/time.h"
#include "base/values.h"
#include "google_apis/gaia/oauth2_api_call_flow.h"
Expand Down Expand Up @@ -334,7 +335,7 @@ class BoxPartFileUploadApiCallFlow : public BoxChunkedUploadBaseApiCallFlow {
private:
void OnSuccessJsonParsed(ParseResult result);
TaskCallback callback_;
const std::string& part_content_;
const raw_ref<const std::string> part_content_;
const std::string content_range_;
base::WeakPtrFactory<BoxPartFileUploadApiCallFlow> weak_factory_{this};
};
Expand Down
14 changes: 8 additions & 6 deletions chrome/browser/extensions/extension_service_sync_unittest.cc
Expand Up @@ -14,6 +14,7 @@
#include "base/files/file_util.h"
#include "base/memory/ptr_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/extensions/chrome_test_extension_loader.h"
#include "chrome/browser/extensions/component_loader.h"
Expand Down Expand Up @@ -1601,7 +1602,8 @@ TEST_F(ExtensionServiceSyncCustomGalleryTest,

struct TestCase {
const char* name; // For failure output only.
const std::string& sync_version; // The version coming in from Sync.
const raw_ref<const std::string>
sync_version; // The version coming in from Sync.
// The disable reason(s) coming in from Sync, or -1 for "not set".
int sync_disable_reasons;
// The expected set of disable reasons after processing the Sync update. The
Expand All @@ -1613,16 +1615,16 @@ TEST_F(ExtensionServiceSyncCustomGalleryTest,
// Sync tells us to re-enable an older version. No permissions should be
// granted, since we can't be sure if the user actually approved the right
// set of permissions.
{"OldVersion", v1, 0,
{"OldVersion", raw_ref(v1), 0,
extensions::disable_reason::DISABLE_PERMISSIONS_INCREASE, false},
// Legacy case: Sync tells us to re-enable the extension, but doesn't
// specify disable reasons. No permissions should be granted.
{"Legacy", v2, -1,
{"Legacy", raw_ref(v2), -1,
extensions::disable_reason::DISABLE_PERMISSIONS_INCREASE, false},
// Sync tells us to re-enable the extension and explicitly removes the
// disable reasons. Now the extension should have its permissions granted.
{"GrantPermissions", v2, 0, extensions::disable_reason::DISABLE_NONE,
true},
{"GrantPermissions", raw_ref(v2), 0,
extensions::disable_reason::DISABLE_NONE, true},
};

for (const TestCase& test_case : test_cases) {
Expand Down Expand Up @@ -1666,7 +1668,7 @@ TEST_F(ExtensionServiceSyncCustomGalleryTest,
sync_pb::ExtensionSpecifics* ext_specifics = specifics.mutable_extension();
ext_specifics->set_id(id);
ext_specifics->set_enabled(true);
ext_specifics->set_version(test_case.sync_version);
ext_specifics->set_version(*test_case.sync_version);
if (test_case.sync_disable_reasons != -1)
ext_specifics->set_disable_reasons(test_case.sync_disable_reasons);

Expand Down
9 changes: 5 additions & 4 deletions chrome/browser/extensions/startup_helper.cc
Expand Up @@ -8,6 +8,7 @@
#include "base/callback_helpers.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/memory/raw_ref.h"
#include "base/run_loop.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
Expand Down Expand Up @@ -138,15 +139,15 @@ class ValidateCrxHelper : public SandboxedUnpackerClient {
DCHECK(GetExtensionFileTaskRunner()->RunsTasksInCurrentSequence());
auto unpacker = base::MakeRefCounted<SandboxedUnpacker>(
mojom::ManifestLocation::kInternal, 0, /* no special creation flags */
temp_dir_, GetExtensionFileTaskRunner().get(), this);
unpacker->StartWithCrx(crx_file_);
*temp_dir_, GetExtensionFileTaskRunner().get(), this);
unpacker->StartWithCrx(*crx_file_);
}

// The file being validated.
const CRXFileInfo& crx_file_;
const raw_ref<const CRXFileInfo> crx_file_;

// The temporary directory where the sandboxed unpacker will do work.
const base::FilePath& temp_dir_;
const raw_ref<const base::FilePath> temp_dir_;

// Closure called upon completion.
base::OnceClosure quit_closure_;
Expand Down

0 comments on commit f59bd14

Please sign in to comment.