Changes the HistoryURLProvider such that if the first pass produced an

inline autocomplete match we don't attempt to fixup the 'what you typed
match' in the second pass. The problem scenario is a url you've typed
like 'pandora.com' and a url you haven't typed, like 'p'. If this
happens, the first pass produces 'pandora.com' with a relevancy of
1400, but the second pass produce 'p' with 1200, and
'pandora.com' would get a much lower relevancy. If the SearchProvider
also provided an inline autocomplete match > 1200, then the omnibox
flickers between the results.

TEST=covered by unit test
BUG=77677
R=pkasting@chromium.org

Review URL: http://codereview.chromium.org/6749039

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80311 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 45 insertions, 4 deletions

diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index 545f98a..3b5354a 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -112,7 +112,8 @@ HistoryURLProviderParams::HistoryURLProviderParams(
       trim_http(trim_http),
       cancel(false),
       failed(false),
-      languages(languages) {
+      languages(languages),
+      dont_suggest_exact_input(false) {
 }
 
 HistoryURLProviderParams::~HistoryURLProviderParams() {}
@@ -235,6 +236,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
   // Checking |is_history_what_you_typed_match| tells us whether
   // SuggestExactInput() succeeded in constructing a valid match.
   if (what_you_typed_match.is_history_what_you_typed_match &&
+      (!backend || !params->dont_suggest_exact_input) &&
       FixupExactSuggestion(db, params->input, &what_you_typed_match,
                            &history_matches)) {
     // Got an exact match for the user's input.  Treat it as the best match
@@ -421,6 +423,13 @@ bool HistoryURLProvider::PromoteMatchForInlineAutocomplete(
        !history::IsHostOnly(match.url_info.url())))
     return false;
 
+  // In the case where the user has typed "foo.com" and visited (but not typed)
+  // "foo/", and the input is "foo", we can reach here for "foo.com" during the
+  // first pass but have the second pass suggest the exact input as a better
+  // URL.  Since we need both passes to agree, and since during the first pass
+  // there's no way to know about "foo/", make reaching this point prevent any
+  // future pass from suggesting the exact input as a better match.
+  params->dont_suggest_exact_input = true;
   params->matches.push_back(HistoryMatchToACMatch(params, match,
                                                   INLINE_AUTOCOMPLETE, 0));
   return true;
diff --git a/chrome/browser/autocomplete/history_url_provider.h b/chrome/browser/autocomplete/history_url_provider.h
index ac93c57..f9b9a9f 100644
--- a/chrome/browser/autocomplete/history_url_provider.h
+++ b/chrome/browser/autocomplete/history_url_provider.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -121,6 +121,9 @@ struct HistoryURLProviderParams {
   // Languages we should pass to gfx::GetCleanStringFromUrl.
   std::string languages;
 
+  // When true, we should avoid calling SuggestExactInput().
+  bool dont_suggest_exact_input;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(HistoryURLProviderParams);
 };
diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc
index 89df897..4d0de7f 100644
--- a/chrome/browser/autocomplete/history_url_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc
@@ -90,6 +90,10 @@ struct TestURLInfo {
   {"http://binky/", "Intranet binky", 2, 2},
   {"http://winky/", "Intranet winky", 2, 2},
   {"http://www.winky.com/", "Internet winky", 5, 0},
+
+  // URLs used by EmptyVisits.
+  {"http://pandora.com/", "Pandora", 2, 2},
+  {"http://p/", "p", 0, 0},
 };
 
 class HistoryURLProviderTest : public TestingBrowserProcessTest,
@@ -131,8 +135,6 @@ class HistoryURLProviderTest : public TestingBrowserProcessTest,
   ACMatches matches_;
   scoped_ptr<TestingProfile> profile_;
   HistoryService* history_service_;
-
- private:
   scoped_refptr<HistoryURLProvider> autocomplete_;
 };
 
@@ -451,6 +453,33 @@ TEST_F(HistoryURLProviderTest, AdjustOffset) {
   RunAdjustOffsetTest(ASCIIToUTF16("http://ms/c++ s"), 15);
 }
 
+// Make sure the results for the input 'p' don't change between the first and
+// second passes.
+TEST_F(HistoryURLProviderTest, EmptyVisits) {
+  // Wait for history to create the in memory DB.
+  profile_->BlockUntilHistoryProcessesPendingRequests();
+
+  AutocompleteInput input(ASCIIToUTF16("p"), string16(), false, false, true,
+                          AutocompleteInput::ALL_MATCHES);
+  autocomplete_->Start(input, false);
+  // HistoryURLProvider shouldn't be done (waiting on async results).
+  EXPECT_FALSE(autocomplete_->done());
+
+  // We should get back an entry for pandora.
+  matches_ = autocomplete_->matches();
+  ASSERT_GT(matches_.size(), 0u);
+  EXPECT_EQ(GURL("http://pandora.com/"), matches_[0].destination_url);
+  int pandora_relevance = matches_[0].relevance;
+
+  // Run the message loop. When |autocomplete_| finishes the loop is quit.
+  MessageLoop::current()->Run();
+  EXPECT_TRUE(autocomplete_->done());
+  matches_ = autocomplete_->matches();
+  ASSERT_GT(matches_.size(), 0u);
+  EXPECT_EQ(GURL("http://pandora.com/"), matches_[0].destination_url);
+  EXPECT_EQ(pandora_relevance, matches_[0].relevance);
+}
+
 TEST_F(HistoryURLProviderTestNoDB, NavigateWithoutDB) {
   // Ensure that we will still produce matches for navigation when there is no
   // database.