Commit 2ff1dae2 authored by Jarryd's avatar Jarryd Committed by Commit Bot

ImportantSites: Remove struct copy constructor

This change removes the copy constructor for ImportantDomainInfo to
prevent accidental copies of instances. The change also gets rid
calls to said constructor.

Bug: 1034140
Change-Id: Ib4fe05701df84922333c864f8c6094c95f8a192e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1967739
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726177}
parent 3708e6de
...@@ -76,7 +76,7 @@ class ImportantSitesUsageCounterTest : public testing::Test { ...@@ -76,7 +76,7 @@ class ImportantSitesUsageCounterTest : public testing::Test {
} }
void FetchCompleted(std::vector<ImportantDomainInfo> domain_info) { void FetchCompleted(std::vector<ImportantDomainInfo> domain_info) {
domain_info_ = domain_info; domain_info_ = std::move(domain_info);
run_loop_->Quit(); run_loop_->Quit();
} }
...@@ -102,8 +102,8 @@ TEST_F(ImportantSitesUsageCounterTest, PopulateUsage) { ...@@ -102,8 +102,8 @@ TEST_F(ImportantSitesUsageCounterTest, PopulateUsage) {
i1.registerable_domain = "example.com"; i1.registerable_domain = "example.com";
ImportantDomainInfo i2; ImportantDomainInfo i2;
i2.registerable_domain = "somethingelse.com"; i2.registerable_domain = "somethingelse.com";
important_sites.push_back(i1); important_sites.push_back(std::move(i1));
important_sites.push_back(i2); important_sites.push_back(std::move(i2));
const std::vector<content::MockOriginData> origins = { const std::vector<content::MockOriginData> origins = {
{"http://example.com/", blink::mojom::StorageType::kTemporary, 1}, {"http://example.com/", blink::mojom::StorageType::kTemporary, 1},
...@@ -125,12 +125,12 @@ TEST_F(ImportantSitesUsageCounterTest, PopulateUsage) { ...@@ -125,12 +125,12 @@ TEST_F(ImportantSitesUsageCounterTest, PopulateUsage) {
->GetDOMStorageContext(); ->GetDOMStorageContext();
ImportantSitesUsageCounter::GetUsage( ImportantSitesUsageCounter::GetUsage(
important_sites, quota_manager, dom_storage_context, std::move(important_sites), quota_manager, dom_storage_context,
base::BindOnce(&ImportantSitesUsageCounterTest::FetchCompleted, base::BindOnce(&ImportantSitesUsageCounterTest::FetchCompleted,
base::Unretained(this))); base::Unretained(this)));
WaitForResult(); WaitForResult();
EXPECT_EQ(important_sites.size(), domain_info().size()); EXPECT_EQ(2U, domain_info().size());
// The first important site is example.com. It uses 1B quota storage for // The first important site is example.com. It uses 1B quota storage for
// http://example.com/, 2B for https://example.com and 4B for // http://example.com/, 2B for https://example.com and 4B for
// https://maps.example.com. On top of that it uses 16B local storage. // https://maps.example.com. On top of that it uses 16B local storage.
......
...@@ -407,7 +407,9 @@ void PopulateInfoMapWithInstalled( ...@@ -407,7 +407,9 @@ void PopulateInfoMapWithInstalled(
ImportantDomainInfo::ImportantDomainInfo() = default; ImportantDomainInfo::ImportantDomainInfo() = default;
ImportantDomainInfo::~ImportantDomainInfo() = default; ImportantDomainInfo::~ImportantDomainInfo() = default;
ImportantDomainInfo::ImportantDomainInfo(const ImportantDomainInfo&) = default; ImportantDomainInfo::ImportantDomainInfo(ImportantDomainInfo&&) = default;
ImportantDomainInfo& ImportantDomainInfo::operator=(ImportantDomainInfo&&) =
default;
std::string ImportantSitesUtil::GetRegisterableDomainOrIP(const GURL& url) { std::string ImportantSitesUtil::GetRegisterableDomainOrIP(const GURL& url) {
return GetRegisterableDomainOrIPFromHost(url.host_piece()); return GetRegisterableDomainOrIPFromHost(url.host_piece());
...@@ -461,7 +463,6 @@ ImportantSitesUtil::GetImportantRegisterableDomains(Profile* profile, ...@@ -461,7 +463,6 @@ ImportantSitesUtil::GetImportantRegisterableDomains(Profile* profile,
std::vector<std::pair<std::string, ImportantDomainInfo>> items; std::vector<std::pair<std::string, ImportantDomainInfo>> items;
for (auto& item : important_info) for (auto& item : important_info)
items.emplace_back(std::move(item)); items.emplace_back(std::move(item));
std::sort(items.begin(), items.end(), &CompareDescendingImportantInfo); std::sort(items.begin(), items.end(), &CompareDescendingImportantInfo);
std::vector<ImportantDomainInfo> final_list; std::vector<ImportantDomainInfo> final_list;
...@@ -472,7 +473,7 @@ ImportantSitesUtil::GetImportantRegisterableDomains(Profile* profile, ...@@ -472,7 +473,7 @@ ImportantSitesUtil::GetImportantRegisterableDomains(Profile* profile,
blacklisted_domains.end()) { blacklisted_domains.end()) {
continue; continue;
} }
final_list.push_back(domain_info.second); final_list.push_back(std::move(domain_info.second));
RECORD_UMA_FOR_IMPORTANT_REASON( RECORD_UMA_FOR_IMPORTANT_REASON(
"Storage.ImportantSites.GeneratedReason", "Storage.ImportantSites.GeneratedReason",
"Storage.ImportantSites.GeneratedReasonCount", "Storage.ImportantSites.GeneratedReasonCount",
...@@ -505,7 +506,7 @@ ImportantSitesUtil::GetInstalledRegisterableDomains( ...@@ -505,7 +506,7 @@ ImportantSitesUtil::GetInstalledRegisterableDomains(
break; break;
if (excluded_domains.find(domain_info.first) != excluded_domains.end()) if (excluded_domains.find(domain_info.first) != excluded_domains.end())
continue; continue;
installed_domains.push_back(domain_info.second); installed_domains.push_back(std::move(domain_info.second));
} }
return installed_domains; return installed_domains;
} }
......
...@@ -33,7 +33,10 @@ class ImportantSitesUtil { ...@@ -33,7 +33,10 @@ class ImportantSitesUtil {
struct ImportantDomainInfo { struct ImportantDomainInfo {
ImportantDomainInfo(); ImportantDomainInfo();
~ImportantDomainInfo(); ~ImportantDomainInfo();
ImportantDomainInfo(const ImportantDomainInfo&); ImportantDomainInfo(ImportantDomainInfo&&);
ImportantDomainInfo(const ImportantDomainInfo&) = delete;
ImportantDomainInfo& operator=(ImportantDomainInfo&&);
ImportantDomainInfo& operator=(const ImportantDomainInfo&) = delete;
std::string registerable_domain; std::string registerable_domain;
GURL example_origin; GURL example_origin;
double engagement_score = 0; double engagement_score = 0;
......
...@@ -181,7 +181,8 @@ void ClearBrowsingDataHandler::GetRecentlyLaunchedInstalledApps( ...@@ -181,7 +181,8 @@ void ClearBrowsingDataHandler::GetRecentlyLaunchedInstalledApps(
void ClearBrowsingDataHandler::OnGotInstalledApps( void ClearBrowsingDataHandler::OnGotInstalledApps(
const std::string& webui_callback_id, const std::string& webui_callback_id,
std::vector<ImportantSitesUtil::ImportantDomainInfo> installed_apps) { const std::vector<ImportantSitesUtil::ImportantDomainInfo>&
installed_apps) {
base::ListValue installed_apps_list; base::ListValue installed_apps_list;
for (const auto& info : installed_apps) { for (const auto& info : installed_apps) {
auto entry = std::make_unique<base::DictionaryValue>(); auto entry = std::make_unique<base::DictionaryValue>();
......
...@@ -55,7 +55,8 @@ class ClearBrowsingDataHandler : public SettingsPageUIHandler, ...@@ -55,7 +55,8 @@ class ClearBrowsingDataHandler : public SettingsPageUIHandler,
// Respond to the WebUI callback with the list of installed apps. // Respond to the WebUI callback with the list of installed apps.
void OnGotInstalledApps( void OnGotInstalledApps(
const std::string& webui_callback_id, const std::string& webui_callback_id,
std::vector<ImportantSitesUtil::ImportantDomainInfo> installed_apps); const std::vector<ImportantSitesUtil::ImportantDomainInfo>&
installed_apps);
// Build a filter of sites to include and exclude from site data removal // Build a filter of sites to include and exclude from site data removal
// based on whether installed apps were marked for deletion by the checkbox on // based on whether installed apps were marked for deletion by the checkbox on
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment