predictors: ResourcePrefetchPredictorTables cleanup.

chrome/browser/predictors/resource_prefetch_predictor_tables.cc

Index: chrome/browser/predictors/resource_prefetch_predictor_tables.cc
diff --git a/chrome/browser/predictors/resource_prefetch_predictor_tables.cc b/chrome/browser/predictors/resource_prefetch_predictor_tables.cc
index c04dc372727ff29f12b9bf4c93f41fa9c9a5a037..2d0145f06800c0f3fba5d78d8dea3b7bfa3ff272 100644
--- a/chrome/browser/predictors/resource_prefetch_predictor_tables.cc
+++ b/chrome/browser/predictors/resource_prefetch_predictor_tables.cc
@@ -4,30 +4,20 @@
 
 #include "chrome/browser/predictors/resource_prefetch_predictor_tables.h"
 
-#include <stdint.h>
-
 #include <algorithm>
-#include <memory>
 #include <utility>
 
-#include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 #include "content/public/browser/browser_thread.h"
-#include "sql/meta_table.h"
 #include "sql/statement.h"
-#include "sql/transaction.h"
 
-using content::BrowserThread;
-using sql::Statement;

[Benoit L, 2016/11/04 15:03:10]

Why?

[alexilin, 2016/11/04 15:29:47]

sql::Statement is more readable than just Statement IMHO.
The namespace has short name so there is no problems with extra lines of code.
Compare with std:: namespace - we don't use usings with it.

+using google::protobuf::MessageLite;
 
 namespace {
 
-using PrefetchData = predictors::PrefetchData;
-using RedirectData = predictors::RedirectData;
-using ::google::protobuf::MessageLite;
-
 const char kMetadataTableName[] = "resource_prefetch_predictor_metadata";
 const char kUrlResourceTableName[] = "resource_prefetch_predictor_url";
 const char kUrlRedirectTableName[] = "resource_prefetch_predictor_url_redirect";
@@ -50,7 +40,7 @@ const char kDeleteProtoTableStatementTemplate[] = "DELETE FROM %s WHERE key=?";
 
 void BindProtoDataToStatement(const std::string& key,
                               const MessageLite& data,
-                              Statement* statement) {
+                              sql::Statement* statement) {
   int size = data.ByteSize();
   DCHECK_GT(size, 0);
   std::vector<char> proto_buffer(size);
@@ -60,7 +50,7 @@ void BindProtoDataToStatement(const std::string& key,
   statement->BindBlob(1, &proto_buffer[0], size);
 }
 
-bool StepAndInitializeProtoData(Statement* statement,
+bool StepAndInitializeProtoData(sql::Statement* statement,
                                 std::string* key,
                                 MessageLite* data) {
   if (!statement->Step())
@@ -80,41 +70,7 @@ bool StepAndInitializeProtoData(Statement* statement,
 
 namespace predictors {
 
-// static
-void ResourcePrefetchPredictorTables::TrimResources(
-    PrefetchData* data,
-    size_t max_consecutive_misses) {
-  auto new_end = std::remove_if(
-      data->mutable_resources()->begin(), data->mutable_resources()->end(),
-      [max_consecutive_misses](const ResourceData& x) {
-        return x.consecutive_misses() >= max_consecutive_misses;
-      });
-  data->mutable_resources()->erase(new_end, data->mutable_resources()->end());
-}
-
-// static
-void ResourcePrefetchPredictorTables::SortResources(PrefetchData* data) {
-  std::sort(data->mutable_resources()->begin(),
-            data->mutable_resources()->end(),
-            [](const ResourceData& x, const ResourceData& y) {
-              // Decreasing score ordering.
-              return ComputeResourceScore(x) > ComputeResourceScore(y);
-            });
-}
-
-// static
-void ResourcePrefetchPredictorTables::TrimRedirects(
-    RedirectData* data,
-    size_t max_consecutive_misses) {
-  auto new_end =
-      std::remove_if(data->mutable_redirect_endpoints()->begin(),
-                     data->mutable_redirect_endpoints()->end(),
-                     [max_consecutive_misses](const RedirectStat& x) {
-                       return x.consecutive_misses() >= max_consecutive_misses;
-                     });
-  data->mutable_redirect_endpoints()->erase(
-      new_end, data->mutable_redirect_endpoints()->end());
-}
+using content::BrowserThread;
 
 void ResourcePrefetchPredictorTables::GetAllData(
     PrefetchDataMap* url_data_map,
@@ -231,7 +187,7 @@ void ResourcePrefetchPredictorTables::DeleteAllData() {
   if (CantAccessDatabase())
     return;
 
-  Statement deleter;
+  sql::Statement deleter;
   for (const char* table_name :
        {kUrlResourceTableName, kUrlRedirectTableName, kHostResourceTableName,
         kHostRedirectTableName}) {
@@ -241,8 +197,88 @@ void ResourcePrefetchPredictorTables::DeleteAllData() {
   }
 }
 
-ResourcePrefetchPredictorTables::ResourcePrefetchPredictorTables()
-    : PredictorTableBase() {}
+// static
+void ResourcePrefetchPredictorTables::TrimResources(
+    PrefetchData* data,
+    size_t max_consecutive_misses) {
+  auto new_end = std::remove_if(
+      data->mutable_resources()->begin(), data->mutable_resources()->end(),
+      [max_consecutive_misses](const ResourceData& x) {
+        return x.consecutive_misses() >= max_consecutive_misses;
+      });
+  data->mutable_resources()->erase(new_end, data->mutable_resources()->end());
+}
+
+// static
+void ResourcePrefetchPredictorTables::SortResources(PrefetchData* data) {
+  std::sort(data->mutable_resources()->begin(),
+            data->mutable_resources()->end(),
+            [](const ResourceData& x, const ResourceData& y) {
+              // Decreasing score ordering.
+              return ComputeResourceScore(x) > ComputeResourceScore(y);
+            });
+}
+
+// static
+float ResourcePrefetchPredictorTables::ComputeResourceScore(
+    const ResourceData& data) {
+  // The ranking is done by considering, in this order:
+  // 1. Resource Priority
+  // 2. Request resource type
+  // 3. Finally, the average position, giving a higher priotity to earlier
+  //    resources.
+
+  int priority_multiplier;
+  switch (data.priority()) {
+    case ResourceData::REQUEST_PRIORITY_HIGHEST:
+      priority_multiplier = 3;
+      break;
+    case ResourceData::REQUEST_PRIORITY_MEDIUM:
+      priority_multiplier = 2;
+      break;
+    case ResourceData::REQUEST_PRIORITY_LOW:
+    case ResourceData::REQUEST_PRIORITY_LOWEST:
+    case ResourceData::REQUEST_PRIORITY_IDLE:
+    default:
+      priority_multiplier = 1;
+      break;
+  }
+
+  int type_multiplier;
+  switch (data.resource_type()) {
+    case ResourceData::RESOURCE_TYPE_STYLESHEET:
+    case ResourceData::RESOURCE_TYPE_SCRIPT:
+      type_multiplier = 3;
+      break;
+    case ResourceData::RESOURCE_TYPE_FONT_RESOURCE:
+      type_multiplier = 2;
+      break;
+    case ResourceData::RESOURCE_TYPE_IMAGE:
+    default:
+      type_multiplier = 1;
+  }
+
+  constexpr int kMaxResourcesPerType = 100;
+  return kMaxResourcesPerType *
+             (priority_multiplier * 100 + type_multiplier * 10) -
+         data.average_position();
+}
+
+// static
+void ResourcePrefetchPredictorTables::TrimRedirects(
+    RedirectData* data,
+    size_t max_consecutive_misses) {
+  auto new_end =
+      std::remove_if(data->mutable_redirect_endpoints()->begin(),
+                     data->mutable_redirect_endpoints()->end(),
+                     [max_consecutive_misses](const RedirectStat& x) {
+                       return x.consecutive_misses() >= max_consecutive_misses;
+                     });
+  data->mutable_redirect_endpoints()->erase(
+      new_end, data->mutable_redirect_endpoints()->end());
+}
+
+ResourcePrefetchPredictorTables::ResourcePrefetchPredictorTables() {}
 
 ResourcePrefetchPredictorTables::~ResourcePrefetchPredictorTables() {}
 
@@ -251,7 +287,7 @@ void ResourcePrefetchPredictorTables::GetAllResourceDataHelper(
     PrefetchDataMap* data_map) {
   // Read the resources table and organize it per primary key.
   const char* table_name = GetTableName(key_type, PrefetchDataType::RESOURCE);
-  Statement resource_reader(DB()->GetUniqueStatement(
+  sql::Statement resource_reader(DB()->GetUniqueStatement(
       base::StringPrintf("SELECT * FROM %s", table_name).c_str()));
 
   PrefetchData data;
@@ -272,7 +308,7 @@ void ResourcePrefetchPredictorTables::GetAllRedirectDataHelper(
     RedirectDataMap* data_map) {
   // Read the redirects table and organize it per primary key.
   const char* table_name = GetTableName(key_type, PrefetchDataType::REDIRECT);
-  Statement redirect_reader(DB()->GetUniqueStatement(
+  sql::Statement redirect_reader(DB()->GetUniqueStatement(
       base::StringPrintf("SELECT * FROM %s", table_name).c_str()));
 
   RedirectData data;
@@ -289,14 +325,14 @@ bool ResourcePrefetchPredictorTables::UpdateDataHelper(
     const std::string& key,
     const MessageLite& data) {
   // Delete the older data from the table.
-  std::unique_ptr<Statement> deleter(
+  std::unique_ptr<sql::Statement> deleter(
       GetTableUpdateStatement(key_type, data_type, TableOperationType::REMOVE));
   deleter->BindString(0, key);
   if (!deleter->Run())
     return false;
 
   // Add the new data to the table.
-  std::unique_ptr<Statement> inserter(
+  std::unique_ptr<sql::Statement> inserter(
       GetTableUpdateStatement(key_type, data_type, TableOperationType::INSERT));
   BindProtoDataToStatement(key, data, inserter.get());
   return inserter->Run();
@@ -307,101 +343,100 @@ void ResourcePrefetchPredictorTables::DeleteDataHelper(
     PrefetchDataType data_type,
     const std::vector<std::string>& keys) {
   for (const std::string& key : keys) {
-    std::unique_ptr<Statement> deleter(GetTableUpdateStatement(
+    std::unique_ptr<sql::Statement> deleter(GetTableUpdateStatement(
         key_type, data_type, TableOperationType::REMOVE));
     deleter->BindString(0, key);
     deleter->Run();
   }
 }
 
-// static
-float ResourcePrefetchPredictorTables::ComputeResourceScore(
-    const ResourceData& data) {
-  // The ranking is done by considering, in this order:
-  // 1. Resource Priority
-  // 2. Request resource type
-  // 3. Finally, the average position, giving a higher priotity to earlier
-  //    resources.
+void ResourcePrefetchPredictorTables::CreateTableIfNonExistent() {
+  DCHECK_CURRENTLY_ON(BrowserThread::DB);
+  if (CantAccessDatabase())
+    return;
 
-  int priority_multiplier;
-  switch (data.priority()) {
-    case ResourceData::REQUEST_PRIORITY_HIGHEST:
-      priority_multiplier = 3;
-      break;
-    case ResourceData::REQUEST_PRIORITY_MEDIUM:
-      priority_multiplier = 2;
-      break;
-    case ResourceData::REQUEST_PRIORITY_LOW:
-    case ResourceData::REQUEST_PRIORITY_LOWEST:
-    case ResourceData::REQUEST_PRIORITY_IDLE:
-    default:
-      priority_multiplier = 1;
-      break;
-  }
+  // Database initialization is all-or-nothing.
+  bool success = DB()->BeginTransaction();
 
-  int type_multiplier;
-  switch (data.resource_type()) {
-    case ResourceData::RESOURCE_TYPE_STYLESHEET:
-    case ResourceData::RESOURCE_TYPE_SCRIPT:
-      type_multiplier = 3;
-      break;
-    case ResourceData::RESOURCE_TYPE_FONT_RESOURCE:
-      type_multiplier = 2;
-      break;
-    case ResourceData::RESOURCE_TYPE_IMAGE:
-    default:
-      type_multiplier = 1;
+  success = success && DropTablesIfOutdated();
+
+  for (const char* table_name :
+       {kUrlResourceTableName, kHostResourceTableName, kUrlRedirectTableName,
+        kHostRedirectTableName}) {
+    success = success &&
+              (DB()->DoesTableExist(table_name) ||
+               DB()->Execute(base::StringPrintf(
+                                 kCreateProtoTableStatementTemplate, table_name)
+                                 .c_str()));
   }
 
-  constexpr int kMaxResourcesPerType = 100;
-  return kMaxResourcesPerType *
-             (priority_multiplier * 100 + type_multiplier * 10) -
-         data.average_position();
+  if (success)
+    success = DB()->CommitTransaction();
+  else
+    DB()->RollbackTransaction();
+
+  if (!success)
+    ResetDB();
 }
 
-// static
-bool ResourcePrefetchPredictorTables::DropTablesIfOutdated(
-    sql::Connection* db) {
-  int version = GetDatabaseVersion(db);
-  bool success = true;
+void ResourcePrefetchPredictorTables::LogDatabaseStats() {
+  DCHECK_CURRENTLY_ON(BrowserThread::DB);
+  if (CantAccessDatabase())
+    return;
+
+  sql::Statement statement(DB()->GetUniqueStatement(
+      base::StringPrintf("SELECT count(*) FROM %s", kUrlResourceTableName)
+          .c_str()));
+  if (statement.Step())
+    UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.UrlTableRowCount",
+                         statement.ColumnInt(0));
+
+  statement.Assign(DB()->GetUniqueStatement(
+      base::StringPrintf("SELECT count(*) FROM %s", kHostResourceTableName)
+          .c_str()));
+  if (statement.Step())
+    UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.HostTableRowCount",
+                         statement.ColumnInt(0));
+}
+
+bool ResourcePrefetchPredictorTables::DropTablesIfOutdated() {
+  int version = GetDatabaseVersion();
   // Too new is also a problem.
   bool incompatible_version = version != kDatabaseVersion;
 
   // These are deprecated tables but they still have to be removed if present.
-  const char kUrlMetadataTableName[] =
+  static const char kUrlMetadataTableName[] =
       "resource_prefetch_predictor_url_metadata";
-  const char kHostMetadataTableName[] =
+  static const char kHostMetadataTableName[] =
       "resource_prefetch_predictor_host_metadata";
 
+  bool success = true;
   if (incompatible_version) {
     for (const char* table_name :
          {kMetadataTableName, kUrlResourceTableName, kHostResourceTableName,
           kUrlRedirectTableName, kHostRedirectTableName, kUrlMetadataTableName,
           kHostMetadataTableName}) {
-      success =
-          success &&
-          db->Execute(base::StringPrintf("DROP TABLE IF EXISTS %s", table_name)
-                          .c_str());
+      success = success &&
+                DB()->Execute(
+                    base::StringPrintf("DROP TABLE IF EXISTS %s", table_name)
+                        .c_str());
     }
-  }
 
-  if (incompatible_version) {
     success =
         success &&
-        db->Execute(base::StringPrintf(kCreateGlobalMetadataStatementTemplate,
-                                       kMetadataTableName)
-                        .c_str());
-    success = success && SetDatabaseVersion(db, kDatabaseVersion);
+        DB()->Execute(base::StringPrintf(kCreateGlobalMetadataStatementTemplate,
+                                         kMetadataTableName)
+                          .c_str());
+    success = success && SetDatabaseVersion(kDatabaseVersion);
   }
 
   return success;
 }
 
-// static
-int ResourcePrefetchPredictorTables::GetDatabaseVersion(sql::Connection* db) {
+int ResourcePrefetchPredictorTables::GetDatabaseVersion() {
   int version = 0;
-  if (db->DoesTableExist(kMetadataTableName)) {
-    sql::Statement statement(db->GetUniqueStatement(
+  if (DB()->DoesTableExist(kMetadataTableName)) {
+    sql::Statement statement(DB()->GetUniqueStatement(
         base::StringPrintf("SELECT value FROM %s WHERE key='version'",
                            kMetadataTableName)
             .c_str()));
@@ -411,10 +446,8 @@ int ResourcePrefetchPredictorTables::GetDatabaseVersion(sql::Connection* db) {
   return version;
 }
 
-// static
-bool ResourcePrefetchPredictorTables::SetDatabaseVersion(sql::Connection* db,
-                                                         int version) {
-  sql::Statement statement(db->GetUniqueStatement(
+bool ResourcePrefetchPredictorTables::SetDatabaseVersion(int version) {
+  sql::Statement statement(DB()->GetUniqueStatement(
       base::StringPrintf(
           "INSERT OR REPLACE INTO %s (key,value) VALUES ('version',%d)",
           kMetadataTableName, version)
@@ -422,58 +455,7 @@ bool ResourcePrefetchPredictorTables::SetDatabaseVersion(sql::Connection* db,
   return statement.Run();
 }
 
-void ResourcePrefetchPredictorTables::CreateTableIfNonExistent() {
-  DCHECK_CURRENTLY_ON(BrowserThread::DB);
-  if (CantAccessDatabase())
-    return;
-
-  // Database initialization is all-or-nothing.
-  sql::Connection* db = DB();
-  sql::Transaction transaction{db};
-  bool success = transaction.Begin();
-
-  success = success && DropTablesIfOutdated(db);
-
-  for (const char* table_name :
-       {kUrlResourceTableName, kHostResourceTableName, kUrlRedirectTableName,
-        kHostRedirectTableName}) {
-    success = success &&
-              (db->DoesTableExist(table_name) ||
-               db->Execute(base::StringPrintf(
-                               kCreateProtoTableStatementTemplate, table_name)
-                               .c_str()));
-  }
-
-  if (success)
-    success = transaction.Commit();
-  else
-    transaction.Rollback();
-
-  if (!success)
-    ResetDB();
-}
-
-void ResourcePrefetchPredictorTables::LogDatabaseStats()  {
-  DCHECK_CURRENTLY_ON(BrowserThread::DB);
-  if (CantAccessDatabase())
-    return;
-
-  Statement statement(DB()->GetUniqueStatement(
-      base::StringPrintf("SELECT count(*) FROM %s",
-                         kUrlResourceTableName).c_str()));
-  if (statement.Step())
-    UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.UrlTableRowCount",
-                         statement.ColumnInt(0));
-
-  statement.Assign(DB()->GetUniqueStatement(
-      base::StringPrintf("SELECT count(*) FROM %s",
-                         kHostResourceTableName).c_str()));
-  if (statement.Step())
-    UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.HostTableRowCount",
-                         statement.ColumnInt(0));
-}
-
-std::unique_ptr<Statement>
+std::unique_ptr<sql::Statement>
 ResourcePrefetchPredictorTables::GetTableUpdateStatement(
     PrefetchKeyType key_type,
     PrefetchDataType data_type,
@@ -484,7 +466,7 @@ ResourcePrefetchPredictorTables::GetTableUpdateStatement(
                                         ? kDeleteProtoTableStatementTemplate
                                         : kInsertProtoTableStatementTemplate);
   const char* table_name = GetTableName(key_type, data_type);
-  return base::MakeUnique<Statement>(DB()->GetCachedStatement(
+  return base::MakeUnique<sql::Statement>(DB()->GetCachedStatement(
       id, base::StringPrintf(statement_template, table_name).c_str()));
 }