[Autofill] Independent Heuristics for Search Fields

Search term fields can now only be predicted if they're adjacent to
an address field. By moving them to an independent parser, this will
not be the case anymore.

Bug: 848413
Change-Id: I5a35e363314b58d75fefddb65b410ddcc840df9a
Reviewed-on: https://chromium-review.googlesource.com/1089371Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Commit-Queue: Parastoo Geranmayeh <parastoog@google.com>
Cr-Commit-Position: refs/heads/master@{#566010}

components/autofill/core/browser/BUILD.gn

@@ -144,6 +144,8 @@ static_library("browser") {
     "region_data_loader_impl.cc",
     "region_data_loader_impl.h",
     "risk_data_loader.h",
+    "search_field.cc",
+    "search_field.h",
     "state_names.cc",
     "state_names.h",
     "subkey_requester.cc",
@@ -417,6 +419,7 @@ source_set("unit_tests") {
     "phone_number_unittest.cc",
     "rationalization_util_unittest.cc",
     "region_combobox_model_unittest.cc",
+    "search_field_unittest.cc",
     "subkey_requester_unittest.cc",
     "ui/card_unmask_prompt_controller_impl_unittest.cc",
     "validation_unittest.cc",

components/autofill/core/browser/address_field.cc

@@ -93,8 +93,6 @@ std::unique_ptr<FormField> AddressField::Parse(AutofillScanner* scanner) {
         begin_trailing_non_labeled_fields = cursor;
       }
 
-      continue;
-    } else if (address_field->ParseSearchTerm(scanner)) {
       continue;
     } else {
       // No field found.
@@ -104,12 +102,11 @@ std::unique_ptr<FormField> AddressField::Parse(AutofillScanner* scanner) {
 
   // If we have identified any address fields in this field then it should be
   // added to the list of fields.
-  // TODO(http://crbug.com/848413): Move search_term_ to its own parser.
   if (address_field->company_ || address_field->address1_ ||
       address_field->address2_ || address_field->address3_ ||
       address_field->street_address_ || address_field->city_ ||
       address_field->state_ || address_field->zip_ || address_field->zip4_ ||
-      address_field->country_ || address_field->search_term_) {
+      address_field->country_) {
     // Don't slurp non-labeled fields at the end into the address.
     if (has_trailing_non_labeled_fields)
       scanner->RewindTo(begin_trailing_non_labeled_fields);
@@ -130,8 +127,7 @@ AddressField::AddressField()
       state_(nullptr),
       zip_(nullptr),
       zip4_(nullptr),
-      country_(nullptr),
-      search_term_(nullptr) {}
+      country_(nullptr) {}
 
 void AddressField::AddClassifications(
     FieldCandidatesMap* field_candidates) const {
@@ -160,15 +156,6 @@ void AddressField::AddClassifications(
                     field_candidates);
   AddClassification(country_, ADDRESS_HOME_COUNTRY, kBaseAddressParserScore,
                     field_candidates);
-  AddClassification(search_term_, SEARCH_TERM, kBaseAddressParserScore,
-                    field_candidates);
-}
-
-bool AddressField::ParseSearchTerm(AutofillScanner* scanner) {
-  if (search_term_ && !search_term_->IsEmpty())
-    return false;
-
-  return ParseField(scanner, UTF8ToUTF16(kSearchTermRe), &search_term_);
 }
 
 bool AddressField::ParseCompany(AutofillScanner* scanner) {

components/autofill/core/browser/address_field.h

@@ -47,10 +47,6 @@ class AddressField : public FormField {
   FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseCountry);
   FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseTwoLineAddressMissingLabel);
   FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseCompany);
-  FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseSearchTermFirst);
-  FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseSearchTermSecond);
-  FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseSearchTermIsolated);
-  FRIEND_TEST_ALL_PREFIXES(AddressFieldTest, ParseNonSearchTermWithSearch);
 
   static const int kZipCodeMatchType;
   static const int kCityMatchType;
@@ -64,7 +60,6 @@ class AddressField : public FormField {
   bool ParseZipCode(AutofillScanner* scanner);
   bool ParseCity(AutofillScanner* scanner);
   bool ParseState(AutofillScanner* scanner);
-  bool ParseSearchTerm(AutofillScanner* scanner);
 
   // Parses the current field pointed to by |scanner|, if it exists, and tries
   // to figure out whether the field's type: city, state, zip, or none of those.
@@ -98,7 +93,6 @@ class AddressField : public FormField {
   AutofillField* zip_;
   AutofillField* zip4_;  // optional ZIP+4; we don't fill this yet.
   AutofillField* country_;
-  AutofillField* search_term_;
 
   DISALLOW_COPY_AND_ASSIGN(AddressField);
 };

components/autofill/core/browser/address_field_unittest.cc

@@ -277,91 +277,4 @@ TEST_F(AddressFieldTest, ParseCompany) {
       field_candidates_map_[ASCIIToUTF16("company1")].BestHeuristicType());
 }
 
-TEST_F(AddressFieldTest, ParseSearchTermFirst) {
-  FormFieldData search_field;
-  search_field.form_control_type = "text";
-
-  search_field.label = ASCIIToUTF16("Search");
-  search_field.name = ASCIIToUTF16("search");
-
-  FormFieldData address_field;
-  address_field.form_control_type = "text";
-
-  address_field.label = ASCIIToUTF16("Address");
-  address_field.name = ASCIIToUTF16("address");
-
-  list_.push_back(
-      std::make_unique<AutofillField>(search_field, ASCIIToUTF16("search1")));
-  list_.push_back(
-      std::make_unique<AutofillField>(address_field, ASCIIToUTF16("address")));
-
-  AutofillScanner scanner(list_);
-  field_ = Parse(&scanner);
-  ASSERT_NE(nullptr, field_.get());
-  field_->AddClassifications(&field_candidates_map_);
-  ASSERT_TRUE(field_candidates_map_.find(ASCIIToUTF16("search1")) !=
-              field_candidates_map_.end());
-  EXPECT_EQ(SEARCH_TERM,
-            field_candidates_map_[ASCIIToUTF16("search1")].BestHeuristicType());
-}
-
-TEST_F(AddressFieldTest, ParseSearchTermSecond) {
-  FormFieldData search_field;
-  search_field.form_control_type = "text";
-
-  search_field.label = ASCIIToUTF16("Search");
-  search_field.name = ASCIIToUTF16("search");
-
-  FormFieldData address_field;
-  address_field.form_control_type = "text";
-
-  address_field.label = ASCIIToUTF16("Address");
-  address_field.name = ASCIIToUTF16("address");
-
-  list_.push_back(
-      std::make_unique<AutofillField>(address_field, ASCIIToUTF16("address")));
-  list_.push_back(
-      std::make_unique<AutofillField>(search_field, ASCIIToUTF16("search1")));
-
-  AutofillScanner scanner(list_);
-  field_ = Parse(&scanner);
-  ASSERT_NE(nullptr, field_.get());
-  field_->AddClassifications(&field_candidates_map_);
-  ASSERT_TRUE(field_candidates_map_.find(ASCIIToUTF16("search1")) !=
-              field_candidates_map_.end());
-  EXPECT_EQ(SEARCH_TERM,
-            field_candidates_map_[ASCIIToUTF16("search1")].BestHeuristicType());
-}
-
-// For a "search xx" phrase, xx has priority to search, if xx is a valid
-// fillable type.
-TEST_F(AddressFieldTest, ParseNonSearchTermWithSearch) {
-  FormFieldData addr_field;
-  addr_field.form_control_type = "text";
-
-  addr_field.label = ASCIIToUTF16("Search Address");
-  addr_field.name = ASCIIToUTF16("search_addr");
-
-  FormFieldData company_field;
-  company_field.form_control_type = "text";
-
-  company_field.label = ASCIIToUTF16("Company");
-  company_field.name = ASCIIToUTF16("company1");
-
-  list_.push_back(
-      std::make_unique<AutofillField>(addr_field, ASCIIToUTF16("search_addr")));
-  list_.push_back(
-      std::make_unique<AutofillField>(company_field, ASCIIToUTF16("company1")));
-
-  AutofillScanner scanner(list_);
-  field_ = Parse(&scanner);
-  ASSERT_NE(nullptr, field_.get());
-  field_->AddClassifications(&field_candidates_map_);
-  ASSERT_TRUE(field_candidates_map_.find(ASCIIToUTF16("search_addr")) !=
-              field_candidates_map_.end());
-  EXPECT_EQ(
-      ADDRESS_HOME_LINE1,
-      field_candidates_map_[ASCIIToUTF16("search_addr")].BestHeuristicType());
-}
-
 }  // namespace autofill

components/autofill/core/browser/form_field.cc

@@ -23,6 +23,7 @@
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/name_field.h"
 #include "components/autofill/core/browser/phone_field.h"
+#include "components/autofill/core/browser/search_field.h"
 #include "components/autofill/core/common/autofill_constants.h"
 #include "components/autofill/core/common/autofill_regexes.h"
 #include "components/autofill/core/common/autofill_util.h"
@@ -30,13 +31,14 @@
 namespace autofill {
 
 // There's an implicit precedence determined by the values assigned here. Email
-// is currently the most important followed by Phone, Address, Credit Card and
-// finally Name.
+// is currently the most important followed by Phone, Address, Credit Card,
+// Name, and Search.
 const float FormField::kBaseEmailParserScore = 1.4f;
 const float FormField::kBasePhoneParserScore = 1.3f;
 const float FormField::kBaseAddressParserScore = 1.2f;
 const float FormField::kBaseCreditCardParserScore = 1.1f;
 const float FormField::kBaseNameParserScore = 1.0f;
+const float FormField::kBaseSearchParserScore = 0.9f;
 
 // static
 FieldCandidatesMap FormField::ParseFormFields(
@@ -77,6 +79,9 @@ FieldCandidatesMap FormField::ParseFormFields(
   // Name pass.
   ParseFormFieldsPass(NameField::Parse, processed_fields, &field_candidates);
 
+  // Search pass.
+  ParseFormFieldsPass(SearchField::Parse, processed_fields, &field_candidates);
+
   // Do not autofill a form if there aren't enough fields. Otherwise, it is
   // very easy to have false positives. See http://crbug.com/447332
   // For <form> tags, make an exception for email fields, which are commonly

components/autofill/core/browser/form_field.h

@@ -37,23 +37,24 @@ class FormField {
   // A bit-field used for matching specific parts of a field in question.
   enum MatchType {
     // Attributes.
-    MATCH_LABEL      = 1 << 0,
-    MATCH_NAME       = 1 << 1,
+    MATCH_LABEL = 1 << 0,
+    MATCH_NAME = 1 << 1,
 
     // Input types.
-    MATCH_TEXT       = 1 << 2,
-    MATCH_EMAIL      = 1 << 3,
-    MATCH_TELEPHONE  = 1 << 4,
-    MATCH_SELECT     = 1 << 5,
-    MATCH_TEXT_AREA  = 1 << 6,
-    MATCH_PASSWORD   = 1 << 7,
-    MATCH_NUMBER     = 1 << 8,
-    MATCH_ALL_INPUTS =
-        MATCH_TEXT | MATCH_EMAIL | MATCH_TELEPHONE | MATCH_SELECT |
-        MATCH_TEXT_AREA | MATCH_PASSWORD | MATCH_NUMBER,
+    MATCH_TEXT = 1 << 2,
+    MATCH_EMAIL = 1 << 3,
+    MATCH_TELEPHONE = 1 << 4,
+    MATCH_SELECT = 1 << 5,
+    MATCH_TEXT_AREA = 1 << 6,
+    MATCH_PASSWORD = 1 << 7,
+    MATCH_NUMBER = 1 << 8,
+    MATCH_SEARCH = 1 << 9,
+    MATCH_ALL_INPUTS = MATCH_TEXT | MATCH_EMAIL | MATCH_TELEPHONE |
+                       MATCH_SELECT | MATCH_TEXT_AREA | MATCH_PASSWORD |
+                       MATCH_NUMBER,
 
     // By default match label and name for input/text types.
-    MATCH_DEFAULT    = MATCH_LABEL | MATCH_NAME | MATCH_TEXT,
+    MATCH_DEFAULT = MATCH_LABEL | MATCH_NAME | MATCH_TEXT,
   };
 
   // Initial values assigned to FieldCandidates by their corresponding parsers.
@@ -62,6 +63,7 @@ class FormField {
   static const float kBaseAddressParserScore;
   static const float kBaseCreditCardParserScore;
   static const float kBaseNameParserScore;
+  static const float kBaseSearchParserScore;
 
   // Only derived classes may instantiate.
   FormField() {}

components/autofill/core/browser/search_field.cc

+// Copyright 2018 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/search_field.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "components/autofill/core/browser/autofill_scanner.h"
+#include "components/autofill/core/common/autofill_regex_constants.h"
+
+namespace autofill {
+
+// static
+std::unique_ptr<FormField> SearchField::Parse(AutofillScanner* scanner) {
+  AutofillField* field;
+  if (ParseFieldSpecifics(scanner, base::UTF8ToUTF16(kSearchTermRe),
+                          MATCH_DEFAULT | MATCH_SEARCH, &field)) {
+    return std::make_unique<SearchField>(field);
+  }
+
+  return nullptr;
+}
+
+SearchField::SearchField(const AutofillField* field) : field_(field) {}
+
+void SearchField::AddClassifications(
+    FieldCandidatesMap* field_candidates) const {
+  AddClassification(field_, SEARCH_TERM, kBaseSearchParserScore,
+                    field_candidates);
+}
+
+}  // namespace autofill

components/autofill/core/browser/search_field.h

+// Copyright 2018 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.
+
+#ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_SEARCH_FIELD_H_
+#define COMPONENTS_AUTOFILL_CORE_BROWSER_SEARCH_FIELD_H_
+
+#include <memory>
+
+#include "base/compiler_specific.h"
+#include "base/macros.h"
+#include "components/autofill/core/browser/form_field.h"
+
+namespace autofill {
+
+// Search fields are not filled by autofill, but identifying them will help
+// to reduce the number of false positives.
+class SearchField : public FormField {
+ public:
+  static std::unique_ptr<FormField> Parse(AutofillScanner* scanner);
+  SearchField(const AutofillField* field);
+
+ protected:
+  void AddClassifications(FieldCandidatesMap* field_candidates) const override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(SearchFieldTest, ParseSearchTerm);
+  FRIEND_TEST_ALL_PREFIXES(SearchFieldTest, ParseNonSearchTerm);
+
+  const AutofillField* field_;
+
+  DISALLOW_COPY_AND_ASSIGN(SearchField);
+};
+
+}  // namespace autofill
+
+#endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_SEARCH_FIELD_H_

components/autofill/core/browser/search_field_unittest.cc

+// Copyright 2018 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/search_field.h"
+
+#include <memory>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/autofill/core/browser/autofill_field.h"
+#include "components/autofill/core/browser/autofill_scanner.h"
+#include "components/autofill/core/common/form_field_data.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using base::ASCIIToUTF16;
+
+namespace autofill {
+
+class SearchFieldTest : public testing::Test {
+ public:
+  SearchFieldTest() {}
+
+ protected:
+  std::vector<std::unique_ptr<AutofillField>> list_;
+  std::unique_ptr<SearchField> field_;
+  FieldCandidatesMap field_candidates_map_;
+
+  // Downcast for tests.
+  static std::unique_ptr<SearchField> Parse(AutofillScanner* scanner) {
+    std::unique_ptr<FormField> field = SearchField::Parse(scanner);
+    return base::WrapUnique(static_cast<SearchField*>(field.release()));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SearchFieldTest);
+};
+
+TEST_F(SearchFieldTest, ParseSearchTerm) {
+  FormFieldData search_field;
+  search_field.form_control_type = "text";
+
+  search_field.label = ASCIIToUTF16("Search");
+  search_field.name = ASCIIToUTF16("search");
+
+  list_.push_back(
+      std::make_unique<AutofillField>(search_field, ASCIIToUTF16("search1")));
+
+  AutofillScanner scanner(list_);
+  field_ = Parse(&scanner);
+  ASSERT_NE(nullptr, field_.get());
+  field_->AddClassifications(&field_candidates_map_);
+  ASSERT_TRUE(field_candidates_map_.find(ASCIIToUTF16("search1")) !=
+              field_candidates_map_.end());
+  EXPECT_EQ(SEARCH_TERM,
+            field_candidates_map_[ASCIIToUTF16("search1")].BestHeuristicType());
+}
+
+TEST_F(SearchFieldTest, ParseNonSearchTerm) {
+  FormFieldData address_field;
+  address_field.form_control_type = "text";
+
+  address_field.label = ASCIIToUTF16("Address");
+  address_field.name = ASCIIToUTF16("address");
+
+  list_.push_back(
+      std::make_unique<AutofillField>(address_field, ASCIIToUTF16("address")));
+
+  AutofillScanner scanner(list_);
+  field_ = Parse(&scanner);
+  ASSERT_EQ(nullptr, field_.get());
+}
+
+}  // namespace autofill

components/autofill/core/common/autofill_regex_constants.cc

@@ -123,6 +123,9 @@ const char kStateRe[] =
     "|地區"                 // zh-TW
     "|^시[·・]?도";         // ko-KR
 
+/////////////////////////////////////////////////////////////////////////////
+// search_field.cc
+/////////////////////////////////////////////////////////////////////////////
 const char kSearchTermRe[] =
     "^q$"
     "|search"