Omnibox - Refactor |relevance_buckets| to Remove Memory Leak on Exit

components/omnibox/browser/scored_history_match.h

 // 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.
 
 #ifndef COMPONENTS_OMNIBOX_BROWSER_SCORED_HISTORY_MATCH_H_
 #define COMPONENTS_OMNIBOX_BROWSER_SCORED_HISTORY_MATCH_H_
 
 #include <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/gtest_prod_util.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "components/history/core/browser/history_match.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/omnibox/browser/in_memory_url_index_types.h"
 
 class ScoredHistoryMatchTest;
 
 // An HistoryMatch that has a score as well as metrics defining where in the
 // history item's URL and/or page title matches have occurred.
 struct ScoredHistoryMatch : public history::HistoryMatch {
   // ScoreMaxRelevance maps from an intermediate-score to the maximum
   // final-relevance score given to a URL for this intermediate score.
-  // This is used to store the score ranges of HQP relevance buckets.
+  // This is used to store the score ranges of relevance buckets.
   // Please see GetFinalRelevancyScore() for details.
   typedef std::pair<double, int> ScoreMaxRelevance;
 
   // Required for STL, we don't use this directly.
   ScoredHistoryMatch();
   ScoredHistoryMatch(const ScoredHistoryMatch& other);
 
   // Initializes the ScoredHistoryMatch with a raw score calculated for the
   // history item given in |row| with recent visits as indicated in |visits|. It
   // first determines if the row qualifies by seeing if all of the terms in
@@ skipping 83 matching lines @@
 
   // Examines the first |max_visits_to_score_| and returns a score (higher is
   // better) based the rate of visits, whether the page is bookmarked, and
   // how often those visits are typed navigations (i.e., explicitly
   // invoked by the user).  |now| is passed in to avoid unnecessarily
   // recomputing it frequently.
   float GetFrequency(const base::Time& now,
                      const bool bookmarked,
                      const VisitInfoVector& visits) const;
 
-  // Combines the two component scores into a final score that's
-  // an appropriate value to use as a relevancy score. Scoring buckets are
-  // specified through |hqp_relevance_buckets|. Please see the function
-  // implementation for more details.
-  static float GetFinalRelevancyScore(
-      float topicality_score,
-      float frequency_score,
-      const std::vector<ScoreMaxRelevance>& hqp_relevance_buckets);
+  // Combines the two component scores into a final score that's an appropriate
+  // value to use as a relevancy score.
+  static float GetFinalRelevancyScore(float topicality_score,
+                                      float frequency_score);
 
-  // Initializes the HQP experimental params: |hqp_relevance_buckets_|
-  // to default buckets. If hqp experimental scoring is enabled, it
-  // fetches the |hqp_experimental_scoring_enabled_|, |topicality_threshold_|
-  // and |hqp_relevance_buckets_| from omnibox field trials.
-  static void InitHQPExperimentalParams();
+  // Helper function that returns the string containing the scoring buckets
+  // (either the default ones or ones specified in an experiment).
+  static std::vector<ScoreMaxRelevance> GetHQPBuckets();

[Peter Kasting, 2016/12/08 01:29:21]

I don't love how often "std::vector<ScoreMaxRelevance>" gets named in these
various files... a type alias might make sense.

[Mark P, 2016/12/09 20:40:56]

Done.

 
-  // Helper function to parse the string containing the scoring buckets.
-  // For example,
-  // String: "0.0:400,1.5:600,12.0:1300,20.0:1399"
-  // Buckets: vector[(0.0, 400),(1.5,600),(12.0,1300),(20.0,1399)]
-  // Returns false, in case if it fail to parse the string.
-  static bool GetHQPBucketsFromString(
-      const std::string& buckets_str,
-      std::vector<ScoreMaxRelevance>* hqp_buckets);
+  // Helper function to parse the string containing the scoring buckets and
+  // return the results.  For example, with |bucket_str| as

[Peter Kasting, 2016/12/08 01:29:21]

Nit: buckets

[Mark P, 2016/12/09 20:40:56]

Done.

+  // "0.0:400,1.5:600,12.0:1300,20.0:1399", it returns [(0.0, 400), (1.5, 600),
+  // (12.0, 1300), (20.0, 1399)]. It returns an empty list in the case of a

[Peter Kasting, 2016/12/08 01:29:22]

Nit: list -> vector

[Mark P, 2016/12/09 20:40:56]

Done.

+  // malformed |buckets_str|.
+  static std::vector<ScoreMaxRelevance> GetHQPBucketsFromString(
+      const std::string& buckets_str);
 
   // If true, assign raw scores to be max(whatever it normally would be, a
   // score that's similar to the score HistoryURL provider would assign).
   static bool also_do_hup_like_scoring_;
 
   // Untyped visits to bookmarked pages score this, compared to 1 for
   // untyped visits to non-bookmarked pages and |typed_value_| for typed visits.
   static float bookmark_value_;
 
   // Typed visits to page score this, compared to 1 for untyped visits.
@@ skipping 13 matching lines @@
   // If true, we allow input terms to match in the TLD (e.g., ".com").
   static bool allow_tld_matches_;
 
   // If true, we allow input terms to match in the scheme (e.g., "http://").
   static bool allow_scheme_matches_;
 
   // The number of title words examined when computing topicality scores.
   // Words beyond this number are ignored.
   static size_t num_title_words_to_allow_;
 
-  // True, if hqp experimental scoring is enabled.
-  static bool hqp_experimental_scoring_enabled_;
-
   // |topicality_threshold_| is used to control the topicality scoring.
-  // If |topicality_threshold_| > 0, then URLs with topicality-score < threshold
-  // are given topicality score of 0. By default it is initalized to -1.
+  // If |topicality_threshold_| > 0, then URLs with topicality-score less than
+  // the threshold are given topicality score of 0.
   static float topicality_threshold_;
 
-  // |hqp_relevance_buckets_str_| is used to control the hqp score ranges.
-  // It is the string representation of |hqp_relevance_buckets_|.
-  static char hqp_relevance_buckets_str_[];
-
-  // |hqp_relevance_buckets_| gives mapping from (topicality*frequency)
-  // to the final relevance scoring. Please see GetFinalRelevancyScore()
-  // for more details and scoring method.
-  static std::vector<ScoreMaxRelevance>* hqp_relevance_buckets_;
+  // Used for testing.  This is a pointer to the static local variable
+  // |relevance_buckets| declared in GetFinalRelevancyScore().  It's not valid
+  // until after the first call to GetFinalRelevancyScore().
+  static std::vector<ScoreMaxRelevance>* relevance_buckets_;
 };
 typedef std::vector<ScoredHistoryMatch> ScoredHistoryMatches;
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_SCORED_HISTORY_MATCH_H_