Use Instant suggested match type for Instant temporary text.

chrome/browser/autocomplete/search_provider.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 <algorithm>
 #include <cmath>
 
 #include "base/auto_reset.h"
@@ skipping 106 matching lines @@
 
 // SearchProvider -------------------------------------------------------------
 
 // static
 const int SearchProvider::kDefaultProviderURLFetcherID = 1;
 // static
 const int SearchProvider::kKeywordProviderURLFetcherID = 2;
 // static
 int SearchProvider::kMinimumTimeBetweenSuggestQueriesMs = 100;
 
+// static
+SearchProvider::Providers SearchProvider::GetProviders(
+    Profile* profile,
+    const AutocompleteInput& input,
+    AutocompleteInput* keyword_input) {
+  TemplateURLService* model = TemplateURLServiceFactory::GetForProfile(profile);
+  Providers providers(model);
+
+  const TemplateURL* keyword_provider = NULL;
+  if (keyword_input) {
+    keyword_provider = KeywordProvider::GetSubstitutingTemplateURLForInput(
+        model, keyword_input);
+    if (keyword_provider == NULL)
+      keyword_input->Clear();
+    else if (keyword_input->text().empty())
+      keyword_provider = NULL;  // No text after the keyword.
+  }
+
+  const TemplateURL* default_provider = model->GetDefaultSearchProvider();
+  if (default_provider && !default_provider->SupportsReplacement())
+    default_provider = NULL;
+
+  if (keyword_provider == default_provider)
+    default_provider = NULL;  // No use in querying the same provider twice.
+
+  providers.set(
+      default_provider ? default_provider->keyword() : string16(),
+      keyword_provider ? keyword_provider->keyword() : string16());
+
+  return providers;
+}
+
+// static
+AutocompleteMatch SearchProvider::CreateSearchSuggestion(
+    Profile* profile,
+    AutocompleteProvider* autocomplete_provider,
+    const Providers& search_providers,
+    const AutocompleteInput& input,
+    const string16& query_string,
+    const string16& input_text,
+    int relevance,
+    AutocompleteMatch::Type type,
+    int accepted_suggestion,
+    bool is_keyword) {
+  AutocompleteMatch match(autocomplete_provider, relevance, false, type);
+
+  // Bail out now if we don't actually have a valid provider.
+  match.keyword = is_keyword ?

[Peter Kasting, 2013/04/27 01:21:21]

The only reason this function needs |search_providers| is to choose the
appropriate keyword.  If instead you pass in the keyword, you can avoid taking a
Providers object.

To calculate this keyword, the OmniboxEditModel can just get the DSP's keyword
directly.  This allows you to avoid exposing GetProviders(), which in turn
allows you to basically revert all the Providers-related changes in this file
(and the header).

Sample code to get the DSP's keyword on the OmniboxEditModel side:

  const TemplateURL* default_provider =
      TemplateURLServiceFactory::GetForProfile(profile_)->
          GetDefaultSearchProvider();
  if (default_provider && default_provider->SupportsReplacement()) {
    ... = CreateSearchSuggestion(..., default_provider->keyword(), ...);
    ...
  } else {
    // Can't create a new search match.  Probably in this case you should just
    // use an AutocompleteMatch with an empty |destination_url| so it can't be
    // navigated; this should realistically never happen anyway.
  }

[sreeram, 2013/04/27 01:25:54]

But I do want to be able to support keyword mode. @dougw is working on
supporting keyword mode for Instant Extended, and I want to make it easy for him
when he gets to this part. Wouldn't he then have to redo some of the logic in
GetProviders() to figure out the keyword provider?

[Peter Kasting, 2013/04/27 01:37:48]

I would say, let's worry about that when we get there.  It's not 100% clear to
me that that will end up needing to run through this particular case in the
omnibox code for keyword searches.

If it does, the right answer will probably be simplified version of
GetProviders() that only returns a string16 that's the keyword for the correct
provider.  The full Providers struct, the idea of ensuring the default and
keyword providers aren't the same, etc. is all needed only by the SearchProvider
internals and doesn't ever need to be exposed publicly.

+      search_providers.keyword_provider() : search_providers.default_provider();
+  const TemplateURL* provider_url = match.GetTemplateURL(profile, false);
+  if (provider_url == NULL)
+    return match;
+
+  match.contents.assign(query_string);
+  // We do intra-string highlighting for suggestions - the suggested segment
+  // will be highlighted, e.g. for input_text = "you" the suggestion may be
+  // "youtube", so we'll bold the "tube" section: you*tube*.
+  if (input_text != query_string) {
+    size_t input_position = match.contents.find(input_text);
+    if (input_position == string16::npos) {
+      // The input text is not a substring of the query string, e.g. input
+      // text is "slasdot" and the query string is "slashdot", so we bold the
+      // whole thing.
+      match.contents_class.push_back(
+          ACMatchClassification(0, ACMatchClassification::MATCH));
+    } else {
+      // TODO(beng): ACMatchClassification::MATCH now seems to just mean
+      //             "bold" this. Consider modifying the terminology.
+      // We don't iterate over the string here annotating all matches because
+      // it looks odd to have every occurrence of a substring that may be as
+      // short as a single character highlighted in a query suggestion result,
+      // e.g. for input text "s" and query string "southwest airlines", it
+      // looks odd if both the first and last s are highlighted.
+      if (input_position != 0) {
+        match.contents_class.push_back(
+            ACMatchClassification(0, ACMatchClassification::NONE));
+      }
+      match.contents_class.push_back(
+          ACMatchClassification(input_position, ACMatchClassification::DIM));
+      size_t next_fragment_position = input_position + input_text.length();
+      if (next_fragment_position < query_string.length()) {
+        match.contents_class.push_back(
+            ACMatchClassification(next_fragment_position,
+                                  ACMatchClassification::NONE));
+      }
+    }
+  } else {
+    // Otherwise, we're dealing with the "default search" result which has no
+    // completion.
+    match.contents_class.push_back(
+        ACMatchClassification(0, ACMatchClassification::NONE));
+  }
+
+  // When the user forced a query, we need to make sure all the fill_into_edit
+  // values preserve that property.  Otherwise, if the user starts editing a
+  // suggestion, non-Search results will suddenly appear.
+  if (input.type() == AutocompleteInput::FORCED_QUERY)
+    match.fill_into_edit.assign(ASCIIToUTF16("?"));
+  if (is_keyword)
+    match.fill_into_edit.append(match.keyword + char16(' '));
+  if (!input.prevent_inline_autocomplete() &&
+      StartsWith(query_string, input_text, false)) {
+    match.inline_autocomplete_offset =
+        match.fill_into_edit.length() + input_text.length();
+  }
+  match.fill_into_edit.append(query_string);
+
+  const TemplateURLRef& search_url = provider_url->url_ref();
+  DCHECK(search_url.SupportsReplacement());
+  match.search_terms_args.reset(
+      new TemplateURLRef::SearchTermsArgs(query_string));
+  match.search_terms_args->original_query = input_text;
+  match.search_terms_args->accepted_suggestion = accepted_suggestion;
+  // This is the destination URL sans assisted query stats.  This must be set
+  // so the AutocompleteController can properly de-dupe; the controller will
+  // eventually overwrite it before it reaches the user.
+  match.destination_url =
+      GURL(search_url.ReplaceSearchTerms(*match.search_terms_args.get()));
+
+  // Search results don't look like URLs.
+  match.transition = is_keyword ?
+      content::PAGE_TRANSITION_KEYWORD : content::PAGE_TRANSITION_GENERATED;
+
+  return match;
+}
+
 SearchProvider::SearchProvider(AutocompleteProviderListener* listener,
                                Profile* profile)
     : AutocompleteProvider(listener, profile,
           AutocompleteProvider::TYPE_SEARCH),
       providers_(TemplateURLServiceFactory::GetForProfile(profile)),
       suggest_results_pending_(0),
       has_default_suggested_relevance_(false),
       has_keyword_suggested_relevance_(false),
       default_verbatim_relevance_(-1),
       keyword_verbatim_relevance_(-1),
@@ skipping 97 matching lines @@
   instant_finalized_ =
       (input.matches_requested() != AutocompleteInput::ALL_MATCHES);
 
   // Can't return search/suggest results for bogus input or without a profile.
   if (!profile_ || (input.type() == AutocompleteInput::INVALID)) {
     Stop(false);
     return;
   }
 
   keyword_input_ = input;
-  const TemplateURL* keyword_provider =
-      KeywordProvider::GetSubstitutingTemplateURLForInput(model,
-                                                          &keyword_input_);
-  if (keyword_provider == NULL)
-    keyword_input_.Clear();
-  else if (keyword_input_.text().empty())
-    keyword_provider = NULL;
+  Providers providers = GetProviders(profile_, input, &keyword_input_);
 
-  const TemplateURL* default_provider = model->GetDefaultSearchProvider();
-  if (default_provider && !default_provider->SupportsReplacement())
-    default_provider = NULL;
-
-  if (keyword_provider == default_provider)
-    default_provider = NULL;  // No use in querying the same provider twice.
-
-  if (!default_provider && !keyword_provider) {
+  if (providers.default_provider().empty() &&
+      providers.keyword_provider().empty()) {
     // No valid providers.
     Stop(false);
     return;
   }
 
   // If we're still running an old query but have since changed the query text
   // or the providers, abort the query.
-  string16 default_provider_keyword(default_provider ?
-      default_provider->keyword() : string16());
-  string16 keyword_provider_keyword(keyword_provider ?
-      keyword_provider->keyword() : string16());
   if (!minimal_changes ||
-      !providers_.equal(default_provider_keyword, keyword_provider_keyword)) {
+      !providers_.equal(providers.default_provider(),
+                        providers.keyword_provider())) {
     // If Instant has not come back with a suggestion, adjust the previous
     // suggestion if possible. If |instant_finalized| is true, we are looking
     // for synchronous matches only, so the suggestion is cleared.
     if (instant_finalized_)
       default_provider_suggestion_ = InstantSuggestion();
     else
       AdjustDefaultProviderSuggestion(input_.text(), input.text());
 
     // Cancel any in-flight suggest requests.
     if (!done_) {
       // The Stop(false) call below clears |default_provider_suggestion_|, but
       // in this instance we do not want to clear cached results, so we
       // restore it.
       base::AutoReset<InstantSuggestion> reset(&default_provider_suggestion_,
                                                InstantSuggestion());
       Stop(false);
     }
   }
 
-  providers_.set(default_provider_keyword, keyword_provider_keyword);
+  providers_ = providers;
 
   if (input.text().empty()) {
     // User typed "?" alone.  Give them a placeholder result indicating what
     // this syntax does.
-    if (default_provider) {
+    if (!providers_.default_provider().empty()) {
       AutocompleteMatch match;
       match.provider = this;
       match.contents.assign(l10n_util::GetStringUTF16(IDS_EMPTY_KEYWORD_VALUE));
       match.contents_class.push_back(
           ACMatchClassification(0, ACMatchClassification::NONE));
       match.keyword = providers_.default_provider();
       matches_.push_back(match);
     }
     Stop(false);
     return;
@@ skipping 1011 matching lines @@
   return std::max(0, base_score - score_discount);
 }
 
 void SearchProvider::AddMatchToMap(const string16& query_string,
                                    const string16& input_text,
                                    int relevance,
                                    AutocompleteMatch::Type type,
                                    int accepted_suggestion,
                                    bool is_keyword,
                                    MatchMap* map) {
-  AutocompleteMatch match(this, relevance, false, type);
-  std::vector<size_t> content_param_offsets;
-  // Bail out now if we don't actually have a valid provider.
-  match.keyword = is_keyword ?
-      providers_.keyword_provider() : providers_.default_provider();
-  const TemplateURL* provider_url = match.GetTemplateURL(profile_, false);
-  if (provider_url == NULL)
+  AutocompleteMatch match = CreateSearchSuggestion(profile_, this, providers_,
+      input_, query_string, input_text, relevance, type, accepted_suggestion,
+      is_keyword);
+  if (!match.destination_url.is_valid())
     return;
 
-  match.contents.assign(query_string);
-  // We do intra-string highlighting for suggestions - the suggested segment
-  // will be highlighted, e.g. for input_text = "you" the suggestion may be
-  // "youtube", so we'll bold the "tube" section: you*tube*.
-  if (input_text != query_string) {
-    size_t input_position = match.contents.find(input_text);
-    if (input_position == string16::npos) {
-      // The input text is not a substring of the query string, e.g. input
-      // text is "slasdot" and the query string is "slashdot", so we bold the
-      // whole thing.
-      match.contents_class.push_back(
-          ACMatchClassification(0, ACMatchClassification::MATCH));
-    } else {
-      // TODO(beng): ACMatchClassification::MATCH now seems to just mean
-      //             "bold" this. Consider modifying the terminology.
-      // We don't iterate over the string here annotating all matches because
-      // it looks odd to have every occurrence of a substring that may be as
-      // short as a single character highlighted in a query suggestion result,
-      // e.g. for input text "s" and query string "southwest airlines", it
-      // looks odd if both the first and last s are highlighted.
-      if (input_position != 0) {
-        match.contents_class.push_back(
-            ACMatchClassification(0, ACMatchClassification::NONE));
-      }
-      match.contents_class.push_back(
-          ACMatchClassification(input_position, ACMatchClassification::DIM));
-      size_t next_fragment_position = input_position + input_text.length();
-      if (next_fragment_position < query_string.length()) {
-        match.contents_class.push_back(
-            ACMatchClassification(next_fragment_position,
-                                  ACMatchClassification::NONE));
-      }
-    }
-  } else {
-    // Otherwise, we're dealing with the "default search" result which has no
-    // completion.
-    match.contents_class.push_back(
-        ACMatchClassification(0, ACMatchClassification::NONE));
-  }
-
-  // When the user forced a query, we need to make sure all the fill_into_edit
-  // values preserve that property.  Otherwise, if the user starts editing a
-  // suggestion, non-Search results will suddenly appear.
-  if (input_.type() == AutocompleteInput::FORCED_QUERY)
-    match.fill_into_edit.assign(ASCIIToUTF16("?"));
-  if (is_keyword)
-    match.fill_into_edit.append(match.keyword + char16(' '));
-  if (!input_.prevent_inline_autocomplete() &&
-      StartsWith(query_string, input_text, false)) {
-    match.inline_autocomplete_offset =
-        match.fill_into_edit.length() + input_text.length();
-  }
-  match.fill_into_edit.append(query_string);
-
-  const TemplateURLRef& search_url = provider_url->url_ref();
-  DCHECK(search_url.SupportsReplacement());
-  match.search_terms_args.reset(
-      new TemplateURLRef::SearchTermsArgs(query_string));
-  match.search_terms_args->original_query = input_text;
-  match.search_terms_args->accepted_suggestion = accepted_suggestion;
-  // This is the destination URL sans assisted query stats.  This must be set
-  // so the AutocompleteController can properly de-dupe; the controller will
-  // eventually overwrite it before it reaches the user.
-  match.destination_url =
-      GURL(search_url.ReplaceSearchTerms(*match.search_terms_args.get()));
-
-  // Search results don't look like URLs.
-  match.transition = is_keyword ?
-      content::PAGE_TRANSITION_KEYWORD : content::PAGE_TRANSITION_GENERATED;
-
   // Try to add |match| to |map|.  If a match for |query_string| is already in
   // |map|, replace it if |match| is more relevant.
   // NOTE: Keep this ToLower() call in sync with url_database.cc.
   const std::pair<MatchMap::iterator, bool> i = map->insert(
       std::pair<string16, AutocompleteMatch>(
           base::i18n::ToLower(query_string), match));
   // NOTE: We purposefully do a direct relevance comparison here instead of
   // using AutocompleteMatch::MoreRelevant(), so that we'll prefer "items added
   // first" rather than "items alphabetically first" when the scores are equal.
   // The only case this matters is when a user has results with the same score
@@ skipping 97 matching lines @@
     it->set_relevance(max_query_relevance);
   }
 }
 
 void SearchProvider::UpdateDone() {
   // We're done when the timer isn't running, there are no suggest queries
   // pending, and we're not waiting on Instant.
   done_ = (!timer_.IsRunning() && (suggest_results_pending_ == 0) &&
            (instant_finalized_ || !chrome::IsInstantEnabled(profile_)));
 }