Adding a fourth omnibox prerender experiment.

chrome/browser/autocomplete/network_action_predictor.cc

Index: chrome/browser/autocomplete/network_action_predictor.cc
diff --git a/chrome/browser/autocomplete/network_action_predictor.cc b/chrome/browser/autocomplete/network_action_predictor.cc
index 31066d2979871e3ddf18861ff9a501fa02d808fc..efdabb5972d0db208d32266bafbd98df155d94ee 100644
--- a/chrome/browser/autocomplete/network_action_predictor.cc
+++ b/chrome/browser/autocomplete/network_action_predictor.cc
@@ -130,6 +130,35 @@ NetworkActionPredictor::~NetworkActionPredictor() {
   db_->OnPredictorDestroyed();
 }
 
+void NetworkActionPredictor::RegisterTransitionalMatches(
+    const string16& user_text,
+    const AutocompleteResult& result) {
+  if (prerender::GetOmniboxHeuristicToUse() !=
+      prerender::OMNIBOX_HEURISTIC_EXACT_FULL) {
+    return;
+  }
+  if (user_text.length() < kMinimumUserTextLength)
+    return;
+  const string16 lower_user_text(base::i18n::ToLower(user_text));
+
+  // Don't register if we already saw this |user_text|.

[Peter Kasting, 2011/11/18 21:17:13]

Are you sure this is what you want?  It's possible for the same user_text to
have many different sets of matches in the same single navigation.  Two ways
this can happen: OnChanged() can get called repeatedly as more providers return
async results to the controller (I think), and the user can delete an inline
autocompletion, resulting in re-running an earlier text query but with
autocomplete disabled (which will result in a different match set).

Maybe you want to union the URLs from all instances with the same text?  I
dunno.

[dominich, 2011/11/18 23:05:51]

I tested this and although I saw OnChanged being called three times for every
character typed (!) I saw the same matches every time. If there's a chance that
they might be different, then yes, I want the union.

+  if (std::find(transitional_matches_.begin(), transitional_matches_.end(),
+                lower_user_text) != transitional_matches_.end())
+    return;
+
+  TransitionalMatch transitional_match;
+  transitional_match.user_text = lower_user_text;
+  for (AutocompleteResult::const_iterator it = result.begin();
+       it != result.end(); ++it) {
+    transitional_match.urls.push_back(it->destination_url);
+  }
+  transitional_matches_.push_back(transitional_match);
+}
+
+void NetworkActionPredictor::ClearTransitionalMatches() {
+  transitional_matches_.clear();
+}
+
 // Given a match, return a recommended action.
 NetworkActionPredictor::Action NetworkActionPredictor::RecommendAction(
     const string16& user_text,
@@ -150,6 +179,7 @@ NetworkActionPredictor::Action NetworkActionPredictor::RecommendAction(
       break;
     }
     case prerender::OMNIBOX_HEURISTIC_EXACT:
+    case prerender::OMNIBOX_HEURISTIC_EXACT_FULL:
       confidence = ExactAlgorithm(user_text, match);
       break;
     default:
@@ -225,40 +255,10 @@ void NetworkActionPredictor::Observe(
     // and those are the events we're most interested in.
     case chrome::NOTIFICATION_OMNIBOX_OPENED_URL: {
       DCHECK(initialized_);
-      AutocompleteLog* log = content::Details<AutocompleteLog>(details).ptr();
-      if (log->text.length() < kMinimumUserTextLength)
-        break;
-
-      const string16 lower_user_text(base::i18n::ToLower(log->text));
-
-      BeginTransaction();
-      for (size_t i = 0; i < log->result.size(); ++i) {
-        const AutocompleteMatch& match(log->result.match_at(i));
-        const DBCacheKey key = { lower_user_text, match.destination_url };
-
-        bool is_hit = (i == log->selected_index);
-
-        NetworkActionPredictorDatabase::Row row;
-        row.user_text = key.user_text;
-        row.url = key.url;
-
-        DBCacheMap::iterator it = db_cache_.find(key);
-        if (it == db_cache_.end()) {
-          row.id = guid::GenerateGUID();
-          row.number_of_hits = is_hit ? 1 : 0;
-          row.number_of_misses = is_hit ? 0 : 1;
-
-          AddRow(key, row);
-        } else {
-          DCHECK(db_id_cache_.find(key) != db_id_cache_.end());
-          row.id = db_id_cache_.find(key)->second;
-          row.number_of_hits = it->second.number_of_hits + (is_hit ? 1 : 0);
-          row.number_of_misses = it->second.number_of_misses + (is_hit ? 0 : 1);
-
-          UpdateRow(it, row);
-        }
-      }
-      CommitTransaction();
+
+      // TODO(dominich): This doesn't need to be synchronous. Investigate
+      // posting it as a task to be run later.
+      OnOmniboxOpenedUrl(*content::Details<AutocompleteLog>(details).ptr());
       break;
     }
 
@@ -278,6 +278,68 @@ void NetworkActionPredictor::Observe(
   }
 }
 
+void NetworkActionPredictor::OnOmniboxOpenedUrl(const AutocompleteLog& log) {
+  if (log.text.length() < kMinimumUserTextLength)
+    return;
+
+  const GURL& opened_url =
+      log.result.match_at(log.selected_index).destination_url;
+
+  const string16 lower_user_text(base::i18n::ToLower(log.text));
+
+  // Add the current match as the only transitional match.
+  if (prerender::GetOmniboxHeuristicToUse() !=
+      prerender::OMNIBOX_HEURISTIC_EXACT_FULL) {
+    DCHECK(transitional_matches_.empty());
+    TransitionalMatch dummy_match;
+    dummy_match.user_text = lower_user_text;
+    dummy_match.urls.push_back(opened_url);
+    transitional_matches_.push_back(dummy_match);
+  }
+
+  BeginTransaction();
+  // Traverse transitional matches for those that have a user_text that is a
+  // prefix of |lower_user_text|.
+  for (std::vector<TransitionalMatch>::const_iterator it =
+       transitional_matches_.begin(); it != transitional_matches_.end();
+       ++it) {
+    if (!StartsWith(lower_user_text, it->user_text, true))
+      continue;
+
+    // Add entries to the database for those matches.
+    for (std::vector<GURL>::const_iterator url_it = it->urls.begin();
+         url_it != it->urls.end(); ++url_it) {
+      DCHECK(it->user_text.length() >= kMinimumUserTextLength);
+      const DBCacheKey key = { it->user_text, *url_it };
+      const bool is_hit = (*url_it == opened_url);
+
+      NetworkActionPredictorDatabase::Row row;
+      row.user_text = key.user_text;
+      row.url = key.url;
+
+      DBCacheMap::iterator it = db_cache_.find(key);
+      if (it == db_cache_.end()) {
+        row.id = guid::GenerateGUID();
+        row.number_of_hits = is_hit ? 1 : 0;
+        row.number_of_misses = is_hit ? 0 : 1;

[Peter Kasting, 2011/11/18 21:17:13]

Nit: Or "= 1 - row.number_of_hits", dunno which is clearer to you

[dominich, 2011/11/18 23:05:51]

I like the symmetry of the existing code - it's clear to me that the values are
orthogonal.

+
+        AddRow(key, row);
+      } else {
+        DCHECK(db_id_cache_.find(key) != db_id_cache_.end());
+        row.id = db_id_cache_.find(key)->second;
+        row.number_of_hits = it->second.number_of_hits + (is_hit ? 1 : 0);
+        row.number_of_misses = it->second.number_of_misses + (is_hit ? 0 : 1);
+
+        UpdateRow(it, row);
+      }
+    }
+  }
+  CommitTransaction();
+
+  ClearTransitionalMatches();
+}
+
+
 void NetworkActionPredictor::DeleteOldIdsFromCaches(
     history::URLDatabase* url_db,
     std::vector<NetworkActionPredictorDatabase::Row::Id>* id_list) {