removing ShouldHideTopMatch

chrome/browser/ui/omnibox/omnibox_controller.cc

 // Copyright 2013 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/ui/omnibox/omnibox_controller.h"
 
 #include "base/metrics/histogram.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h"
 #include "chrome/browser/net/predictor.h"
@@ skipping 21 matching lines @@
 // Returns the AutocompleteMatch that the InstantController should prefetch, if
 // any.
 //
 // The SearchProvider may mark some suggestions to be prefetched based on
 // instructions from the suggest server. If such a match ranks sufficiently
 // highly or if kAllowPrefetchNonDefaultMatch field trial is enabled, we'll
 // return it.
 //
 // If the kAllowPrefetchNonDefaultMatch field trial is enabled we return the
 // prefetch suggestion even if it is not the default match. Otherwise we only
-// care about matches that are the default or the very first entry in the
-// dropdown (which can happen for non-default matches only if we're hiding a top
-// verbatim match) or the second entry in the dropdown (which can happen for
-// non-default matches when a top verbatim match is shown); for other matches,
-// we think the likelihood of the user selecting them is low enough that
-// prefetching isn't worth doing.
+// care about matches that are the default; for other matches, we think the
+// likelihood of the user selecting them is low enough that prefetching isn't
+// worth doing.
 const AutocompleteMatch* GetMatchToPrefetch(const AutocompleteResult& result) {
   if (chrome::ShouldAllowPrefetchNonDefaultMatch()) {
     const AutocompleteResult::const_iterator prefetch_match = std::find_if(
         result.begin(), result.end(), SearchProvider::ShouldPrefetch);
     return prefetch_match != result.end() ? &(*prefetch_match) : NULL;
   }
 
   // If the default match should be prefetched, do that.
-  const AutocompleteResult::const_iterator default_match(
-      result.default_match());
+  const auto default_match = result.default_match();
   if ((default_match != result.end()) &&
       SearchProvider::ShouldPrefetch(*default_match))
     return &(*default_match);
 
-  // Otherwise, if the top match is a verbatim match and the very next match

[Mark P, 2015/06/01 19:23:02]

I don't understand why you're removing this block.

[Peter Kasting, 2015/06/01 20:18:01]

kmadhusu said the TopMatchIsStandaloneVerbatimMatch() stuff should be removed.

[kmadhusu, 2015/06/01 20:24:32]

pkasting@, I am assuming default match is always the top match. Is my assumption
correct?

[Peter Kasting, 2015/06/01 20:26:57]

Default match is always top match.

What removing this block entirely will do is, previously if the default match
was the verbatim input and the next match was a prefetchable non-verbatim match,
this would prefetch it.  By removing it we'll no longer do that.  Are you sure
you want this gone?  You said it was part of an unlaunched experiment but I
didn't see anything about this that looked experiment-gated.

[kmadhusu, 2015/06/01 20:37:21]

I see. Thanks for the details. I think we need this block.

Use case: 
(1) User typed "south" in the omnibox.
(2) Omnibox suggestions:
   south
   southwest
   www.southwest.com
   www.southwest.com/xyz
   www.southwest.com/abc

In this case, we want to check if "southwest" is marked for prefetching. 
    

[dschuyler, 2015/06/05 20:33:54]

Thanks for going over that!

Done.

-  // is prefetchable, fetch that.
-  if ((result.ShouldHideTopMatch() ||
-       result.TopMatchIsStandaloneVerbatimMatch()) &&
-      (result.size() > 1) &&
-      SearchProvider::ShouldPrefetch(result.match_at(1)))
-    return &result.match_at(1);
-
   return NULL;
 }
 
 // Calls back to the OmniboxController when the requested image is downloaded.
 // This is a separate class instead of being implemented on OmniboxController
 // because BitmapFetcherService currently takes ownership of this object.
 // TODO(dschuyler): Make BitmapFetcherService use the more typical non-owning
 // ObserverList pattern and have OmniboxController implement the Observer call
 // directly.
 class AnswerImageObserver : public BitmapFetcherService::Observer {
@@ skipping 134 matching lines @@
     // We could prefetch the alternate nav URL, if any, but because there
     // can be many of these as a user types an initial series of characters,
     // the OS DNS cache could suffer eviction problems for minimal gain.
   }
 }
 
 void OmniboxController::SetAnswerBitmap(const SkBitmap& bitmap) {
   request_id_ = BitmapFetcherService::REQUEST_ID_INVALID;
   popup_->SetAnswerBitmap(bitmap);
 }