Commit e4bf55e8 authored by alexilin's avatar alexilin Committed by Commit bot

predictors: ResourcePrefetchPredictorTables cleanup.

Make resource score display in webui correctly.

ResourcePrefetchPredictorTables changes:
- Organize includes and usings.
- Get rid of mocking friend at the cost of moving ctor/dtor from private to
protected section.
- Make ComputeResourceScore public to use it in webui.

BUG=631966

Review-Url: https://codereview.chromium.org/2478823002
Cr-Commit-Position: refs/heads/master@{#430245}
parent 006274cc
...@@ -4,30 +4,20 @@ ...@@ -4,30 +4,20 @@
#include "chrome/browser/predictors/resource_prefetch_predictor_tables.h" #include "chrome/browser/predictors/resource_prefetch_predictor_tables.h"
#include <stdint.h>
#include <algorithm> #include <algorithm>
#include <memory>
#include <utility> #include <utility>
#include "base/logging.h" #include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "sql/meta_table.h"
#include "sql/statement.h" #include "sql/statement.h"
#include "sql/transaction.h"
using content::BrowserThread; using google::protobuf::MessageLite;
using sql::Statement;
namespace { namespace {
using PrefetchData = predictors::PrefetchData;
using RedirectData = predictors::RedirectData;
using ::google::protobuf::MessageLite;
const char kMetadataTableName[] = "resource_prefetch_predictor_metadata"; const char kMetadataTableName[] = "resource_prefetch_predictor_metadata";
const char kUrlResourceTableName[] = "resource_prefetch_predictor_url"; const char kUrlResourceTableName[] = "resource_prefetch_predictor_url";
const char kUrlRedirectTableName[] = "resource_prefetch_predictor_url_redirect"; const char kUrlRedirectTableName[] = "resource_prefetch_predictor_url_redirect";
...@@ -50,7 +40,7 @@ const char kDeleteProtoTableStatementTemplate[] = "DELETE FROM %s WHERE key=?"; ...@@ -50,7 +40,7 @@ const char kDeleteProtoTableStatementTemplate[] = "DELETE FROM %s WHERE key=?";
void BindProtoDataToStatement(const std::string& key, void BindProtoDataToStatement(const std::string& key,
const MessageLite& data, const MessageLite& data,
Statement* statement) { sql::Statement* statement) {
int size = data.ByteSize(); int size = data.ByteSize();
DCHECK_GT(size, 0); DCHECK_GT(size, 0);
std::vector<char> proto_buffer(size); std::vector<char> proto_buffer(size);
...@@ -60,7 +50,7 @@ void BindProtoDataToStatement(const std::string& key, ...@@ -60,7 +50,7 @@ void BindProtoDataToStatement(const std::string& key,
statement->BindBlob(1, &proto_buffer[0], size); statement->BindBlob(1, &proto_buffer[0], size);
} }
bool StepAndInitializeProtoData(Statement* statement, bool StepAndInitializeProtoData(sql::Statement* statement,
std::string* key, std::string* key,
MessageLite* data) { MessageLite* data) {
if (!statement->Step()) if (!statement->Step())
...@@ -80,6 +70,8 @@ bool StepAndInitializeProtoData(Statement* statement, ...@@ -80,6 +70,8 @@ bool StepAndInitializeProtoData(Statement* statement,
namespace predictors { namespace predictors {
using content::BrowserThread;
// static // static
void ResourcePrefetchPredictorTables::TrimResources( void ResourcePrefetchPredictorTables::TrimResources(
PrefetchData* data, PrefetchData* data,
...@@ -231,7 +223,7 @@ void ResourcePrefetchPredictorTables::DeleteAllData() { ...@@ -231,7 +223,7 @@ void ResourcePrefetchPredictorTables::DeleteAllData() {
if (CantAccessDatabase()) if (CantAccessDatabase())
return; return;
Statement deleter; sql::Statement deleter;
for (const char* table_name : for (const char* table_name :
{kUrlResourceTableName, kUrlRedirectTableName, kHostResourceTableName, {kUrlResourceTableName, kUrlRedirectTableName, kHostResourceTableName,
kHostRedirectTableName}) { kHostRedirectTableName}) {
...@@ -241,8 +233,7 @@ void ResourcePrefetchPredictorTables::DeleteAllData() { ...@@ -241,8 +233,7 @@ void ResourcePrefetchPredictorTables::DeleteAllData() {
} }
} }
ResourcePrefetchPredictorTables::ResourcePrefetchPredictorTables() ResourcePrefetchPredictorTables::ResourcePrefetchPredictorTables() {}
: PredictorTableBase() {}
ResourcePrefetchPredictorTables::~ResourcePrefetchPredictorTables() {} ResourcePrefetchPredictorTables::~ResourcePrefetchPredictorTables() {}
...@@ -251,7 +242,7 @@ void ResourcePrefetchPredictorTables::GetAllResourceDataHelper( ...@@ -251,7 +242,7 @@ void ResourcePrefetchPredictorTables::GetAllResourceDataHelper(
PrefetchDataMap* data_map) { PrefetchDataMap* data_map) {
// Read the resources table and organize it per primary key. // Read the resources table and organize it per primary key.
const char* table_name = GetTableName(key_type, PrefetchDataType::RESOURCE); 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())); base::StringPrintf("SELECT * FROM %s", table_name).c_str()));
PrefetchData data; PrefetchData data;
...@@ -272,7 +263,7 @@ void ResourcePrefetchPredictorTables::GetAllRedirectDataHelper( ...@@ -272,7 +263,7 @@ void ResourcePrefetchPredictorTables::GetAllRedirectDataHelper(
RedirectDataMap* data_map) { RedirectDataMap* data_map) {
// Read the redirects table and organize it per primary key. // Read the redirects table and organize it per primary key.
const char* table_name = GetTableName(key_type, PrefetchDataType::REDIRECT); 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())); base::StringPrintf("SELECT * FROM %s", table_name).c_str()));
RedirectData data; RedirectData data;
...@@ -289,14 +280,14 @@ bool ResourcePrefetchPredictorTables::UpdateDataHelper( ...@@ -289,14 +280,14 @@ bool ResourcePrefetchPredictorTables::UpdateDataHelper(
const std::string& key, const std::string& key,
const MessageLite& data) { const MessageLite& data) {
// Delete the older data from the table. // 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)); GetTableUpdateStatement(key_type, data_type, TableOperationType::REMOVE));
deleter->BindString(0, key); deleter->BindString(0, key);
if (!deleter->Run()) if (!deleter->Run())
return false; return false;
// Add the new data to the table. // 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)); GetTableUpdateStatement(key_type, data_type, TableOperationType::INSERT));
BindProtoDataToStatement(key, data, inserter.get()); BindProtoDataToStatement(key, data, inserter.get());
return inserter->Run(); return inserter->Run();
...@@ -307,7 +298,7 @@ void ResourcePrefetchPredictorTables::DeleteDataHelper( ...@@ -307,7 +298,7 @@ void ResourcePrefetchPredictorTables::DeleteDataHelper(
PrefetchDataType data_type, PrefetchDataType data_type,
const std::vector<std::string>& keys) { const std::vector<std::string>& keys) {
for (const std::string& key : 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)); key_type, data_type, TableOperationType::REMOVE));
deleter->BindString(0, key); deleter->BindString(0, key);
deleter->Run(); deleter->Run();
...@@ -368,9 +359,9 @@ bool ResourcePrefetchPredictorTables::DropTablesIfOutdated( ...@@ -368,9 +359,9 @@ bool ResourcePrefetchPredictorTables::DropTablesIfOutdated(
bool incompatible_version = version != kDatabaseVersion; bool incompatible_version = version != kDatabaseVersion;
// These are deprecated tables but they still have to be removed if present. // 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"; "resource_prefetch_predictor_url_metadata";
const char kHostMetadataTableName[] = static const char kHostMetadataTableName[] =
"resource_prefetch_predictor_host_metadata"; "resource_prefetch_predictor_host_metadata";
if (incompatible_version) { if (incompatible_version) {
...@@ -429,9 +420,7 @@ void ResourcePrefetchPredictorTables::CreateTableIfNonExistent() { ...@@ -429,9 +420,7 @@ void ResourcePrefetchPredictorTables::CreateTableIfNonExistent() {
// Database initialization is all-or-nothing. // Database initialization is all-or-nothing.
sql::Connection* db = DB(); sql::Connection* db = DB();
sql::Transaction transaction{db}; bool success = db->BeginTransaction();
bool success = transaction.Begin();
success = success && DropTablesIfOutdated(db); success = success && DropTablesIfOutdated(db);
for (const char* table_name : for (const char* table_name :
...@@ -445,35 +434,35 @@ void ResourcePrefetchPredictorTables::CreateTableIfNonExistent() { ...@@ -445,35 +434,35 @@ void ResourcePrefetchPredictorTables::CreateTableIfNonExistent() {
} }
if (success) if (success)
success = transaction.Commit(); success = db->CommitTransaction();
else else
transaction.Rollback(); db->RollbackTransaction();
if (!success) if (!success)
ResetDB(); ResetDB();
} }
void ResourcePrefetchPredictorTables::LogDatabaseStats() { void ResourcePrefetchPredictorTables::LogDatabaseStats() {
DCHECK_CURRENTLY_ON(BrowserThread::DB); DCHECK_CURRENTLY_ON(BrowserThread::DB);
if (CantAccessDatabase()) if (CantAccessDatabase())
return; return;
Statement statement(DB()->GetUniqueStatement( sql::Statement statement(DB()->GetUniqueStatement(
base::StringPrintf("SELECT count(*) FROM %s", base::StringPrintf("SELECT count(*) FROM %s", kUrlResourceTableName)
kUrlResourceTableName).c_str())); .c_str()));
if (statement.Step()) if (statement.Step())
UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.UrlTableRowCount", UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.UrlTableRowCount",
statement.ColumnInt(0)); statement.ColumnInt(0));
statement.Assign(DB()->GetUniqueStatement( statement.Assign(DB()->GetUniqueStatement(
base::StringPrintf("SELECT count(*) FROM %s", base::StringPrintf("SELECT count(*) FROM %s", kHostResourceTableName)
kHostResourceTableName).c_str())); .c_str()));
if (statement.Step()) if (statement.Step())
UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.HostTableRowCount", UMA_HISTOGRAM_COUNTS("ResourcePrefetchPredictor.HostTableRowCount",
statement.ColumnInt(0)); statement.ColumnInt(0));
} }
std::unique_ptr<Statement> std::unique_ptr<sql::Statement>
ResourcePrefetchPredictorTables::GetTableUpdateStatement( ResourcePrefetchPredictorTables::GetTableUpdateStatement(
PrefetchKeyType key_type, PrefetchKeyType key_type,
PrefetchDataType data_type, PrefetchDataType data_type,
...@@ -484,7 +473,7 @@ ResourcePrefetchPredictorTables::GetTableUpdateStatement( ...@@ -484,7 +473,7 @@ ResourcePrefetchPredictorTables::GetTableUpdateStatement(
? kDeleteProtoTableStatementTemplate ? kDeleteProtoTableStatementTemplate
: kInsertProtoTableStatementTemplate); : kInsertProtoTableStatementTemplate);
const char* table_name = GetTableName(key_type, data_type); 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())); id, base::StringPrintf(statement_template, table_name).c_str()));
} }
......
...@@ -5,8 +5,7 @@ ...@@ -5,8 +5,7 @@
#ifndef CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_ #ifndef CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_
#define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_ #define CHROME_BROWSER_PREDICTORS_RESOURCE_PREFETCH_PREDICTOR_TABLES_H_
#include <stddef.h> #include <cstddef>
#include <map> #include <map>
#include <memory> #include <memory>
#include <string> #include <string>
...@@ -14,13 +13,9 @@ ...@@ -14,13 +13,9 @@
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "chrome/browser/predictors/predictor_table_base.h" #include "chrome/browser/predictors/predictor_table_base.h"
#include "chrome/browser/predictors/resource_prefetch_common.h" #include "chrome/browser/predictors/resource_prefetch_common.h"
#include "chrome/browser/predictors/resource_prefetch_predictor.pb.h" #include "chrome/browser/predictors/resource_prefetch_predictor.pb.h"
#include "content/public/common/resource_type.h"
#include "net/base/request_priority.h"
#include "url/gurl.h"
namespace sql { namespace sql {
class Statement; class Statement;
...@@ -91,6 +86,9 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase { ...@@ -91,6 +86,9 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase {
// Sorts the resources by score, decreasing. // Sorts the resources by score, decreasing.
static void SortResources(PrefetchData* data); static void SortResources(PrefetchData* data);
// Computes score of |data|.
static float ComputeResourceScore(const ResourceData& data);
// Removes the redirects with more than |max_consecutive_misses| consecutive // Removes the redirects with more than |max_consecutive_misses| consecutive
// misses from |data|. // misses from |data|.
static void TrimRedirects(RedirectData* data, size_t max_consecutive_misses); static void TrimRedirects(RedirectData* data, size_t max_consecutive_misses);
...@@ -98,6 +96,12 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase { ...@@ -98,6 +96,12 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase {
// The maximum length of the string that can be stored in the DB. // The maximum length of the string that can be stored in the DB.
static constexpr size_t kMaxStringLength = 1024; static constexpr size_t kMaxStringLength = 1024;
protected:
// Protected for testing. Use PredictorDatabase::resource_prefetch_tables()
// instead of this constructor.
ResourcePrefetchPredictorTables();
~ResourcePrefetchPredictorTables() override;
private: private:
// Represents the type of information that is stored in prefetch database. // Represents the type of information that is stored in prefetch database.
enum class PrefetchDataType { RESOURCE, REDIRECT }; enum class PrefetchDataType { RESOURCE, REDIRECT };
...@@ -105,16 +109,14 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase { ...@@ -105,16 +109,14 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase {
enum class TableOperationType { INSERT, REMOVE }; enum class TableOperationType { INSERT, REMOVE };
friend class PredictorDatabaseInternal; friend class PredictorDatabaseInternal;
friend class MockResourcePrefetchPredictorTables;
FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTablesTest, FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTablesTest,
DatabaseVersionIsSet); DatabaseVersionIsSet);
FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTablesTest, FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTablesTest,
DatabaseIsResetWhenIncompatible); DatabaseIsResetWhenIncompatible);
FRIEND_TEST_ALL_PREFIXES(ResourcePrefetchPredictorTablesTest,
ComputeResourceScore);
ResourcePrefetchPredictorTables(); // Database version. Always increment it when any change is made to the data
~ResourcePrefetchPredictorTables() override; // schema (including the .proto).
static constexpr int kDatabaseVersion = 5;
// Helper functions below help perform functions on the Url and host table // Helper functions below help perform functions on the Url and host table
// using the same code. // using the same code.
...@@ -130,22 +132,15 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase { ...@@ -130,22 +132,15 @@ class ResourcePrefetchPredictorTables : public PredictorTableBase {
PrefetchDataType data_type, PrefetchDataType data_type,
const std::vector<std::string>& keys); const std::vector<std::string>& keys);
// Computes score of |data|. // PredictorTableBase:
static float ComputeResourceScore(const ResourceData& data);
// PredictorTableBase methods.
void CreateTableIfNonExistent() override; void CreateTableIfNonExistent() override;
void LogDatabaseStats() override; void LogDatabaseStats() override;
// Database version. Always increment it when any change is made to the data
// schema (including the .proto).
static constexpr int kDatabaseVersion = 5;
static bool DropTablesIfOutdated(sql::Connection* db); static bool DropTablesIfOutdated(sql::Connection* db);
static int GetDatabaseVersion(sql::Connection* db); static int GetDatabaseVersion(sql::Connection* db);
static bool SetDatabaseVersion(sql::Connection* db, int version); static bool SetDatabaseVersion(sql::Connection* db, int version);
// Helper to return Statements for cached Statements. // Helper to return cached Statements.
std::unique_ptr<sql::Statement> GetTableUpdateStatement( std::unique_ptr<sql::Statement> GetTableUpdateStatement(
PrefetchKeyType key_type, PrefetchKeyType key_type,
PrefetchDataType data_type, PrefetchDataType data_type,
......
...@@ -653,6 +653,8 @@ TEST_F(ResourcePrefetchPredictorTablesTest, DatabaseIsResetWhenIncompatible) { ...@@ -653,6 +653,8 @@ TEST_F(ResourcePrefetchPredictorTablesTest, DatabaseIsResetWhenIncompatible) {
&host_redirect_data); &host_redirect_data);
EXPECT_TRUE(url_data.empty()); EXPECT_TRUE(url_data.empty());
EXPECT_TRUE(host_data.empty()); EXPECT_TRUE(host_data.empty());
EXPECT_TRUE(url_redirect_data.empty());
EXPECT_TRUE(host_redirect_data.empty());
} }
TEST_F(ResourcePrefetchPredictorTablesReopenTest, GetAllData) { TEST_F(ResourcePrefetchPredictorTablesReopenTest, GetAllData) {
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/browser/ui/webui/predictors/predictors_handler.h" #include "chrome/browser/ui/webui/predictors/predictors_handler.h"
#include <memory> #include <memory>
#include <string>
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
...@@ -128,6 +129,8 @@ void PredictorsHandler::AddPrefetchDataMapToListValue( ...@@ -128,6 +129,8 @@ void PredictorsHandler::AddPrefetchDataMapToListValue(
resource->SetInteger("number_of_misses", r.number_of_misses()); resource->SetInteger("number_of_misses", r.number_of_misses());
resource->SetInteger("consecutive_misses", r.consecutive_misses()); resource->SetInteger("consecutive_misses", r.consecutive_misses());
resource->SetDouble("position", r.average_position()); resource->SetDouble("position", r.average_position());
resource->SetDouble(
"score", ResourcePrefetchPredictorTables::ComputeResourceScore(r));
resources->Append(std::move(resource)); resources->Append(std::move(resource));
} }
main->Set("resources", resources); main->Set("resources", resources);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment