Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(415)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: components/precache/core/precache_database_unittest.cc

Issue 2586813004: Report downloaded resources at most once (Closed)
Patch Set: Fix race condition in PrecacheFetcherTest due to MaybePost Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/precache/core/precache_database.cc ('k') | components/precache/core/precache_fetcher.h » ('j') | components/precache/core/precache_url_table.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/precache/core/precache_database_unittest.cc
diff --git a/components/precache/core/precache_database_unittest.cc b/components/precache/core/precache_database_unittest.cc
index 8d00c79f6f6c0c36bc49e985454d235f5c118dd3..6a1f752756ffcb050751784b1b1d315232800c4f 100644
--- a/components/precache/core/precache_database_unittest.cc
+++ b/components/precache/core/precache_database_unittest.cc
@@ -9,6 +9,8 @@
#include <map>
#include <memory>
+#include "base/bind.h"
+#include "base/bind_helpers.h"
#include "base/containers/hash_tables.h"
#include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.h"
@@ -91,6 +93,11 @@ class PrecacheDatabaseTest : public testing::Test {
PrecacheDatabaseTest() {}
~PrecacheDatabaseTest() override {}
+ void SqlErrorCallback(int line, sql::Statement* statement) {
+ LOG(INFO) << "SQL error at line " << line << ": "
+ << statement->GetSQLStatement();
+ }
+
protected:
void SetUp() override {
precache_database_.reset(new PrecacheDatabase());
@@ -99,6 +106,8 @@ class PrecacheDatabaseTest : public testing::Test {
base::FilePath db_path = scoped_temp_dir_.GetPath().Append(
base::FilePath(FILE_PATH_LITERAL("precache_database")));
ASSERT_TRUE(precache_database_->Init(db_path));
+ precache_database_->db_->set_error_callback(base::Bind(
+ &PrecacheDatabaseTest::SqlErrorCallback, base::Unretained(this)));
}
void TearDown() override { precache_url_table()->DeleteAll(); }
@@ -263,7 +272,7 @@ TEST_F(PrecacheDatabaseTest, PrecacheOverNetwork) {
}
TEST_F(PrecacheDatabaseTest, PrecacheFromCacheWithURLTableEntry) {
- precache_url_table()->AddURL(kURL, kReferrerID, true, kOldFetchTime);
+ precache_url_table()->AddURL(kURL, kReferrerID, true, true, kOldFetchTime);
RecordPrecacheFromCache(kURL, kFetchTime, kSize);
// The URL table entry should have been updated to have |kFetchTime| as the
@@ -326,7 +335,7 @@ TEST_F(PrecacheDatabaseTest, FetchOverNetwork_Cellular) {
}
TEST_F(PrecacheDatabaseTest, FetchOverNetworkWithURLTableEntry) {
- precache_url_table()->AddURL(kURL, kReferrerID, true, kOldFetchTime);
+ precache_url_table()->AddURL(kURL, kReferrerID, true, true, kOldFetchTime);
RecordFetchFromNetwork(kURL, kLatency, kFetchTime, kSize);
// The URL table entry should have been deleted.
@@ -343,7 +352,7 @@ TEST_F(PrecacheDatabaseTest, FetchOverNetworkWithURLTableEntry) {
}
TEST_F(PrecacheDatabaseTest, FetchFromCacheWithURLTableEntry_NonCellular) {
- precache_url_table()->AddURL(kURL, kReferrerID, true, kOldFetchTime);
+ precache_url_table()->AddURL(kURL, kReferrerID, true, true, kOldFetchTime);
RecordFetchFromCache(kURL, kFetchTime, kSize);
// The URL table entry should have been deleted.
@@ -361,7 +370,7 @@ TEST_F(PrecacheDatabaseTest, FetchFromCacheWithURLTableEntry_NonCellular) {
}
TEST_F(PrecacheDatabaseTest, FetchFromCacheWithURLTableEntry_Cellular) {
- precache_url_table()->AddURL(kURL, kReferrerID, true, kOldFetchTime);
+ precache_url_table()->AddURL(kURL, kReferrerID, true, true, kOldFetchTime);
RecordFetchFromCacheCellular(kURL, kFetchTime, kSize);
// The URL table entry should have been deleted.
@@ -397,9 +406,9 @@ TEST_F(PrecacheDatabaseTest, DeleteExpiredPrecacheHistory) {
const base::Time k61DaysAgo = kToday - base::TimeDelta::FromDays(61);
precache_url_table()->AddURL(GURL("http://expired-precache.com"), kReferrerID,
- true, k61DaysAgo);
+ true, true, k61DaysAgo);
precache_url_table()->AddURL(GURL("http://old-precache.com"), kReferrerID,
- true, k59DaysAgo);
+ true, true, k59DaysAgo);
precache_database_->DeleteExpiredPrecacheHistory(kToday);
@@ -533,6 +542,24 @@ TEST_F(PrecacheDatabaseTest, GetURLListForReferrerHost) {
kNewFetchTime);
precache_database_->UpdatePrecacheReferrerHost("empty.com", 3, kNewFetchTime);
+ // Add two resources that shouldn't appear in downloaded.
+ Flush(); // We need to write the referrer_host_id.
+ const std::string already_reported_and_not_refetch =
+ "http://foo.com/already-reported-and-not-refetch.js";
+
+ precache_database_->RecordURLPrefetch(GURL(already_reported_and_not_refetch),
+ "foo.com", kPrecacheTime, false, kSize);
+ const std::string already_reported_and_in_cache =
+ "http://foo.com/already-reported-and-in-cache.js";
+ precache_database_->RecordURLPrefetch(GURL(already_reported_and_in_cache),
+ "foo.com", kPrecacheTime, false, kSize);
+ {
+ std::vector<GURL> unused_a, unused_b;
+ auto id = precache_database_->GetReferrerHost("foo.com").id;
+ CHECK_NE(PrecacheReferrerHostEntry::kInvalidId, id);
twifkak 2016/12/20 01:41:59 Per the last bullet of https://chromium.googlesour
jamartin 2016/12/20 21:09:34 Done.
+ precache_database_->GetURLListForReferrerHost(id, &unused_a, &unused_b);
twifkak 2016/12/20 01:41:59 I assume the purpose of this call is to mark the a
jamartin 2016/12/20 21:09:34 Done.
+ }
+
struct test_resource_info {
std::string url;
bool is_user_browsed;
@@ -582,6 +609,8 @@ TEST_F(PrecacheDatabaseTest, GetURLListForReferrerHost) {
kPrecacheTime, false, kSize);
}
}
+ precache_database_->RecordURLPrefetch(GURL(already_reported_and_in_cache),
+ "foo.com", kPrecacheTime, true, kSize);
// Update some resources as used due to user browsing.
for (const auto& test : tests) {
for (const auto& resource : test.resource_info) {
@@ -600,35 +629,35 @@ TEST_F(PrecacheDatabaseTest, GetURLListForReferrerHost) {
}
}
Flush();
- // Verify the used and unused resources.
+ // Verify the used and downloaded resources.
for (const auto& test : tests) {
- std::vector<GURL> expected_used_urls, expected_unused_urls;
+ std::vector<GURL> expected_used_urls, expected_downloaded_urls;
for (const auto& resource : test.resource_info) {
if (resource.expected_in_used) {
expected_used_urls.push_back(GURL(resource.url));
- } else {
- expected_unused_urls.push_back(GURL(resource.url));
}
+ expected_downloaded_urls.push_back(GURL(resource.url));
}
- std::vector<GURL> actual_used_urls, actual_unused_urls;
+ std::vector<GURL> actual_used_urls, actual_downloaded_urls;
auto referrer_id = precache_database_->GetReferrerHost(test.hostname).id;
EXPECT_NE(PrecacheReferrerHostEntry::kInvalidId, referrer_id);
precache_database_->GetURLListForReferrerHost(
- referrer_id, &actual_used_urls, &actual_unused_urls);
- EXPECT_THAT(expected_used_urls, ::testing::ContainerEq(actual_used_urls));
- EXPECT_THAT(expected_unused_urls,
- ::testing::ContainerEq(actual_unused_urls));
+ referrer_id, &actual_used_urls, &actual_downloaded_urls);
+ EXPECT_THAT(actual_used_urls, ::testing::ContainerEq(expected_used_urls));
+ EXPECT_THAT(actual_downloaded_urls,
+ ::testing::ContainerEq(expected_downloaded_urls))
+ << "Host: " << test.hostname;
}
- // Subsequent manifest updates should clear the used and unused resources.
+ // Subsequent manifest updates should clear the used and downloaded resources.
for (const auto& test : tests) {
precache_database_->UpdatePrecacheReferrerHost(test.hostname, 100,
kNewFetchTime);
Flush();
- std::vector<GURL> actual_used_urls, actual_unused_urls;
+ std::vector<GURL> actual_used_urls, actual_downloaded_urls;
auto referrer_id = precache_database_->GetReferrerHost(test.hostname).id;
EXPECT_NE(PrecacheReferrerHostEntry::kInvalidId, referrer_id);
precache_database_->GetURLListForReferrerHost(
- referrer_id, &actual_used_urls, &actual_unused_urls);
+ referrer_id, &actual_used_urls, &actual_downloaded_urls);
EXPECT_TRUE(actual_used_urls.empty());
}
// Resources that were precached previously and not seen in user browsing
@@ -640,6 +669,9 @@ TEST_F(PrecacheDatabaseTest, GetURLListForReferrerHost) {
expected_url_table_map[GURL(resource.url)] = kPrecacheTime;
}
}
+ expected_url_table_map[GURL(already_reported_and_not_refetch)] =
+ kPrecacheTime;
+ expected_url_table_map[GURL(already_reported_and_in_cache)] = kPrecacheTime;
EXPECT_EQ(expected_url_table_map, GetActualURLTableMap());
}
« no previous file with comments | « components/precache/core/precache_database.cc ('k') | components/precache/core/precache_fetcher.h » ('j') | components/precache/core/precache_url_table.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698