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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: components/precache/content/precache_manager_unittest.cc

Issue 1961153003: Add pause/resume functionality to precache (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Moved members to proto and updated UMA Created 4 years, 7 months 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
« components/precache/content/precache_manager.cc ('K') | « components/precache/content/precache_manager.cc ('k') | components/precache/core/BUILD.gn » ('j') | components/precache/core/precache_fetcher.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/precache/content/precache_manager_unittest.cc
diff --git a/components/precache/content/precache_manager_unittest.cc b/components/precache/content/precache_manager_unittest.cc
index fb5dbfdfa5ddf5e03f58d2b475baa2c87d67dfa8..5c2cc666efc7fdb923b7c3bcef8363fffdbebf2a 100644
--- a/components/precache/content/precache_manager_unittest.cc
+++ b/components/precache/content/precache_manager_unittest.cc
@@ -7,6 +7,7 @@
#include <stddef.h>
#include <map>
+#include <memory>
#include <set>
#include <string>
@@ -14,14 +15,19 @@
#include "base/callback.h"
#include "base/command_line.h"
#include "base/compiler_specific.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
#include "base/location.h"
+#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/test/histogram_tester.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/history/core/browser/history_constants.h"
#include "components/history/core/browser/history_service.h"
#include "components/history/core/browser/history_types.h"
+#include "components/precache/core/precache_database.h"
#include "components/precache/core/precache_switches.h"
+#include "components/precache/core/proto/unfinished_work.pb.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_context.h"
#include "content/public/test/test_browser_thread_bundle.h"
@@ -49,6 +55,8 @@ const char kConfigURL[] = "http://config-url.com";
const char kManifestURLPrefix[] = "http://manifest-url-prefix.com/";
const char kGoodManifestURL[] =
"http://manifest-url-prefix.com/good-manifest.com";
+const char kEvilManifestURL[] =
+ "http://manifest-url-prefix.com/evil-manifest.com";
class TestURLFetcherCallback {
public:
@@ -117,11 +125,17 @@ class TestPrecacheCompletionCallback {
class PrecacheManagerUnderTest : public PrecacheManager {
public:
- PrecacheManagerUnderTest(content::BrowserContext* browser_context,
- const sync_driver::SyncService* const sync_service,
- const history::HistoryService* const history_service)
- : PrecacheManager(browser_context, sync_service, history_service),
- control_group_(false) {}
+ PrecacheManagerUnderTest(
+ content::BrowserContext* browser_context,
+ const sync_driver::SyncService* const sync_service,
+ const history::HistoryService* const history_service,
+ const base::FilePath& db_path,
+ std::unique_ptr<PrecacheDatabase> precache_database)
+ : PrecacheManager(
+ browser_context, sync_service, history_service,
+ db_path, std::move(precache_database)),
+ control_group_(false) {
+ }
bool IsInExperimentGroup() const override { return !control_group_; }
bool IsInControlGroup() const override { return control_group_; }
bool IsPrecachingAllowed() const override { return true; }
@@ -136,10 +150,7 @@ class PrecacheManagerUnderTest : public PrecacheManager {
class PrecacheManagerTest : public testing::Test {
public:
PrecacheManagerTest()
- : precache_manager_(&browser_context_,
- nullptr /* sync_service */,
- &history_service_),
- factory_(nullptr,
+ : factory_(nullptr,
base::Bind(&TestURLFetcherCallback::CreateURLFetcher,
base::Unretained(&url_callback_))) {}
@@ -158,18 +169,32 @@ class PrecacheManagerTest : public testing::Test {
switches::kPrecacheConfigSettingsURL, kConfigURL);
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
+ std::unique_ptr<PrecacheDatabase> precache_database(
+ new PrecacheDatabase());
+ precache_database_ = precache_database.get();
+
+ ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
+ base::FilePath db_path = scoped_temp_dir_.path().Append(
+ base::FilePath(FILE_PATH_LITERAL("precache_database")));
// Make the fetch of the precache configuration settings fail. Precaching
// should still complete normally in this case.
factory_.SetFakeResponse(GURL(kConfigURL), "",
net::HTTP_INTERNAL_SERVER_ERROR,
net::URLRequestStatus::FAILED);
+ precache_manager_.reset(
+ new PrecacheManagerUnderTest(
+ &browser_context_, nullptr /* sync_service */,
+ &history_service_, db_path, std::move(precache_database)));
+ base::MessageLoop::current()->RunUntilIdle();
}
// Must be declared first so that it is destroyed last.
content::TestBrowserThreadBundle test_browser_thread_bundle_;
+ base::ScopedTempDir scoped_temp_dir_;
+ PrecacheDatabase* precache_database_;
content::TestBrowserContext browser_context_;
- PrecacheManagerUnderTest precache_manager_;
+ std::unique_ptr<PrecacheManagerUnderTest> precache_manager_;
TestURLFetcherCallback url_callback_;
net::FakeURLFetcherFactory factory_;
TestPrecacheCompletionCallback precache_callback_;
@@ -178,7 +203,7 @@ class PrecacheManagerTest : public testing::Test {
};
TEST_F(PrecacheManagerTest, StartAndFinishPrecaching) {
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
MockHistoryService::TopHostsCallback top_hosts_callback;
EXPECT_CALL(history_service_, TopHosts(NumTopHosts(), _))
@@ -187,14 +212,14 @@ TEST_F(PrecacheManagerTest, StartAndFinishPrecaching) {
factory_.SetFakeResponse(GURL(kGoodManifestURL), "", net::HTTP_OK,
net::URLRequestStatus::SUCCESS);
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
-
- EXPECT_TRUE(precache_manager_.IsPrecaching());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
top_hosts_callback.Run(
history::TopHostsList(1, std::make_pair("good-manifest.com", 1)));
base::MessageLoop::current()->RunUntilIdle(); // For PrecacheFetcher.
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
EXPECT_TRUE(precache_callback_.was_on_done_called());
std::multiset<GURL> expected_requested_urls;
@@ -203,71 +228,114 @@ TEST_F(PrecacheManagerTest, StartAndFinishPrecaching) {
EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
}
+TEST_F(PrecacheManagerTest, StartAndFinishPrecachingWithUnfinishedHosts) {
+ std::unique_ptr<PrecacheUnfinishedWork> unfinished_work(
+ new PrecacheUnfinishedWork());
+ unfinished_work->add_top_host()->set_hostname("evil-manifest.com");
+ unfinished_work->set_start_time(base::Time::Now().ToInternalValue());
+ precache_database_->SaveUnfinishedWork(std::move(unfinished_work));
+
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
+
+ factory_.SetFakeResponse(
+ GURL(kEvilManifestURL), "",
+ net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
+
+ base::MessageLoop::current()->RunUntilIdle(); // For PrecacheFetcher.
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
+ EXPECT_TRUE(precache_callback_.was_on_done_called());
+
+ std::multiset<GURL> expected_requested_urls;
+ expected_requested_urls.insert(GURL(kConfigURL));
+ expected_requested_urls.insert(GURL(kEvilManifestURL));
+ EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
+}
+
+TEST_F(PrecacheManagerTest,
+ StartAndCancelPrecachingBeforeUnfinishedWorkRetrieved) {
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
+
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
+ precache_manager_->CancelPrecaching();
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
+
+ base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_FALSE(precache_callback_.was_on_done_called());
+}
+
TEST_F(PrecacheManagerTest, StartAndCancelPrecachingBeforeTopHostsCompleted) {
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
MockHistoryService::TopHostsCallback top_hosts_callback;
EXPECT_CALL(history_service_, TopHosts(NumTopHosts(), _))
.WillOnce(SaveArg<1>(&top_hosts_callback));
- precache_manager_.SetInControlGroup(true);
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
- EXPECT_TRUE(precache_manager_.IsPrecaching());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
- precache_manager_.CancelPrecaching();
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ precache_manager_->CancelPrecaching();
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
+ base::MessageLoop::current()->RunUntilIdle();
top_hosts_callback.Run(
history::TopHostsList(1, std::make_pair("starting-url.com", 1)));
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
EXPECT_FALSE(precache_callback_.was_on_done_called());
}
TEST_F(PrecacheManagerTest, StartAndCancelPrecachingBeforeURLsReceived) {
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
MockHistoryService::TopHostsCallback top_hosts_callback;
EXPECT_CALL(history_service_, TopHosts(NumTopHosts(), _))
.WillOnce(SaveArg<1>(&top_hosts_callback));
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
- EXPECT_TRUE(precache_manager_.IsPrecaching());
-
- precache_manager_.CancelPrecaching();
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
+ precache_manager_->CancelPrecaching();
+ base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
top_hosts_callback.Run(
history::TopHostsList(1, std::make_pair("starting-url.com", 1)));
base::MessageLoop::current()->RunUntilIdle(); // For PrecacheFetcher.
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
EXPECT_FALSE(precache_callback_.was_on_done_called());
EXPECT_TRUE(url_callback_.requested_urls().empty());
}
TEST_F(PrecacheManagerTest, StartAndCancelPrecachingAfterURLsReceived) {
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
+
+ PrecacheManifest good_manifest;
+ good_manifest.add_resource()->set_url("http://good-resource.com");
EXPECT_CALL(history_service_, TopHosts(NumTopHosts(), _))
.WillOnce(ReturnHosts(
history::TopHostsList(1, std::make_pair("starting-url.com", 1))));
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
- // Since the |history_service_| ran the callback immediately, Start() has
- // been called on the PrecacheFetcher, and the precache config settings have
- // been requested. The response has not yet been received though, so
- // precaching is still in progress.
- EXPECT_TRUE(precache_manager_.IsPrecaching());
-
- precache_manager_.CancelPrecaching();
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
+ // Run a task to get unfinished work, and to get hosts.
+ for (int i = 0; i < 2; ++i) {
+ base::RunLoop run_loop;
+ base::MessageLoop::current()->PostTask(FROM_HERE, run_loop.QuitClosure());
+ run_loop.Run();
+ }
+ //base::MessageLoop::current()->RunUntilIdle();
+ precache_manager_->CancelPrecaching();
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
base::MessageLoop::current()->RunUntilIdle(); // For PrecacheFetcher.
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
EXPECT_FALSE(precache_callback_.was_on_done_called());
- // Even though the response for the precache config settings should not have
- // been received, the request should still have been made.
std::multiset<GURL> expected_requested_urls;
expected_requested_urls.insert(GURL(kConfigURL));
EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
@@ -275,7 +343,7 @@ TEST_F(PrecacheManagerTest, StartAndCancelPrecachingAfterURLsReceived) {
TEST_F(PrecacheManagerTest, RecordStatsForFetchWithSizeZero) {
// Fetches with size 0 should be ignored.
- precache_manager_.RecordStatsForFetch(GURL("http://url.com"), GURL(),
+ precache_manager_->RecordStatsForFetch(GURL("http://url.com"), GURL(),
base::TimeDelta(), base::Time(), 0,
false);
base::MessageLoop::current()->RunUntilIdle();
@@ -284,7 +352,7 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchWithSizeZero) {
TEST_F(PrecacheManagerTest, RecordStatsForFetchWithNonHTTP) {
// Fetches for URLs with schemes other than HTTP or HTTPS should be ignored.
- precache_manager_.RecordStatsForFetch(GURL("ftp://ftp.com"), GURL(),
+ precache_manager_->RecordStatsForFetch(GURL("ftp://ftp.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::MessageLoop::current()->RunUntilIdle();
@@ -293,7 +361,7 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchWithNonHTTP) {
TEST_F(PrecacheManagerTest, RecordStatsForFetchWithEmptyURL) {
// Fetches for empty URLs should be ignored.
- precache_manager_.RecordStatsForFetch(GURL(), GURL(), base::TimeDelta(),
+ precache_manager_->RecordStatsForFetch(GURL(), GURL(), base::TimeDelta(),
base::Time(), 1000, false);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."), IsEmpty());
@@ -303,14 +371,14 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchDuringPrecaching) {
EXPECT_CALL(history_service_, TopHosts(NumTopHosts(), _))
.WillOnce(ReturnHosts(history::TopHostsList()));
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
- EXPECT_TRUE(precache_manager_.IsPrecaching());
- precache_manager_.RecordStatsForFetch(GURL("http://url.com"), GURL(),
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
+ precache_manager_->RecordStatsForFetch(GURL("http://url.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
-
- precache_manager_.CancelPrecaching();
+ base::MessageLoop::current()->RunUntilIdle();
+ precache_manager_->CancelPrecaching();
// For PrecacheFetcher and RecordURLPrecached.
base::MessageLoop::current()->RunUntilIdle();
@@ -324,7 +392,7 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchDuringPrecaching) {
}
TEST_F(PrecacheManagerTest, RecordStatsForFetchHTTP) {
- precache_manager_.RecordStatsForFetch(GURL("http://http-url.com"), GURL(),
+ precache_manager_->RecordStatsForFetch(GURL("http://http-url.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::MessageLoop::current()->RunUntilIdle();
@@ -336,7 +404,7 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchHTTP) {
}
TEST_F(PrecacheManagerTest, RecordStatsForFetchHTTPS) {
- precache_manager_.RecordStatsForFetch(GURL("https://https-url.com"), GURL(),
+ precache_manager_->RecordStatsForFetch(GURL("https://https-url.com"), GURL(),
base::TimeDelta(), base::Time(), 1000,
false);
base::MessageLoop::current()->RunUntilIdle();
@@ -354,7 +422,7 @@ TEST_F(PrecacheManagerTest, RecordStatsForFetchInTopHosts) {
[](const GURL& url, const base::Callback<void(int)>& callback) {
callback.Run(0);
}));
- precache_manager_.RecordStatsForFetch(
+ precache_manager_->RecordStatsForFetch(
GURL("http://http-url.com"), GURL("http://referrer.com"),
base::TimeDelta(), base::Time(), 1000, false);
base::MessageLoop::current()->RunUntilIdle();
@@ -375,17 +443,17 @@ TEST_F(PrecacheManagerTest, DeleteExpiredPrecacheHistory) {
.Times(2)
.WillRepeatedly(ReturnHosts(history::TopHostsList()));
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
- EXPECT_TRUE(precache_manager_.IsPrecaching());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
// Precache a bunch of URLs, with different fetch times.
- precache_manager_.RecordStatsForFetch(
+ precache_manager_->RecordStatsForFetch(
GURL("http://old-fetch.com"), GURL(), base::TimeDelta(),
kCurrentTime - base::TimeDelta::FromDays(61), 1000, false);
- precache_manager_.RecordStatsForFetch(
+ precache_manager_->RecordStatsForFetch(
GURL("http://recent-fetch.com"), GURL(), base::TimeDelta(),
kCurrentTime - base::TimeDelta::FromDays(59), 1000, false);
- precache_manager_.RecordStatsForFetch(
+ precache_manager_->RecordStatsForFetch(
GURL("http://yesterday-fetch.com"), GURL(), base::TimeDelta(),
kCurrentTime - base::TimeDelta::FromDays(1), 1000, false);
expected_histogram_count_map["Precache.DownloadedPrecacheMotivated"] += 3;
@@ -394,29 +462,40 @@ TEST_F(PrecacheManagerTest, DeleteExpiredPrecacheHistory) {
expected_histogram_count_map["Precache.Fetch.ResponseBytes.Total"]++;
expected_histogram_count_map["Precache.Fetch.TimeToComplete"]++;
expected_histogram_count_map["Precache.Latency.Prefetch"] += 3;
+ base::MessageLoop::current()->RunUntilIdle();
+
+ precache_manager_->CancelPrecaching();
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // Disable pause-resume.
+ precache_database_->DeleteUnfinishedWork();
+ base::MessageLoop::current()->RunUntilIdle();
- precache_manager_.CancelPrecaching();
// For PrecacheFetcher and RecordURLPrecached.
base::MessageLoop::current()->RunUntilIdle();
EXPECT_THAT(histograms_.GetTotalCountsForPrefix("Precache."),
ContainerEq(expected_histogram_count_map));
// The expired precache will be deleted during precaching this time.
- precache_manager_.StartPrecaching(precache_callback_.GetCallback());
- EXPECT_TRUE(precache_manager_.IsPrecaching());
+ precache_manager_->StartPrecaching(precache_callback_.GetCallback());
+ EXPECT_TRUE(precache_manager_->IsPrecaching());
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // The precache fetcher runs until done, which records these histograms,
+ // and then cancels precaching, which records these histograms again.
+ // In practice
expected_histogram_count_map["Precache.Fetch.PercentCompleted"]++;
expected_histogram_count_map["Precache.Fetch.ResponseBytes.Network"]++;
expected_histogram_count_map["Precache.Fetch.ResponseBytes.Total"]++;
-
- precache_manager_.CancelPrecaching();
+ //precache_manager_->CancelPrecaching();
sclittle 2016/05/20 21:57:07 nit: remove commented line?
bengr 2016/05/21 00:16:32 Done.
// For PrecacheFetcher and RecordURLPrecached.
base::MessageLoop::current()->RunUntilIdle();
- EXPECT_FALSE(precache_manager_.IsPrecaching());
+ EXPECT_FALSE(precache_manager_->IsPrecaching());
// A fetch for the same URL as the expired precache was served from the cache,
// but it isn't reported as saved bytes because it had expired in the precache
// history.
- precache_manager_.RecordStatsForFetch(GURL("http://old-fetch.com"), GURL(),
+ precache_manager_->RecordStatsForFetch(GURL("http://old-fetch.com"), GURL(),
base::TimeDelta(), kCurrentTime, 1000,
true);
expected_histogram_count_map["Precache.Fetch.TimeToComplete"]++;
@@ -429,10 +508,10 @@ TEST_F(PrecacheManagerTest, DeleteExpiredPrecacheHistory) {
// The other precaches should not have expired, so the following fetches from
// the cache should count as saved bytes.
- precache_manager_.RecordStatsForFetch(GURL("http://recent-fetch.com"), GURL(),
+ precache_manager_->RecordStatsForFetch(GURL("http://recent-fetch.com"), GURL(),
base::TimeDelta(), kCurrentTime, 1000,
true);
- precache_manager_.RecordStatsForFetch(GURL("http://yesterday-fetch.com"),
+ precache_manager_->RecordStatsForFetch(GURL("http://yesterday-fetch.com"),
GURL(), base::TimeDelta(), kCurrentTime,
1000, true);
expected_histogram_count_map["Precache.Latency.NonPrefetch"] += 2;
« components/precache/content/precache_manager.cc ('K') | « components/precache/content/precache_manager.cc ('k') | components/precache/core/BUILD.gn » ('j') | components/precache/core/precache_fetcher.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698