components/autofill/core/browser/autofill_metrics_unittest.cc - Issue 2403773002: Remove stl_util's STLDeleteContainerPointers from autofill.

Unified Diff: components/autofill/core/browser/autofill_metrics_unittest.cc

Issue 2403773002: Remove stl_util's STLDeleteContainerPointers from autofill. (Closed)

Patch Set: rebase Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« components/autofill/core/browser/autofill_manager.h ('K') | « components/autofill/core/browser/autofill_merge_unittest.cc ('k') | components/autofill/core/browser/form_structure_unittest.cc » ('j') | components/autofill/core/browser/personal_data_manager_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: components/autofill/core/browser/autofill_metrics_unittest.cc

diff --git a/components/autofill/core/browser/autofill_metrics_unittest.cc b/components/autofill/core/browser/autofill_metrics_unittest.cc

index 73c9ae8fd4a4a7915996259c8ff2bd32752b5dd8..3a972f4f888e7acee5d05a68dd08cf7a34ffb4db 100644

--- a/components/autofill/core/browser/autofill_metrics_unittest.cc

+++ b/components/autofill/core/browser/autofill_metrics_unittest.cc

@@ -10,6 +10,7 @@

#include <vector>

#include "base/macros.h"

+#include "base/memory/ptr_util.h"

#include "base/run_loop.h"

#include "base/strings/string16.h"

#include "base/strings/utf_string_conversions.h"

@@ -64,40 +65,47 @@ class TestPersonalDataManager : public PersonalDataManager {

// for the side-effect of logging the profile count.

void LoadProfiles() override {

{

- std::vector<AutofillProfile*> profiles;

- web_profiles_.release(&profiles);

- WDResult<std::vector<AutofillProfile*> > result(AUTOFILL_PROFILES_RESULT,

- profiles);

+ std::vector<std::unique_ptr<AutofillProfile>> profiles;

+ web_profiles_.swap(profiles);

+ std::unique_ptr<WDTypedResult> result = base::MakeUnique<

+ WDResult<std::vector<std::unique_ptr<AutofillProfile>>>>(

+ AUTOFILL_PROFILES_RESULT, std::move(profiles));

pending_profiles_query_ = 123;

- OnWebDataServiceRequestDone(pending_profiles_query_, &result);

+ OnWebDataServiceRequestDone(pending_profiles_query_, std::move(result));

}

{

- std::vector<AutofillProfile*> profiles;

- server_profiles_.release(&profiles);

- WDResult<std::vector<AutofillProfile*> > result(AUTOFILL_PROFILES_RESULT,

- profiles);

+ std::vector<std::unique_ptr<AutofillProfile>> profiles;

+ server_profiles_.swap(profiles);

+ std::unique_ptr<WDTypedResult> result = base::MakeUnique<

+ WDResult<std::vector<std::unique_ptr<AutofillProfile>>>>(

+ AUTOFILL_PROFILES_RESULT, std::move(profiles));

pending_server_profiles_query_ = 124;

- OnWebDataServiceRequestDone(pending_server_profiles_query_, &result);

+ OnWebDataServiceRequestDone(pending_server_profiles_query_,

+ std::move(result));

}

// Overridden to avoid a trip to the database.

void LoadCreditCards() override {

{

- std::vector<CreditCard*> credit_cards;

- local_credit_cards_.release(&credit_cards);

- WDResult<std::vector<CreditCard*> > result(

- AUTOFILL_CREDITCARDS_RESULT, credit_cards);

+ std::vector<std::unique_ptr<CreditCard>> credit_cards;

+ local_credit_cards_.swap(credit_cards);

+ std::unique_ptr<WDTypedResult> result =

+ base::MakeUnique<WDResult<std::vector<std::unique_ptr<CreditCard>>>>(

+ AUTOFILL_CREDITCARDS_RESULT, std::move(credit_cards));

pending_creditcards_query_ = 125;

- OnWebDataServiceRequestDone(pending_creditcards_query_, &result);

+ OnWebDataServiceRequestDone(pending_creditcards_query_,

+ std::move(result));

}

{

- std::vector<CreditCard*> credit_cards;

- server_credit_cards_.release(&credit_cards);

- WDResult<std::vector<CreditCard*> > result(

- AUTOFILL_CREDITCARDS_RESULT, credit_cards);

+ std::vector<std::unique_ptr<CreditCard>> credit_cards;

+ server_credit_cards_.swap(credit_cards);

+ std::unique_ptr<WDTypedResult> result =

+ base::MakeUnique<WDResult<std::vector<std::unique_ptr<CreditCard>>>>(

+ AUTOFILL_CREDITCARDS_RESULT, std::move(credit_cards));

pending_server_creditcards_query_ = 126;

- OnWebDataServiceRequestDone(pending_server_creditcards_query_, &result);

+ OnWebDataServiceRequestDone(pending_server_creditcards_query_,

+ std::move(result));

}

@@ -108,10 +116,8 @@ class TestPersonalDataManager : public PersonalDataManager {

// Only need to copy all the profiles. This adds any new profiles created at

// form submission.

web_profiles_.clear();

- for (std::vector<AutofillProfile>::iterator iter = profiles->begin();

- iter != profiles->end(); ++iter) {

- web_profiles_.push_back(new AutofillProfile(*iter));

- }

+ for (auto iter = profiles->begin(); iter != profiles->end(); ++iter)

vabr (Chromium) 2016/10/14 07:24:43 optional: Since you already change the for-loop, w

Avi (use Gerrit) 2016/10/16 20:05:53 Done.

+ web_profiles_.push_back(base::MakeUnique<AutofillProfile>(*iter));

}

void set_autofill_enabled(bool autofill_enabled) {

@@ -119,60 +125,61 @@ class TestPersonalDataManager : public PersonalDataManager {

}

// Removes all existing profiles and creates 0 or 1 local profiles and 0 or 1

- // server profile according to the paramters.

+ // server profile according to the parameters.

void RecreateProfiles(bool include_local_profile,

bool include_server_profile) {

web_profiles_.clear();

server_profiles_.clear();

if (include_local_profile) {

- AutofillProfile* profile = new AutofillProfile;

- test::SetProfileInfo(profile, "Elvis", "Aaron",

- "Presley", "theking@gmail.com", "RCA",

- "3734 Elvis Presley Blvd.", "Apt. 10",

- "Memphis", "Tennessee", "38116", "US",

- "12345678901");

+ std::unique_ptr<AutofillProfile> profile =

+ base::MakeUnique<AutofillProfile>();

+ test::SetProfileInfo(profile.get(), "Elvis", "Aaron", "Presley",

+ "theking@gmail.com", "RCA",

+ "3734 Elvis Presley Blvd.", "Apt. 10", "Memphis",

+ "Tennessee", "38116", "US", "12345678901");

profile->set_guid("00000000-0000-0000-0000-000000000001");

- web_profiles_.push_back(profile);

+ web_profiles_.push_back(std::move(profile));

}

if (include_server_profile) {

- AutofillProfile* profile = new AutofillProfile(

- AutofillProfile::SERVER_PROFILE, "server_id");

- test::SetProfileInfo(profile, "Charles", "Hardin",

- "Holley", "buddy@gmail.com", "Decca",

- "123 Apple St.", "unit 6", "Lubbock",

- "Texas", "79401", "US", "2345678901");

+ std::unique_ptr<AutofillProfile> profile =

+ base::MakeUnique<AutofillProfile>(AutofillProfile::SERVER_PROFILE,

+ "server_id");

+ test::SetProfileInfo(profile.get(), "Charles", "Hardin", "Holley",

+ "buddy@gmail.com", "Decca", "123 Apple St.",

+ "unit 6", "Lubbock", "Texas", "79401", "US",

+ "2345678901");

profile->set_guid("00000000-0000-0000-0000-000000000002");

- server_profiles_.push_back(profile);

+ server_profiles_.push_back(std::move(profile));

}

Refresh();

}

// Removes all existing credit cards and creates 0 or 1 local profiles and

- // 0 or 1 server profile according to the paramters.

+ // 0 or 1 server profile according to the parameters.

void RecreateCreditCards(bool include_local_credit_card,

bool include_masked_server_credit_card,

bool include_full_server_credit_card) {

local_credit_cards_.clear();

server_credit_cards_.clear();

if (include_local_credit_card) {

- std::unique_ptr<CreditCard> credit_card(new CreditCard(

- "10000000-0000-0000-0000-000000000001", std::string()));

+ std::unique_ptr<CreditCard> credit_card = base::MakeUnique<CreditCard>(

+ "10000000-0000-0000-0000-000000000001", std::string());

test::SetCreditCardInfo(credit_card.get(), nullptr, "4111111111111111",

"12", "24");

- local_credit_cards_.push_back(credit_card.release());

+ local_credit_cards_.push_back(std::move(credit_card));

}

if (include_masked_server_credit_card) {

- std::unique_ptr<CreditCard> credit_card(new CreditCard(

- CreditCard::MASKED_SERVER_CARD, "server_id"));

+ std::unique_ptr<CreditCard> credit_card = base::MakeUnique<CreditCard>(

+ CreditCard::MASKED_SERVER_CARD, "server_id");

credit_card->set_guid("10000000-0000-0000-0000-000000000002");

credit_card->SetTypeForMaskedCard(kDiscoverCard);

- server_credit_cards_.push_back(credit_card.release());

+ server_credit_cards_.push_back(std::move(credit_card));

}

if (include_full_server_credit_card) {

- std::unique_ptr<CreditCard> credit_card(new CreditCard(

- CreditCard::FULL_SERVER_CARD, "server_id"));

+ std::unique_ptr<CreditCard> credit_card = base::MakeUnique<CreditCard>(

+ CreditCard::FULL_SERVER_CARD, "server_id");

credit_card->set_guid("10000000-0000-0000-0000-000000000003");

- server_credit_cards_.push_back(credit_card.release());

+ server_credit_cards_.push_back(std::move(credit_card));

}

Refresh();

}

@@ -180,22 +187,22 @@ class TestPersonalDataManager : public PersonalDataManager {

bool IsAutofillEnabled() const override { return autofill_enabled_; }

private:

- void CreateTestAutofillProfiles(ScopedVector<AutofillProfile>* profiles) {

- AutofillProfile* profile = new AutofillProfile;

- test::SetProfileInfo(profile, "Elvis", "Aaron",

- "Presley", "theking@gmail.com", "RCA",

- "3734 Elvis Presley Blvd.", "Apt. 10",

- "Memphis", "Tennessee", "38116", "US",

+ void CreateTestAutofillProfiles(

+ std::vector<std::unique_ptr<AutofillProfile>>* profiles) {

+ std::unique_ptr<AutofillProfile> profile =

+ base::MakeUnique<AutofillProfile>();

+ test::SetProfileInfo(profile.get(), "Elvis", "Aaron", "Presley",

+ "theking@gmail.com", "RCA", "3734 Elvis Presley Blvd.",

+ "Apt. 10", "Memphis", "Tennessee", "38116", "US",

"12345678901");

profile->set_guid("00000000-0000-0000-0000-000000000001");

- profiles->push_back(profile);

- profile = new AutofillProfile;

- test::SetProfileInfo(profile, "Charles", "Hardin",

- "Holley", "buddy@gmail.com", "Decca",

- "123 Apple St.", "unit 6", "Lubbock",

- "Texas", "79401", "US", "2345678901");

+ profiles->push_back(std::move(profile));

+ profile = base::MakeUnique<AutofillProfile>();

+ test::SetProfileInfo(profile.get(), "Charles", "Hardin", "Holley",

+ "buddy@gmail.com", "Decca", "123 Apple St.", "unit 6",

+ "Lubbock", "Texas", "79401", "US", "2345678901");

profile->set_guid("00000000-0000-0000-0000-000000000002");

- profiles->push_back(profile);

+ profiles->push_back(std::move(profile));

}

bool autofill_enabled_;

@@ -250,10 +257,10 @@ class TestAutofillManager : public AutofillManager {

empty_form.fields[i].value = base::string16();

}

- // |form_structure| will be owned by |form_structures()|.

- TestFormStructure* form_structure = new TestFormStructure(empty_form);

+ std::unique_ptr<TestFormStructure> form_structure =

+ base::MakeUnique<TestFormStructure>(empty_form);

form_structure->SetFieldTypes(heuristic_types, server_types);

- form_structures()->push_back(form_structure);

+ form_structures()->push_back(std::move(form_structure));

}

// Calls AutofillManager::OnWillSubmitForm and waits for it to complete.

@@ -737,9 +744,11 @@ TEST_F(AutofillMetricsTest, QualityMetrics_BasedOnAutocomplete) {

field.autocomplete_attribute = "";

form.fields.push_back(field);

- TestFormStructure* form_structure = new TestFormStructure(form);

+ std::unique_ptr<TestFormStructure> form_structure =

+ base::MakeUnique<TestFormStructure>(form);

+ TestFormStructure* form_structure_ptr = form_structure.get();

form_structure->DetermineHeuristicTypes();

- autofill_manager_->form_structures()->push_back(form_structure);

+ autofill_manager_->form_structures()->push_back(std::move(form_structure));

AutofillQueryResponseContents response;

// Server response will match with autocomplete.

@@ -755,7 +764,7 @@ TEST_F(AutofillMetricsTest, QualityMetrics_BasedOnAutocomplete) {

ASSERT_TRUE(response.SerializeToString(&response_string));

std::vector<std::string> signatures;

- signatures.push_back(form_structure->FormSignatureAsStr());

+ signatures.push_back(form_structure_ptr->FormSignatureAsStr());

base::HistogramTester histogram_tester;

autofill_manager_->OnLoadedServerPredictions(response_string, signatures);

@@ -768,10 +777,11 @@ TEST_F(AutofillMetricsTest, QualityMetrics_BasedOnAutocomplete) {

AutofillMetrics::QUERY_RESPONSE_PARSED, 1);

// Autocomplete-derived types are eventually what's inferred.

- EXPECT_EQ(NAME_LAST, form_structure->field(0)->Type().GetStorableType());

- EXPECT_EQ(NAME_MIDDLE, form_structure->field(1)->Type().GetStorableType());

+ EXPECT_EQ(NAME_LAST, form_structure_ptr->field(0)->Type().GetStorableType());

+ EXPECT_EQ(NAME_MIDDLE,

+ form_structure_ptr->field(1)->Type().GetStorableType());

EXPECT_EQ(ADDRESS_HOME_ZIP,

- form_structure->field(2)->Type().GetStorableType());

+ form_structure_ptr->field(2)->Type().GetStorableType());

// Heuristic predictions.

// Unknown:

@@ -3996,14 +4006,15 @@ class AutofillMetricsParseQueryResponseTest : public testing::Test {

field.name = ASCIIToUTF16("address");

form.fields.push_back(field);

- // Checkable fields should be ignored in parsing

+ // Checkable fields should be ignored in parsing.

FormFieldData checkable_field;

checkable_field.label = ASCIIToUTF16("radio_button");

checkable_field.form_control_type = "radio";

checkable_field.check_status = FormFieldData::CHECKABLE_BUT_UNCHECKED;

form.fields.push_back(checkable_field);

- forms_.push_back(new FormStructure(form));

+ owned_forms_.push_back(base::MakeUnique<FormStructure>(form));

+ forms_.push_back(owned_forms_.back().get());

field.label = ASCIIToUTF16("email");

field.name = ASCIIToUTF16("email");

@@ -4014,12 +4025,14 @@ class AutofillMetricsParseQueryResponseTest : public testing::Test {

field.form_control_type = "password";

form.fields.push_back(field);

- forms_.push_back(new FormStructure(form));

+ owned_forms_.push_back(base::MakeUnique<FormStructure>(form));

+ forms_.push_back(owned_forms_.back().get());

}

protected:

TestRapporService rappor_service_;

- ScopedVector<FormStructure> forms_;

+ std::vector<std::unique_ptr<FormStructure>> owned_forms_;

+ std::vector<FormStructure*> forms_;

};

TEST_F(AutofillMetricsParseQueryResponseTest, ServerHasData) {

@@ -4033,8 +4046,7 @@ TEST_F(AutofillMetricsParseQueryResponseTest, ServerHasData) {

ASSERT_TRUE(response.SerializeToString(&response_string));

base::HistogramTester histogram_tester;

- FormStructure::ParseQueryResponse(response_string, forms_.get(),

- &rappor_service_);

+ FormStructure::ParseQueryResponse(response_string, forms_, &rappor_service_);

EXPECT_THAT(

histogram_tester.GetAllSamples("Autofill.ServerResponseHasDataForForm"),

ElementsAre(Bucket(true, 2)));

@@ -4057,8 +4069,7 @@ TEST_F(AutofillMetricsParseQueryResponseTest, OneFormNoServerData) {

ASSERT_TRUE(response.SerializeToString(&response_string));

base::HistogramTester histogram_tester;

- FormStructure::ParseQueryResponse(response_string, forms_.get(),

- &rappor_service_);

+ FormStructure::ParseQueryResponse(response_string, forms_, &rappor_service_);

EXPECT_THAT(

histogram_tester.GetAllSamples("Autofill.ServerResponseHasDataForForm"),

ElementsAre(Bucket(false, 1), Bucket(true, 1)));

@@ -4084,8 +4095,7 @@ TEST_F(AutofillMetricsParseQueryResponseTest, AllFormsNoServerData) {

ASSERT_TRUE(response.SerializeToString(&response_string));

base::HistogramTester histogram_tester;

- FormStructure::ParseQueryResponse(response_string, forms_.get(),

- &rappor_service_);

+ FormStructure::ParseQueryResponse(response_string, forms_, &rappor_service_);

EXPECT_THAT(

histogram_tester.GetAllSamples("Autofill.ServerResponseHasDataForForm"),

ElementsAre(Bucket(false, 2)));

@@ -4114,8 +4124,7 @@ TEST_F(AutofillMetricsParseQueryResponseTest, PartialNoServerData) {

ASSERT_TRUE(response.SerializeToString(&response_string));

base::HistogramTester histogram_tester;

- FormStructure::ParseQueryResponse(response_string, forms_.get(),

- &rappor_service_);

+ FormStructure::ParseQueryResponse(response_string, forms_, &rappor_service_);

EXPECT_THAT(

histogram_tester.GetAllSamples("Autofill.ServerResponseHasDataForForm"),

ElementsAre(Bucket(true, 2)));