Skip to content

Commit

Permalink
File System: Stop using operator new for ObfuscatedFileUtil.
Browse files Browse the repository at this point in the history
This clarifies the fact that each ObfuscatedFileUtil instance has a
clear unique owner.

Change-Id: Iee7580b6a7fed10a2a01ed816babc7c7ee8b47ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2928909
Commit-Queue: Austin Sullivan <asully@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/master@{#888455}
  • Loading branch information
pwnall authored and Chromium LUCI CQ committed Jun 2, 2021
1 parent 9787b39 commit 8bfb964
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 20 deletions.
4 changes: 2 additions & 2 deletions storage/browser/file_system/obfuscated_file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) ObfuscatedFileUtil
// Helper method to create an obfuscated file util for regular
// (temporary, persistent) file systems. Used only for testing.
// Note: this is implemented in sandbox_file_system_backend_delegate.cc.
static ObfuscatedFileUtil* CreateForTesting(
SpecialStoragePolicy* special_storage_policy,
static std::unique_ptr<ObfuscatedFileUtil> CreateForTesting(
scoped_refptr<SpecialStoragePolicy> special_storage_policy,
const base::FilePath& file_system_directory,
leveldb::Env* env_override,
bool is_incognito);
Expand Down
19 changes: 9 additions & 10 deletions storage/browser/file_system/obfuscated_file_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,11 @@ class ObfuscatedFileUtilTest : public testing::Test,
}

std::unique_ptr<ObfuscatedFileUtil> CreateObfuscatedFileUtil(
SpecialStoragePolicy* storage_policy) {
return std::unique_ptr<ObfuscatedFileUtil>(
ObfuscatedFileUtil::CreateForTesting(
storage_policy, data_dir_path(),
is_incognito() ? incognito_leveldb_environment_.get() : nullptr,
is_incognito()));
scoped_refptr<SpecialStoragePolicy> storage_policy) {
return ObfuscatedFileUtil::CreateForTesting(
std::move(storage_policy), data_dir_path(),
is_incognito() ? incognito_leveldb_environment_.get() : nullptr,
is_incognito());
}

ObfuscatedFileUtil* ofu() {
Expand Down Expand Up @@ -698,7 +697,7 @@ class ObfuscatedFileUtilTest : public testing::Test,

void MaybeDropDatabasesAliveCaseTestBody() {
std::unique_ptr<ObfuscatedFileUtil> file_util =
CreateObfuscatedFileUtil(nullptr);
CreateObfuscatedFileUtil(/*storage_policy=*/nullptr);
file_util->InitOriginDatabase(Origin(), true /*create*/);
ASSERT_TRUE(file_util->origin_database_ != nullptr);

Expand All @@ -716,7 +715,7 @@ class ObfuscatedFileUtilTest : public testing::Test,
// doesn't cause a crash for use after free.
{
std::unique_ptr<ObfuscatedFileUtil> file_util =
CreateObfuscatedFileUtil(nullptr);
CreateObfuscatedFileUtil(/*storage_policy=*/nullptr);
file_util->InitOriginDatabase(Origin(), true /*create*/);
file_util->db_flush_delay_seconds_ = 0;
file_util->MarkUsed();
Expand All @@ -729,7 +728,7 @@ class ObfuscatedFileUtilTest : public testing::Test,
void DestroyDirectoryDatabase_IsolatedTestBody() {
storage_policy_->AddIsolated(origin_.GetURL());
std::unique_ptr<ObfuscatedFileUtil> file_util =
CreateObfuscatedFileUtil(storage_policy_.get());
CreateObfuscatedFileUtil(/*storage_policy=*/storage_policy_);
const FileSystemURL url = FileSystemURL::CreateForTest(
origin_, kFileSystemTypePersistent, base::FilePath());

Expand All @@ -747,7 +746,7 @@ class ObfuscatedFileUtilTest : public testing::Test,
void GetDirectoryDatabase_IsolatedTestBody() {
storage_policy_->AddIsolated(origin_.GetURL());
std::unique_ptr<ObfuscatedFileUtil> file_util =
CreateObfuscatedFileUtil(storage_policy_.get());
CreateObfuscatedFileUtil(storage_policy_);
const FileSystemURL url = FileSystemURL::CreateForTest(
origin_, kFileSystemTypePersistent, base::FilePath());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ class QuotaBackendImplTest : public testing::Test,
void SetUp() override {
ASSERT_TRUE(data_dir_.CreateUniqueTempDir());
in_memory_env_ = leveldb_chrome::NewMemEnv("quota");
file_util_.reset(ObfuscatedFileUtil::CreateForTesting(
nullptr, data_dir_.GetPath(), in_memory_env_.get(), is_incognito()));
file_util_ = ObfuscatedFileUtil::CreateForTesting(
/*special_storage_policy=*/nullptr, data_dir_.GetPath(),
in_memory_env_.get(), is_incognito());
backend_ = std::make_unique<QuotaBackendImpl>(
file_task_runner(), file_util_.get(), &file_system_usage_cache_,
quota_manager_proxy_.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -705,15 +705,15 @@ SandboxFileSystemBackendDelegate::memory_file_util_delegate() {

// Declared in obfuscated_file_util.h.
// static
ObfuscatedFileUtil* ObfuscatedFileUtil::CreateForTesting(
SpecialStoragePolicy* special_storage_policy,
std::unique_ptr<ObfuscatedFileUtil> ObfuscatedFileUtil::CreateForTesting(
scoped_refptr<SpecialStoragePolicy> special_storage_policy,
const base::FilePath& file_system_directory,
leveldb::Env* env_override,
bool is_incognito) {
return new ObfuscatedFileUtil(special_storage_policy, file_system_directory,
env_override,
base::BindRepeating(&GetTypeStringForURL),
GetKnownTypeStrings(), nullptr, is_incognito);
return std::make_unique<ObfuscatedFileUtil>(
std::move(special_storage_policy), file_system_directory, env_override,
base::BindRepeating(&GetTypeStringForURL), GetKnownTypeStrings(),
/*sandbox_delegate=*/nullptr, is_incognito);
}

} // namespace storage

0 comments on commit 8bfb964

Please sign in to comment.