Improve Autocomplete Matches and Handling of Large Results Sets

chrome/browser/autocomplete/history_quick_provider.cc

 // Copyright (c) 2011 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 "chrome/browser/autocomplete/history_quick_provider.h"
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/i18n/break_iterator.h"
@@ skipping 41 matching lines @@
   // BEST_MATCH and there is no inline autocompletion because none of the HQP
   // matches can score highly enough to qualify.
   if ((input.type() == AutocompleteInput::INVALID) ||
       (input.type() == AutocompleteInput::FORCED_QUERY) ||
       (input.matches_requested() == AutocompleteInput::BEST_MATCH &&
        input.prevent_inline_autocomplete()))
     return;
 
   autocomplete_input_ = input;
 
-  // Do some fixup on the user input before matching against it, so we provide
-  // good results for local file paths, input with spaces, etc.
-  if (!FixupUserInput(&autocomplete_input_))

[Peter Kasting, 2011/11/21 20:31:02]

I think removing this may be worse than keeping it.  Without this, you're going
to have trouble matching in URLs with escaped characters, punycode domain names,
etc.

Maybe you can give me some concrete examples of problems this causes and we can
figure out how to fight them?

[mrossetti, 2011/11/21 21:38:25]

Great! I'd appreciate the advice...

When give a search string like '/r/' (as described in the bug), calling
FixupUserInput results in the string 'file:\/\/\/r\/'. This, of course, causes
the HQP to search for history items with the words 'file' and 'r' and then does
a final filter for that exact string ('file:///r/').

It also sounds to me like the unit tests for this should be enhanced by adding
URLs with escaped characters, etc. Could you suggest a few for completeness?

Since the user input can be any combination of plain words and an URL it's
important I get this right.

On 2011/11/21 20:31:02, Peter Kasting wrote:

[Peter Kasting, 2011/11/21 22:09:00]

Hmm.  I didn't think about the fact that we could have input where it's not
clear which tokens go together and which don't.  If my original URL is
"http://who/peter%20kasting", for example, does "peter kasting" mean "peter
kasting" or "peter%20kasting"?  We can't know.  And if we have something like
"frank google.com\mail" then as humans it's probably obvious that we should
treat "google.com\mail" as a URL and fix up the '\' to be a '/', but I don't
know how to code that.

So maybe you're right and this should go away.

My biggest worry is probably unicode domain names.  I can't remember how the
history backend stores those, but if it stores them in punycode, then users
won't be able to type them in and get any HQP matches at all.  (I think similar
issues made the HUP less effective than it ideally would be and I wanted to
change how the backend worked, but it was too big a task or something.)  I don't
know how to address this perfectly.

My only idea here blows up matching time and thus is probably not terribly
feasible:
  * For each whitespace-delimited token, try to fix it up somehow.  Maybe
FixupUserInput(), maybe something that does less?
  * If the fixed up version is different than the original, then allow matching
either version

The fixup stage here we want to do things like convert unicode hostnames,
convert backslashes to forward slashes, and escape characters in URL paths, but
we don't need it to add schemes, convert numbers to dotted quads, etc.  A lot of
this FixupUserInput() goes back and reverses after the fact (like the dotted
quads thing) but perhaps there's a different way to go...

-    return;
-
   // TODO(pkasting): We should just block here until this loads.  Any time
   // someone unloads the history backend, we'll get inconsistent inline
   // autocomplete behavior here.
   if (GetIndex()) {
     base::TimeTicks start_time = base::TimeTicks::Now();
     DoAutocomplete();
     if (input.text().length() < 6) {
       base::TimeTicks end_time = base::TimeTicks::Now();
       std::string name = "HistoryQuickProvider.QueryIndexTime." +
           base::IntToString(input.text().length());
       base::Histogram* counter = base::Histogram::FactoryGet(
           name, 1, 1000, 50, base::Histogram::kUmaTargetedHistogramFlag);
       counter->Add(static_cast<int>((end_time - start_time).InMilliseconds()));
     }
     UpdateStarredStateOfMatches();
   }
 }
 
 // TODO(mrossetti): Implement this function. (Will happen in next CL.)
 void HistoryQuickProvider::DeleteMatch(const AutocompleteMatch& match) {}
 
 void HistoryQuickProvider::DoAutocomplete() {
   // Get the matching URLs from the DB.
   string16 term_string = autocomplete_input_.text();
-  term_string = net::UnescapeURLComponent(term_string,
-      net::UnescapeRule::SPACES | net::UnescapeRule::URL_SPECIAL_CHARS);
-  history::String16Vector terms(
-      history::String16VectorFromString16(term_string, false));
-  ScoredHistoryMatches matches = GetIndex()->HistoryItemsForTerms(terms);
+  ScoredHistoryMatches matches = GetIndex()->HistoryItemsForTerms(term_string);
   if (matches.empty())
     return;
 
   // Artificially reduce the score of high-scoring matches which should not be
   // inline autocompletd. Each such result gets the next available
   // |max_match_score|. Upon use of |max_match_score| it is decremented.
   // All subsequent matches must be clamped to retain match results ordering.
   int max_match_score = autocomplete_input_.prevent_inline_autocomplete() ?
       (AutocompleteResult::kLowestDefaultScore - 1) : -1;
   for (ScoredHistoryMatches::const_iterator match_iter = matches.begin();
@@ skipping 102 matching lines @@
     do {
       offset += matches[i].length;
       ++i;
     } while ((i < match_count) && (offset == matches[i].offset));
     if (offset < text_length)
       spans.push_back(ACMatchClassification(offset, url_style));
   }
 
   return spans;
 }