Removes the link in the omnibox for searching history.

Removes the link in the omnibox for searching history.

BUG=58975
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71963

[Peter Kasting, 2011-01-19 20:02:36]

LGTM

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/autocomplete.cc (right):

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete.cc:608: std::sort(matches_.begin(),
matches_.end(), &AutocompleteMatch::MoreRelevant);
Nit: Line too long.

Maybe we should just condense these two blocks into one:

std::partial_sort(matches_.begin(), matches_.begin() + std::min(kMaxMatches,
matches_.size()), ...);
matches_.resize(kMaxMatches);

?

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/autocomplete.h (right):

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete.h:649: HistoryContentsProvider*
history_contents_provider_;
Seems like we can get rid of this and its references.

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/autocomplete_match.cc (right):

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_match.cc:86: // A negative relevance
indicates the real relevance can be determined by
Nit: The negative stuff can all go away?

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/autocomplete_match.h (right):

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_match.h:136: // If the relevance is
negative, it will only be displayed if there are not
Nit: This paragraph can go away?

[sky, 2011-01-19 23:59:04]

All addressed. New snapshot uploaded.

  -Scott

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/autocomplete.cc (right):

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete.cc:608: std::sort(matches_.begin(),
matches_.end(), &AutocompleteMatch::MoreRelevant);
On 2011/01/19 20:02:36, Peter Kasting wrote:
> Nit: Line too long.
> 
> Maybe we should just condense these two blocks into one:
> 
> std::partial_sort(matches_.begin(), matches_.begin() + std::min(kMaxMatches,
> matches_.size()), ...);
> matches_.resize(kMaxMatches);
> 
> ?

Sure. I think you mean:


  const size_t num_matches = std::min(kMaxMatches, matches_.size());
  std::partial_sort(matches_.begin(), matches_.begin() + num_matches,
                    matches_.end(), &AutocompleteMatch::MoreRelevant);
  // Remove matches so that we have at most kMaxMatches.
  matches_.resize(num_matches);

[Peter Kasting, 2011-01-20 00:08:33]

LGTM

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
File chrome/browser/autocomplete/autocomplete.cc (right):

http://codereview.chromium.org/6268005/diff/3001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete.cc:608: std::sort(matches_.begin(),
matches_.end(), &AutocompleteMatch::MoreRelevant);
On 2011/01/19 23:59:04, sky wrote:
> Sure. I think you mean:
> 
> 
>   const size_t num_matches = std::min(kMaxMatches, matches_.size());
>   std::partial_sort(matches_.begin(), matches_.begin() + num_matches,
>                     matches_.end(), &AutocompleteMatch::MoreRelevant);
>   // Remove matches so that we have at most kMaxMatches.
>   matches_.resize(num_matches);

Yes.

http://codereview.chromium.org/6268005/diff/7002/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete.cc:601: if (matches_.size() >
kMaxMatches) {
Nit: I'd just combine the two comments into a single "Trim to the most relevant
kMaxMatches matches."