[Autofill] Added utils for creating LabelFormatters.

Making a new branch and CL to resolve issues with git and try jobs.

Original CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1498614

The utils for making various LabelFormatters sometimes favor std::set over
std::vector so that poorly formed forms, e.g. forms with duplicate fields, do
not interfere with processing later on. Sets allow us to safely assume that
there is just one of a particular ServerFieldType or FieldTypeGroup.

It is worth noting that ContactFormLabelFormatters store their FieldTypeGroups
while AddressFormLabelFormatters do not. This is because address forms are
guaranteed to have ADDRESS_HOME and NAME; however, contact forms could have
NAME and PHONE_HOME, EMAIL, or both PHONE_HOME and EMAIL. It is useful for a
ContactFormLabelFormatter to know which subset of FieldTypeGroups correspond
to its fields.

Change-Id: I6164081759848b263c3d9dd5cfcda190388a3d5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1506116
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638682}

components/autofill/core/browser/BUILD.gn

@@ -129,6 +129,7 @@ jumbo_static_library("browser") {
     "form_structure.h",
     "form_types.cc",
     "form_types.h",
+    "label_formatter.cc",
     "label_formatter.h",
     "label_formatter_utils.cc",
     "label_formatter_utils.h",

components/autofill/core/browser/address_form_label_formatter.cc

@@ -10,13 +10,11 @@ AddressFormLabelFormatter::AddressFormLabelFormatter(
     const std::string& app_locale,
     ServerFieldType focused_field_type,
     const std::vector<ServerFieldType>& field_types)
-    : app_locale_(app_locale),
-      focused_field_type_(focused_field_type),
-      field_types_(field_types) {
+    : LabelFormatter(app_locale, focused_field_type, field_types) {
   for (const ServerFieldType& type : field_types) {
-    if ((type != ADDRESS_HOME_COUNTRY) && (type != ADDRESS_BILLING_COUNTRY) &&
-        (type != focused_field_type_)) {
-      filtered_field_types_.push_back(type);
+    if (type != focused_field_type && type != ADDRESS_HOME_COUNTRY &&
+        type != ADDRESS_BILLING_COUNTRY) {
+      field_types_for_labels_.push_back(type);
     }
   }
 }

components/autofill/core/browser/address_form_label_formatter.h

@@ -19,33 +19,19 @@ namespace autofill {
 // with name and address fields and without email or phone fields.
 class AddressFormLabelFormatter : public LabelFormatter {
  public:
-  explicit AddressFormLabelFormatter(
-      const std::string& app_locale,
-      ServerFieldType focused_field_type,
-      const std::vector<ServerFieldType>& field_types);
+  AddressFormLabelFormatter(const std::string& app_locale,
+                            ServerFieldType focused_field_type,
+                            const std::vector<ServerFieldType>& field_types);
+
   ~AddressFormLabelFormatter() override;
 
   std::vector<base::string16> GetLabels(
       const std::vector<AutofillProfile*>& profiles) const override;
 
  private:
-  // The locale for which to generate labels. This reflects the language and
-  // country for which the application is translated, e.g. en_AU for Austalian
-  // English.
-  std::string app_locale_;
-
-  // The field on which the user is currently focused.
-  ServerFieldType focused_field_type_;
-
-  // A collection of meaningful field types in the form with which the user is
-  // interacting. The NO_SERVER_DATA and UNKNOWN_TYPE field types are not
-  // considered meaningful.
-  std::vector<ServerFieldType> field_types_;
-
-  // A collection of meaningful field types excluding the focused_field_type_
-  // and ADDRESS_HOME_COUNTRY and ADDRESS_BILLING_COUNTRY. These types are used
-  // to construct the labels.
-  std::vector<ServerFieldType> filtered_field_types_;
+  // A collection of field types that can be used to make labels. This
+  // collection excludes the focused_field_type_ and address countries.
+  std::vector<ServerFieldType> field_types_for_labels_;
 };
 
 }  // namespace autofill

components/autofill/core/browser/contact_form_label_formatter.cc

@@ -9,15 +9,19 @@ namespace autofill {
 ContactFormLabelFormatter::ContactFormLabelFormatter(
     const std::string& app_locale,
     ServerFieldType focused_field_type,
-    const std::vector<ServerFieldType>& field_types)
-    : app_locale_(app_locale),
-      focused_field_type_(focused_field_type),
-      field_types_(field_types) {
+    const std::vector<ServerFieldType>& field_types,
+    const std::set<FieldTypeGroup>& field_type_groups)
+    : LabelFormatter(app_locale, focused_field_type, field_types),
+      field_type_groups_(field_type_groups),
+      filtered_field_type_groups_(field_type_groups) {
   for (const ServerFieldType& type : field_types) {
-    if (type != focused_field_type_) {
-      filtered_field_types_.push_back(type);
+    if (type != focused_field_type) {
+      field_types_for_labels_.push_back(type);
     }
   }
+  const FieldTypeGroup group =
+      AutofillType(AutofillType(focused_field_type).GetStorableType()).group();
+  filtered_field_type_groups_.erase(group);
 }
 
 ContactFormLabelFormatter::~ContactFormLabelFormatter() {}

components/autofill/core/browser/contact_form_label_formatter.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_CONTACT_FORM_LABEL_FORMATTER_H_
 #define COMPONENTS_AUTOFILL_CORE_BROWSER_CONTACT_FORM_LABEL_FORMATTER_H_
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -19,32 +20,28 @@ namespace autofill {
 // containing name and phone or email fields.
 class ContactFormLabelFormatter : public LabelFormatter {
  public:
-  explicit ContactFormLabelFormatter(
-      const std::string& app_locale,
-      ServerFieldType focused_field_type,
-      const std::vector<ServerFieldType>& field_types);
+  ContactFormLabelFormatter(const std::string& app_locale,
+                            ServerFieldType focused_field_type,
+                            const std::vector<ServerFieldType>& field_types,
+                            const std::set<FieldTypeGroup>& field_type_groups);
+
   ~ContactFormLabelFormatter() override;
 
   std::vector<base::string16> GetLabels(
       const std::vector<AutofillProfile*>& profiles) const override;
 
  private:
-  // The locale for which to generate labels. This reflects the language and
-  // country for which the application is translated, e.g. en_AU for Austalian
-  // English.
-  std::string app_locale_;
-
-  // The field on which the user is currently focused.
-  ServerFieldType focused_field_type_;
-
-  // A collection of meaningful field types in the form with which the user is
-  // interacting. The NO_SERVER_DATA and UNKNOWN_TYPE field types are not
-  // considered meaningful.
-  std::vector<ServerFieldType> field_types_;
-
-  // A collection of meaningful field types excluding the focused_field_type_.
-  // These types are used to construct the labels.
-  std::vector<ServerFieldType> filtered_field_types_;
+  // A collection of field types that can be used to make labels. This
+  // collection excludes the focused_field_type_.
+  std::vector<ServerFieldType> field_types_for_labels_;
+
+  // A collection of meaningful FieldTypeGroups in the form with which the user
+  // is interacting.
+  std::set<FieldTypeGroup> field_type_groups_;
+
+  // A collection of meaningful FieldTypeGroups in the form with which the user
+  // is interacting minus the focused field's corresponding FieldTypeGroup.
+  std::set<FieldTypeGroup> filtered_field_type_groups_;
 };
 
 }  // namespace autofill

components/autofill/core/browser/label_formatter.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/autofill/core/browser/label_formatter.h"
+
+namespace autofill {
+
+LabelFormatter::LabelFormatter(const std::string& app_locale,
+                               ServerFieldType focused_field_type,
+                               const std::vector<ServerFieldType>& field_types)
+    : app_locale_(app_locale),
+      focused_field_type_(focused_field_type),
+      field_types_(field_types) {}
+LabelFormatter::~LabelFormatter() = default;
+
+}  // namespace autofill

components/autofill/core/browser/label_formatter.h

@@ -17,11 +17,35 @@ namespace autofill {
 // Handles the creation of Suggestions' disambiguating labels.
 class LabelFormatter {
  public:
-  virtual ~LabelFormatter() = default;
+  LabelFormatter(const std::string& app_locale,
+                 ServerFieldType focused_field_type,
+                 const std::vector<ServerFieldType>& field_types);
+  virtual ~LabelFormatter();
+
   // Returns a collection of |labels| formed by extracting useful disambiguating
   // information from a collection of |profiles|.
   virtual std::vector<base::string16> GetLabels(
       const std::vector<AutofillProfile*>& profiles) const = 0;
+
+ protected:
+  const std::string& app_locale() const { return app_locale_; }
+  ServerFieldType focused_field_type() const { return focused_field_type_; }
+  const std::vector<ServerFieldType>& field_types() const {
+    return field_types_;
+  }
+
+ private:
+  // The locale for which to generate labels. This reflects the language and
+  // country for which the application is translated, e.g. en_AU for Austalian
+  // English.
+  std::string app_locale_;
+
+  // The field on which the user is currently focused.
+  ServerFieldType focused_field_type_;
+
+  // A collection of meaningful field types in the form with which the user is
+  // interacting.
+  std::vector<ServerFieldType> field_types_;
 };
 
 }  // namespace autofill

components/autofill/core/browser/label_formatter_utils.cc

@@ -7,22 +7,64 @@
 #include <memory>
 #include <set>
 
+#include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/address_form_label_formatter.h"
+#include "components/autofill/core/browser/autofill_data_util.h"
 #include "components/autofill/core/browser/contact_form_label_formatter.h"
+#include "components/autofill/core/browser/validation.h"
 
 namespace autofill {
 namespace {
 
-// Returns true if |type| belongs to the NAME FieldTypeGroup. Note that billing
-// ServerFieldTypes are converted to their non-billing types.
-bool FieldTypeIsName(const ServerFieldType& type) {
-  FieldTypeGroup focused_field_group =
-      AutofillType(AutofillType(type).GetStorableType()).group();
-  return focused_field_group == NAME;
+bool ContainsName(uint32_t groups) {
+  return groups & label_formatter_groups::kName;
+}
+
+bool ContainsAddress(uint32_t groups) {
+  return groups & label_formatter_groups::kAddress;
+}
+
+bool ContainsEmail(uint32_t groups) {
+  return groups & label_formatter_groups::kEmail;
+}
+
+bool ContainsPhone(uint32_t groups) {
+  return groups & label_formatter_groups::kPhone;
+}
+
+std::set<FieldTypeGroup> GetFieldTypeGroups(uint32_t groups) {
+  std::set<FieldTypeGroup> field_type_groups;
+  if (ContainsName(groups)) {
+    field_type_groups.insert(NAME);
+  }
+  if (ContainsAddress(groups)) {
+    field_type_groups.insert(ADDRESS_HOME);
+  }
+  if (ContainsEmail(groups)) {
+    field_type_groups.insert(EMAIL);
+  }
+  if (ContainsPhone(groups)) {
+    field_type_groups.insert(PHONE_HOME);
+  }
+  return field_type_groups;
 }
 
 }  // namespace
 
+std::vector<ServerFieldType> FilterFieldTypes(
+    const std::vector<ServerFieldType>& field_types) {
+  std::vector<ServerFieldType> filtered_field_types;
+  for (const ServerFieldType& field_type : field_types) {
+    const FieldTypeGroup group =
+        AutofillType(AutofillType(field_type).GetStorableType()).group();
+    if (group == NAME || group == ADDRESS_HOME || group == EMAIL ||
+        group == PHONE_HOME) {
+      filtered_field_types.push_back(field_type);
+    }
+  }
+  return filtered_field_types;
+}
+
 uint32_t DetermineGroups(const std::vector<ServerFieldType>& field_types) {
   uint32_t group_bitmask = 0;
   for (const ServerFieldType& type : field_types) {
@@ -42,7 +84,7 @@ uint32_t DetermineGroups(const std::vector<ServerFieldType>& field_types) {
         group_bitmask |= label_formatter_groups::kPhone;
         break;
       default:
-        group_bitmask |= label_formatter_groups::kUnsupported;
+        break;
     }
   }
   return group_bitmask;
@@ -52,34 +94,22 @@ std::unique_ptr<LabelFormatter> Create(
     const std::string& app_locale,
     ServerFieldType focused_field_type,
     const std::vector<ServerFieldType>& field_types) {
-  if (!FieldTypeIsName(focused_field_type)) {
-    return nullptr;
-  }
+  const uint32_t groups = DetermineGroups(field_types);
 
-  std::vector<ServerFieldType> filtered_field_types;
-  for (const ServerFieldType& field_type : field_types) {
-    // NO_SERVER_DATA fields are frequently found in the collection of field
-    // types sent from the frontend. UKNOWN_TYPE fields represent various form
-    // elements, e.g. checkboxes. Neither field type is useful to the formatter,
-    // so they are excluded from its collection of field types.
-    if (field_type != NO_SERVER_DATA && field_type != UNKNOWN_TYPE) {
-      filtered_field_types.push_back(field_type);
-    }
+  if (!ContainsName(groups)) {
+    return nullptr;
   }
-
-  const uint32_t groups = DetermineGroups(filtered_field_types);
-  if (groups ==
-      (label_formatter_groups::kName | label_formatter_groups::kAddress)) {
+  if (ContainsAddress(groups) && !ContainsEmail(groups) &&
+      !ContainsPhone(groups)) {
     return std::make_unique<AddressFormLabelFormatter>(
-        app_locale, focused_field_type, filtered_field_types);
+        app_locale, focused_field_type, FilterFieldTypes(field_types));
   }
-
-  if (groups ==
-      (label_formatter_groups::kName | label_formatter_groups::kPhone |
-       label_formatter_groups::kEmail)) {
+  if (ContainsEmail(groups) || ContainsPhone(groups)) {
     return std::make_unique<ContactFormLabelFormatter>(
-        app_locale, focused_field_type, filtered_field_types);
+        app_locale, focused_field_type, FilterFieldTypes(field_types),
+        GetFieldTypeGroups(groups));
   }
   return nullptr;
 }
+
 }  // namespace autofill

components/autofill/core/browser/label_formatter_utils.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_AUTOFILL_CORE_BROWSER_LABEL_FORMATTER_UTILS_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -16,30 +17,37 @@ namespace autofill {
 namespace label_formatter_groups {
 
 // Bits for FieldTypeGroup options.
-// The form contains an unsupported field.
-constexpr uint32_t kUnsupported = 1 << 0;
 // The form contains at least one field associated with the NAME_HOME or
 // NAME_BILLING FieldTypeGroups.
-constexpr uint32_t kName = 1 << 1;
+constexpr uint32_t kName = 1 << 0;
 // The form contains at least one field associated with the ADDRESS_HOME or
 // ADDRESS_BILLING FieldTypeGroups.
-constexpr uint32_t kAddress = 1 << 2;
+constexpr uint32_t kAddress = 1 << 1;
 // The form contains at least one field associated with the EMAIL
 // FieldTypeGroup.
-constexpr uint32_t kEmail = 1 << 3;
+constexpr uint32_t kEmail = 1 << 2;
 // The form contains at least one field associated with the PHONE_HOME or
 // PHONE_BILLING FieldTypeGroup.
-constexpr uint32_t kPhone = 1 << 4;
+constexpr uint32_t kPhone = 1 << 3;
 
 }  // namespace label_formatter_groups
 
-// Returns a bitmask indicating the FieldTypeGroups associated with the given
-// |field_types|.
+// Returns a subset of meaningful ServerFieldTypes found in |field_types|.
+// ServerFieldTypes like NO_SERVER_DATA and UNKNOWN_TYPE are frequently found in
+// the collection of types sent from the frontend. Other types, e.g.
+// COMPANY_NAME and PHONE_FAX_WHOLE_NUMBER, are also sometimes present. These
+// types are not useful to LabelFormatters, so only types related to names,
+// addresses, emails, and phone numbers are in the results.
+std::vector<ServerFieldType> FilterFieldTypes(
+    const std::vector<ServerFieldType>& field_types);
+
+// Returns a bitmask indicating whether the NAME, ADDRESS_HOME, EMAIL, and
+// PHONE_HOME FieldTypeGroups are associated with the given |field_types|.
 uint32_t DetermineGroups(const std::vector<ServerFieldType>& field_types);
 
 // Creates a form-specific LabelFormatter according to |field_types|. If the
-// given |focused_field_type| and |field_types| do not correspond to a
-// LabelFormatter, then nullptr will be returned.
+// given |field_types| do not correspond to a LabelFormatter, then nullptr will
+// be returned.
 std::unique_ptr<LabelFormatter> Create(
     const std::string& app_locale,
     ServerFieldType focused_field_type,

components/autofill/core/browser/label_formatter_utils_unittest.cc

@@ -13,9 +13,35 @@ using label_formatter_groups::kAddress;
 using label_formatter_groups::kEmail;
 using label_formatter_groups::kName;
 using label_formatter_groups::kPhone;
-using label_formatter_groups::kUnsupported;
+
 }  // namespace
 
+TEST(LabelFormatterUtilsTest, FilterFieldTypesNoFiltering) {
+  const std::vector<ServerFieldType> field_types{
+      NAME_LAST,           NAME_BILLING_LAST, ADDRESS_HOME_ZIP,
+      ADDRESS_BILLING_ZIP, EMAIL_ADDRESS,     PHONE_HOME_NUMBER,
+      PHONE_BILLING_NUMBER};
+  const std::vector<ServerFieldType> filtered_field_types =
+      FilterFieldTypes(field_types);
+  EXPECT_EQ(field_types, filtered_field_types);
+}
+
+TEST(LabelFormatterUtilsTest, FilterFieldTypesFilterCompany) {
+  const std::vector<ServerFieldType> field_types{NAME_LAST, COMPANY_NAME};
+  const std::vector<ServerFieldType> expected_filtered_field_types{NAME_LAST};
+  const std::vector<ServerFieldType> filtered_field_types =
+      FilterFieldTypes(field_types);
+  EXPECT_EQ(expected_filtered_field_types, filtered_field_types);
+}
+
+TEST(LabelFormatterUtilsTest, FilterFieldTypesForNoGivenFieldTypes) {
+  const std::vector<ServerFieldType> field_types =
+      std::vector<ServerFieldType>();
+  const std::vector<ServerFieldType> filtered_field_types =
+      FilterFieldTypes(field_types);
+  EXPECT_EQ(field_types, filtered_field_types);
+}
+
 TEST(LabelFormatterUtilsTest, DetermineGroupsForHomeNameAndAddress) {
   const std::vector<ServerFieldType> field_types{
       NAME_FIRST,        NAME_LAST,          ADDRESS_HOME_LINE1,
@@ -36,7 +62,7 @@ TEST(LabelFormatterUtilsTest, DetermineGroupsForBillingNameAndAddress) {
   EXPECT_EQ(expected_group_bitmask, group_bitmask);
 }
 
-TEST(LabelFormatterUtilsTest, DetermineGroupsForNameHomePhoneAndEmail) {
+TEST(LabelFormatterUtilsTest, DetermineGroupsForHomeNamePhoneAndEmail) {
   const std::vector<ServerFieldType> field_types{
       NAME_FULL, PHONE_HOME_CITY_AND_NUMBER, EMAIL_ADDRESS};
 
@@ -45,7 +71,7 @@ TEST(LabelFormatterUtilsTest, DetermineGroupsForNameHomePhoneAndEmail) {
   EXPECT_EQ(expected_group_bitmask, group_bitmask);
 }
 
-TEST(LabelFormatterUtilsTest, DetermineGroupsForNameBillingPhoneAndEmail) {
+TEST(LabelFormatterUtilsTest, DetermineGroupsForBillingNamePhoneAndEmail) {
   const std::vector<ServerFieldType> field_types{
       NAME_BILLING_FULL, PHONE_BILLING_WHOLE_NUMBER, EMAIL_ADDRESS};
 
@@ -58,7 +84,7 @@ TEST(LabelFormatterUtilsTest, DetermineGroupsForUnknownServerFieldType) {
   const std::vector<ServerFieldType> field_types{UNKNOWN_TYPE, NAME_FULL,
                                                  ADDRESS_HOME_ZIP};
 
-  const uint32_t expected_group_bitmask = kName | kAddress | kUnsupported;
+  const uint32_t expected_group_bitmask = kName | kAddress;
   const uint32_t group_bitmask = DetermineGroups(field_types);
   EXPECT_EQ(expected_group_bitmask, group_bitmask);
 }
@@ -71,4 +97,4 @@ TEST(LabelFormatterUtilsTest, DetermineGroupsForNoServerFieldTypes) {
   EXPECT_EQ(expected_group_bitmask, group_bitmask);
 }
 
-}  // namespace autofill
+}  // namespace autofill
\ No newline at end of file