autocomplete: Make ContactProvider rank contacts by affinity

chrome/browser/autocomplete/contact_provider_chromeos.cc

Index: chrome/browser/autocomplete/contact_provider_chromeos.cc
diff --git a/chrome/browser/autocomplete/contact_provider_chromeos.cc b/chrome/browser/autocomplete/contact_provider_chromeos.cc
index 2e7ff9a81dc49710d63f21b169109d4a0306d94a..e25b0a4fd66780d6eb0da5c7c9646445a0365ee9 100644
--- a/chrome/browser/autocomplete/contact_provider_chromeos.cc
+++ b/chrome/browser/autocomplete/contact_provider_chromeos.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/autocomplete/contact_provider_chromeos.h"
 
 #include <algorithm>
+#include <cmath>
 
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/string_search.h"
@@ -18,6 +19,9 @@
 
 namespace {
 
+// Default affinity assigned to contacts whose |affinity| field is unset.
+float kDefaultAffinity = 1.0;

[Peter Kasting, 2012/09/13 23:32:37]

Isn't this kind of high?  I guess I don't totally understand how affinity is
calculated.

[Daniel Erat, 2012/09/14 15:31:50]

Dirty secret: We're not getting affinity data yet, so I set this high so we'll
see contacts in the app list results when the flag is set.  Added a TODO.

+
 // Returns true if |word_to_find| is a prefix of |name_to_search| and marks the
 // matching text in |classifications| (which corresponds to the contact's full
 // name).  |name_index_in_full_name| contains |name_to_search|'s index within
@@ -46,23 +50,35 @@ bool WordIsNamePrefix(const string16& word_to_find,
   return true;
 }
 
+// Returns true if |a|'s affinity is greater than |b|'s affinity.
+bool CompareContactAffinity(const contacts::Contact* a,
+                            const contacts::Contact* b) {
+  float affinity_a = a->has_affinity() ? a->affinity() : kDefaultAffinity;

[Peter Kasting, 2012/09/13 23:32:37]

Super tiny nit: Seems like |a_affinity| would be just a teensy bit better than
|affinity_a|

[Daniel Erat, 2012/09/14 15:31:50]

Done.

+  float affinity_b = b->has_affinity() ? b->affinity() : kDefaultAffinity;
+  return affinity_a > affinity_b;
+}
+
 }  // namespace
 
 // static
 const char ContactProvider::kMatchContactIdKey[] = "contact_id";
+const int ContactProvider::kBaseRelevance = 1300;

[Daniel Erat, 2012/09/13 23:10:08]

I just pulled numbers out of the air for this.  Since this is off by default and
we have no plans to expose these results anywhere outside the app list (which
itself will soon only return apps rather than giving the full omnibox
experience), I'm not sure how much these numbers matter.

+const int ContactProvider::kAffinityRelevanceBoost = 200;
 
 // Cached information about a contact.
 struct ContactProvider::ContactData {
   ContactData(const string16& full_name,
               const string16& given_name,
               const string16& family_name,
-              const std::string& contact_id)
+              const std::string& contact_id,
+              float affinity)
       : full_name(full_name),
         given_name(given_name),
         family_name(family_name),
         given_name_index(string16::npos),
         family_name_index(string16::npos),
-        contact_id(contact_id) {
+        contact_id(contact_id),
+        affinity(affinity) {
     base::i18n::StringSearchIgnoringCaseAndAccents(
         given_name, full_name, &given_name_index, NULL);
     base::i18n::StringSearchIgnoringCaseAndAccents(
@@ -80,6 +96,9 @@ struct ContactProvider::ContactData {
 
   // Unique ID used to look up additional contact information.
   std::string contact_id;
+
+  // Affinity between the user and this contact, in the range [0.0, 1.0].
+  float affinity;
 };
 
 ContactProvider::ContactProvider(
@@ -115,8 +134,11 @@ void ContactProvider::Start(const AutocompleteInput& input,
     }
   }
 
+  // |contacts_| is ordered by descending affinity.  Since affinity is currently
+  // the only signal used for computing relevance, we can stop after we've found
+  // kMaxMatches results.
   for (ContactDataVector::const_iterator it = contacts_.begin();
-       it != contacts_.end(); ++it)
+       it != contacts_.end() && matches_.size() < kMaxMatches; ++it)
     AddContactIfMatched(input, input_words, *it);
 }
 
@@ -139,6 +161,7 @@ void ContactProvider::RefreshContacts() {
 
   scoped_ptr<contacts::ContactPointers> contacts =
       contact_manager_->GetAllContacts(profile_);
+  std::sort(contacts->begin(), contacts->end(), CompareContactAffinity);

[Peter Kasting, 2012/09/13 23:32:37]

Nit: If we move this down to the bottom of the function, we can get rid of the
need to check for missing affinity from CompareContactAffinity(), because the
code below will have taken care of that

[Daniel Erat, 2012/09/14 15:31:50]

I went back and forth on this.  Sorting pointers seemed better than sorting
structs, but that's a pretty specious argument for me to take (if it were really
a big deal performance-wise, I shouldn't be storing the structs directly in a
vector, or I should at least be calling reserve() first).  Updated.

 
   contacts_.clear();
   for (contacts::ContactPointers::const_iterator it = contacts->begin();
@@ -150,11 +173,13 @@ void ContactProvider::RefreshContacts() {
         AutocompleteMatch::SanitizeString(UTF8ToUTF16(contact.given_name()));
     string16 family_name =
         AutocompleteMatch::SanitizeString(UTF8ToUTF16(contact.family_name()));
+    float affinity =
+        contact.has_affinity() ? contact.affinity() : kDefaultAffinity;
 
     if (!full_name.empty()) {
       contacts_.push_back(
-          ContactData(
-              full_name, given_name, family_name, contact.contact_id()));
+          ContactData(full_name, given_name, family_name, contact.contact_id(),
+                      affinity));
     }
   }
 }
@@ -200,8 +225,8 @@ AutocompleteMatch ContactProvider::CreateAutocompleteMatch(
   match.inline_autocomplete_offset = string16::npos;
   match.contents = contact.full_name;
   match.fill_into_edit = match.contents;
-  // TODO(derat): Implement ranking.
-  match.relevance = 1;
+  match.relevance = kBaseRelevance +
+      static_cast<int>(roundf(kAffinityRelevanceBoost * contact.affinity));
   match.RecordAdditionalInfo(kMatchContactIdKey, contact.contact_id);
   return match;
 }