[Autofill] Don't set PHONE_HOME_CITY_AND_NUMBER directly. [chromium/src : main]
findit-for-me@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Norge Vizcay.
This change meets the code coverage requirements.
Patch set 1:Code-Coverage +1
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Matthias Körber (Gerrit)
Attention is currently required from: Florian Leimgruber, Norge Vizcay, Timofey Chudakov.
2 comments:
File components/autofill/core/browser/data_model/phone_number.cc:
Patch Set #1, Line 95:
SetRawInfoWithVerificationStatusShould we add a test to cover the difference?
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}I wonder if we should add a CHECK_NO_CRASH here just to make sure we are not using other types in any import logic.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Norge Vizcay.
2 comments:
File components/autofill/core/browser/data_model/phone_number.cc:
Patch Set #1, Line 95:
SetRawInfoWithVerificationStatusShould we add a test to cover the difference?
Done
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
I wonder if we should add a CHECK_NO_CRASH here just to make sure we are not using other types in an […]
I don't quite understand what exactly can potentially crash?
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
findit-for-me@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Norge Vizcay, Timofey Chudakov.
This change meets the code coverage requirements.
Patch set 2:Code-Coverage +1
Florian Leimgruber (Gerrit)
Attention is currently required from: Matthias Körber, Norge Vizcay, Timofey Chudakov.
Patch set 2:Code-Review +1
3 comments:
Patchset:
I haven't check all callpaths, but I think this is safe. See my [other comment](https://chromium-review.googlesource.com/c/chromium/src/+/4773760/comment/f1df4ec9_bc40cc48/) for details.
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
I don't quite understand what exactly can potentially crash?
I think Matthias wants to make sure that in case some logic relies on `SetRawInfo(PHONE_HOME_CITY_AND_NUMBER)`, we notice.
The importing logic in particular [doesn't rely](https://source.chromium.org/chromium/chromium/src/+/main:components/autofill/core/browser/form_data_importer.cc;l=254;drc=554d54a93697d889655a178d6f122d01b4de7280) on SetRawInfo(), but SetInfo(). And it only sets the PHONE_HOME_WHOLE_NUMBER, since it manually concatenates the different phone number parts.
The only three instances I can think of that actually use SetRawInfo() are:
- Settings
- AutofillTable
- Sync proto conversion
And in all three cases, we only reference the PHONE_HOME_WHOLE_NUMBER (the stored type).
Finally: CHECK_NO_CRASH() doesn't seem to exist?
File components/autofill/core/browser/data_model/phone_number.cc:
As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
I think this part of the comment can be removed now (I added it to explain why PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX is not part of the if, even though PHONE_HOME_CITY_AND_NUMBER is).
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Norge Vizcay (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Timofey Chudakov.
2 comments:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
I think Matthias wants to make sure that in case some logic relies on `SetRawInfo(PHONE_HOME_CITY_AN […]
Florian, one option might be missing. From what I see on the import test tool example `chrome/test/data/autofill/profiles/profiles.json` we suggest `PHONE_HOME_CITY_AND_NUMBER` can be used, and the tool uses `SetRawInfoWithVerificationStatus` directly. We probably need to update the example.
That said, +1 to Matthias' suggestion. We could add something like
`CHECK(type != PHONE_HOME_CITY_AND_NUMBER) << "Invalid set operation for phone number"` to catch such cases.
File components/autofill/core/browser/data_model/phone_number_unittest.cc:
PHONE_HOME_NUMBER, PHONE_HOME_CITY_CODE, PHONE_HOME_COUNTRY_CODE,
PHONE_HOME_CITY_AND_NUMBER, PHONE_HOME_EXTENSION,
PHONE_HOME_CITY_CODE_WITH_TRUNK_PREFIX,
PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX,
PHONE_HOME_NUMBER_PREFIX, PHONE_HOME_NUMBER_SUFFIXInstead of listing them directly, how about listing them through `PhoneNumber::GetSupportedTypes` and skipping the one that are not interesting to you.
It would avoid potential errors when new types are introduced.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Florian Leimgruber (Gerrit)
Attention is currently required from: Matthias Körber, Timofey Chudakov.
Patch set 2:-Code-Review
The change is no longer submittable: Code-Owners and Code-Review are unsatisfied now.
3 comments:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
Florian, one option might be missing
Good point, I missed that. Thanks for noticing! Yes, that should be updated.
CHECK(type != PHONE_HOME_CITY_AND_NUMBER)
The problem with CHECK() is that is will crash the browser - we should avoid that.
File components/autofill/core/browser/data_model/phone_number_unittest.cc:
PHONE_HOME_NUMBER, PHONE_HOME_CITY_CODE, PHONE_HOME_COUNTRY_CODE,
PHONE_HOME_CITY_AND_NUMBER, PHONE_HOME_EXTENSION,
PHONE_HOME_CITY_CODE_WITH_TRUNK_PREFIX,
PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX,
PHONE_HOME_NUMBER_PREFIX, PHONE_HOME_NUMBER_SUFFIX
Instead of listing them directly, how about listing them through `PhoneNumber::GetSupportedTypes` an […]
+1
Patch Set #2, Line 513:
ASSERT_TRUE(phone_number.GetRawInfo(type).empty());`EXPECT_TRUE(!phone_number.HasRawInfo(type))`
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Florian Leimgruber (Gerrit)
Attention is currently required from: Matthias Körber, Norge Vizcay, Timofey Chudakov.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
> Florian, one option might be missing […]
The import test tool is used by manual rater. We need to [update their JSON files](https://drive.google.com/corp/drive/folders/1R3iNdOKiuwfr168EhTkNx0tslzCv0yl1) first. Otherwise, their test profiles will stop containing a phone number. Again, thanks for the great catch, Norge.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Norge Vizcay (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Timofey Chudakov.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}> Florian, one option might be missing […]
`CHECK`s are advised when asserting an invariant [1], which is probably the case here?
CCing @maximsh...@google.com regarding updating the test JSON files. Although he is OOO and I'm not sure if anyone else has ownership on these files.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay.
2 comments:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
`CHECK`s are advised when asserting an invariant [1], which is probably the case here? […]
I don't need to land this asap, this can wait a long time. Although this change is helpful, it doesn't affect I18n.
File components/autofill/core/browser/data_model/phone_number.cc:
As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
I think this part of the comment can be removed now (I added it to explain why PHONE_HOME_CITY_AND_N […]
Done
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay.
2 comments:
File components/autofill/core/browser/data_model/phone_number_unittest.cc:
PHONE_HOME_NUMBER, PHONE_HOME_CITY_CODE, PHONE_HOME_COUNTRY_CODE,
PHONE_HOME_CITY_AND_NUMBER, PHONE_HOME_EXTENSION,
PHONE_HOME_CITY_CODE_WITH_TRUNK_PREFIX,
PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX,
PHONE_HOME_NUMBER_PREFIX, PHONE_HOME_NUMBER_SUFFIX
+1
That method is protected. After discussing online, I created a bug to investigate if that method can be made public.
Patch Set #2, Line 513:
ASSERT_TRUE(phone_number.GetRawInfo(type).empty());
`EXPECT_TRUE(!phone_number. […]
Done
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Maxim Sheshukov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Norge Vizcay, Timofey Chudakov.
2 comments:
File chrome/test/data/autofill/profiles/profiles.json:
Patch Set #8, Line 19:
601-952-1325This is an international phone number now. Should it start with +1?
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
I don't need to land this asap, this can wait a long time. […]
Thanks for letting me know. I'll update profiles.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Florian Leimgruber (Gerrit)
Attention is currently required from: Matthias Körber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
2 comments:
File chrome/test/data/autofill/profiles/profiles.json:
This is an international phone number now. […]
PHONE_HOME_WHOLE_NUMBER is just "everything we've stored" - which is not necessarily an international number. In the past, we tried changing the code to make it an international number. But users weren't happy about this, since many people prefer nationally formatted numbers. So I think it's reasonable to keep this nationally formatted.
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
Thanks for letting me know. I'll update profiles.
Thanks!
---
Coming back to Matthias' comment: Should we add a (non-crashing) check here? Just in case we missed something, this way we would at least get bug reports.
I think NOTREACHED() [would work](https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/c++/checks.md).
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Maxim Sheshukov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Norge Vizcay, Timofey Chudakov.
1 comment:
File chrome/test/data/autofill/profiles/profiles.json:
PHONE_HOME_WHOLE_NUMBER is just "everything we've stored" - which is not necessarily an internationa […]
Acknowledged
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
> Thanks for letting me know. I'll update profiles. […]
Unfortunately it's not possible, cause we're going to need the fact that it doesn't set that fact while calling this function with `PHONE_HOME_CITY_AND_NUMBER`.
This will be the case with I18n: we'll pass all supported fields to the platform code and receive all fields back, so `PHONE_HOME_CITY_AND_NUMBER` will be passed to this function.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Florian Leimgruber (Gerrit)
Attention is currently required from: Matthias Körber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
we'll pass all supported fields to the platform code
Are you sure this is necessary? PHONE_HOME_CITY_AND_NUMBER is a [derived type](https://source.chromium.org/chromium/chromium/src/+/main:components/autofill/README.md;l=318-333;drc=eba114c1afd8c9f5899e311df4fd34a4901bf455). I'd expect that we only ever want to show stored types in the UI. If the user edits a derived type, but not its corresponding stored type, after restarting the browser, the change will be gone.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Matthias Körber (Gerrit)
Attention is currently required from: Florian Leimgruber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
> we'll pass all supported fields to the platform code […]
Exactly, we do not want to make an implementation mistake and set a derived type.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
findit-for-me@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Florian Leimgruber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay.
Patch set 12:Commit-Queue +1
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
Exactly, we do not want to make an implementation mistake and set a derived type.
Ok, I ended up forwarding only storable fields to the platform code. Please take a look now 😊
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Florian Leimgruber (Gerrit)
Attention is currently required from: Matthias Körber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
Patch set 12:Code-Review +1
6 comments:
Patchset:
Assuming that the JSON files are updated, this LGTM
File chrome/test/data/autofill/profiles/profiles.json:
Acknowledged
(just an FYI) I talked with Dominic about this issue yesterday and we concluded that "whole number" should eventually become "international number". We should ultimately improve our prediction or filling logic to decide between "whole number" and "city and number". (At the moment, local heuristics always classify single field phone number inputs as "whole number")
For now, I think the best option is keeping this a nationally formatted number, until we fixed the prediction/filling logic. Otherwise we might break the experience for manual testers.
File components/autofill/core/browser/data_model/phone_number.cc:
nit: go/no-we
Patch Set #12, Line 103:
NOTREACHED();NOTREACHED_NORETURN(). See [here](https://source.chromium.org/chromium/chromium/src/+/main:base/notreached.h;l=34;drc=78ea882471b0219cbce7fe8020df8fa0245a3efe).
File components/autofill/core/browser/data_model/phone_number_unittest.cc:
PHONE_HOME_NUMBER, PHONE_HOME_CITY_CODE, PHONE_HOME_COUNTRY_CODE,
PHONE_HOME_CITY_AND_NUMBER, PHONE_HOME_EXTENSION,
PHONE_HOME_CITY_CODE_WITH_TRUNK_PREFIX,
PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX,
PHONE_HOME_NUMBER_PREFIX, PHONE_HOME_NUMBER_SUFFIX
That method is protected. […]
Another alternative could be something like this:
```
ServerFieldTypeSet types;
profile.GetSupportedTypes(&types);
base::EraseIf(types, [](ServerFieldType type) {
return AutofillType(type).group() != FieldTypeGroup::kPhone ||
base::Contains(AutofillTable::GetStoredTypesForAutofillProfile(), type);
});
(at this point `types` only contains derived phone number types)
base::ranges::for_each(types, ...);
```
File components/autofill/core/browser/data_model/phone_number_unittest.cc:
nit: U (upper case)
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Norge Vizcay (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Timofey Chudakov.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
Ok, I ended up forwarding only storable fields to the platform code. […]
Regarding forwarding only derived types. Although I agree that we in principle would only want to forward storable fields, I think there are some exceptions.
For example, ADDRESS_LINE types are currently accounted as derived (not storable), but their `set` operation has an impact on the storable field (e.g. ADDRESS_HOME_STREET_ADDRESS). Moreover, iOS seems to show address lines as fields on their address editors and forward those back.
I can't think of other exceptions, so maybe we should consider ADDRESS_LINE(s) as storable types?
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Matthias Körber (Gerrit)
Attention is currently required from: Florian Leimgruber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
1 comment:
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
Regarding forwarding only derived types. […]
middle name initial can also be storable if there is no middle name yet.
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay.
5 comments:
File chrome/test/data/autofill/profiles/profiles.json:
(just an FYI) I talked with Dominic about this issue yesterday and we concluded that "whole number" […]
Acknowledged
File components/autofill/core/browser/data_model/phone_number.cc:
if (type != PHONE_HOME_WHOLE_NUMBER) {
// Only full phone numbers should be set directly. The remaining field types
// are read-only. As PHONE_HOME_CITY_AND_NUMBER_WITHOUT_TRUNK_PREFIX
// generally doesn't represent a dialable number, it is not accessible
// either.
return;
}
middle name initial can also be storable if there is no middle name yet.
Ack regarding the storable types. IIUC IOS should get rid of ADDRESS_LINE fields in the nearest future.
I would discuss this separately cause this has little to do with this CL.
File components/autofill/core/browser/data_model/phone_number.cc:
nit: go/no-we
Done
Patch Set #12, Line 103:
NOTREACHED();
NOTREACHED_NORETURN(). See [here](https://source.chromium. […]
Done
File components/autofill/core/browser/data_model/phone_number_unittest.cc:
nit: U (upper case)
Done
To view, visit change 4773760. To unsubscribe, or for help writing mail filters, visit settings.
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay.
Patch set 13:Commit-Queue +2
Timofey Chudakov (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
Patch set 14:Commit-Queue +2
findit-for-me@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Florian Leimgruber, Matthias Körber, Maxim Sheshukov, Norge Vizcay, Timofey Chudakov.
This change meets the code coverage requirements.
Patch set 14:Code-Coverage +1
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
12 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: components/autofill/core/browser/data_model/phone_number.cc
Insertions: 4, Deletions: 5.
The diff is too large to show. Please review the diff.
```
```
The name of the file: components/autofill/core/browser/data_model/phone_number_unittest.cc
Insertions: 13, Deletions: 12.
The diff is too large to show. Please review the diff.
```
[Autofill] Don't set PHONE_HOME_CITY_AND_NUMBER directly.
AutofillProfile::GetRawInfo(PHONE_HOME_CITY_AND_NUMBER) always returns
empty string. This is inconsistent with how SetRawInfo works:
SetRawInfo handles PHONE_HOME_CITY_AND_NUMBER as a stored type
and allows settings it.
This CL removes this inconsistency.
Bug: 1441904
Change-Id: Ie8118e4b1b58f5a9299e85dd0d3191a4a39f77b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4773760
Reviewed-by: Florian Leimgruber <fleim...@google.com>
Commit-Queue: Timofey Chudakov <tchu...@google.com>
Code-Coverage: findit...@appspot.gserviceaccount.com <findit...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1192952}
---
M chrome/test/data/autofill/profiles/profiles.json
M components/autofill/core/browser/data_model/phone_number.cc
M components/autofill/core/browser/data_model/phone_number_unittest.cc
3 files changed, 26 insertions(+), 7 deletions(-)