[omnibox] Fix bug: All matches culled if none allowed to be default

In the status quo, all matches are culled if none of them have
allowed_to_be_default_match() true.

That's a bug, because there's legitimate exotic situations where
truly none of the matches should be pre-selected:
 - DefaultSearchProviderEnabled policy == false
 - NTP on-focus suggestions (Enter should do nothing)

It also removes a DCHECK that's not true. The DCHECK assumes that this
can only occur for NTP on-focus suggestions. This also can occur for
Enterprises that have disallowed by policy the default search provider.

Note: Since we previously just culled all matches in the
none-allowed-default case, it probably papered over some bugs. This CL
is likely to expose those bugs, but I intend to fix them all as soon
as we find them.

If there's a crash bisected to this CL, don't revert this CL,
message me instead.

Bug: 1016845, 363656
Change-Id: I5b4333d3260f21d65500a5e49e3a5b77cd0735ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872485
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709656}

components/omnibox/browser/autocomplete_result.cc

@@ -28,6 +28,7 @@
 #include "components/omnibox/browser/omnibox_pedal.h"
 #include "components/omnibox/browser/omnibox_pedal_provider.h"
 #include "components/omnibox/common/omnibox_features.h"
+#include "components/search_engines/template_url_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/url_fixer.h"
 #include "third_party/metrics_proto/omnibox_event.pb.h"
@@ -296,14 +297,23 @@ void AutocompleteResult::SortAndCull(
              : base::string16()) +
         base::ASCIIToUTF16(", input=") + input.text();
 
-    // We should only get here with an empty omnibox for automatic suggestions
-    // on focus on the NTP; in these cases hitting enter should do nothing, so
-    // there should be no default match.  Otherwise, we're doing automatic
-    // suggestions for the currently visible URL (and hitting enter should
-    // reload it), or the user is typing; in either of these cases, there should
-    // be a default match.
-    DCHECK_NE(input.text().empty(), default_match_->allowed_to_be_default_match)
-        << debug_info;
+    // It's unusual if |default_match_| is not |allowed_to_be_default_match|.
+    // This can occur in two situations:
+    //  - Empty-textfield on-focus suggestions (i.e. NTP, ChromeOS launcher)
+    //  - Enterprise policy prohibiting a default search provider
+    //
+    // In those cases, hitting Enter should do nothing, so there should be
+    // legitimately no default match.
+    //
+    // TODO(tommycli): It seems odd that we are still setting |default_match_|
+    // in that case. We should fix that.
+    if (!default_match_->allowed_to_be_default_match) {
+      bool default_search_provider_exists =
+          template_url_service &&
+          template_url_service->GetDefaultSearchProvider();
+      DCHECK(input.text().empty() || !default_search_provider_exists)
+          << debug_info;
+    }
 
     // For navigable default matches, make sure the destination type is what the
     // user would expect given the input.
@@ -768,6 +778,10 @@ void AutocompleteResult::MaybeCullTailSuggestions(ACMatches* matches) {
       matches->begin(), matches->end(), [&](const AutocompleteMatch& match) {
         return match.allowed_to_be_default_match && !is_tail(match);
       });
+  bool tail_default_exists = std::any_of(
+      matches->begin(), matches->end(), [&](const AutocompleteMatch& match) {
+        return match.allowed_to_be_default_match && is_tail(match);
+      });
   // If the only default matches are tail suggestions, let them remain and
   // instead remove the non-tail suggestions.  This is necessary because we do
   // not want to display tail suggestions mixed with other suggestions in the
@@ -776,7 +790,7 @@ void AutocompleteResult::MaybeCullTailSuggestions(ACMatches* matches) {
   // default match--the non-tail ones much go.  This situation though is
   // unlikely, as we normally would expect the search-what-you-typed suggestion
   // as a default match (and that's a non-tail suggestion).
-  if (non_tail_default == matches->end()) {
+  if (tail_default_exists && non_tail_default == matches->end()) {
     base::EraseIf(*matches, [&is_tail](const AutocompleteMatch& match) {
       return !is_tail(match);
     });

components/omnibox/browser/autocomplete_result_unittest.cc

@@ -552,6 +552,32 @@ TEST_F(AutocompleteResultTest, SortAndCullOnlyTailSuggestions) {
               result.match_at(i)->type);
 }
 
+TEST_F(AutocompleteResultTest, SortAndCullNoMatchesAllowedToBeDefault) {
+  // clang-format off
+  TestData data[] = {
+      {1, 1, 500,  false},  // Not allowed_to_be_default_match
+      {2, 1, 1100, false},  // Not allowed_to_be_default_match
+      {3, 1, 1000, false},  // Not allowed_to_be_default_match
+  };
+  // clang-format on
+
+  ACMatches matches;
+  PopulateAutocompleteMatches(data, base::size(data), &matches);
+
+  AutocompleteInput input(base::string16(), metrics::OmniboxEventProto::OTHER,
+                          TestSchemeClassifier());
+  AutocompleteResult result;
+  result.AppendMatches(input, matches);
+  result.SortAndCull(input, template_url_service_.get());
+
+  EXPECT_EQ(3UL, result.size());
+  EXPECT_EQ(matches[1].destination_url, result.match_at(0)->destination_url);
+  EXPECT_EQ(matches[2].destination_url, result.match_at(1)->destination_url);
+  EXPECT_EQ(matches[0].destination_url, result.match_at(2)->destination_url);
+  for (size_t i = 0; i < 3; ++i)
+    EXPECT_FALSE(result.match_at(i)->allowed_to_be_default_match);
+}
+
 TEST_F(AutocompleteResultTest, SortAndCullDuplicateSearchURLs) {
   // Register a template URL that corresponds to 'foo' search engine.
   TemplateURLData url_data;