Restrict Omnibox Prerender to URL matches.

We don't want to prerender search results even in the rare case that they would match something in the user's recent history. We do still want to preconnect, however.

This CL also has the benefit of explicitly logging 0 confidence results to UMA which helps with statistical analysis. Previously, some corner cases would return 0 without logging so the recorded counts were inconsistent.

BUG=None
TEST=NetworkActionPredictor.*


Review URL: http://codereview.chromium.org/8137031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@104303 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 155 insertions, 99 deletions

diff --git a/chrome/browser/autocomplete/network_action_predictor.cc b/chrome/browser/autocomplete/network_action_predictor.cc
index f24f0bf..923f1c8 100644
--- a/chrome/browser/autocomplete/network_action_predictor.cc
+++ b/chrome/browser/autocomplete/network_action_predictor.cc
@@ -66,6 +66,20 @@ double ConservativeAlgorithm(const history::URLRow& url_row) {
   return base_score / exp(decay_exponent);
 }
 
+bool GetURLRowForAutocompleteMatch(Profile* profile,
+                                   const AutocompleteMatch& match,
+                                   history::URLRow* url_row) {
+  DCHECK(url_row);
+
+  HistoryService* history_service =
+      profile->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (!history_service)
+    return false;
+
+  history::URLDatabase* url_db = history_service->InMemoryDatabase();
+  return url_db && (url_db->GetRowForURL(match.destination_url, url_row) != 0);
+}
+
 }
 
 NetworkActionPredictor::NetworkActionPredictor(Profile* profile)
@@ -76,32 +90,24 @@ NetworkActionPredictor::~NetworkActionPredictor() {
 }
 
 // Given a match, return a recommended action.
-NetworkActionPredictor::Action
-    NetworkActionPredictor::RecommendAction(
-        const string16& user_text, const AutocompleteMatch& match) const {
-  HistoryService* history_service =
-      profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  if (!history_service)
-    return ACTION_NONE;
-
-  history::URLDatabase* url_db = history_service->InMemoryDatabase();
-  if (!url_db)
-    return ACTION_NONE;
-
-  history::URLRow url_row;
-  history::URLID url_id = url_db->GetRowForURL(match.destination_url, &url_row);
-
-  if (url_id == 0)
-    return ACTION_NONE;
-
+NetworkActionPredictor::Action NetworkActionPredictor::RecommendAction(
+    const string16& user_text,
+    const AutocompleteMatch& match) const {
   double confidence = 0.0;
+
   switch (prerender::GetOmniboxHeuristicToUse()) {
-    case prerender::OMNIBOX_HEURISTIC_ORIGINAL:
-      confidence = OriginalAlgorithm(url_row);
+    case prerender::OMNIBOX_HEURISTIC_ORIGINAL: {
+      history::URLRow url_row;
+      if (GetURLRowForAutocompleteMatch(profile_, match, &url_row))
+        confidence = OriginalAlgorithm(url_row);
       break;
-    case prerender::OMNIBOX_HEURISTIC_CONSERVATIVE:
-      confidence = ConservativeAlgorithm(url_row);
+    }
+    case prerender::OMNIBOX_HEURISTIC_CONSERVATIVE: {
+      history::URLRow url_row;
+      if (GetURLRowForAutocompleteMatch(profile_, match, &url_row))
+        confidence = ConservativeAlgorithm(url_row);
       break;
+    }
     default:
       NOTREACHED();
       break;
@@ -113,10 +119,24 @@ NetworkActionPredictor::Action
                            prerender::GetOmniboxHistogramSuffix(),
                            confidence * 100);
 
-  for (int i = 0; i < LAST_PREDICT_ACTION; ++i)
-    if (confidence >= kConfidenceCutoff[i])
-      return static_cast<Action>(i);
-  return ACTION_NONE;
+  // Map the confidence to an action.
+  Action action = ACTION_NONE;
+  for (int i = 0; i < LAST_PREDICT_ACTION; ++i) {
+    if (confidence >= kConfidenceCutoff[i]) {
+      action = static_cast<Action>(i);
+      break;
+    }
+  }
+
+  // Downgrade prerender to preconnect if this is a search match.
+  if (action == ACTION_PRERENDER &&
+      (match.type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED ||
+       match.type == AutocompleteMatch::SEARCH_SUGGEST ||
+       match.type == AutocompleteMatch::SEARCH_OTHER_ENGINE)) {
+    action = ACTION_PRECONNECT;
+  }
+
+  return action;
 }
 
 // Return true if the suggestion type warrants a TCP/IP preconnection.
diff --git a/chrome/browser/autocomplete/network_action_predictor_unittest.cc b/chrome/browser/autocomplete/network_action_predictor_unittest.cc
index b49bd92..fbca1a2 100644
--- a/chrome/browser/autocomplete/network_action_predictor_unittest.cc
+++ b/chrome/browser/autocomplete/network_action_predictor_unittest.cc
@@ -14,89 +14,125 @@
 #include "content/browser/browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-typedef testing::Test NetworkActionPredictorTest;
-
-TEST_F(NetworkActionPredictorTest, RecommendAction) {
-  MessageLoop loop(MessageLoop::TYPE_DEFAULT);
-  BrowserThread ui_thread(BrowserThread::UI, &loop);
-  BrowserThread file_thread(BrowserThread::FILE, &loop);
-
-  TestingProfile profile;
-  profile.CreateHistoryService(true, false);
-  profile.BlockUntilHistoryProcessesPendingRequests();
-
-  NetworkActionPredictor predictor(&profile);
-
-  HistoryService* history = profile.GetHistoryService(Profile::EXPLICIT_ACCESS);
-  CHECK(history);
-  history::URLDatabase* url_db = history->InMemoryDatabase();
-  CHECK(url_db);
-
-  struct TestUrlInfo {
-    GURL url;
-    string16 title;
-    int typed_count;
-    int days_from_now;
-    string16 user_text;
-    NetworkActionPredictor::Action expected_action;
-  } test_url_db[] = {
-    { GURL("http://www.testsite.com/a.html"),
-      ASCIIToUTF16("Test - site - just a test"), 1, 1,
-      ASCIIToUTF16("just"),
-      NetworkActionPredictor::ACTION_PRERENDER },
-    { GURL("http://www.testsite.com/b.html"),
-      ASCIIToUTF16("Test - site - just a test"), 0, 1,
-      ASCIIToUTF16("just"),
-      NetworkActionPredictor::ACTION_PRERENDER },
-    { GURL("http://www.testsite.com/c.html"),
-      ASCIIToUTF16("Test - site - just a test"), 1, 5,
-      ASCIIToUTF16("just"),
-      NetworkActionPredictor::ACTION_PRECONNECT },
-    { GURL("http://www.testsite.com/d.html"),
-      ASCIIToUTF16("Test - site - just a test"), 2, 5,
-      ASCIIToUTF16("just"),
-      NetworkActionPredictor::ACTION_PRERENDER },
-    { GURL("http://www.testsite.com/e.html"),
-      ASCIIToUTF16("Test - site - just a test"), 1, 8,
-      ASCIIToUTF16("just"),
-      NetworkActionPredictor::ACTION_PRECONNECT },
-    { GURL("http://www.testsite.com/f.html"),
-      ASCIIToUTF16("Test - site - just a test"), 4, 8,
-      ASCIIToUTF16("just"),
-      NetworkActionPredictor::ACTION_PRERENDER },
-    { GURL("http://www.testsite.com/g.html"),
-      ASCIIToUTF16("Test - site - just a test"), 1, 12,
-      ASCIIToUTF16("just a"),
-      NetworkActionPredictor::ACTION_NONE },
-    { GURL("http://www.testsite.com/h.html"),
-      ASCIIToUTF16("Test - site - just a test"), 2, 21,
-      ASCIIToUTF16("just a test"),
-      NetworkActionPredictor::ACTION_NONE },
-    { GURL("http://www.testsite.com/i.html"),
-      ASCIIToUTF16("Test - site - just a test"), 3, 28,
-      ASCIIToUTF16("just a test"),
-      NetworkActionPredictor::ACTION_NONE }
-  };
+namespace {
 
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_url_db); ++i) {
-    const base::Time visit_time =
-        base::Time::Now() - base::TimeDelta::FromDays(
-            test_url_db[i].days_from_now);
+struct TestUrlInfo {
+  GURL url;
+  string16 title;
+  int typed_count;
+  int days_from_now;
+  string16 user_text;
+  NetworkActionPredictor::Action expected_action;
+} test_url_db[] = {
+  { GURL("http://www.testsite.com/a.html"),
+    ASCIIToUTF16("Test - site - just a test"), 1, 1,
+    ASCIIToUTF16("just"),
+    NetworkActionPredictor::ACTION_PRERENDER },
+  { GURL("http://www.testsite.com/b.html"),
+    ASCIIToUTF16("Test - site - just a test"), 0, 1,
+    ASCIIToUTF16("just"),
+    NetworkActionPredictor::ACTION_PRERENDER },
+  { GURL("http://www.testsite.com/c.html"),
+    ASCIIToUTF16("Test - site - just a test"), 1, 5,
+    ASCIIToUTF16("just"),
+    NetworkActionPredictor::ACTION_PRECONNECT },
+  { GURL("http://www.testsite.com/d.html"),
+    ASCIIToUTF16("Test - site - just a test"), 2, 5,
+    ASCIIToUTF16("just"),
+    NetworkActionPredictor::ACTION_PRERENDER },
+  { GURL("http://www.testsite.com/e.html"),
+    ASCIIToUTF16("Test - site - just a test"), 1, 8,
+    ASCIIToUTF16("just"),
+    NetworkActionPredictor::ACTION_PRECONNECT },
+  { GURL("http://www.testsite.com/f.html"),
+    ASCIIToUTF16("Test - site - just a test"), 4, 8,
+    ASCIIToUTF16("just"),
+    NetworkActionPredictor::ACTION_PRERENDER },
+  { GURL("http://www.testsite.com/g.html"),
+    ASCIIToUTF16("Test - site - just a test"), 1, 12,
+    ASCIIToUTF16("just a"),
+    NetworkActionPredictor::ACTION_NONE },
+  { GURL("http://www.testsite.com/h.html"),
+    ASCIIToUTF16("Test - site - just a test"), 2, 21,
+    ASCIIToUTF16("just a test"),
+    NetworkActionPredictor::ACTION_NONE },
+  { GURL("http://www.testsite.com/i.html"),
+    ASCIIToUTF16("Test - site - just a test"), 3, 28,
+    ASCIIToUTF16("just a test"),
+    NetworkActionPredictor::ACTION_NONE }
+};
 
-    history::URLRow row(test_url_db[i].url);
-    row.set_title(test_url_db[i].title);
-    row.set_typed_count(test_url_db[i].typed_count);
-    row.set_last_visit(visit_time);
+}  // end namespace
 
-    CHECK(url_db->AddURL(row));
+class NetworkActionPredictorTest : public testing::Test {
+ public:
+  NetworkActionPredictorTest()
+      : loop_(MessageLoop::TYPE_DEFAULT),
+        ui_thread_(BrowserThread::UI, &loop_),
+        file_thread_(BrowserThread::FILE, &loop_),
+        predictor_(&profile_) {
   }
 
+  void SetUp() {
+    profile_.CreateHistoryService(true, false);
+    profile_.BlockUntilHistoryProcessesPendingRequests();
+
+    HistoryService* history =
+        profile_.GetHistoryService(Profile::EXPLICIT_ACCESS);
+    CHECK(history);
+    history::URLDatabase* url_db = history->InMemoryDatabase();
+    CHECK(url_db);
+
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_url_db); ++i) {
+      const base::Time visit_time =
+          base::Time::Now() - base::TimeDelta::FromDays(
+              test_url_db[i].days_from_now);
+
+      history::URLRow row(test_url_db[i].url);
+      row.set_title(test_url_db[i].title);
+      row.set_typed_count(test_url_db[i].typed_count);
+      row.set_last_visit(visit_time);
+
+      CHECK(url_db->AddURL(row));
+    }
+  }
+
+  const NetworkActionPredictor& predictor() const { return predictor_; }
+
+ private:
+  MessageLoop loop_;
+  BrowserThread ui_thread_;
+  BrowserThread file_thread_;
+  TestingProfile profile_;
+  NetworkActionPredictor predictor_;
+};
+
+TEST_F(NetworkActionPredictorTest, RecommendActionURL) {
   AutocompleteMatch match;
+  match.type = AutocompleteMatch::HISTORY_URL;
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_url_db); ++i) {
     match.destination_url = GURL(test_url_db[i].url);
     EXPECT_EQ(test_url_db[i].expected_action,
-              predictor.RecommendAction(test_url_db[i].user_text, match))
+              predictor().RecommendAction(test_url_db[i].user_text, match))
+        << "Unexpected action for " << match.destination_url;
+  }
+}
+
+TEST_F(NetworkActionPredictorTest, RecommendActionSearch) {
+  AutocompleteMatch match;
+  match.type = AutocompleteMatch::SEARCH_WHAT_YOU_TYPED;
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_url_db); ++i) {
+    match.destination_url = GURL(test_url_db[i].url);
+    const NetworkActionPredictor::Action expected =
+        (test_url_db[i].expected_action ==
+         NetworkActionPredictor::ACTION_PRERENDER) ?
+            NetworkActionPredictor::ACTION_PRECONNECT :
+            test_url_db[i].expected_action;
+
+    EXPECT_EQ(expected,
+              predictor().RecommendAction(test_url_db[i].user_text, match))
         << "Unexpected action for " << match.destination_url;
   }
 }