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