app_list search results relevance scores are clamped to [0, 1] range.

Since the scores come from an external service, they should be forced
into the [0, 1] range, instead of assuming they are in that range. (We
cannot accept any arbitrary score, because then that would allow the
service to prioritize its results over other services, and would also
break the "known results" prioritization scheme.)

BUG=456641

Review URL: https://codereview.chromium.org/906133003

Cr-Commit-Position: refs/heads/master@{#315275}

ui/app_list/search/mixer.cc

@@ -63,10 +63,13 @@ class Mixer::Group {
 
     for (const SearchProvider* provider : providers_) {
       for (SearchResult* result : provider->results()) {
-        DCHECK_GE(result->relevance(), 0.0);
-        DCHECK_LE(result->relevance(), 1.0);
         DCHECK(!result->id().empty());
 
+        // We cannot rely on providers to give relevance scores in the range
+        // [0.0, 1.0] (e.g., PeopleProvider directly gives values from the
+        // Google+ API). Clamp to that range.
+        double relevance = std::min(std::max(result->relevance(), 0.0), 1.0);
+
         double boost = boost_;
         KnownResults::const_iterator known_it =
             known_results.find(result->id());
@@ -94,7 +97,7 @@ class Mixer::Group {
         if (is_voice_query && result->voice_result())
           boost += 4.0;
 
-        results_.push_back(SortData(result, result->relevance() + boost));
+        results_.push_back(SortData(result, relevance + boost));
       }
     }
 

ui/app_list/search/mixer_unittest.cc

@@ -57,7 +57,7 @@ int TestSearchResult::instantiation_count = 0;
 class TestSearchProvider : public SearchProvider {
  public:
   explicit TestSearchProvider(const std::string& prefix)
-      : prefix_(prefix), count_(0) {}
+      : prefix_(prefix), count_(0), bad_relevance_range_(false) {}
   ~TestSearchProvider() override {}
 
   // SearchProvider overrides:
@@ -66,7 +66,11 @@ class TestSearchProvider : public SearchProvider {
     for (size_t i = 0; i < count_; ++i) {
       const std::string id =
           base::StringPrintf("%s%d", prefix_.c_str(), static_cast<int>(i));
-      const double relevance = 1.0 - i / 10.0;
+      double relevance = 1.0 - i / 10.0;
+      // If bad_relevance_range_, change the relevances to give results outside
+      // of the canonical [0.0, 1.0] range.
+      if (bad_relevance_range_)
+        relevance = 10.0 - i * 10;
       TestSearchResult* result = new TestSearchResult(id, relevance);
       if (voice_result_indices.find(i) != voice_result_indices.end())
         result->set_voice_result(true);
@@ -78,10 +82,12 @@ class TestSearchProvider : public SearchProvider {
   void set_prefix(const std::string& prefix) { prefix_ = prefix; }
   void set_count(size_t count) { count_ = count; }
   void set_as_voice_result(size_t index) { voice_result_indices.insert(index); }
+  void set_bad_relevance_range() { bad_relevance_range_ = true; }
 
  private:
   std::string prefix_;
   size_t count_;
+  bool bad_relevance_range_;
   // Indices of results that will have the |voice_result| flag set.
   std::set<size_t> voice_result_indices;
 
@@ -264,6 +270,25 @@ TEST_F(MixerTest, VoiceQuery) {
   EXPECT_EQ("omnibox1,omnibox2,omnibox0", GetResults());
 }
 
+TEST_F(MixerTest, BadRelevanceRange) {
+  // This gives relevance scores: (10.0, 0.0). Even though providers are
+  // supposed to give scores within the range [0.0, 1.0], we cannot rely on
+  // providers to do this, since they retrieve results from disparate and
+  // unreliable sources (like the Google+ API).
+  people_provider()->set_bad_relevance_range();
+  people_provider()->set_count(2);
+
+  // Give a massive boost to the second result.
+  AddKnownResult("people1", PERFECT_PRIMARY);
+
+  RunQuery();
+
+  // If the results are correctly clamped to the range [0.0, 1.0], the boost to
+  // "people1" will push it over the first result. If not, the massive base
+  // score of "people0" will erroneously keep it on top.
+  EXPECT_EQ("people1,people0", GetResults());
+}
+
 TEST_F(MixerTest, Publish) {
   scoped_ptr<SearchResult> result1(new TestSearchResult("app1", 0));
   scoped_ptr<SearchResult> result2(new TestSearchResult("app2", 0));