Omnibox: Prevent Asynchronous Suggestions from Changing Default Match

chrome/browser/autocomplete/search_provider_unittest.cc

 // Copyright 2012 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/search_provider.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
@@ skipping 121 matching lines @@
     const bool allowed_to_be_default_match;
     const base::string16 fill_into_edit;
   };
 
   struct TestData {
     const base::string16 input;
     const size_t num_results;
     const ResultInfo output[3];
   };
 
+  struct ExpectedMatch {
+    std::string contents;
+    bool allowed_to_be_default_match;
+  };
+
   SearchProviderTest()
       : default_t_url_(NULL),
         term1_(ASCIIToUTF16("term1")),
         keyword_t_url_(NULL),
         keyword_term_(ASCIIToUTF16("keyword")),
         run_loop_(NULL) {
     ResetFieldTrialList();
   }
 
   // See description above class for what this registers.
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
 
   void RunTest(TestData* cases, int num_cases, bool prefer_keyword);
 
  protected:
   // Needed for AutocompleteFieldTrial::ActivateStaticTrials();
   scoped_ptr<base::FieldTrialList> field_trial_list_;
 
-  // Default value used for testing.
+  // Default values used for testing.
   static const std::string kNotApplicable;
+  static const ExpectedMatch kEmptyExpectedMatch;

[msw, 2014/08/27 18:44:39]

nit: use |kEmptyMatch| for less line wrapping with this.

[Mark P, 2014/08/27 23:08:19]

Can't do that without changing the name of countless kEmptyMatch local variables
for various tests below.

[msw, 2014/08/28 19:21:40]

Acknowledged.

 
   // Adds a search for |term|, using the engine |t_url| to the history, and
   // returns the URL for that search.
   GURL AddSearchToHistory(TemplateURL* t_url, base::string16 term, int visit_count);
 
   // Looks for a match in |provider_| with |contents| equal to |contents|.
   // Sets |match| to it if found.  Returns whether |match| was set.
   bool FindMatchWithContents(const base::string16& contents,
                              AutocompleteMatch* match);
 
@@ skipping 27 matching lines @@
   // Runs SearchProvider on |input|, for which the suggest server replies
   // with |json|, and expects that the resulting matches' contents equals
   // that in |matches|.  An empty entry in |matches| means no match should
   // be returned in that position.  Reports any errors with a message that
   // includes |error_description|.
   void ForcedQueryTestHelper(const std::string& input,
                              const std::string& json,
                              const std::string matches[3],
                              const std::string& error_description);
 
+  // Verifies that |matches| and |expected_matches| agree on the first
+  // |num_expected_matches|, displaying an error message that includes
+  // |description| for any disagreement.
+  void CheckMatches(const std::string& description,
+                    const size_t num_expected_matches,
+                    const ExpectedMatch expected_matches[],
+                    const ACMatches& matches);
+
   void ResetFieldTrialList();
 
   // Create a field trial, with ZeroSuggest activation based on |enabled|.
   base::FieldTrial* CreateZeroSuggestFieldTrial(bool enabled);
 
   void ClearAllResults();
 
   // See description above class for details of these fields.
   TemplateURL* default_t_url_;
   const base::string16 term1_;
@@ skipping 14 matching lines @@
   scoped_refptr<SearchProviderForTest> provider_;
 
   // If non-NULL, OnProviderUpdate quits the current |run_loop_|.
   base::RunLoop* run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchProviderTest);
 };
 
 // static
 const std::string SearchProviderTest::kNotApplicable = "Not Applicable";
+const SearchProviderTest::ExpectedMatch
+    SearchProviderTest::kEmptyExpectedMatch = { kNotApplicable, false };
 
 void SearchProviderTest::SetUp() {
   // Make sure that fetchers are automatically ungregistered upon destruction.
   test_factory_.set_remove_fetcher_on_delete(true);
 
   // We need both the history service and template url model loaded.
   ASSERT_TRUE(profile_.CreateHistoryService(true, false));
   TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
       &profile_, &TemplateURLServiceFactory::BuildInstanceFor);
 
@@ skipping 180 matching lines @@
   // Tell the SearchProvider the default suggest query is done.
   default_fetcher->set_response_code(200);
   default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
 }
 
 void SearchProviderTest::ForcedQueryTestHelper(
     const std::string& input,
     const std::string& json,
     const std::string expected_matches[3],
     const std::string& error_description) {
-  QueryForInput(ASCIIToUTF16(input), false, false);
-  net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
-      SearchProvider::kDefaultProviderURLFetcherID);
-  ASSERT_TRUE(fetcher);
-  fetcher->set_response_code(200);
-  fetcher->SetResponseString(json);
-  fetcher->delegate()->OnURLFetchComplete(fetcher);
-  RunTillProviderDone();
+  // Send the query twice in order to have a synchronous pass after the first
+  // reply is received.  This is necessary because SearchProvider doesn't allow
+  // an asynchronous reply to change the default match.
+  for (size_t i = 0; i < 2; i++) {

[msw, 2014/08/27 18:44:38]

nit: ++i

[Mark P, 2014/08/27 23:08:19]

Done throughout.

+    QueryForInput(ASCIIToUTF16(input), false, false);
+    net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
+        SearchProvider::kDefaultProviderURLFetcherID);
+    ASSERT_TRUE(fetcher);
+    fetcher->set_response_code(200);
+    fetcher->SetResponseString(json);
+    fetcher->delegate()->OnURLFetchComplete(fetcher);
+    RunTillProviderDone();
+  }
 
   const ACMatches& matches = provider_->matches();
   ASSERT_LE(matches.size(), 3u);
   size_t i = 0;
   // Ensure that the returned matches equal the expectations.
   for (; i < matches.size(); ++i) {
     EXPECT_EQ(ASCIIToUTF16(expected_matches[i]), matches[i].contents) <<
         error_description;
   }
   // Ensure that no expected matches are missing.
   for (; i < 3u; ++i) {
     EXPECT_EQ(std::string(), expected_matches[i]) <<
         "Case #" << i << ": " << error_description;
   }
 }
 
+void SearchProviderTest::CheckMatches(const std::string& description,
+                                      const size_t num_expected_matches,
+                                      const ExpectedMatch expected_matches[],
+                                      const ACMatches& matches) {
+  ASSERT_FALSE(matches.empty());
+  ASSERT_LE(matches.size(), num_expected_matches);
+  size_t j = 0;

[msw, 2014/08/27 18:44:38]

nit: use |i|

[Mark P, 2014/08/27 23:08:20]

Done.  (refactoring error)

+  // Ensure that the returned matches equal the expectations.
+  for (; j < matches.size(); ++j) {
+    EXPECT_EQ(ASCIIToUTF16(expected_matches[j].contents),
+              matches[j].contents) << description << " Case # " << j;
+    EXPECT_EQ(expected_matches[j].allowed_to_be_default_match,
+              matches[j].allowed_to_be_default_match) << description
+                                                      << " Case # " << j;
+  }
+  // Ensure that no expected matches are missing.
+  for (; j < num_expected_matches; ++j) {
+    EXPECT_EQ(kNotApplicable, expected_matches[j].contents) <<
+        "Case # " << j << " " << description;
+  }
+}
+
 void SearchProviderTest::ResetFieldTrialList() {
   // Destroy the existing FieldTrialList before creating a new one to avoid
   // a DCHECK.
   field_trial_list_.reset();
   field_trial_list_.reset(new base::FieldTrialList(
       new metrics::SHA1EntropyProvider("foo")));
   variations::testing::ClearAllVariationParams();
   base::FieldTrial* trial = base::FieldTrialList::CreateFieldTrial(
       "AutocompleteDynamicTrial_0", "DefaultGroup");
   trial->group();
@@ skipping 645 matching lines @@
   fetcher->delegate()->OnURLFetchComplete(fetcher);
   fetcher = NULL;
 
   // Run till the history results complete.
   RunTillProviderDone();
 
   // Make sure the only match is 'a.com' and it doesn't have a template_url.
   AutocompleteMatch nav_match;
   EXPECT_TRUE(FindMatchWithDestination(GURL("http://a.com"), &nav_match));
   EXPECT_TRUE(nav_match.keyword.empty());
-  EXPECT_TRUE(nav_match.allowed_to_be_default_match);
+  EXPECT_FALSE(nav_match.allowed_to_be_default_match);
   EXPECT_FALSE(FindMatchWithDestination(GURL("http://a.com/b"), &nav_match));
 }
 
 // Verifies that the most relevant suggest results are added properly.
 TEST_F(SearchProviderTest, SuggestRelevance) {
   QueryForInput(ASCIIToUTF16("a"), false, false);
 
   // Make sure the default provider's suggest service was queried.
   net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
       SearchProvider::kDefaultProviderURLFetcherID);
@@ skipping 12 matching lines @@
   AutocompleteMatch verbatim, match_a1, match_a2, match_a3, match_a4;
   EXPECT_TRUE(FindMatchWithContents(ASCIIToUTF16("a"), &verbatim));
   EXPECT_TRUE(FindMatchWithContents(ASCIIToUTF16("a1"), &match_a1));
   EXPECT_TRUE(FindMatchWithContents(ASCIIToUTF16("a2"), &match_a2));
   EXPECT_TRUE(FindMatchWithContents(ASCIIToUTF16("a3"), &match_a3));
   EXPECT_FALSE(FindMatchWithContents(ASCIIToUTF16("a4"), &match_a4));
   EXPECT_GT(verbatim.relevance, match_a1.relevance);
   EXPECT_GT(match_a1.relevance, match_a2.relevance);
   EXPECT_GT(match_a2.relevance, match_a3.relevance);
   EXPECT_TRUE(verbatim.allowed_to_be_default_match);
-  EXPECT_TRUE(match_a1.allowed_to_be_default_match);
-  EXPECT_TRUE(match_a2.allowed_to_be_default_match);
-  EXPECT_TRUE(match_a3.allowed_to_be_default_match);
+  EXPECT_FALSE(match_a1.allowed_to_be_default_match);
+  EXPECT_FALSE(match_a2.allowed_to_be_default_match);
+  EXPECT_FALSE(match_a3.allowed_to_be_default_match);
 }
 
 // Verifies that the default provider abandons suggested relevance scores
 // when in keyword mode.  This should happen regardless of whether the
 // keyword provider returns suggested relevance scores.
 TEST_F(SearchProviderTest, DefaultProviderNoSuggestRelevanceInKeywordMode) {
   struct {
     const std::string default_provider_json;
     const std::string keyword_provider_json;
     const std::string matches[5];
@@ skipping 12 matching lines @@
       "{\"google:verbatimrelevance\":9700,"
       "\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"],"
       "\"google:suggestrelevance\":[9900, 9800]}]",
       "[\"a\",[\"akeyword-query\"],[],[],{\"google:suggesttype\":[\"QUERY\"],"
       "\"google:verbatimrelevance\":9500,"
       "\"google:suggestrelevance\":[9600]}]",
       { "akeyword-query", "a", "k a", "adefault.com", "k adefault-query" } }
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
-    QueryForInput(ASCIIToUTF16("k a"), false, true);
-    net::TestURLFetcher* default_fetcher =
-        test_factory_.GetFetcherByID(
-            SearchProvider::kDefaultProviderURLFetcherID);
-    ASSERT_TRUE(default_fetcher);
-    default_fetcher->set_response_code(200);
-    default_fetcher->SetResponseString(cases[i].default_provider_json);
-    default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
-    net::TestURLFetcher* keyword_fetcher =
-        test_factory_.GetFetcherByID(
-            SearchProvider::kKeywordProviderURLFetcherID);
-    ASSERT_TRUE(keyword_fetcher);
-    keyword_fetcher->set_response_code(200);
-    keyword_fetcher->SetResponseString(cases[i].keyword_provider_json);
-    keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher);
-    RunTillProviderDone();
+    // Send the query twice in order to have a synchronous pass after the first
+    // reply is received.  This is necessary because SearchProvider doesn't
+    // allow an asynchronous reply to change the default match.
+    for (size_t j = 0; j < 2; j++) {

[msw, 2014/08/27 18:44:38]

nit: ++j

[Mark P, 2014/08/27 23:08:20]

Done.

+      QueryForInput(ASCIIToUTF16("k a"), false, true);
+      net::TestURLFetcher* default_fetcher =
+          test_factory_.GetFetcherByID(
+              SearchProvider::kDefaultProviderURLFetcherID);
+      ASSERT_TRUE(default_fetcher);
+      default_fetcher->set_response_code(200);
+      default_fetcher->SetResponseString(cases[i].default_provider_json);
+      default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
+      net::TestURLFetcher* keyword_fetcher =
+          test_factory_.GetFetcherByID(
+              SearchProvider::kKeywordProviderURLFetcherID);
+      ASSERT_TRUE(keyword_fetcher);
+      keyword_fetcher->set_response_code(200);
+      keyword_fetcher->SetResponseString(cases[i].keyword_provider_json);
+      keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher);
+      RunTillProviderDone();
+    }
 
     const std::string description = "for input with default_provider_json=" +
         cases[i].default_provider_json + " and keyword_provider_json=" +
         cases[i].keyword_provider_json;
     const ACMatches& matches = provider_->matches();
     ASSERT_LE(matches.size(), ARRAYSIZE_UNSAFE(cases[i].matches));
     size_t j = 0;
     // Ensure that the returned matches equal the expectations.

[msw, 2014/08/27 18:44:39]

Too bad this doesn't check |allowed_to_be_default_match|, otherwise you could
use CheckMatches. Maybe you could generalize that or extract the checking code
from ForcedQueryTestHelper to use here and elsewhere? (okay to postpone cleanup)

[Mark P, 2014/08/27 23:08:19]

It would be nice if the helper functions in the unit tests would be refactored
for less code duplication.  It's not high on my list, and I'd rather not do it
for this (already big) changelist.  Postponed.

[msw, 2014/08/28 19:21:39]

Acknowledged.

     for (; j < matches.size(); ++j) {
       EXPECT_EQ(ASCIIToUTF16(cases[i].matches[j]), matches[j].contents) <<
           description;
     }
     // Ensure that no expected matches are missing.
     for (; j < ARRAYSIZE_UNSAFE(cases[i].matches); ++j)
       EXPECT_EQ(std::string(), cases[i].matches[j]) << description;
   }
 }
 
@@ skipping 29 matching lines @@
     { "[\"a\",[\"http://b.com\", \"http://c.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"]}]",
       { { "a", true }, { "b.com", false }, kEmptyMatch, kEmptyMatch,
         kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // Ensure that verbatimrelevance scores reorder or suppress verbatim.
     // Negative values will have no effect; the calculated value will be used.
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":9999,"
                              "\"google:suggestrelevance\":[9998]}]",
-      { { "a", true}, { "a1", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
+      { { "a", true}, { "a1", false }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       std::string() },
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":9998,"
                              "\"google:suggestrelevance\":[9999]}]",
       { { "a1", true }, { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "1" },
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":0,"
                              "\"google:suggestrelevance\":[9999]}]",
       { { "a1", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "1" },
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":-1,"
                              "\"google:suggestrelevance\":[9999]}]",
       { { "a1", true }, { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "1" },
     { "[\"a\",[\"http://a.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:verbatimrelevance\":9999,"
         "\"google:suggestrelevance\":[9998]}]",
-      { { "a", true }, { "a.com", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
-        kEmptyMatch },
+      { { "a", true }, { "a.com", false }, kEmptyMatch, kEmptyMatch,
+        kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:verbatimrelevance\":9998,"
         "\"google:suggestrelevance\":[9999]}]",
       { { "a.com", true }, { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       ".com" },
     { "[\"a\",[\"http://a.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:verbatimrelevance\":0,"
         "\"google:suggestrelevance\":[9999]}]",
       { { "a.com", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       ".com" },
     { "[\"a\",[\"http://a.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:verbatimrelevance\":-1,"
         "\"google:suggestrelevance\":[9999]}]",
       { { "a.com", true }, { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       ".com" },
 
     // Ensure that both types of relevance scores reorder matches together.
     { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[9999, 9997],"
                                      "\"google:verbatimrelevance\":9998}]",
-      { { "a1", true }, { "a", true }, { "a2", true }, kEmptyMatch, kEmptyMatch,
-        kEmptyMatch },
+      { { "a1", true }, { "a", true }, { "a2", false }, kEmptyMatch,
+        kEmptyMatch, kEmptyMatch },
       "1" },
 
-    // Allow non-inlineable matches to be the highest-scoring match but,
-    // if the result set lacks a single inlineable result, abandon suggested
-    // relevance scores entirely.
+    // Check that an inlineable result appears first regardless of its score.
+    // Also, if the result set lacks a single inlineable result, abandon the
+    // request to suppress verbatim (verbatim_relevance=0), which will then
+    // cause verbatim to appear (first).
     { "[\"a\",[\"b\"],[],[],{\"google:suggestrelevance\":[9999]}]",
-      { { "b", false }, { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
+      { { "a", true }, { "b", false }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       std::string() },
     { "[\"a\",[\"b\"],[],[],{\"google:suggestrelevance\":[9999],"
                             "\"google:verbatimrelevance\":0}]",
       { { "a", true }, { "b", false }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://b.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:suggestrelevance\":[9999]}]",
-      { { "b.com", false }, { "a", true }, kEmptyMatch, kEmptyMatch,
+      { { "a", true }, { "b.com", false }, kEmptyMatch, kEmptyMatch,
         kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://b.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:suggestrelevance\":[9999],"
         "\"google:verbatimrelevance\":0}]",
       { { "a", true }, { "b.com", false }, kEmptyMatch, kEmptyMatch,
         kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // Allow low-scoring matches.
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":0}]",
       { { "a1", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "1" },
-    { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":1}]",
+    { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":10}]",
       { { "a1", true }, { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "1" },
-    { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[1],"
+    { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[10],"
                              "\"google:verbatimrelevance\":0}]",
       { { "a1", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "1" },
-    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[1, 2],"
+    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[10, 20],"
                                      "\"google:verbatimrelevance\":0}]",
-      { { "a2", true }, { "a1", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
+      { { "a2", true }, { "a1", false }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       "2" },
-    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[1, 3],"
-      "\"google:verbatimrelevance\":2}]",
-      { { "a2", true }, { "a", true }, { "a1", true }, kEmptyMatch, kEmptyMatch,
-        kEmptyMatch },
+    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[10, 30],"
+      "\"google:verbatimrelevance\":20}]",
+      { { "a2", true }, { "a", true }, { "a1", false }, kEmptyMatch,
+        kEmptyMatch, kEmptyMatch },
       "2" },
     { "[\"a\",[\"http://a.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
-        "\"google:suggestrelevance\":[1],"
+        "\"google:suggestrelevance\":[10],"
         "\"google:verbatimrelevance\":0}]",
       { { "a.com", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       ".com" },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
-        "\"google:suggestrelevance\":[1, 2],"
+        "\"google:suggestrelevance\":[10, 20],"
         "\"google:verbatimrelevance\":0}]",
-      { { "a2.com", true }, { "a1.com", true }, kEmptyMatch, kEmptyMatch,
+      { { "a2.com", true }, { "a1.com", false }, kEmptyMatch, kEmptyMatch,
         kEmptyMatch, kEmptyMatch },
       "2.com" },
 
     // Ensure that all suggestions are considered, regardless of order.
     { "[\"a\",[\"b\", \"c\", \"d\", \"e\", \"f\", \"g\", \"h\"],[],[],"
-       "{\"google:suggestrelevance\":[1, 2, 3, 4, 5, 6, 7]}]",
+       "{\"google:suggestrelevance\":[10, 20, 30, 40, 50, 60, 70]}]",
       { { "a", true }, { "h", false }, { "g", false }, { "f", false },
         { "e", false }, { "d", false } },
       std::string() },
     { "[\"a\",[\"http://b.com\", \"http://c.com\", \"http://d.com\","
               "\"http://e.com\", \"http://f.com\", \"http://g.com\","
               "\"http://h.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\","
                                 "\"NAVIGATION\", \"NAVIGATION\","
                                 "\"NAVIGATION\", \"NAVIGATION\","
                                 "\"NAVIGATION\"],"
-        "\"google:suggestrelevance\":[1, 2, 3, 4, 5, 6, 7]}]",
+        "\"google:suggestrelevance\":[10, 20, 30, 40, 50, 60, 70]}]",
       { { "a", true }, { "h.com", false }, { "g.com", false },
         { "f.com", false }, { "e.com", false }, { "d.com", false } },
       std::string() },
 
     // Ensure that incorrectly sized suggestion relevance lists are ignored.
-    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[1]}]",
-      { { "a", true }, { "a1", true }, { "a2", true }, kEmptyMatch, kEmptyMatch,
-        kEmptyMatch },
+    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[10]}]",
+      { { "a", true }, { "a1", false }, { "a2", false }, kEmptyMatch,
+        kEmptyMatch, kEmptyMatch },
       std::string() },
-    { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[9999, 1]}]",
-      { { "a", true }, { "a1", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
+    { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[9999, 10]}]",
+      { { "a", true }, { "a1", false }, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
-        "\"google:suggestrelevance\":[1]}]",
-      { { "a", true }, { "a1.com", true }, kEmptyMatch, kEmptyMatch,
+        "\"google:suggestrelevance\":[10]}]",
+      { { "a", true }, { "a1.com", false }, kEmptyMatch, kEmptyMatch,
         kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a1.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
-       "\"google:suggestrelevance\":[9999, 1]}]",
-      { { "a", true }, { "a1.com", true }, kEmptyMatch, kEmptyMatch,
+       "\"google:suggestrelevance\":[9999, 10]}]",
+      { { "a", true }, { "a1.com", false }, kEmptyMatch, kEmptyMatch,
         kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // Ensure that all 'verbatim' results are merged with their maximum score.
     { "[\"a\",[\"a\", \"a1\", \"a2\"],[],[],"
        "{\"google:suggestrelevance\":[9998, 9997, 9999]}]",
-      { { "a2", true }, { "a", true }, { "a1", true }, kEmptyMatch, kEmptyMatch,
-        kEmptyMatch },
+      { { "a2", true }, { "a", true }, { "a1", false }, kEmptyMatch,
+        kEmptyMatch, kEmptyMatch },
       "2" },
     { "[\"a\",[\"a\", \"a1\", \"a2\"],[],[],"
        "{\"google:suggestrelevance\":[9998, 9997, 9999],"
         "\"google:verbatimrelevance\":0}]",
-      { { "a2", true }, { "a", true }, { "a1", true }, kEmptyMatch, kEmptyMatch,
-        kEmptyMatch },
+      { { "a2", true }, { "a", true }, { "a1", false }, kEmptyMatch,
+        kEmptyMatch, kEmptyMatch },
       "2" },
 
     // Ensure that verbatim is always generated without other suggestions.
     // TODO(msw): Ensure verbatimrelevance is respected (except suppression).
     { "[\"a\",[],[],[],{\"google:verbatimrelevance\":1}]",
       { { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       std::string() },
     { "[\"a\",[],[],[],{\"google:verbatimrelevance\":0}]",
       { { "a", true }, kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch,
         kEmptyMatch },
       std::string() },
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
-    QueryForInput(ASCIIToUTF16("a"), false, false);
-    net::TestURLFetcher* fetcher =
-        test_factory_.GetFetcherByID(
-            SearchProvider::kDefaultProviderURLFetcherID);
-    ASSERT_TRUE(fetcher);
-    fetcher->set_response_code(200);
-    fetcher->SetResponseString(cases[i].json);
-    fetcher->delegate()->OnURLFetchComplete(fetcher);
-    RunTillProviderDone();
+    // Send the query twice in order to have a synchronous pass after the first
+    // reply is received.  This is necessary because SearchProvider doesn't
+    // allow an asynchronous reply to change the default match.
+    for (size_t j = 0; j < 2; j++) {

[msw, 2014/08/27 18:44:38]

nit: ++j

[Mark P, 2014/08/27 23:08:20]

Done.

+      QueryForInput(ASCIIToUTF16("a"), false, false);
+      net::TestURLFetcher* fetcher =
+          test_factory_.GetFetcherByID(
+              SearchProvider::kDefaultProviderURLFetcherID);
+      ASSERT_TRUE(fetcher);
+      fetcher->set_response_code(200);
+      fetcher->SetResponseString(cases[i].json);
+      fetcher->delegate()->OnURLFetchComplete(fetcher);
+      RunTillProviderDone();
+    }
 
     const std::string description = "for input with json=" + cases[i].json;
     const ACMatches& matches = provider_->matches();
     ASSERT_FALSE(matches.empty());
     // Find the first match that's allowed to be the default match and check
     // its inline_autocompletion.
     ACMatches::const_iterator it = FindDefaultMatch(matches);
     ASSERT_NE(matches.end(), it);
     EXPECT_EQ(ASCIIToUTF16(cases[i].inline_autocompletion),
               it->inline_autocompletion) << description;
 
     ASSERT_LE(matches.size(), ARRAYSIZE_UNSAFE(cases[i].matches));
     size_t j = 0;
     // Ensure that the returned matches equal the expectations.

[msw, 2014/08/27 18:44:38]

Can this use CheckMatches?

[Mark P, 2014/08/27 23:08:20]

Yes.  Good catch.  Done.

     for (; j < matches.size(); ++j) {
       EXPECT_EQ(ASCIIToUTF16(cases[i].matches[j].contents),
                 matches[j].contents) << description;
       EXPECT_EQ(cases[i].matches[j].allowed_to_be_default_match,
                 matches[j].allowed_to_be_default_match) << description;
     }
     // Ensure that no expected matches are missing.
     for (; j < ARRAYSIZE_UNSAFE(cases[i].matches); ++j)
       EXPECT_EQ(kNotApplicable, cases[i].matches[j].contents) <<
           "Case # " << i << " " << description;
@@ skipping 55 matching lines @@
         { "b.com", false, false },
         { "k a",   false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // Ensure that verbatimrelevance scores reorder or suppress verbatim.
     // Negative values will have no effect; the calculated value will be used.
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":9999,"
                              "\"google:suggestrelevance\":[9998]}]",
       { { "a",   true,  true },
-        { "a1",  true,  true },
+        { "a1",  true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":9998,"
                              "\"google:suggestrelevance\":[9999]}]",
       { { "a1",  true,  true },
         { "a",   true,  true },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       "1" },
@@ skipping 18 matching lines @@
         { "a.com", false, false },
         { "k a",   false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // Ensure that both types of relevance scores reorder matches together.
     { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[9999, 9997],"
                                      "\"google:verbatimrelevance\":9998}]",
       { { "a1",  true,  true },
         { "a",   true,  true },
-        { "a2",  true,  true },
+        { "a2",  true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch },
       "1" },
 
-    // Check that non-inlinable matches may be ranked as the highest result
-    // if there is at least one inlineable match.
+    // Check that an inlinable match appears first regardless of its score.

[msw, 2014/08/27 18:44:38]

nit: "inlineable" seems more comment, fix up all the "inlinable" spellings.

[Mark P, 2014/08/27 23:08:20]

Fixed all (two).

     { "[\"a\",[\"b\"],[],[],{\"google:suggestrelevance\":[9999]}]",
-      { { "b",   true,  false },
-        { "a",   true,  true },
+      { { "a",   true,  true },
+        { "b",   true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://b.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:suggestrelevance\":[9999]}]",
-      { { "b.com", false, false },
-        { "a",     true,  true },
+      { { "a",     true,  true },
+        { "b.com", false, false },
         { "k a",   false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
-    // On the other hand, if there is no inlineable match, restore
-    // the keyword verbatim score.
+    // If there is no inlineable match, restore the keyword verbatim score.
+    // The keyword verbatim match will then appear first.
     { "[\"a\",[\"b\"],[],[],{\"google:suggestrelevance\":[9999],"
                             "\"google:verbatimrelevance\":0}]",
-      { { "b",   true,  false },
-        { "a",   true,  true },
+      { { "a",   true,  true },
+        { "b",   true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://b.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:suggestrelevance\":[9999],"
         "\"google:verbatimrelevance\":0}]",
-      { { "b.com", false, false },
-        { "a",     true,  true },
+      { { "a",     true,  true },
+        { "b.com", false, false },
         { "k a",   false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // The top result does not have to score as highly as calculated
     // verbatim.  i.e., there are no minimum score restrictions in
     // this provider.
     { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":0}]",
       { { "a1",  true,  true },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch },
       "1" },
-    { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":1}]",
+    { "[\"a\",[\"a1\"],[],[],{\"google:verbatimrelevance\":10}]",
       { { "a1",  true,  true },
         { "k a", false, false },
         { "a",   true,  true },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       "1" },
-    { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[1],"
+    { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[10],"
                              "\"google:verbatimrelevance\":0}]",
-      { { "k a", false, false },
-        { "a1",   true, true },
+      { { "a1",  true,  true },
+        { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch },
       "1" },
-    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[1, 2],"
+    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[10, 20],"
                                      "\"google:verbatimrelevance\":0}]",
-      {
+      { { "a2",  true,  true },
         { "k a", false, false },
-        { "a2",  true,  true },
-        { "a1",  true,  true },
+        { "a1",  true,  false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       "2" },
-    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[1, 3],"
-      "\"google:verbatimrelevance\":2}]",
-      { { "k a", false, false },
-        { "a2",  true,  true },
+    { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[10, 30],"
+      "\"google:verbatimrelevance\":20}]",
+      { { "a2",  true,  true },
+        { "k a", false, false },
         { "a",   true,  true },
-        { "a1",  true,  true },
+        { "a1",  true,  false },
         kEmptyMatch, kEmptyMatch },
       "2" },
 
     // Ensure that all suggestions are considered, regardless of order.
     { "[\"a\",[\"b\", \"c\", \"d\", \"e\", \"f\", \"g\", \"h\"],[],[],"
-       "{\"google:suggestrelevance\":[1, 2, 3, 4, 5, 6, 7]}]",
+       "{\"google:suggestrelevance\":[10, 20, 30, 40, 50, 60, 70]}]",
       { { "a",   true,  true },
         { "k a", false, false },
         { "h",   true,  false },
         { "g",   true,  false },
         { "f",   true,  false },
         { "e",   true,  false } },
       std::string() },
     { "[\"a\",[\"http://b.com\", \"http://c.com\", \"http://d.com\","
               "\"http://e.com\", \"http://f.com\", \"http://g.com\","
               "\"http://h.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\","
                                 "\"NAVIGATION\", \"NAVIGATION\","
                                 "\"NAVIGATION\", \"NAVIGATION\","
                                 "\"NAVIGATION\"],"
-        "\"google:suggestrelevance\":[1, 2, 3, 4, 5, 6, 7]}]",
+        "\"google:suggestrelevance\":[10, 20, 30, 40, 50, 60, 70]}]",
       { { "a",     true,  true },
         { "k a",   false, false },
         { "h.com", false, false },
         { "g.com", false, false },
         { "f.com", false, false },
         { "e.com", false, false } },
       std::string() },
 
     // Ensure that incorrectly sized suggestion relevance lists are ignored.
     // Note that keyword suggestions by default (not in suggested relevance
     // mode) score more highly than the default verbatim.
     { "[\"a\",[\"a1\", \"a2\"],[],[],{\"google:suggestrelevance\":[1]}]",
       { { "a",   true,  true },
-        { "a1",  true,  true },
-        { "a2",  true,  true },
+        { "a1",  true,  false },
+        { "a2",  true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"a1\"],[],[],{\"google:suggestrelevance\":[9999, 1]}]",
       { { "a",   true,  true },
-        { "a1",  true,  true },
+        { "a1",  true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     // In this case, ignoring the suggested relevance scores means we keep
     // only one navsuggest result.
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
         "\"google:suggestrelevance\":[1]}]",
       { { "a",      true,  true },
         { "a1.com", false, false },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a1.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
        "\"google:suggestrelevance\":[9999, 1]}]",
       { { "a",      true,  true },
         { "a1.com", false, false },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // Ensure that all 'verbatim' results are merged with their maximum score.
     { "[\"a\",[\"a\", \"a1\", \"a2\"],[],[],"
        "{\"google:suggestrelevance\":[9998, 9997, 9999]}]",
       { { "a2",  true,  true },
         { "a",   true,  true },
-        { "a1",  true,  true },
+        { "a1",  true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch },
       "2" },
     { "[\"a\",[\"a\", \"a1\", \"a2\"],[],[],"
        "{\"google:suggestrelevance\":[9998, 9997, 9999],"
         "\"google:verbatimrelevance\":0}]",
       { { "a2",  true,  true },
         { "a",   true,  true },
-        { "a1",  true,  true },
+        { "a1",  true,  false },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch },
       "2" },
 
     // Ensure that verbatim is always generated without other suggestions.
     // TODO(mpearson): Ensure the value of verbatimrelevance is respected
     // (except when suggested relevances are ignored).
     { "[\"a\",[],[],[],{\"google:verbatimrelevance\":1}]",
-      { { "k a", false, false },
-        { "a",   true,  true },
+      { { "a",   true,  true },
+        { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[],[],[],{\"google:verbatimrelevance\":0}]",
       { { "a",   true,  true },
         { "k a", false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
 
     // In reorder mode, navsuggestions will not need to be demoted (because
     // they are marked as not allowed to be default match and will be
     // reordered as necessary).
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9998, 9999]}]",
-      { { "a2.com", false, false },
+      { { "a",      true,  true },
+        { "a2.com", false, false },
         { "a1.com", false, false },
-        { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9999, 9998]}]",
-      { { "a1.com", false, false },
+      { { "a",      true,  true },
+        { "a1.com", false, false },
         { "a2.com", false, false },
-        { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"https://a/\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\"],"
         "\"google:suggestrelevance\":[9999]}]",
-      { { "https://a", false, false },
-        { "a",         true,  true },
+      { { "a",         true,  true },
+        { "https://a", false, false },
         { "k a",       false, false },
         kEmptyMatch, kEmptyMatch, kEmptyMatch },
       std::string() },
     // Check when navsuggest scores more than verbatim and there is query
     // suggestion but it scores lower.
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9998, 9999, 1300]}]",
-      { { "a2.com", false, false },
+      { { "a",      true,  true },
+        { "a2.com", false, false },
         { "a1.com", false, false },
-        { "a",      true,  true },
-        { "a3",     true,  true },
+        { "a3",     true,  false },
         { "k a",    false, false },
         kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9999, 9998, 1300]}]",
-      { { "a1.com", false, false },
+      { { "a",      true,  true },
+        { "a1.com", false, false },
         { "a2.com", false, false },
-        { "a",      true,  true },
-        { "a3",     true,  true },
+        { "a3",     true,  false },
         { "k a",    false, false },
         kEmptyMatch },
       std::string() },
     // Check when navsuggest scores more than a query suggestion.  There is
     // a verbatim but it scores lower.
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9998, 9999, 9997]}]",
-      { { "a2.com", false, false },
+      { { "a3",     true,  true },
+        { "a2.com", false, false },
         { "a1.com", false, false },
-        { "a3",     true,  true },
         { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch },
       "3" },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9999, 9998, 9997]}]",
-      { { "a1.com", false, false },
+      { { "a3",     true,  true },
+        { "a1.com", false, false },
         { "a2.com", false, false },
-        { "a3",     true,  true },
         { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch },
       "3" },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":0,"
         "\"google:suggestrelevance\":[9998, 9999, 9997]}]",
-      { { "a2.com", false, false },
+      { { "a3",     true,  true },
+        { "a2.com", false, false },
         { "a1.com", false, false },
-        { "a3",     true,  true },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch },
       "3" },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":0,"
         "\"google:suggestrelevance\":[9999, 9998, 9997]}]",
-      { { "a1.com", false, false },
+      { { "a3",     true,  true },
+        { "a1.com", false, false },
         { "a2.com", false, false },
-        { "a3",     true,  true },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch },
       "3" },
     // Check when there is neither verbatim nor a query suggestion that,
     // because we can't demote navsuggestions below a query suggestion,
     // we restore the keyword verbatim score.
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
         "\"google:verbatimrelevance\":0,"
         "\"google:suggestrelevance\":[9998, 9999]}]",
-      { { "a2.com", false, false },
+      { { "a",      true,  true },
+        { "a2.com", false, false },
         { "a1.com", false, false },
-        { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch },
       std::string() },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
         "\"google:verbatimrelevance\":0,"
         "\"google:suggestrelevance\":[9999, 9998]}]",
-      { { "a1.com", false, false },
+      { { "a",      true,  true },
+        { "a1.com", false, false },
         { "a2.com", false, false },
-        { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch, kEmptyMatch },
       std::string() },
     // More checks that everything works when it's not necessary to demote.
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9997, 9998, 9999]}]",
       { { "a3",     true,  true },
         { "a2.com", false, false },
         { "a1.com", false, false },
         { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch },
       "3" },
     { "[\"a\",[\"http://a1.com\", \"http://a2.com\", \"a3\"],[],[],"
        "{\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\", \"QUERY\"],"
         "\"google:verbatimrelevance\":9990,"
         "\"google:suggestrelevance\":[9998, 9997, 9999]}]",
       { { "a3",     true,  true },
         { "a1.com", false, false },
         { "a2.com", false, false },
         { "a",      true,  true },
         { "k a",    false, false },
         kEmptyMatch },
       "3" },
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
-    QueryForInput(ASCIIToUTF16("k a"), false, true);
+    // Send the query twice in order to have a synchronous pass after the first
+    // reply is received.  This is necessary because SearchProvider doesn't
+    // allow an asynchronous reply to change the default match.
+    for (size_t j = 0; j < 2; j++) {

[msw, 2014/08/27 18:44:38]

nit: ++j

[Mark P, 2014/08/27 23:08:19]

Done.

+      QueryForInput(ASCIIToUTF16("k a"), false, true);
 
-    // Set up a default fetcher with no results.
-    net::TestURLFetcher* default_fetcher =
-        test_factory_.GetFetcherByID(
-            SearchProvider::kDefaultProviderURLFetcherID);
-    ASSERT_TRUE(default_fetcher);
-    default_fetcher->set_response_code(200);
-    default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
-    default_fetcher = NULL;
+      // Set up a default fetcher with no results.
+      net::TestURLFetcher* default_fetcher =
+          test_factory_.GetFetcherByID(
+              SearchProvider::kDefaultProviderURLFetcherID);
+      ASSERT_TRUE(default_fetcher);
+      default_fetcher->set_response_code(200);
+      default_fetcher->delegate()->OnURLFetchComplete(default_fetcher);
+      default_fetcher = NULL;
 
-    // Set up a keyword fetcher with provided results.
-    net::TestURLFetcher* keyword_fetcher =
-        test_factory_.GetFetcherByID(
-            SearchProvider::kKeywordProviderURLFetcherID);
-    ASSERT_TRUE(keyword_fetcher);
-    keyword_fetcher->set_response_code(200);
-    keyword_fetcher->SetResponseString(cases[i].json);
-    keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher);
-    keyword_fetcher = NULL;
-    RunTillProviderDone();
+      // Set up a keyword fetcher with provided results.
+      net::TestURLFetcher* keyword_fetcher =
+          test_factory_.GetFetcherByID(
+              SearchProvider::kKeywordProviderURLFetcherID);
+      ASSERT_TRUE(keyword_fetcher);
+      keyword_fetcher->set_response_code(200);
+      keyword_fetcher->SetResponseString(cases[i].json);
+      keyword_fetcher->delegate()->OnURLFetchComplete(keyword_fetcher);
+      keyword_fetcher = NULL;
+      RunTillProviderDone();
+    }
 
     const std::string description = "for input with json=" + cases[i].json;
     const ACMatches& matches = provider_->matches();
     ASSERT_FALSE(matches.empty());
     // Find the first match that's allowed to be the default match and check
     // its inline_autocompletion.
     ACMatches::const_iterator it = FindDefaultMatch(matches);
     ASSERT_NE(matches.end(), it);
     EXPECT_EQ(ASCIIToUTF16(cases[i].inline_autocompletion),
               it->inline_autocompletion) << description;
 
     ASSERT_LE(matches.size(), ARRAYSIZE_UNSAFE(cases[i].matches));
     size_t j = 0;
     // Ensure that the returned matches equal the expectations.

[msw, 2014/08/27 18:44:38]

Ditto, too bad this also checks |from_keyword|.

[Mark P, 2014/08/27 23:08:19]

I suppose I could add from_keyword to CheckMatches and maybe match_type (per
another comment by you), but then we have a lot of extraneous stuff in
initializers elsewhere.  I wonder if templates would help or if there's a more
elegant way.
Regardless, cleanup postponed.

[msw, 2014/08/28 19:21:40]

Acknowledged.

     for (; j < matches.size(); ++j) {
       EXPECT_EQ(ASCIIToUTF16(cases[i].matches[j].contents),
                 matches[j].contents) << description;
       EXPECT_EQ(cases[i].matches[j].from_keyword,
                 matches[j].keyword == ASCIIToUTF16("k")) << description;
       EXPECT_EQ(cases[i].matches[j].allowed_to_be_default_match,
                 matches[j].allowed_to_be_default_match) << description;
     }
     // Ensure that no expected matches are missing.
     for (; j < ARRAYSIZE_UNSAFE(cases[i].matches); ++j)
       EXPECT_EQ(kNotApplicable, cases[i].matches[j].contents) <<
           "Case # " << i << " " << description;
   }
 }
 
+TEST_F(SearchProviderTest, DontInlineAutocompleteAsynchronously) {
+  // This test sends two separate queries, each receiving different JSON
+  // replies, and checks that at each stage of processing (receiving first
+  // asynchronous reply, handling new keystroke synchronously / sending the
+  // second request, and receiving the second asynchronous reply) we have the
+  // expected matches.  In particular, receiving the second reply shouldn't
+  // cause an unexpected inline autcompletion.
+  struct {
+    const std::string first_json;
+    const ExpectedMatch first_async_matches[4];
+    const ExpectedMatch sync_matches[4];
+    const std::string second_json;
+    const ExpectedMatch second_async_matches[4];
+  } cases[] = {
+    // A simple test that verifies we don't inline autocomplete after the
+    // first asynchronous reply, but we do at the next keystroke if the
+    // reply's results were good enough.  Furthermore, we should continue
+    // inline autocompleting after the second asynchronous reply if the new
+    // top suggestion is the same as the old inline autocompleted suggestion.
+    { "[\"a\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"

[msw, 2014/08/27 18:44:39]

nit: most tests avoid numbers near this threshold, consider using 999x.

[Mark P, 2014/08/27 23:08:19]

Done.

+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "a", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch },
+      { { "ab1", true }, { "ab2", true }, { "ab", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "ab1", true }, { "ab2", false }, { "ab", true },
+        kEmptyExpectedMatch } },
+    // Ditto, just for a navigation suggestion.
+    { "[\"a\",[\"ab1.com\", \"ab2.com\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "a", true }, { "ab1.com", false }, { "ab2.com", false },
+        kEmptyExpectedMatch },
+      { { "ab1.com", true }, { "ab2.com", true }, { "ab", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1.com\", \"ab2.com\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "ab1.com", true }, { "ab2.com", false }, { "ab", true },
+        kEmptyExpectedMatch } },
+    // A more realistic test of the same situation.
+    { "[\"a\",[\"abcdef\", \"abcdef.com\", \"abc\"],[],[],"
+       "{\"google:verbatimrelevance\":900,"
+        "\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\", \"QUERY\"],"
+        "\"google:suggestrelevance\":[1250, 1200, 1000]}]",
+      { { "a", true }, { "abcdef", false }, { "abcdef.com", false },
+        { "abc", false } },
+      { { "abcdef", true }, { "abcdef.com", true }, { "abc", true },
+        { "ab", true } },
+      "[\"ab\",[\"abcdef\", \"abcdef.com\", \"abc\"],[],[],"
+       "{\"google:verbatimrelevance\":900,"
+        "\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\", \"QUERY\"],"
+        "\"google:suggestrelevance\":[1250, 1200, 1000]}]",
+      { { "abcdef", true }, { "abcdef.com", false }, { "abc", false },
+        { "ab", true } } },
+
+    // Without an original inline autcompletion, a new inline autcompletion
+    // should be rejected.
+    { "[\"a\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[900, 800]}]",
+      { { "a", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch },
+      { { "ab", true }, { "ab1", true }, { "ab2", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "ab", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch } },
+    { "[\"a\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[900, 800]}]",
+      { { "a", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch },
+      { { "ab", true }, { "ab1", true }, { "ab2", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1301, 1302]}]",

[msw, 2014/08/27 18:44:38]

This case only differs from above in this scoring of the second async results.
That seems a bit superfluous to me, but maybe I'm missing something that adds
more value in these similar cases? Did you intend to do something more complex
like give initial scores that inline "ab1" and followup scores that inline
"ab2"? (but that seems covered by your line 2155 set of cases).

[Mark P, 2014/08/27 23:08:20]

No, I didn't intend to do something more complex.  I merely wanted to make sure
that for results we decide not to inline, we order them according to their new
scores, not their earlier scores.  Added comment.

[msw, 2014/08/28 19:21:39]

Acknowledged.

+      { { "ab", true }, { "ab2", false }, { "ab1", false },
+        kEmptyExpectedMatch } },
+    // Now, the same verifications but with the new inline autocompletion as a
+    // navsuggestion.  The new autocompletion should still be rejected.
+    { "[\"a\",[\"ab1.com\", \"ab2.com\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
+        "\"google:suggestrelevance\":[900, 800]}]",
+      { { "a", true }, { "ab1.com", false }, { "ab2.com", false },
+        kEmptyExpectedMatch },
+      { { "ab", true }, { "ab1.com", true }, { "ab2.com", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1.com\", \"ab2.com\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "ab", true }, { "ab1.com", false }, { "ab2.com", false },
+        kEmptyExpectedMatch } },
+    { "[\"a\",[\"ab1.com\", \"ab2.com\"],[],[],"

[msw, 2014/08/27 18:44:39]

Ditto, similar case.

[Mark P, 2014/08/27 23:08:19]

Acknowledged.

+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
+        "\"google:suggestrelevance\":[900, 800]}]",
+      { { "a", true }, { "ab1.com", false }, { "ab2.com", false },
+        kEmptyExpectedMatch },
+      { { "ab", true }, { "ab1.com", true }, { "ab2.com", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1.com\", \"ab2.com\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggesttype\":[\"NAVIGATION\", \"NAVIGATION\"],"
+        "\"google:suggestrelevance\":[1301, 1302]}]",
+      { { "ab", true }, { "ab2.com", false }, { "ab1.com", false },
+        kEmptyExpectedMatch } },
+
+    // It's okay to abandon an inline autocompletion asynchronously.

[msw, 2014/08/27 18:44:39]

Wait, but why?* If I were about to hit enter for the inline "ab1" suggestion and
the async reply yanks it out from under me as soon as the reply is received, how
is that more okay than other behavior you're fixing here? (*a fun blog, btw).

I guess we can't guarantee that the prior top suggestion is sent again, so maybe
my expectation to retain the inline match on an async reply is unreasonable?

[Mark P, 2014/08/27 23:08:20]

From discussion with the suggest folks, this should happen rarely if at all.  We
decided not to put this in the initial version of the code.  You'll see some
mention of this in the bug discussion.  We'll decide later if it's worth the
trouble to implement.  (It's harder to implement than this change, and this
change wasn't easy.)

[msw, 2014/08/28 19:21:40]

Acknowledged.

+    { "[\"a\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "a", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch },
+      { { "ab1", true }, { "ab2", true }, { "ab", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[900, 800]}]",
+      { { "ab", true }, { "ab1", true }, { "ab2", false },
+        kEmptyExpectedMatch } },
+
+    // Note: it's possible that the suggest server returns a suggestion with
+    // an inline autocompletion (that as usual we delay in allowing it to
+    // be displayed as an inline autocompletion until the next keystroke),
+    // then, in reply to the next keystroke, the server returns a different
+    // suggestion as an inline autocompletion.  This is not likely to happen.
+    // Regardless, if it does, one could imagine three different behaviors:
+    // - keep the original inline autocompletion until the next keystroke
+    //   (i.e., don't abandon an inline autocompletion asynchronously)
+    // - abandon all inline autocompletions

[msw, 2014/08/27 18:44:38]

nit: if you want to expound upon these behaviors (which may be unnecessary), you
should explain when the second and third changes would occur (on the next
keystroke or on the async reply).

[Mark P, 2014/08/27 23:08:20]

Done.  I found expanding it useful for me, and likely handy for reference.

[msw, 2014/08/28 19:21:39]

Acknowledged.

+    // - ignore the new inline autocompletion, yet possibly keep the original
+    //   if it scores well in the most recent reply
+    // All of these behaviors are reasonable.  The main thing we want to
+    // ensure is that the second asynchronous reply shouldn't cause _a new_
+    // inline autocompletion to be displayed.  We test that here.
+    // (BTW, it happens that the code does the third of those bullets.)
+    { "[\"a\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "a", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch },
+      { { "ab1", true }, { "ab2", true }, { "ab", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1\", \"ab3\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1302, 1400]}]",
+      { { "ab1", true }, { "ab3", false }, { "ab", true },
+        kEmptyExpectedMatch } },
+    { "[\"a\",[\"ab1\", \"ab2\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[1302, 1301]}]",
+      { { "a", true }, { "ab1", false }, { "ab2", false },
+        kEmptyExpectedMatch },
+      { { "ab1", true }, { "ab2", true }, { "ab", true },
+        kEmptyExpectedMatch },
+      "[\"ab\",[\"ab1\", \"ab3\"],[],[],"
+       "{\"google:verbatimrelevance\":1300,"
+        "\"google:suggestrelevance\":[900, 1400]}]",
+      { { "ab", true }, { "ab3", false }, { "ab1", true },
+        kEmptyExpectedMatch } },
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {

[msw, 2014/08/27 18:44:38]

nit: ++i

[Mark P, 2014/08/27 23:08:20]

Done.

+    // First, send the query "a" and receive the provided JSON reply,

[msw, 2014/08/27 18:44:39]

nit: remove "provided" for a one-liner.

[Mark P, 2014/08/27 23:08:19]

Done.

+    // |first_json|.
+    ClearAllResults();
+    QueryForInput(ASCIIToUTF16("a"), false, false);

[msw, 2014/08/27 18:44:38]

This [clear, query, set response, get provider, run till done] pattern is common
and repeated in this file, can [a subset of] it be made into a helper function?

[Mark P, 2014/08/27 23:08:20]

Done.  The helper function can get (and does) now get widely used. :-)

[msw, 2014/08/28 19:21:40]

Thank you for this nice cleanup!

+    net::TestURLFetcher* first_fetcher =
+        test_factory_.GetFetcherByID(
+            SearchProvider::kDefaultProviderURLFetcherID);
+    ASSERT_TRUE(first_fetcher);
+    first_fetcher->set_response_code(200);
+    first_fetcher->SetResponseString(cases[i].first_json);
+    first_fetcher->delegate()->OnURLFetchComplete(first_fetcher);
+    RunTillProviderDone();
+
+    // Verify that the matches after the asynchronous results are as expected.
+    std::string description = "first asynchronous reply for input with "

[msw, 2014/08/27 18:44:39]

nit: s/reply/response/ to match other comments? these could be improved a bit.

[Mark P, 2014/08/27 23:08:20]

Consistency is a virtue.  Fixed all.

+        "first_json=" + cases[i].first_json;
+    CheckMatches(description, ARRAYSIZE_UNSAFE(cases[i].first_async_matches),
+                 cases[i].first_async_matches, provider_->matches());
+
+    // Then, send the query "ab" and check the synchronous matches.
+    description = "synchronous response after first keystroke after input "

[msw, 2014/08/27 18:44:38]

nit: "the first keystroke"

[Mark P, 2014/08/27 23:08:19]

Done.

+        "with first_json=" + cases[i].first_json;
+    QueryForInput(ASCIIToUTF16("ab"), false, false);
+    CheckMatches(description, ARRAYSIZE_UNSAFE(cases[i].sync_matches),
+                 cases[i].sync_matches, provider_->matches());
+
+    // Finally, get the provided JSON reply, |second_json|, and verify the
+    // matches after the second asynchronous reply are as expected.
+    description = "second asynchronous response after input with first_json=" +
+        cases[i].first_json + " and second_json=" + cases[i].second_json;
+    net::TestURLFetcher* second_fetcher =
+        test_factory_.GetFetcherByID(
+            SearchProvider::kDefaultProviderURLFetcherID);
+    ASSERT_TRUE(second_fetcher);
+    second_fetcher->set_response_code(200);
+    second_fetcher->SetResponseString(cases[i].second_json);
+    second_fetcher->delegate()->OnURLFetchComplete(second_fetcher);
+    RunTillProviderDone();
+    CheckMatches(description, ARRAYSIZE_UNSAFE(cases[i].second_async_matches),
+                 cases[i].second_async_matches, provider_->matches());
+  }
+}
+
 TEST_F(SearchProviderTest, LocalAndRemoteRelevances) {
   // We hardcode the string "term1" below, so ensure that the search term that
   // got added to history already is that string.
   ASSERT_EQ(ASCIIToUTF16("term1"), term1_);
   base::string16 term = term1_.substr(0, term1_.length() - 1);
 
   AddSearchToHistory(default_t_url_, term + ASCIIToUTF16("2"), 2);
   profile_.BlockUntilHistoryProcessesPendingRequests();
 
   struct {
@@ skipping 58 matching lines @@
     fetcher->delegate()->OnURLFetchComplete(fetcher);
     RunTillProviderDone();
 
     const std::string description = "for input with json=" + cases[i].json;
     const ACMatches& matches = provider_->matches();
 
     // Ensure no extra matches are present.
     ASSERT_LE(matches.size(), ARRAYSIZE_UNSAFE(cases[i].matches));
 
     size_t j = 0;
     // Ensure that the returned matches equal the expectations.

[msw, 2014/08/27 18:44:38]

Ditto, too bad this doesn't check |allowed_to_be_default_match|

[Mark P, 2014/08/27 23:08:19]

Acknowledged.

     for (; j < matches.size(); ++j)
       EXPECT_EQ(ASCIIToUTF16(cases[i].matches[j]),
                 matches[j].contents) << description;
     // Ensure that no expected matches are missing.
     for (; j < ARRAYSIZE_UNSAFE(cases[i].matches); ++j)
       EXPECT_EQ(kNotApplicable, cases[i].matches[j]) <<
           "Case # " << i << " " << description;
   }
 }
 
 // Verifies suggest relevance behavior for URL input.
 TEST_F(SearchProviderTest, DefaultProviderSuggestRelevanceScoringUrlInput) {
   struct DefaultFetcherUrlInputMatch {
     const std::string match_contents;
     AutocompleteMatch::Type match_type;
     bool allowed_to_be_default_match;
   };
   const DefaultFetcherUrlInputMatch kEmptyMatch =
       { kNotApplicable, AutocompleteMatchType::NUM_TYPES, false };
   struct {
     const std::string input;
     const std::string json;
     const DefaultFetcherUrlInputMatch output[4];
   } cases[] = {
-    // Ensure NAVIGATION matches are allowed to be listed first for URL
-    // input regardless of whether the match is inlineable.  Note that
-    // non-inlineable matches should not be allowed to be the default match.
+    // Ensure NAVIGATION matches are allowed to be listed first for URL input.
+    // Non-inlineable matches should not be allowed to be the default match.
+    // Note that the top-scoring inlineable match is moved to the top
+    // regardless of its score.
     { "a.com", "[\"a.com\",[\"http://b.com/\"],[],[],"
                 "{\"google:suggesttype\":[\"NAVIGATION\"],"
                  "\"google:suggestrelevance\":[9999]}]",
-      { { "b.com",   AutocompleteMatchType::NAVSUGGEST,            false },
-        { "a.com",   AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
+      { { "a.com",   AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
+        { "b.com",   AutocompleteMatchType::NAVSUGGEST,            false },
         kEmptyMatch, kEmptyMatch } },
     { "a.com", "[\"a.com\",[\"https://b.com\"],[],[],"
                 "{\"google:suggesttype\":[\"NAVIGATION\"],"
                  "\"google:suggestrelevance\":[9999]}]",
-      { { "https://b.com", AutocompleteMatchType::NAVSUGGEST,           false },
-        { "a.com",         AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
+      { { "a.com",         AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
+        { "https://b.com", AutocompleteMatchType::NAVSUGGEST,           false },
         kEmptyMatch, kEmptyMatch } },
     { "a.com", "[\"a.com\",[\"http://a.com/a\"],[],[],"
                 "{\"google:suggesttype\":[\"NAVIGATION\"],"
                  "\"google:suggestrelevance\":[9999]}]",
       { { "a.com/a", AutocompleteMatchType::NAVSUGGEST,            true },
         { "a.com",   AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
         kEmptyMatch, kEmptyMatch } },
     { "a.com", "[\"a.com\",[\"https://a.com\"],[],[],"
                 "{\"google:suggesttype\":[\"NAVIGATION\"],"
                  "\"google:suggestrelevance\":[9999]}]",
       { { "https://a.com", AutocompleteMatchType::NAVSUGGEST,            true },
         { "a.com",         AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
         kEmptyMatch, kEmptyMatch } },
 
     // Ensure topmost inlineable SUGGEST matches are NOT allowed for URL
     // input.  SearchProvider disregards search and verbatim suggested
     // relevances.
     { "a.com", "[\"a.com\",[\"a.com info\"],[],[],"
                 "{\"google:suggestrelevance\":[9999]}]",
-      { { "a.com",      AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
-        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        true },
+      { { "a.com",      AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true  },
+        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        false },
         kEmptyMatch, kEmptyMatch } },
     { "a.com", "[\"a.com\",[\"a.com info\"],[],[],"
                 "{\"google:suggestrelevance\":[9999]}]",
-      { { "a.com",   AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,   true },
-        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        true },
+      { { "a.com",      AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true  },
+        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        false },
         kEmptyMatch, kEmptyMatch } },
 
     // Ensure the fallback mechanism allows inlinable NAVIGATION matches.
     { "a.com", "[\"a.com\",[\"a.com info\", \"http://a.com/b\"],[],[],"
                 "{\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"],"
                  "\"google:suggestrelevance\":[9999, 9998]}]",
-      { { "a.com/b",    AutocompleteMatchType::NAVSUGGEST,            true },
-        { "a.com",      AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
-        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        true },
+      { { "a.com/b",    AutocompleteMatchType::NAVSUGGEST,            true  },
+        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        false },
+        { "a.com",      AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true  },
         kEmptyMatch } },
     { "a.com", "[\"a.com\",[\"a.com info\", \"http://a.com/b\"],[],[],"
                 "{\"google:suggesttype\":[\"QUERY\", \"NAVIGATION\"],"
                  "\"google:suggestrelevance\":[9998, 9997],"
                  "\"google:verbatimrelevance\":9999}]",
       { { "a.com/b",    AutocompleteMatchType::NAVSUGGEST,            true },
         { "a.com",      AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
-        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        true },
+        { "a.com info", AutocompleteMatchType::SEARCH_SUGGEST,        false },
         kEmptyMatch } },
 
-    // Ensure topmost non-inlineable SUGGEST matches are allowed for URL
-    // input assuming the top inlineable match is not a query (i.e., is a
-    // NAVSUGGEST).
+    // Ensure non-inlineable SUGGEST matches are allowed for URL input
+    // assuming the best inlineable match is not a query (i.e., is a
+    // NAVSUGGEST).  The best inlineable match will be at the top of the
+    // list regardless of its score.
     { "a.com", "[\"a.com\",[\"info\"],[],[],"
                 "{\"google:suggestrelevance\":[9999]}]",
-      { { "info",  AutocompleteMatchType::SEARCH_SUGGEST,        false },
-        { "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
+      { { "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true  },
+        { "info",  AutocompleteMatchType::SEARCH_SUGGEST,        false },
         kEmptyMatch, kEmptyMatch } },
     { "a.com", "[\"a.com\",[\"info\"],[],[],"
                 "{\"google:suggestrelevance\":[9999]}]",
-      { { "info",  AutocompleteMatchType::SEARCH_SUGGEST,        false },
-        { "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true },
+      { { "a.com", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, true  },
+        { "info",  AutocompleteMatchType::SEARCH_SUGGEST,        false },
         kEmptyMatch, kEmptyMatch } },
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
-    QueryForInput(ASCIIToUTF16(cases[i].input), false, false);
-    net::TestURLFetcher* fetcher =
-        test_factory_.GetFetcherByID(
-            SearchProvider::kDefaultProviderURLFetcherID);
-    ASSERT_TRUE(fetcher);
-    fetcher->set_response_code(200);
-    fetcher->SetResponseString(cases[i].json);
-    fetcher->delegate()->OnURLFetchComplete(fetcher);
-    RunTillProviderDone();
+    // Send the query twice in order to have a synchronous pass after the first
+    // reply is received.  This is necessary because SearchProvider doesn't
+    // allow an asynchronous reply to change the default match.
+    for (size_t j = 0; j < 2; j++) {

[msw, 2014/08/27 18:44:39]

nit: ++j

[Mark P, 2014/08/27 23:08:19]

Done.

+      QueryForInput(ASCIIToUTF16(cases[i].input), false, false);
+      net::TestURLFetcher* fetcher =
+          test_factory_.GetFetcherByID(
+              SearchProvider::kDefaultProviderURLFetcherID);
+      ASSERT_TRUE(fetcher);
+      fetcher->set_response_code(200);
+      fetcher->SetResponseString(cases[i].json);
+      fetcher->delegate()->OnURLFetchComplete(fetcher);
+      RunTillProviderDone();
+    }
 
+    const std::string description = "input=" + cases[i].input + " json=" +
+        cases[i].json;
     size_t j = 0;
     const ACMatches& matches = provider_->matches();
-    ASSERT_LE(matches.size(), ARRAYSIZE_UNSAFE(cases[i].output));
+    ASSERT_LE(matches.size(), ARRAYSIZE_UNSAFE(cases[i].output)) <<
+        description;

[msw, 2014/08/27 18:44:39]

nit: SCOPED_TRACE may be your friend here and in similar cases.

[Mark P, 2014/08/27 23:08:20]

Good idea.  Fixed all << whatever in this file to SCOPED_TRACE where it seemed
reasonable.  (If it wouldn't actually save any lines of code, I didn't bother to
do it.)

[msw, 2014/08/28 19:21:40]

Acknowledged.

     // Ensure that the returned matches equal the expectations.

[msw, 2014/08/27 18:44:38]

Ditto, too bad this also checks |match_type|.

[Mark P, 2014/08/27 23:08:19]

Acknowledged.

     for (; j < matches.size(); ++j) {
       EXPECT_EQ(ASCIIToUTF16(cases[i].output[j].match_contents),
-                matches[j].contents);
-      EXPECT_EQ(cases[i].output[j].match_type, matches[j].type);
+                matches[j].contents) << description;
+      EXPECT_EQ(cases[i].output[j].match_type, matches[j].type) << description;
       EXPECT_EQ(cases[i].output[j].allowed_to_be_default_match,
-                matches[j].allowed_to_be_default_match);
+                matches[j].allowed_to_be_default_match) << description;
     }
     // Ensure that no expected matches are missing.
     for (; j < ARRAYSIZE_UNSAFE(cases[i].output); ++j) {
-      EXPECT_EQ(kNotApplicable, cases[i].output[j].match_contents);
+      EXPECT_EQ(kNotApplicable, cases[i].output[j].match_contents) <<
+          description;
       EXPECT_EQ(AutocompleteMatchType::NUM_TYPES,
-                cases[i].output[j].match_type);
-      EXPECT_FALSE(cases[i].output[j].allowed_to_be_default_match);
+                cases[i].output[j].match_type) << description;
+      EXPECT_FALSE(cases[i].output[j].allowed_to_be_default_match) <<
+          description;
     }
   }
 }
 
 // A basic test that verifies the field trial triggered parsing logic.
 TEST_F(SearchProviderTest, FieldTrialTriggeredParsing) {
   QueryForInput(ASCIIToUTF16("foo"), false, false);
 
   // Make sure the default providers suggest service was queried.
   net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
@@ skipping 192 matching lines @@
                                "?www.abc.com", "c.com", true, false },
     { "?ab",             "http://www.abc.com",
                                "?www.abc.com", "c.com", true, false },
     { "?abc.com",        "http://www.abc.com",
                                "?www.abc.com", "",      true, true },
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
     // First test regular mode.
     QueryForInput(ASCIIToUTF16(cases[i].input), false, false);
-    AutocompleteMatch match(
-        provider_->NavigationToMatch(SearchSuggestionParser::NavigationResult(
-            ChromeAutocompleteSchemeClassifier(&profile_), GURL(cases[i].url),
-            AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
-            false, 0, false, ASCIIToUTF16(cases[i].input), std::string())));
+    SearchSuggestionParser::NavigationResult result(
+        ChromeAutocompleteSchemeClassifier(&profile_), GURL(cases[i].url),
+        AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
+        false, 0, false, ASCIIToUTF16(cases[i].input), std::string());
+    result.set_received_after_last_keystroke(false);
+    AutocompleteMatch match(provider_->NavigationToMatch(result));
     EXPECT_EQ(ASCIIToUTF16(cases[i].inline_autocompletion),
               match.inline_autocompletion);
     EXPECT_EQ(ASCIIToUTF16(cases[i].fill_into_edit), match.fill_into_edit);
     EXPECT_EQ(cases[i].allowed_to_be_default_match_in_regular_mode,
               match.allowed_to_be_default_match);
 
     // Then test prevent-inline-autocomplete mode.
     QueryForInput(ASCIIToUTF16(cases[i].input), true, false);
+    SearchSuggestionParser::NavigationResult result_prevent_inline(
+        ChromeAutocompleteSchemeClassifier(&profile_), GURL(cases[i].url),
+        AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
+        false, 0, false, ASCIIToUTF16(cases[i].input), std::string());
+    result_prevent_inline.set_received_after_last_keystroke(false);
     AutocompleteMatch match_prevent_inline(
-        provider_->NavigationToMatch(SearchSuggestionParser::NavigationResult(
-            ChromeAutocompleteSchemeClassifier(&profile_), GURL(cases[i].url),
-            AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
-            false, 0, false, ASCIIToUTF16(cases[i].input), std::string())));
+        provider_->NavigationToMatch(result_prevent_inline));
     EXPECT_EQ(ASCIIToUTF16(cases[i].inline_autocompletion),
               match_prevent_inline.inline_autocompletion);
     EXPECT_EQ(ASCIIToUTF16(cases[i].fill_into_edit),
               match_prevent_inline.fill_into_edit);
     EXPECT_EQ(cases[i].allowed_to_be_default_match_in_prevent_inline_mode,
               match_prevent_inline.allowed_to_be_default_match);
   }
 }
 
 // Verifies that "http://" is not trimmed for input that is a leading substring.
 TEST_F(SearchProviderTest, NavigationInlineSchemeSubstring) {
   const base::string16 input(ASCIIToUTF16("ht"));
   const base::string16 url(ASCIIToUTF16("http://a.com"));
-  const SearchSuggestionParser::NavigationResult result(
+  SearchSuggestionParser::NavigationResult result(
       ChromeAutocompleteSchemeClassifier(&profile_), GURL(url),
       AutocompleteMatchType::NAVSUGGEST,
       base::string16(), std::string(), false, 0, false, input, std::string());
+  result.set_received_after_last_keystroke(false);
 
   // Check the offset and strings when inline autocompletion is allowed.
   QueryForInput(input, false, false);
   AutocompleteMatch match_inline(provider_->NavigationToMatch(result));
   EXPECT_EQ(url, match_inline.fill_into_edit);
   EXPECT_EQ(url.substr(2), match_inline.inline_autocompletion);
   EXPECT_TRUE(match_inline.allowed_to_be_default_match);
   EXPECT_EQ(url, match_inline.contents);
 
   // Check the same strings when inline autocompletion is prevented.
   QueryForInput(input, true, false);
   AutocompleteMatch match_prevent(provider_->NavigationToMatch(result));
   EXPECT_EQ(url, match_prevent.fill_into_edit);
   EXPECT_FALSE(match_prevent.allowed_to_be_default_match);
   EXPECT_EQ(url, match_prevent.contents);
 }
 
 // Verifies that input "w" marks a more significant domain label than "www.".
 TEST_F(SearchProviderTest, NavigationInlineDomainClassify) {
   QueryForInput(ASCIIToUTF16("w"), false, false);
-  AutocompleteMatch match(
-      provider_->NavigationToMatch(SearchSuggestionParser::NavigationResult(
-          ChromeAutocompleteSchemeClassifier(&profile_),
-          GURL("http://www.wow.com"),
-          AutocompleteMatchType::NAVSUGGEST, base::string16(), std::string(),
-          false, 0, false, ASCIIToUTF16("w"), std::string())));
+  SearchSuggestionParser::NavigationResult result(
+      ChromeAutocompleteSchemeClassifier(&profile_),
+      GURL("http://www.wow.com"), AutocompleteMatchType::NAVSUGGEST,
+      base::string16(), std::string(), false, 0, false, ASCIIToUTF16("w"),
+      std::string());
+  result.set_received_after_last_keystroke(false);
+  AutocompleteMatch match(provider_->NavigationToMatch(result));
   EXPECT_EQ(ASCIIToUTF16("ow.com"), match.inline_autocompletion);
   EXPECT_TRUE(match.allowed_to_be_default_match);
   EXPECT_EQ(ASCIIToUTF16("www.wow.com"), match.fill_into_edit);
   EXPECT_EQ(ASCIIToUTF16("www.wow.com"), match.contents);
 
   // Ensure that the match for input "w" is marked on "wow" and not "www".
   ASSERT_EQ(3U, match.contents_class.size());
   EXPECT_EQ(0U, match.contents_class[0].offset);
   EXPECT_EQ(AutocompleteMatch::ACMatchClassification::URL,
             match.contents_class[0].style);
@@ skipping 815 matching lines @@
   // Mismatching input will return empty prefetch data.
   AutocompleteInput input2(base::ASCIIToUTF16("weather n"),
                            base::string16::npos, base::string16(), GURL(),
                            metrics::OmniboxEventProto::INVALID_SPEC, false,
                            false, true, true,
                            ChromeAutocompleteSchemeClassifier(&profile_));
   provider_->DoAnswersQuery(input2);
   EXPECT_TRUE(provider_->prefetch_data_.full_query_text.empty());
   EXPECT_TRUE(provider_->prefetch_data_.query_type.empty());
 }