Refactoring, moving and renaming the NetworkActionPredictor.

chrome/browser/predictors/autocomplete_action_predictor.cc

Index: chrome/browser/predictors/autocomplete_action_predictor.cc
diff --git a/chrome/browser/autocomplete/network_action_predictor.cc b/chrome/browser/predictors/autocomplete_action_predictor.cc
similarity index 73%
rename from chrome/browser/autocomplete/network_action_predictor.cc
rename to chrome/browser/predictors/autocomplete_action_predictor.cc
index 7afc9f82eb941dbc614baa07436d45ee7751d5a5..ecb196e14384126e3908bbdfe2c760bb86e6a099 100644
--- a/chrome/browser/autocomplete/network_action_predictor.cc
+++ b/chrome/browser/predictors/autocomplete_action_predictor.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/autocomplete/network_action_predictor.h"
+#include "chrome/browser/predictors/autocomplete_action_predictor.h"
 
 #include <math.h>
 
@@ -16,10 +16,11 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
-#include "chrome/browser/autocomplete/network_action_predictor_database.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_notifications.h"
 #include "chrome/browser/history/in_memory_database.h"
+#include "chrome/browser/predictors/predictor_database.h"
+#include "chrome/browser/predictors/predictor_database_factory.h"
 #include "chrome/browser/prerender/prerender_field_trial.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
@@ -42,7 +43,7 @@ const size_t kMinimumUserTextLength = 1;
 const int kMinimumNumberOfHits = 3;
 
 COMPILE_ASSERT(arraysize(kConfidenceCutoff) ==
-               NetworkActionPredictor::LAST_PREDICT_ACTION,
+               predictors::AutocompleteActionPredictor::LAST_PREDICT_ACTION,
                ConfidenceCutoff_count_mismatch);
 
 enum DatabaseAction {
@@ -68,19 +69,19 @@ bool IsAutocompleteMatchSearchType(const AutocompleteMatch& match) {
   }
 }
 
-}
+}  // namespace
+
+namespace predictors {
 
-const int NetworkActionPredictor::kMaximumDaysToKeepEntry = 14;
+const int AutocompleteActionPredictor::kMaximumDaysToKeepEntry = 14;
 
-double NetworkActionPredictor::hit_weight_ = 1.0;
+double AutocompleteActionPredictor::hit_weight_ = 1.0;
 
-NetworkActionPredictor::NetworkActionPredictor(Profile* profile)
+AutocompleteActionPredictor::AutocompleteActionPredictor(Profile* profile)
     : profile_(profile),
-      db_(new NetworkActionPredictorDatabase(profile)),
+      db_(PredictorDatabaseFactory::GetForProfile(
+          profile)->autocomplete_table()),
       initialized_(false) {
-  content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::Initialize, db_));
-
   // Request the in-memory database from the history to force it to load so it's
   // available as soon as possible.
   HistoryService* history_service =
@@ -91,20 +92,21 @@ NetworkActionPredictor::NetworkActionPredictor(Profile* profile)
   // Create local caches using the database as loaded. We will garbage collect
   // rows from the caches and the database once the history service is
   // available.
-  std::vector<NetworkActionPredictorDatabase::Row>* rows =
-      new std::vector<NetworkActionPredictorDatabase::Row>();
+  std::vector<AutocompleteActionPredictorTable::Row>* rows =
+      new std::vector<AutocompleteActionPredictorTable::Row>();
   content::BrowserThread::PostTaskAndReply(
       content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::GetAllRows, db_, rows),
-      base::Bind(&NetworkActionPredictor::CreateCaches, AsWeakPtr(),
+      base::Bind(&AutocompleteActionPredictorTable::GetAllRows,
+                 db_,
+                 rows),
+      base::Bind(&AutocompleteActionPredictor::CreateCaches, AsWeakPtr(),
                  base::Owned(rows)));
-
 }
 
-NetworkActionPredictor::~NetworkActionPredictor() {
+AutocompleteActionPredictor::~AutocompleteActionPredictor() {
 }
 
-void NetworkActionPredictor::RegisterTransitionalMatches(
+void AutocompleteActionPredictor::RegisterTransitionalMatches(
     const string16& user_text,
     const AutocompleteResult& result) {
   if (user_text.length() < kMinimumUserTextLength)
@@ -132,25 +134,26 @@ void NetworkActionPredictor::RegisterTransitionalMatches(
   }
 }
 
-void NetworkActionPredictor::ClearTransitionalMatches() {
+void AutocompleteActionPredictor::ClearTransitionalMatches() {
   transitional_matches_.clear();
 }
 
 // Given a match, return a recommended action.
-NetworkActionPredictor::Action NetworkActionPredictor::RecommendAction(
-    const string16& user_text,
-    const AutocompleteMatch& match) const {
+AutocompleteActionPredictor::Action
+    AutocompleteActionPredictor::RecommendAction(
+        const string16& user_text,
+        const AutocompleteMatch& match) const {
   bool is_in_db = false;
   const double confidence = CalculateConfidence(user_text, match, &is_in_db);
   DCHECK(confidence >= 0.0 && confidence <= 1.0);
 
-  UMA_HISTOGRAM_BOOLEAN("NetworkActionPredictor.MatchIsInDb", is_in_db);
+  UMA_HISTOGRAM_BOOLEAN("AutocompleteActionPredictor.MatchIsInDb", is_in_db);
 
   if (is_in_db) {
     // Multiple enties with the same URL are fine as the confidence may be
     // different.
     tracked_urls_.push_back(std::make_pair(match.destination_url, confidence));
-    UMA_HISTOGRAM_COUNTS_100("NetworkActionPredictor.Confidence",
+    UMA_HISTOGRAM_COUNTS_100("AutocompleteActionPredictor.Confidence",
                              confidence * 100);
   }
 
@@ -181,15 +184,15 @@ NetworkActionPredictor::Action NetworkActionPredictor::RecommendAction(
 // Return true if the suggestion type warrants a TCP/IP preconnection.
 // i.e., it is now quite likely that the user will select the related domain.
 // static
-bool NetworkActionPredictor::IsPreconnectable(const AutocompleteMatch& match) {
+bool AutocompleteActionPredictor::IsPreconnectable(
+    const AutocompleteMatch& match) {
   return IsAutocompleteMatchSearchType(match);
 }
 
-void NetworkActionPredictor::Shutdown() {
-  db_->OnPredictorDestroyed();
+void AutocompleteActionPredictor::Shutdown() {
 }
 
-void NetworkActionPredictor::Observe(
+void AutocompleteActionPredictor::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
@@ -234,7 +237,8 @@ void NetworkActionPredictor::Observe(
   }
 }
 
-void NetworkActionPredictor::OnOmniboxOpenedUrl(const AutocompleteLog& log) {
+void AutocompleteActionPredictor::OnOmniboxOpenedUrl(
+    const AutocompleteLog& log) {
   if (log.text.length() < kMinimumUserTextLength)
     return;
 
@@ -259,9 +263,11 @@ void NetworkActionPredictor::OnOmniboxOpenedUrl(const AutocompleteLog& log) {
 
   const string16 lower_user_text(base::i18n::ToLower(log.text));
 
-  BeginTransaction();
   // Traverse transitional matches for those that have a user_text that is a
   // prefix of |lower_user_text|.
+  std::vector<AutocompleteActionPredictorTable::Row> rows_to_add;
+  std::vector<AutocompleteActionPredictorTable::Row> rows_to_update;
+
   for (std::vector<TransitionalMatch>::const_iterator it =
        transitional_matches_.begin(); it != transitional_matches_.end();
        ++it) {
@@ -275,7 +281,7 @@ void NetworkActionPredictor::OnOmniboxOpenedUrl(const AutocompleteLog& log) {
       const DBCacheKey key = { it->user_text, *url_it };
       const bool is_hit = (*url_it == opened_url);
 
-      NetworkActionPredictorDatabase::Row row;
+      AutocompleteActionPredictorTable::Row row;
       row.user_text = key.user_text;
       row.url = key.url;
 
@@ -285,18 +291,36 @@ void NetworkActionPredictor::OnOmniboxOpenedUrl(const AutocompleteLog& log) {
         row.number_of_hits = is_hit ? 1 : 0;
         row.number_of_misses = is_hit ? 0 : 1;
 
-        AddRow(key, row);
+        DBCacheValue value = { row.number_of_hits, row.number_of_misses };
+        db_cache_[key] = value;
+        db_id_cache_[key] = row.id;
+
+        rows_to_add.push_back(row);
+        UMA_HISTOGRAM_ENUMERATION(
+            "AutocompleteActionPredictor.DatabaseAction",
+            DATABASE_ACTION_ADD, DATABASE_ACTION_COUNT);
       } 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);
+        it->second.number_of_hits = row.number_of_hits;
+        it->second.number_of_misses = row.number_of_misses;
+
+        rows_to_update.push_back(row);
+        UMA_HISTOGRAM_ENUMERATION(
+            "AutocompleteActionPredictor.DatabaseAction",
+            DATABASE_ACTION_UPDATE, DATABASE_ACTION_COUNT);
       }
     }
   }
-  CommitTransaction();
+  if (rows_to_add.size() > 0 || rows_to_update.size() > 0)
+    content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
+        base::Bind(&AutocompleteActionPredictorTable::AddAndUpdateRows,
+                   db_,
+                   rows_to_add,
+                   rows_to_update));
 
   ClearTransitionalMatches();
 
@@ -306,16 +330,16 @@ void NetworkActionPredictor::OnOmniboxOpenedUrl(const AutocompleteLog& log) {
        tracked_urls_.begin(); it != tracked_urls_.end();
        ++it) {
     if (opened_url == it->first) {
-      UMA_HISTOGRAM_COUNTS_100("NetworkActionPredictor.AccurateCount",
+      UMA_HISTOGRAM_COUNTS_100("AutocompleteActionPredictor.AccurateCount",
                                it->second * 100);
     }
   }
   tracked_urls_.clear();
 }
 
-void NetworkActionPredictor::DeleteOldIdsFromCaches(
+void AutocompleteActionPredictor::DeleteOldIdsFromCaches(
     history::URLDatabase* url_db,
-    std::vector<NetworkActionPredictorDatabase::Row::Id>* id_list) {
+    std::vector<AutocompleteActionPredictorTable::Row::Id>* id_list) {
   CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(url_db);
   DCHECK(id_list);
@@ -337,15 +361,17 @@ void NetworkActionPredictor::DeleteOldIdsFromCaches(
   }
 }
 
-void NetworkActionPredictor::DeleteOldEntries(history::URLDatabase* url_db) {
+void AutocompleteActionPredictor::DeleteOldEntries(
+    history::URLDatabase* url_db) {
   CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(!initialized_);
 
-  std::vector<NetworkActionPredictorDatabase::Row::Id> ids_to_delete;
+  std::vector<AutocompleteActionPredictorTable::Row::Id> ids_to_delete;
   DeleteOldIdsFromCaches(url_db, &ids_to_delete);
 
   content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::DeleteRows, db_,
+      base::Bind(&AutocompleteActionPredictorTable::DeleteRows,
+                 db_,
                  ids_to_delete));
 
   // Register for notifications and set the |initialized_| flag.
@@ -356,14 +382,14 @@ void NetworkActionPredictor::DeleteOldEntries(history::URLDatabase* url_db) {
   initialized_ = true;
 }
 
-void NetworkActionPredictor::CreateCaches(
-    std::vector<NetworkActionPredictorDatabase::Row>* rows) {
+void AutocompleteActionPredictor::CreateCaches(
+    std::vector<AutocompleteActionPredictorTable::Row>* rows) {
   CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK(!initialized_);
   DCHECK(db_cache_.empty());
   DCHECK(db_id_cache_.empty());
 
-  for (std::vector<NetworkActionPredictorDatabase::Row>::const_iterator it =
+  for (std::vector<AutocompleteActionPredictorTable::Row>::const_iterator it =
        rows->begin(); it != rows->end(); ++it) {
     const DBCacheKey key = { it->user_text, it->url };
     const DBCacheValue value = { it->number_of_hits, it->number_of_misses };
@@ -382,7 +408,7 @@ void NetworkActionPredictor::CreateCaches(
   }
 }
 
-bool NetworkActionPredictor::TryDeleteOldEntries(HistoryService* service) {
+bool AutocompleteActionPredictor::TryDeleteOldEntries(HistoryService* service) {
   if (!service)
     return false;
 
@@ -394,7 +420,7 @@ bool NetworkActionPredictor::TryDeleteOldEntries(HistoryService* service) {
   return true;
 }
 
-double NetworkActionPredictor::CalculateConfidence(
+double AutocompleteActionPredictor::CalculateConfidence(
     const string16& user_text,
     const AutocompleteMatch& match,
     bool* is_in_db) const {
@@ -412,7 +438,7 @@ double NetworkActionPredictor::CalculateConfidence(
   return CalculateConfidenceForDbEntry(iter);
 }
 
-double NetworkActionPredictor::CalculateConfidenceForDbEntry(
+double AutocompleteActionPredictor::CalculateConfidenceForDbEntry(
     DBCacheMap::const_iterator iter) const {
   const DBCacheValue& value = iter->second;
   if (value.number_of_hits < kMinimumNumberOfHits)
@@ -422,9 +448,9 @@ double NetworkActionPredictor::CalculateConfidenceForDbEntry(
   return number_of_hits / (number_of_hits + value.number_of_misses);
 }
 
-void NetworkActionPredictor::AddRow(
+void AutocompleteActionPredictor::AddRow(
     const DBCacheKey& key,
-    const NetworkActionPredictorDatabase::Row& row) {
+    const AutocompleteActionPredictorTable::Row& row) {
   if (!initialized_)
     return;
 
@@ -432,15 +458,17 @@ void NetworkActionPredictor::AddRow(
   db_cache_[key] = value;
   db_id_cache_[key] = row.id;
   content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::AddRow, db_, row));
+      base::Bind(&AutocompleteActionPredictorTable::AddRow,
+                 db_,
+                 row));
 
-  UMA_HISTOGRAM_ENUMERATION("NetworkActionPredictor.DatabaseAction",
+  UMA_HISTOGRAM_ENUMERATION("AutocompleteActionPredictor.DatabaseAction",
                             DATABASE_ACTION_ADD, DATABASE_ACTION_COUNT);
 }
 
-void NetworkActionPredictor::UpdateRow(
+void AutocompleteActionPredictor::UpdateRow(

[Scott Hess - ex-Googler, 2012/03/15 00:13:33]

Having an iterator argument seems to implicitly assume the old structure of
iterating to call UpdateRow() and AddRow() one by one.  But now you collect
things and call AddAndUpdateRows() once.  Can these two functions go away
entirely?

[If so, then can the called UpdateRow() and AddRow() also be removed?]

[Shishir, 2012/03/15 07:55:48]

These two calls are needed only for unittests to be able to manipulate the cache
and the DB in a consistent fashion. I have combined them into a single function.
The only assumption in the function is the rows that are being updates or added
need to have unique keys, which seems to be the case.. Maybe I should check that
explicitly.

     DBCacheMap::iterator it,
-    const NetworkActionPredictorDatabase::Row& row) {
+    const AutocompleteActionPredictorTable::Row& row) {
   if (!initialized_)
     return;
 
@@ -448,28 +476,32 @@ void NetworkActionPredictor::UpdateRow(
   it->second.number_of_hits = row.number_of_hits;
   it->second.number_of_misses = row.number_of_misses;
   content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::UpdateRow, db_, row));
-  UMA_HISTOGRAM_ENUMERATION("NetworkActionPredictor.DatabaseAction",
+      base::Bind(&AutocompleteActionPredictorTable::UpdateRow,
+                 db_,
+                 row));
+  UMA_HISTOGRAM_ENUMERATION("AutocompleteActionPredictor.DatabaseAction",
                             DATABASE_ACTION_UPDATE, DATABASE_ACTION_COUNT);
 }
 
-void NetworkActionPredictor::DeleteAllRows() {
+void AutocompleteActionPredictor::DeleteAllRows() {
   if (!initialized_)
     return;
 
   db_cache_.clear();
   db_id_cache_.clear();
   content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::DeleteAllRows, db_));
-  UMA_HISTOGRAM_ENUMERATION("NetworkActionPredictor.DatabaseAction",
+      base::Bind(&AutocompleteActionPredictorTable::DeleteAllRows,
+                 db_));
+  UMA_HISTOGRAM_ENUMERATION("AutocompleteActionPredictor.DatabaseAction",
                             DATABASE_ACTION_DELETE_ALL, DATABASE_ACTION_COUNT);
 }
 
-void NetworkActionPredictor::DeleteRowsWithURLs(const std::set<GURL>& urls) {
+void AutocompleteActionPredictor::DeleteRowsWithURLs(
+    const std::set<GURL>& urls) {
   if (!initialized_)
     return;
 
-  std::vector<NetworkActionPredictorDatabase::Row::Id> id_list;
+  std::vector<AutocompleteActionPredictorTable::Row::Id> id_list;
 
   for (DBCacheMap::iterator it = db_cache_.begin(); it != db_cache_.end();) {
     if (urls.find(it->first.url) != urls.end()) {
@@ -484,29 +516,17 @@ void NetworkActionPredictor::DeleteRowsWithURLs(const std::set<GURL>& urls) {
   }
 
   content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::DeleteRows, db_, id_list));
-  UMA_HISTOGRAM_ENUMERATION("NetworkActionPredictor.DatabaseAction",
+      base::Bind(&AutocompleteActionPredictorTable::DeleteRows,
+                 db_,
+                 id_list));
+  UMA_HISTOGRAM_ENUMERATION("AutocompleteActionPredictor.DatabaseAction",
                             DATABASE_ACTION_DELETE_SOME, DATABASE_ACTION_COUNT);
 }
 
-void NetworkActionPredictor::BeginTransaction() {
-  if (!initialized_)
-    return;
-
-  content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::BeginTransaction, db_));
-}
-
-void NetworkActionPredictor::CommitTransaction() {
-  if (!initialized_)
-    return;
-
-  content::BrowserThread::PostTask(content::BrowserThread::DB, FROM_HERE,
-      base::Bind(&NetworkActionPredictorDatabase::CommitTransaction, db_));
+AutocompleteActionPredictor::TransitionalMatch::TransitionalMatch() {
 }
 
-NetworkActionPredictor::TransitionalMatch::TransitionalMatch() {
+AutocompleteActionPredictor::TransitionalMatch::~TransitionalMatch() {
 }
 
-NetworkActionPredictor::TransitionalMatch::~TransitionalMatch() {
-}
+}  // namespace predictors