Omnibox: Make URLs of Bookmarks Searchable - Try 2

chrome/browser/autocomplete/bookmark_provider.cc

 // Copyright (c) 2012 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/autocomplete/bookmark_provider.h"
 
 #include <algorithm>
 #include <functional>
 #include <vector>
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
 #include "chrome/browser/autocomplete/history_provider.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/bookmarks/browser/bookmark_match.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/metrics/proto/omnibox_input_type.pb.h"
 #include "components/omnibox/autocomplete_result.h"
-#include "components/omnibox/omnibox_field_trial.h"
 #include "components/omnibox/url_prefix.h"
 #include "net/base/net_util.h"
 
 using bookmarks::BookmarkMatch;
 
 typedef std::vector<BookmarkMatch> BookmarkMatches;
 
 namespace {
 
 // Removes leading spaces from |title| before displaying, otherwise it looks
@@ skipping 16 matching lines @@
   }
 }
 
 }  // namespace
 
 // BookmarkProvider ------------------------------------------------------------
 
 BookmarkProvider::BookmarkProvider(Profile* profile)
     : AutocompleteProvider(AutocompleteProvider::TYPE_BOOKMARK),
       profile_(profile),
-      bookmark_model_(NULL),
-      score_using_url_matches_(OmniboxFieldTrial::BookmarksIndexURLsValue()) {
+      bookmark_model_(NULL) {
   if (profile) {
     bookmark_model_ = BookmarkModelFactory::GetForProfile(profile);
     languages_ = profile_->GetPrefs()->GetString(prefs::kAcceptLanguages);
   }
 }
 
 void BookmarkProvider::Start(const AutocompleteInput& input,
                              bool minimal_changes) {
   if (minimal_changes)
     return;
@@ skipping 169 matching lines @@
   //     giving more points for matches that appear earlier in the string:
   //       ((string_length - position of match start) / string_length).
   //
   //  Example: Given a bookmark title of 'abcde fghijklm', with a title length
   //     of 14, and two different search terms, 'abcde' and 'fghij', with
   //     start positions of 0 and 6, respectively, 'abcde' will score higher
   //     (with a a partial factor of (14-0)/14 = 1.000 ) than 'fghij' (with
   //     a partial factor of (14-6)/14 = 0.571 ).  (In this example neither
   //     term matches in the URL.)
   //
-  // Once all match factors have been calculated they are summed.  If URL
-  // matches are not considered, the resulting sum will never be greater than
-  // the length of the bookmark title because of the way the bookmark model
-  // matches and removes overlaps.  (In particular, the bookmark model only
+  // Once all match factors have been calculated they are summed.  If there
+  // are no URL matches, the resulting sum will never be greater than the
+  // length of the bookmark title because of the way the bookmark model matches
+  // and removes overlaps.  (In particular, the bookmark model only
   // matches terms to the beginning of words and it removes all overlapping
   // matches, keeping only the longest.  Together these mean that each
-  // character is included in at most one match.)  If URL matches are
-  // considered, the sum can be greater.
+  // character is included in at most one match.)  If there are matches in the
+  // URL, the sum can be greater.
   //
-  // This sum is then normalized by the length of the bookmark title (if URL
-  // matches are not considered) or by the length of the bookmark title + 10
-  // (if URL matches are considered) and capped at 1.0.  (If URL matches
-  // are considered, we want to expand the scoring range so fewer bookmarks
-  // will hit the 1.0 cap and hence lose all ability to distinguish between
-  // these high-quality bookmarks.)
+  // This sum is then normalized by the length of the bookmark title + 10
+  // and capped at 1.0.  The +10 is to expand the scoring range so fewer
+  // bookmarks will hit the 1.0 cap and hence lose all ability to distinguish
+  // between these high-quality bookmarks.
   //
   // The normalized value is multiplied against the scoring range available,
   // which is 299.  The 299 is calculated by subtracting the minimum possible
   // score, 900, from the maximum possible score, 1199.  This product, ranging
   // from 0 to 299, is added to the minimum possible score, 900, giving the
   // preliminary score.
   //
   // If the preliminary score is less than the maximum possible score, 1199,
   // it can be boosted up to that maximum possible score if the URL referenced
   // by the bookmark is also referenced by any of the user's other bookmarks.
   // A count of how many times the bookmark's URL is referenced is determined
   // and, for each additional reference beyond the one for the bookmark being
   // scored up to a maximum of three, the score is boosted by a fixed amount
   // given by |kURLCountBoost|, below.
   //
-  if (score_using_url_matches_) {
-    // Pretend empty titles are identical to the URL.
-    if (title.empty())
-      title = base::ASCIIToUTF16(url.spec());
-  } else {
-    DCHECK(!title.empty());
-  }
+
+  // Pretend empty titles are identical to the URL.
+  if (title.empty())
+    title = base::ASCIIToUTF16(url.spec());
   ScoringFunctor title_position_functor =
       for_each(bookmark_match.title_match_positions.begin(),
                bookmark_match.title_match_positions.end(),
                ScoringFunctor(title.size()));
   ScoringFunctor url_position_functor =
       for_each(bookmark_match.url_match_positions.begin(),
                bookmark_match.url_match_positions.end(),
                ScoringFunctor(bookmark_match.node->url().spec().length()));
   const double summed_factors = title_position_functor.ScoringFactor() +
-      (score_using_url_matches_ ? url_position_functor.ScoringFactor() : 0);
-  const double normalized_sum = std::min(
-      summed_factors / (title.size() + (score_using_url_matches_ ? 10 : 0)),
-      1.0);
+      url_position_functor.ScoringFactor();
+  const double normalized_sum =
+      std::min(summed_factors / (title.size() + 10), 1.0);
   const int kBaseBookmarkScore = 900;
   const int kMaxBookmarkScore = 1199;
   const double kBookmarkScoreRange =
       static_cast<double>(kMaxBookmarkScore - kBaseBookmarkScore);
   match.relevance = static_cast<int>(normalized_sum * kBookmarkScoreRange) +
       kBaseBookmarkScore;
   // Don't waste any time searching for additional referenced URLs if we
   // already have a perfect title match.
   if (match.relevance >= kMaxBookmarkScore)
     return match;
@@ skipping 10 matching lines @@
 
 // static
 ACMatchClassifications BookmarkProvider::ClassificationsFromMatch(
     const query_parser::Snippet::MatchPositions& positions,
     size_t text_length,
     bool is_url) {
   ACMatchClassification::Style url_style =
       is_url ? ACMatchClassification::URL : ACMatchClassification::NONE;
   ACMatchClassifications classifications;
   if (positions.empty()) {
-    classifications.push_back(
-        ACMatchClassification(0, url_style));
+    if (text_length > 0)

[Mark P, 2014/08/26 21:33:35]

This has changed from the previous review.

+      classifications.push_back(ACMatchClassification(0, url_style));
     return classifications;
   }
 
   for (query_parser::Snippet::MatchPositions::const_iterator i =
            positions.begin();
        i != positions.end();
        ++i) {
     AutocompleteMatch::ACMatchClassifications new_class;
     AutocompleteMatch::ClassifyLocationInString(i->first, i->second - i->first,
         text_length, url_style, &new_class);
     classifications = AutocompleteMatch::MergeClassifications(
         classifications, new_class);
   }
   return classifications;
 }