Loosen DCHECK for Omnibox Extensions

chrome/browser/autocomplete/search_provider.cc

Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index c059c6ec2030c57e826980c33907b53beb5e7646..4bd5fb553da38ef65e38c2fc2102bc318cefff6d 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -409,10 +409,15 @@ void SearchProvider::UpdateMatches() {
     // These blocks attempt to repair undesirable behavior by suggested
     // relevances with minimal impact, preserving other suggested relevances.
 
-    if ((providers_.GetKeywordProviderURL() != NULL) &&
+    const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
+    const bool is_extension_keyword =
+        (keyword_url->GetType() == TemplateURL::OMNIBOX_API_EXTENSION);

[msw, 2014/08/22 17:45:56]

This should be keyword_url != NULL && ...

[Mark P, 2014/08/22 17:52:42]

No wonder the tests are failing.  (I hadn't looked yet.)

+    if ((keyword_url != NULL) && !is_extension_keyword &&

[msw, 2014/08/22 17:45:56]

fyi: you'll still need this NULL check too.

[Mark P, 2014/08/22 17:52:42]

Acknowledged.

         (FindTopMatch() == matches_.end())) {
-      // In keyword mode, disregard the keyword verbatim suggested relevance
-      // if necessary, so at least one match is allowed to be default.
+      // In non-extension keyword mode, disregard the keyword verbatim suggested
+      // relevance if necessary, so at least one match is allowed to be default.
+      // (In extension keyword mode this is not necessary because the extension
+      // will return a default match.)
       keyword_results_.verbatim_relevance = -1;
       ConvertResultsToAutocompleteMatches();
     }
@@ -427,15 +432,19 @@ void SearchProvider::UpdateMatches() {
       keyword_results_.verbatim_relevance = -1;
       ConvertResultsToAutocompleteMatches();
     }
-    if (FindTopMatch() == matches_.end()) {
-      // Guarantee that SearchProvider returns a legal default match.  (The
-      // omnibox always needs at least one legal default match, and it relies
-      // on SearchProvider to always return one.)
+    if ((FindTopMatch() == matches_.end()) &&
+        ((keyword_url == NULL) || !is_extension_keyword)) {

[msw, 2014/08/22 17:45:56]

nit: nix (keyword_url == NULL), use |is_extension_keyword|'s NULL check.

[Mark P, 2014/08/22 17:52:42]

Done.

+      // Guarantee that SearchProvider returns a legal default match (except
+      // when in n extension-based keyword mode).  The omnibox always needs

[msw, 2014/08/22 17:45:56]

Remove stray 'n'.

[Mark P, 2014/08/22 17:52:42]

Done.

+      // at least one legal default match, and it relies on SearchProvider in
+      // combination with KeywordProvider (for extension-based keywords) to
+      // always return one.
       ApplyCalculatedRelevance();
       ConvertResultsToAutocompleteMatches();
     }
     DCHECK(!IsTopMatchSearchWithURLInput());
-    DCHECK(FindTopMatch() != matches_.end());
+    DCHECK((FindTopMatch() != matches_.end()) ||
+           ((keyword_url != NULL) && is_extension_keyword));

[msw, 2014/08/22 17:45:56]

nit: nix (keyword_url == NULL), use |is_extension_keyword|'s NULL check.

[Mark P, 2014/08/22 17:52:41]

Done.

   }
   UMA_HISTOGRAM_CUSTOM_COUNTS(
       "Omnibox.SearchProviderMatches", matches_.size(), 1, 6, 7);