Change TrustedSourcesManager to return unique_ptr instead of raw pointer
Bug: 916176
Change-Id: I3418fdbceea18391bb09e957e4846accf1b28447
Reviewed-on: https://chromium-review.googlesource.com/c/1413072
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626216}diff --git a/chrome/browser/download/download_path_reservation_tracker_unittest.cc b/chrome/browser/download/download_path_reservation_tracker_unittest.cc
index 7da9207..1d3cd925 100644
--- a/chrome/browser/download/download_path_reservation_tracker_unittest.cc
+++ b/chrome/browser/download/download_path_reservation_tracker_unittest.cc
@@ -44,7 +44,7 @@
void SetUp() override;
void TearDown() override;
- MockDownloadItem* CreateDownloadItem(int32_t id);
+ std::unique_ptr<MockDownloadItem> CreateDownloadItem(int32_t id);
base::FilePath GetPathInDownloadsDirectory(
const base::FilePath::CharType* suffix);
bool IsPathInUse(const base::FilePath& path);
@@ -91,9 +91,9 @@
content::RunAllTasksUntilIdle();
}
-MockDownloadItem* DownloadPathReservationTrackerTest::CreateDownloadItem(
- int32_t id) {
- MockDownloadItem* item = new ::testing::StrictMock<MockDownloadItem>;
+std::unique_ptr<MockDownloadItem>
+DownloadPathReservationTrackerTest::CreateDownloadItem(int32_t id) {
+ auto item = std::make_unique<::testing::StrictMock<MockDownloadItem>>();
EXPECT_CALL(*item, GetId())
.WillRepeatedly(Return(id));
EXPECT_CALL(*item, GetTargetFilePath())
@@ -180,7 +180,7 @@
// A basic reservation is acquired and committed.
TEST_F(DownloadPathReservationTrackerTest, BasicReservation) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
ASSERT_FALSE(IsPathInUse(path));
@@ -205,7 +205,7 @@
// A download that is interrupted should lose its reservation.
TEST_F(DownloadPathReservationTrackerTest, InterruptedDownload) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
ASSERT_FALSE(IsPathInUse(path));
@@ -229,7 +229,7 @@
// A completed download should also lose its reservation.
TEST_F(DownloadPathReservationTrackerTest, CompleteDownload) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
ASSERT_FALSE(IsPathInUse(path));
@@ -257,7 +257,7 @@
// If there are files on the file system, a unique reservation should uniquify
// around it.
TEST_F(DownloadPathReservationTrackerTest, ConflictingFiles) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
base::FilePath path1(
@@ -295,7 +295,7 @@
// If there are conflicting files on the file system, an overwriting reservation
// should succeed without altering the target path.
TEST_F(DownloadPathReservationTrackerTest, ConflictingFiles_Overwrite) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
// Create a file at |path|.
@@ -322,7 +322,7 @@
// If the source is a file:// URL that is in the download directory, then Chrome
// could download the file onto itself. Test that this is flagged by DPRT.
TEST_F(DownloadPathReservationTrackerTest, ConflictWithSource) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
ASSERT_EQ(0, base::WriteFile(path, "", 0));
@@ -346,7 +346,7 @@
// Multiple reservations for the same path should uniquify around each other.
TEST_F(DownloadPathReservationTrackerTest, ConflictingReservations) {
- std::unique_ptr<MockDownloadItem> item1(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item1 = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
base::FilePath uniquified_path(
@@ -368,7 +368,7 @@
{
// Requesting a reservation for the same path with uniquification results in
// a uniquified path.
- std::unique_ptr<MockDownloadItem> item2(CreateDownloadItem(2));
+ std::unique_ptr<MockDownloadItem> item2 = CreateDownloadItem(2);
base::FilePath reserved_path2;
CallGetReservedPath(item2.get(), path, create_directory, conflict_action,
&reserved_path2, &result);
@@ -384,7 +384,7 @@
{
// Since the previous download item was removed, requesting a reservation
// for the same path should result in the same uniquified path.
- std::unique_ptr<MockDownloadItem> item2(CreateDownloadItem(2));
+ std::unique_ptr<MockDownloadItem> item2 = CreateDownloadItem(2);
base::FilePath reserved_path2;
CallGetReservedPath(item2.get(), path, create_directory, conflict_action,
&reserved_path2, &result);
@@ -397,7 +397,7 @@
// Now acquire an overwriting reservation. We should end up with the same
// non-uniquified path for both reservations.
- std::unique_ptr<MockDownloadItem> item3(CreateDownloadItem(2));
+ std::unique_ptr<MockDownloadItem> item3 = CreateDownloadItem(2);
base::FilePath reserved_path3;
conflict_action = DownloadPathReservationTracker::OVERWRITE;
CallGetReservedPath(item3.get(), path, create_directory, conflict_action,
@@ -415,8 +415,8 @@
// Two active downloads shouldn't be able to reserve paths that only differ by
// case.
TEST_F(DownloadPathReservationTrackerTest, ConflictingCaseReservations) {
- std::unique_ptr<MockDownloadItem> item1(CreateDownloadItem(1));
- std::unique_ptr<MockDownloadItem> item2(CreateDownloadItem(2));
+ std::unique_ptr<MockDownloadItem> item1 = CreateDownloadItem(1);
+ std::unique_ptr<MockDownloadItem> item2 = CreateDownloadItem(2);
base::FilePath path_foo =
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt"));
@@ -479,7 +479,7 @@
expected_path =
path.InsertBeforeExtensionASCII(" - 2019-01-23T163530.020");
}
- items[i].reset(CreateDownloadItem(i));
+ items[i] = CreateDownloadItem(i);
EXPECT_FALSE(IsPathInUse(expected_path));
CallGetReservedPath(items[i].get(), path, create_directory, conflict_action,
&reserved_path, &result);
@@ -488,8 +488,8 @@
EXPECT_EQ(PathValidationResult::SUCCESS, result);
}
// The next reservation for |path| will fail to be unique.
- std::unique_ptr<MockDownloadItem> item(
- CreateDownloadItem(DownloadPathReservationTracker::kMaxUniqueFiles + 2));
+ std::unique_ptr<MockDownloadItem> item =
+ CreateDownloadItem(DownloadPathReservationTracker::kMaxUniqueFiles + 2);
base::FilePath reserved_path;
PathValidationResult result = PathValidationResult::NAME_TOO_LONG;
CallGetReservedPath(item.get(), path, create_directory, conflict_action,
@@ -505,7 +505,7 @@
// If the target directory is unwriteable, then callback should be notified that
// verification failed.
TEST_F(DownloadPathReservationTrackerTest, UnwriteableDirectory) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
base::FilePath dir(path.DirName());
@@ -541,7 +541,7 @@
bool create_directory = false;
{
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath reserved_path;
PathValidationResult result = PathValidationResult::NAME_TOO_LONG;
CallGetReservedPath(item.get(), path, create_directory, conflict_action,
@@ -552,7 +552,7 @@
}
ASSERT_FALSE(IsPathInUse(path));
{
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath reserved_path;
PathValidationResult result = PathValidationResult::NAME_TOO_LONG;
set_default_download_path(dir);
@@ -568,7 +568,7 @@
// If the target path of the download item changes, the reservation should be
// updated to match.
TEST_F(DownloadPathReservationTrackerTest, UpdatesToTargetPath) {
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(
GetPathInDownloadsDirectory(FILE_PATH_LITERAL("foo.txt")));
ASSERT_FALSE(IsPathInUse(path));
@@ -626,7 +626,7 @@
const size_t max_length = real_max_length - 11;
#endif // defined(OS_WIN)
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(GetLongNamePathInDownloadsDirectory(
max_length, FILE_PATH_LITERAL(".txt")));
ASSERT_FALSE(IsPathInUse(path));
@@ -657,7 +657,7 @@
const size_t max_length = real_max_length - 11;
#endif // defined(OS_WIN)
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(GetLongNamePathInDownloadsDirectory(
max_length, FILE_PATH_LITERAL(".txt")));
base::FilePath path0(GetLongNamePathInDownloadsDirectory(
@@ -696,7 +696,7 @@
const size_t max_length = real_max_length - 11;
#endif // defined(OS_WIN)
- std::unique_ptr<MockDownloadItem> item(CreateDownloadItem(1));
+ std::unique_ptr<MockDownloadItem> item = CreateDownloadItem(1);
base::FilePath path(GetPathInDownloadsDirectory(
(FILE_PATH_LITERAL("a.") +
base::FilePath::StringType(max_length, 'b')).c_str()));
diff --git a/chrome/browser/download/download_prefs.cc b/chrome/browser/download/download_prefs.cc
index cd6f9c1..84ce429 100644
--- a/chrome/browser/download/download_prefs.cc
+++ b/chrome/browser/download/download_prefs.cc
@@ -305,7 +305,7 @@
bool DownloadPrefs::IsFromTrustedSource(const download::DownloadItem& item) {
if (!trusted_sources_manager_)
- trusted_sources_manager_.reset(TrustedSourcesManager::Create());
+ trusted_sources_manager_ = TrustedSourcesManager::Create();
return trusted_sources_manager_->IsFromTrustedSource(item.GetURL());
}
diff --git a/chrome/browser/download/trusted_sources_manager.h b/chrome/browser/download/trusted_sources_manager.h
index 62a9aa6..99c5075 100644
--- a/chrome/browser/download/trusted_sources_manager.h
+++ b/chrome/browser/download/trusted_sources_manager.h
@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_DOWNLOAD_TRUSTED_SOURCES_MANAGER_H_
#define CHROME_BROWSER_DOWNLOAD_TRUSTED_SOURCES_MANAGER_H_
+#include <memory>
+
#include "base/macros.h"
#include "net/proxy_resolution/proxy_bypass_rules.h"
@@ -29,7 +31,7 @@
// the security zone mapping is used instead to determine whether the source
// is trusted or not.
//
- static TrustedSourcesManager* Create();
+ static std::unique_ptr<TrustedSourcesManager> Create();
// Returns true if the source of this URL is part of the trusted sources.
virtual bool IsFromTrustedSource(const GURL& url) const;
diff --git a/chrome/browser/download/trusted_sources_manager_posix.cc b/chrome/browser/download/trusted_sources_manager_posix.cc
index 0c0adab..cd8f2b97 100644
--- a/chrome/browser/download/trusted_sources_manager_posix.cc
+++ b/chrome/browser/download/trusted_sources_manager_posix.cc
@@ -4,7 +4,9 @@
#include "chrome/browser/download/trusted_sources_manager.h"
+#include "base/memory/ptr_util.h"
+
// static
-TrustedSourcesManager* TrustedSourcesManager::Create() {
- return new TrustedSourcesManager;
+std::unique_ptr<TrustedSourcesManager> TrustedSourcesManager::Create() {
+ return base::WrapUnique(new TrustedSourcesManager);
}
diff --git a/chrome/browser/download/trusted_sources_manager_win.cc b/chrome/browser/download/trusted_sources_manager_win.cc
index 95254447..d88d096 100644
--- a/chrome/browser/download/trusted_sources_manager_win.cc
+++ b/chrome/browser/download/trusted_sources_manager_win.cc
@@ -8,6 +8,7 @@
#include <wrl/client.h>
#include "base/logging.h"
+#include "base/memory/ptr_util.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "url/gurl.h"
@@ -49,6 +50,6 @@
} // namespace
// static
-TrustedSourcesManager* TrustedSourcesManager::Create() {
- return new TrustedSourcesManagerWin;
+std::unique_ptr<TrustedSourcesManager> TrustedSourcesManager::Create() {
+ return base::WrapUnique(new TrustedSourcesManagerWin);
}
