[Autofill/Autocomplete Feature] Substring matching instead of prefix matching.

components/autofill/core/common/autofill_util.cc

+// Copyright 2015 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/common/autofill_util.h"
+
+#include <vector>
+
+#include "base/command_line.h"
+#include "base/i18n/case_conversion.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "components/autofill/core/common/autofill_switches.h"
+
+namespace autofill {
+
+namespace {
+
+base::string16 kTokenSeperatorsUTF16 = base::ASCIIToUTF16(" .,-_@");

[vabr (Chromium), 2015/03/31 09:38:07]

I'm surprised none of the compilers complained about static variable with a
non-trivial destructor.
The guide complains, however --
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Static_and_Gl...
forbids static variables of class type.

[Pritam Nikam, 2015/03/31 14:26:12]

Done.

Removed it and used in-place substitution.

+
+}  // namespace
+
+bool IsFeatureSubstringMatchEnabled() {
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableSuggestionsWithSubstringMatch);
+}
+
+bool IsContentsPrefixOfSuggestionToken(const base::string16& field_suggestion,
+                                       const base::string16& field_contents,
+                                       bool case_sensitive) {
+  if (field_suggestion.empty() || field_contents.empty()) {
+    return false;
+  }
+
+  bool result = false;
+  base::string16 search_string =
+      case_sensitive ? field_contents : base::i18n::ToLower(field_contents);
+  base::string16 suggestion_string =
+      case_sensitive ? field_suggestion : base::i18n::ToLower(field_suggestion);
+  base::string16::size_type offset = suggestion_string.find(search_string);

[vabr (Chromium), 2015/03/31 09:38:07]

Why are you doing this pre-search?
If it is just for performance, I'm not so sure -- converting to lowercase on
lines 37-40 might also cost something.

Keeping the code to minimum helps eliminate future bugs and incerases
readability, so there is a value in getting rid of lines 37-42. If performance
is your only reason to keep them, could you please do a benchmark to justify the
need for them?

[Pritam Nikam, 2015/03/31 14:26:11]

Done.

+  if (base::string16::npos != offset) {
+    std::vector<base::string16> suggestion_tokens;
+    Tokenize(suggestion_string, kTokenSeperatorsUTF16, &suggestion_tokens);
+
+    // Check whether the |search_string| prefixes any of the
+    // |suggestion_string|'s token.
+    if (suggestion_tokens.size() != 0) {

[vabr (Chromium), 2015/03/31 09:38:07]

You do not really need this if. For size() == 0 the for loop below won't
execute.
Also -- next time you need to check if a collection is empty, use .empty(), not
comparing .size() to 0 (which is less readable and generally less efficient).

[Pritam Nikam, 2015/03/31 14:26:12]

Done.

+      for (auto token : suggestion_tokens) {
+        result = StartsWith(token, search_string, case_sensitive);
+        if (result)
+          break;
+      }
+    }
+  }
+
+  return result;
+}
+
+base::string16::size_type ComputeRange(const base::string16& field_suggestion,
+                                       const base::string16& field_contents,
+                                       size_t* start,
+                                       size_t* end) {
+  base::string16 user_input = base::i18n::ToLower(field_contents);
+  base::string16 suggestion = base::i18n::ToLower(field_suggestion);
+  base::string16::size_type offset = suggestion.find(user_input);
+  if (base::string16::npos != offset) {
+    *start = offset + user_input.size();
+    *end = suggestion.size();
+  }
+
+  return offset;
+}
+
+}  // namespace autofill