removing ShouldHideTopMatch

chrome/browser/ui/omnibox/omnibox_controller.cc

Index: chrome/browser/ui/omnibox/omnibox_controller.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_controller.cc b/chrome/browser/ui/omnibox/omnibox_controller.cc
index db2aaa1065b0ac936923369827b870051ea63dbf..b855aa5381586c9dce341d66b52e33c1e7b741fa 100644
--- a/chrome/browser/ui/omnibox/omnibox_controller.cc
+++ b/chrome/browser/ui/omnibox/omnibox_controller.cc
@@ -39,12 +39,9 @@ namespace {
 //
 // 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(
@@ -53,20 +50,12 @@ const AutocompleteMatch* GetMatchToPrefetch(const AutocompleteResult& result) {
   }
 
   // If the default match should be prefetched, do that.
-  const AutocompleteResult::const_iterator default_match(
-      result.default_match());
+  const AutocompleteResult::const_iterator default_match =
+      result.default_match();

[Peter Kasting, 2015/05/28 23:06:24]

Nit: Not sure why the change of initializer format... but if you're mucking with
this line anyway I might change to use "const auto" here.

[dschuyler, 2015/05/29 20:38:39]

Groby requested (above in this CL) that it be changed.
I went ahead and changed it.  I am wondering why auto would be preferred though.
 Can you tell me more about the <auto vs explicit type> preference or direct me
to something to read to help me understand this nit?

Done.

[groby-ooo-7-16, 2015/05/29 20:46:04]

Presumably because I asked why we use the copy-ctor here :)

What I meant by that is, why do we make a second instance at all? Can't we just
use result.default_match() *instead* of default_match()?

[Peter Kasting, 2015/05/29 21:10:12]

We totally can.  This was just intended to be a shorter alias.

I'm fine with removing this entirely and writing result.default_match()
explicitly below if it's not too cumbersome.

On 2015/05/29 20:38:39, dschuyler wrote:
Check out the internal style guide's section on "auto" for more, but basically,
use auto when the real type name is verbose and doesn't really add clarity for
the reader (either the type is obvious or it's not important to know the precise
type).

[groby-ooo-7-16, 2015/05/29 21:11:21]

auto vs. explicit in a nutshell: Use auto if the specific type doesn't add
clarity. (It doesn't here - it's some kind of iterator, and we pass through it's
value. The specific kind is irrelevant).

In general, discussion of C++11 features in Chromium is at
https://chromium-cpp.appspot.com/
For all features, there's a link to a long discussion on chromium-dev why/how to
use this.

   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
-  // 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;
 }