Use GURL instead of a plain string for the Wallet management URL The goal of this refactoring is to slightly improve code clarity by using a more specific type and to ensure consistency with the handling of source URLs, which have been updated to use GURL in crrev.com/c/8420529 Bug: b:560061580 Change-Id: I54dc7718e2a203fce942734efbf8df5a6a6a6964 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8427610 Commit-Queue: Thomas Schickinger <schickin@google.com> Reviewed-by: Norge Vizcay <vizcay@google.com> Reviewed-by: Vidhan Jain <vidhanj@google.com> Cr-Commit-Position: refs/heads/main@{#1706771}
diff --git a/chrome/browser/autofill/android/entity_instance_android.cc b/chrome/browser/autofill/android/entity_instance_android.cc index 00ed3f6..1669f64 100644 --- a/chrome/browser/autofill/android/entity_instance_android.cc +++ b/chrome/browser/autofill/android/entity_instance_android.cc
@@ -152,7 +152,8 @@ case EntityInstance::RecordType::kServerWallet: // If there was no existing entity, the newly created entity cannot yet // have a management URL, which is provided by the Wallet server. - return EntityInstance::WalletRecordTypePayload{.management_url = ""}; + return EntityInstance::WalletRecordTypePayload{.management_url = + GURL()}; case EntityInstance::RecordType::kPersonalContext: // pContext entities cannot be created from settings. NOTREACHED();
diff --git a/chrome/browser/autofill/android/entity_instance_android_unittest.cc b/chrome/browser/autofill/android/entity_instance_android_unittest.cc index 2501d5aa..df34b28 100644 --- a/chrome/browser/autofill/android/entity_instance_android_unittest.cc +++ b/chrome/browser/autofill/android/entity_instance_android_unittest.cc
@@ -167,7 +167,7 @@ EntityInstance passport = test::GetPassportEntityInstance( {.record_type = EntityInstance::WalletRecordTypePayload{ .management_url = - "https://wallet.google.com/synthetic_pass?id=fake123"}}); + GURL("https://wallet.google.com/synthetic_pass?id=fake123")}}); EntityInstanceAndroid entity_instance_android( passport, /*is_enabled=*/true, /*is_eligible_for_wallet_storage=*/true, /*requires_reauth_to_see=*/true); @@ -202,9 +202,10 @@ EXPECT_EQ(entity_instance.record_type(), EntityInstance::RecordType::kServerWallet); - EXPECT_EQ(entity_instance.record_type_data(), - EntityInstance::RecordTypeData( - EntityInstance::WalletRecordTypePayload{.management_url = ""})); + EXPECT_EQ( + entity_instance.record_type_data(), + EntityInstance::RecordTypeData( + EntityInstance::WalletRecordTypePayload{.management_url = GURL()})); } // Test that the Wallet record type payload is added when converting an @@ -214,7 +215,7 @@ EntityInstance existing_entity = test::GetPassportEntityInstance( {.record_type = EntityInstance::WalletRecordTypePayload{ .management_url = - "https://wallet.google.com/synthetic_pass?id=fake123"}}); + GURL("https://wallet.google.com/synthetic_pass?id=fake123")}}); EntityInstance entity_without_payload = test::GetPassportEntityInstance(); EntityInstanceAndroid entity_instance_android(
diff --git a/chrome/browser/extensions/api/autofill_private/autofill_ai_util.cc b/chrome/browser/extensions/api/autofill_private/autofill_ai_util.cc index 58b6b01e..7908ba4e5 100644 --- a/chrome/browser/extensions/api/autofill_private/autofill_ai_util.cc +++ b/chrome/browser/extensions/api/autofill_private/autofill_ai_util.cc
@@ -215,7 +215,8 @@ // Wallet passes can only be created (not updated) from settings, at // which point the server-provided management URL is not yet known. ? EntityInstance::RecordTypeData( - EntityInstance::WalletRecordTypePayload{.management_url = ""}) + EntityInstance::WalletRecordTypePayload{.management_url = + GURL()}) : EntityInstance::RecordTypeData( EntityInstance::LocalRecordTypePayload{});
diff --git a/components/autofill/core/browser/data_model/autofill_ai/entity_instance.cc b/components/autofill/core/browser/data_model/autofill_ai/entity_instance.cc index 84bf090b..765e7fd 100644 --- a/components/autofill/core/browser/data_model/autofill_ai/entity_instance.cc +++ b/components/autofill/core/browser/data_model/autofill_ai/entity_instance.cc
@@ -509,7 +509,7 @@ } }, [&os](const EntityInstance::WalletRecordTypePayload& p) { - if (!p.management_url.empty()) { + if (!p.management_url.is_empty()) { os << "- management url: \"" << p.management_url << '"' << std::endl; } @@ -789,7 +789,7 @@ // the management URL is empty because it needs to be provisioned // by the Google Wallet servers. new_entity.record_type_data_ = - WalletRecordTypePayload{.management_url = ""}; + WalletRecordTypePayload{.management_url = GURL()}; break; case RecordType::kPersonalContext: // TODO(crbug.com/542083924): Converting to a pContext entity is currently
diff --git a/components/autofill/core/browser/data_model/autofill_ai/entity_instance.h b/components/autofill/core/browser/data_model/autofill_ai/entity_instance.h index d05bc30..9a09cca 100644 --- a/components/autofill/core/browser/data_model/autofill_ai/entity_instance.h +++ b/components/autofill/core/browser/data_model/autofill_ai/entity_instance.h
@@ -312,7 +312,7 @@ // Canonical management URL for this entity on wallet.google.com. // Empty if not provided by the server or before sync propagation. // TODO(crbug.com/560061580): This field is not yet populated. - std::string management_url = internal::IsRequired(); + GURL management_url = internal::IsRequired(); friend bool operator==(const WalletRecordTypePayload&, const WalletRecordTypePayload&) = default;
diff --git a/components/autofill/core/browser/data_model/autofill_ai/entity_instance_unittest.cc b/components/autofill/core/browser/data_model/autofill_ai/entity_instance_unittest.cc index b3edd2919..5dc3235 100644 --- a/components/autofill/core/browser/data_model/autofill_ai/entity_instance_unittest.cc +++ b/components/autofill/core/browser/data_model/autofill_ai/entity_instance_unittest.cc
@@ -167,13 +167,12 @@ std::get_if<EntityInstance::WalletRecordTypePayload>( &converted_wallet_entity.record_type_data()); ASSERT_NE(converted_payload, nullptr); - EXPECT_EQ(converted_payload->management_url, ""); + EXPECT_EQ(converted_payload->management_url, GURL()); } TEST_F(AutofillEntityInstanceTest, CopyWithNewRecordTypeSameTypeCopiesThePayload) { - constexpr char kWalletUrl[] = - "https://wallet.google.com/synthetic_pass?id=fake123"; + const GURL kWalletUrl("https://wallet.google.com/synthetic_pass?id=fake123"); EntityInstance wallet_entity = test::GetPassportEntityInstance( {.record_type = EntityInstance::WalletRecordTypePayload{.management_url = kWalletUrl}});
diff --git a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_import_util.cc b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_import_util.cc index 3dda4b3..9758854 100644 --- a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_import_util.cc +++ b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_import_util.cc
@@ -237,7 +237,7 @@ // exist on Google Wallet servers yet. Their management URL // is empty until the pass is created by the Wallet service. EntityInstance::WalletRecordTypePayload{.management_url = - ""}) + GURL()}) : EntityInstance::RecordTypeData( EntityInstance::LocalRecordTypePayload{}); EntityInstance entity = EntityInstance(
diff --git a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_manager.cc b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_manager.cc index 0cae4190..a9f931a 100644 --- a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_manager.cc +++ b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_manager.cc
@@ -151,7 +151,7 @@ &saved_entity.record_type_data()); return EntityInstance::WalletRecordTypePayload{ .management_url = - saved_payload ? saved_payload->management_url : ""}; + saved_payload ? saved_payload->management_url : GURL()}; } case EntityInstance::RecordType::kPersonalContext: // pContext entities are read-only.
diff --git a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.cc b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.cc index 918145d8..bf9ab4c4 100644 --- a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.cc +++ b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.cc
@@ -159,8 +159,7 @@ NOTREACHED(); } -bool IsValidWalletManagementUrl(std::string_view url_string) { - GURL url(url_string); +bool IsValidWalletManagementUrl(const GURL& url) { return url.SchemeIs(url::kHttpsScheme) && google_util::IsGoogleDomainUrl( url, google_util::ALLOW_SUBDOMAIN,
diff --git a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.h b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.h index 817c2ca..1b746d0 100644 --- a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.h +++ b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util.h
@@ -41,9 +41,9 @@ // Returns the URL of a Wallet `entity`'s management page on wallet.google.com. std::string GetWalletManagementURL(const EntityInstance& entity); -// Returns true if `url_string` is a valid, secure Google Wallet management URL +// Returns true if `url` is a valid, secure Google Wallet management URL // (i.e. valid GURL, HTTPS scheme, Google domain, and standard port). -bool IsValidWalletManagementUrl(std::string_view url_string); +bool IsValidWalletManagementUrl(const GURL& url); // Logs a `sync_pb::UserConsentTypes::WalletPrivatePassConsent` with // `accepted_consent_string_id` and `accept_button_string_id` as its consent
diff --git a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util_unittest.cc b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util_unittest.cc index fe90f92..979621bc 100644 --- a/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util_unittest.cc +++ b/components/autofill/core/browser/integrators/autofill_ai/autofill_ai_wallet_util_unittest.cc
@@ -343,7 +343,8 @@ TEST_P(IsValidWalletManagementUrlTest, Validation) { const IsValidWalletManagementUrlTestCase& test_case = GetParam(); - EXPECT_EQ(IsValidWalletManagementUrl(test_case.url), test_case.is_valid); + EXPECT_EQ(IsValidWalletManagementUrl(GURL(test_case.url)), + test_case.is_valid); } INSTANTIATE_TEST_SUITE_P(
diff --git a/components/autofill/core/browser/manual_testing_import.cc b/components/autofill/core/browser/manual_testing_import.cc index 0d4d7e4b..da90cc41 100644 --- a/components/autofill/core/browser/manual_testing_import.cc +++ b/components/autofill/core/browser/manual_testing_import.cc
@@ -319,7 +319,7 @@ case EntityInstance::RecordType::kServerWallet: { const std::string* management_url = dict.FindString(kKeyManagementUrl); return EntityInstance::WalletRecordTypePayload{ - .management_url = management_url ? *management_url : ""}; + .management_url = management_url ? GURL(*management_url) : GURL()}; } case EntityInstance::RecordType::kPersonalContext: return EntityInstance::PersonalContextRecordTypePayload{
diff --git a/components/autofill/core/browser/manual_testing_import_unittest.cc b/components/autofill/core/browser/manual_testing_import_unittest.cc index 41e4396..c1d280c 100644 --- a/components/autofill/core/browser/manual_testing_import_unittest.cc +++ b/components/autofill/core/browser/manual_testing_import_unittest.cc
@@ -474,7 +474,7 @@ const auto* payload0 = std::get_if<WalletRecordTypePayload>(&entities->at(0).record_type_data()); ASSERT_TRUE(payload0); - EXPECT_EQ(*payload0, WalletRecordTypePayload{.management_url = ""}); + EXPECT_EQ(*payload0, WalletRecordTypePayload{.management_url = GURL()}); ASSERT_EQ(entities->at(1).record_type(), EntityInstance::RecordType::kServerWallet); @@ -483,8 +483,8 @@ ASSERT_TRUE(payload1); EXPECT_EQ(*payload1, (WalletRecordTypePayload{ - .management_url = - "https://wallet.google.com/synthetic_pass?id=fake123"})); + .management_url = GURL( + "https://wallet.google.com/synthetic_pass?id=fake123")})); } // Tests that invalid entity record_type fails import. @@ -551,7 +551,7 @@ EntityInstance::EntityId(base::Uuid::GenerateRandomV4()), /*nickname=*/"", base::Time::Now(), /*use_count=*/0, /*use_date=*/base::Time(), - EntityInstance::WalletRecordTypePayload{.management_url = ""}, + EntityInstance::WalletRecordTypePayload{.management_url = GURL()}, EntityInstance::AreAttributesReadOnly(false), /*frecency_override=*/""); @@ -571,7 +571,7 @@ EntityInstance::EntityId(base::Uuid::GenerateRandomV4()), /*nickname=*/"", base::Time::Now(), /*use_count=*/0, /*use_date=*/base::Time(), - EntityInstance::WalletRecordTypePayload{.management_url = ""}, + EntityInstance::WalletRecordTypePayload{.management_url = GURL()}, EntityInstance::AreAttributesReadOnly(false), /*frecency_override=*/"");
diff --git a/components/autofill/core/browser/test_utils/entity_data_test_util.cc b/components/autofill/core/browser/test_utils/entity_data_test_util.cc index b0a479d30..1d1a062 100644 --- a/components/autofill/core/browser/test_utils/entity_data_test_util.cc +++ b/components/autofill/core/browser/test_utils/entity_data_test_util.cc
@@ -486,7 +486,7 @@ return EntityInstance::LocalRecordTypePayload{}; case EntityInstance::RecordType::kServerWallet: return EntityInstance::WalletRecordTypePayload{.management_url = - ""}; + GURL()}; case EntityInstance::RecordType::kPersonalContext: return EntityInstance::PersonalContextRecordTypePayload{ .sources = {}};
diff --git a/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util.cc b/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util.cc index 2dfdc493..4ff5190 100644 --- a/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util.cc +++ b/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util.cc
@@ -807,7 +807,7 @@ /*use_date=*/{}, // TODO(crbug.com/560061580): Add logic to import the management URL // from the synced specifics. - EntityInstance::WalletRecordTypePayload{.management_url = ""}, + EntityInstance::WalletRecordTypePayload{.management_url = GURL()}, EntityInstance::AreAttributesReadOnly(!specifics.is_editable()), std::move(frecency_overwrite)); };
diff --git a/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util_unittest.cc b/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util_unittest.cc index 78590b00..2f4c568 100644 --- a/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util_unittest.cc +++ b/components/autofill/core/browser/webdata/autofill_ai/entity_sync_util_unittest.cc
@@ -417,7 +417,7 @@ EntityType(EntityTypeName::kVehicle), std::move(attributes), EntityInstance::EntityId("00000000-0000-4000-8000-200000000000"), /*nickname=*/"", /*date_modified=*/{}, /*use_count=*/0, /*use_date=*/{}, - EntityInstance::WalletRecordTypePayload{.management_url = ""}, + EntityInstance::WalletRecordTypePayload{.management_url = GURL()}, EntityInstance::AreAttributesReadOnly(false), /*frecency_override=*/"");
diff --git a/components/autofill/core/browser/webdata/autofill_ai/entity_table.cc b/components/autofill/core/browser/webdata/autofill_ai/entity_table.cc index 64c5ceb..148908d8 100644 --- a/components/autofill/core/browser/webdata/autofill_ai/entity_table.cc +++ b/components/autofill/core/browser/webdata/autofill_ai/entity_table.cc
@@ -344,7 +344,9 @@ SQL_FROM_HERE, *db(), s, wallet_metadata::kTableName, {wallet_metadata::kEntityGuid, wallet_metadata::kManagementUrl}); s.BindString(0, *guid); - s.BindString(1, payload.management_url); + // Note that this will store an empty URL and create a non-fatal dump + // if the payload is corrupted for some reason. + s.BindString(1, payload.management_url.spec()); return s.Run(); } @@ -585,11 +587,10 @@ while (s.Step()) { EntityInstance::EntityId entity_guid(s.ColumnString(0)); - std::string management_url = s.ColumnString(1); wallet_metadata_records.emplace( std::move(entity_guid), - EntityInstance::WalletRecordTypePayload{.management_url = - std::move(management_url)}); + EntityInstance::WalletRecordTypePayload{ + .management_url = GURL(s.ColumnStringView(1))}); } if (!s.Succeeded()) { return {}; @@ -809,8 +810,8 @@ return EntityInstance::LocalRecordTypePayload{}; case EntityInstance::RecordType::kServerWallet: return std::move(wallet_payload) - .value_or( - EntityInstance::WalletRecordTypePayload{.management_url = ""}); + .value_or(EntityInstance::WalletRecordTypePayload{.management_url = + GURL()}); case EntityInstance::RecordType::kPersonalContext: // pContext entities are not stored in `EntityTable`. NOTREACHED();
diff --git a/components/autofill/core/browser/webdata/autofill_ai/entity_table_unittest.cc b/components/autofill/core/browser/webdata/autofill_ai/entity_table_unittest.cc index ccdc2c8..2860796 100644 --- a/components/autofill/core/browser/webdata/autofill_ai/entity_table_unittest.cc +++ b/components/autofill/core/browser/webdata/autofill_ai/entity_table_unittest.cc
@@ -169,7 +169,8 @@ .guid = "00000000-0000-4000-8000-123000000000", .record_type = EntityInstance::WalletRecordTypePayload{ - .management_url = "https://wallet.google.com/wallet/passes/123"}, + .management_url = + GURL("https://wallet.google.com/wallet/passes/123")}, }); EntityInstance local_vr = GetVehicleEntityInstance({ .guid = "00000000-0000-4000-8000-456000000000", @@ -531,7 +532,8 @@ .guid = "00000000-0000-4000-8000-123000000000", .record_type = EntityInstance::WalletRecordTypePayload{ - .management_url = "https://wallet.google.com/wallet/passes/123"}, + .management_url = + GURL("https://wallet.google.com/wallet/passes/123")}, }); ASSERT_TRUE(table().AddOrUpdateEntityInstance(wallet_vr)); EXPECT_THAT(table().GetEntityInstances(), ElementsAre(wallet_vr)); @@ -541,7 +543,8 @@ .guid = "00000000-0000-4000-8000-123000000000", .record_type = EntityInstance::WalletRecordTypePayload{ - .management_url = "https://wallet.google.com/wallet/passes/456"}, + .management_url = + GURL("https://wallet.google.com/wallet/passes/456")}, }); ASSERT_TRUE(table().AddOrUpdateEntityInstance(updated_wallet_vr)); EXPECT_THAT(table().GetEntityInstances(), ElementsAre(updated_wallet_vr)); @@ -568,7 +571,8 @@ .guid = "00000000-0000-4000-8000-123000000000", .record_type = EntityInstance::WalletRecordTypePayload{ - .management_url = "https://wallet.google.com/wallet/passes/123"}, + .management_url = + GURL("https://wallet.google.com/wallet/passes/123")}, }); ASSERT_TRUE(table().AddOrUpdateEntityInstance(wallet_vr)); @@ -580,7 +584,7 @@ EntityInstance expected_fallback_vr = GetVehicleEntityInstance({ .guid = "00000000-0000-4000-8000-123000000000", .record_type = - EntityInstance::WalletRecordTypePayload{.management_url = ""}, + EntityInstance::WalletRecordTypePayload{.management_url = GURL()}, }); EXPECT_THAT(table().GetEntityInstances(), ElementsAre(expected_fallback_vr)); }
diff --git a/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator.mm b/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator.mm index a94a9acd..65439f1 100644 --- a/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator.mm +++ b/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator.mm
@@ -52,7 +52,7 @@ // Empty entities that are created in the settings cannot yet have a // management_url, which is later provided by the Wallet server. return autofill::EntityInstance::WalletRecordTypePayload{ - .management_url = ""}; + .management_url = GURL()}; case autofill::EntityInstance::RecordType::kPersonalContext: // pContext entities cannot be created from settings. NOTREACHED();
diff --git a/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator_unittest.mm b/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator_unittest.mm index e334c6c..cd9dfef 100644 --- a/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator_unittest.mm +++ b/ios/chrome/browser/settings/autofill/autofill_ai/coordinator/autofill_ai_entity_edit_coordinator_unittest.mm
@@ -176,8 +176,8 @@ // Create an explicit Server Wallet entity instance. autofill::EntityType type(autofill::EntityTypeName::kVehicle); autofill::EntityInstanceBuilder builder(type); - builder.SetRecordTypeData( - autofill::EntityInstance::WalletRecordTypePayload{.management_url = ""}); + builder.SetRecordTypeData(autofill::EntityInstance::WalletRecordTypePayload{ + .management_url = GURL()}); for (autofill::AttributeType attr_type : type.attributes()) { builder.AddAttribute(autofill::AttributeInstance(attr_type));