Chromium Code Reviews| Index: components/precache/core/precache_fetcher_unittest.cc |
| diff --git a/components/precache/core/precache_fetcher_unittest.cc b/components/precache/core/precache_fetcher_unittest.cc |
| index 708476671a358989d686d2b94f30793d983c9790..4296c3259e19795cb822bb68064fcbf04a2eb82e 100644 |
| --- a/components/precache/core/precache_fetcher_unittest.cc |
| +++ b/components/precache/core/precache_fetcher_unittest.cc |
| @@ -16,6 +16,8 @@ |
| #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/memory/ptr_util.h" |
| #include "base/memory/ref_counted.h" |
| #include "base/memory/weak_ptr.h" |
| @@ -24,6 +26,7 @@ |
| #include "base/strings/stringprintf.h" |
| #include "base/test/histogram_tester.h" |
| #include "base/threading/thread_task_runner_handle.h" |
| +#include "components/precache/core/precache_database.h" |
| #include "components/precache/core/precache_switches.h" |
| #include "components/precache/core/proto/precache.pb.h" |
| #include "components/precache/core/proto/unfinished_work.pb.h" |
| @@ -90,9 +93,7 @@ class TestURLFetcherCallback { |
| return fetcher; |
| } |
| - const std::multiset<GURL>& requested_urls() const { |
| - return requested_urls_; |
| - } |
| + std::multiset<GURL>& requested_urls() { return requested_urls_; } |
|
sclittle
2016/08/11 22:52:36
You shouldn't return a non-const reference here. U
Raj
2016/08/12 19:04:21
Done.
|
| int total_response_bytes() const { return total_response_bytes_; } |
| @@ -230,10 +231,10 @@ TEST_F(PrecacheFetcherFetcherTest, Config) { |
| NotNull()))); |
| PrecacheFetcher::Fetcher precache_fetcher( |
| - request_context_.get(), url, callback_, false /* is_resource_request */, |
| - SIZE_MAX); |
| + request_context_.get(), url, url.host(), callback_, |
| + false /* is_resource_request */, SIZE_MAX); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| ASSERT_NE(nullptr, fetcher); |
| EXPECT_EQ(kNoTracking, fetcher->GetLoadFlags()); |
| @@ -252,10 +253,10 @@ TEST_F(PrecacheFetcherFetcherTest, ResourceNotInCache) { |
| .Times(1); |
| PrecacheFetcher::Fetcher precache_fetcher( |
| - request_context_.get(), url, callback_, true /* is_resource_request */, |
| - SIZE_MAX); |
| + request_context_.get(), url, url.host(), callback_, |
| + true /* is_resource_request */, SIZE_MAX); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| ASSERT_NE(nullptr, fetcher1); |
| EXPECT_EQ(net::LOAD_ONLY_FROM_CACHE | kNoTracking, fetcher1->GetLoadFlags()); |
| @@ -275,10 +276,10 @@ TEST_F(PrecacheFetcherFetcherTest, ResourceHasValidators) { |
| NotNull()))); |
| PrecacheFetcher::Fetcher precache_fetcher( |
| - request_context_.get(), url, callback_, true /* is_resource_request */, |
| - SIZE_MAX); |
| + request_context_.get(), url, url.host(), callback_, |
| + true /* is_resource_request */, SIZE_MAX); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| ASSERT_NE(nullptr, fetcher1); |
| EXPECT_EQ(net::LOAD_ONLY_FROM_CACHE | kNoTracking, fetcher1->GetLoadFlags()); |
| @@ -297,10 +298,10 @@ TEST_F(PrecacheFetcherFetcherTest, ResourceHasNoValidators) { |
| nullptr))); // It never reached the network. |
| PrecacheFetcher::Fetcher precache_fetcher( |
| - request_context_.get(), url, callback_, true /* is_resource_request */, |
| - SIZE_MAX); |
| + request_context_.get(), url, url.host(), callback_, |
| + true /* is_resource_request */, SIZE_MAX); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| EXPECT_EQ(net::LOAD_ONLY_FROM_CACHE | kNoTracking, fetcher->GetLoadFlags()); |
| } |
| @@ -324,16 +325,17 @@ TEST_F(PrecacheFetcherFetcherTest, ResourceTooBig) { |
| nullptr))); |
| PrecacheFetcher::Fetcher precache_fetcher( |
| - request_context_.get(), url, callback_, true /* is_resource_request */, |
| - 99 /* max_bytes */); |
| + request_context_.get(), url, url.host(), callback_, |
| + true /* is_resource_request */, 99 /* max_bytes */); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| } |
| class PrecacheFetcherTest : public testing::Test { |
| public: |
| PrecacheFetcherTest() |
| - : request_context_(new net::TestURLRequestContextGetter( |
| + : task_runner_(base::ThreadTaskRunnerHandle::Get()), |
| + request_context_(new net::TestURLRequestContextGetter( |
| base::ThreadTaskRunnerHandle::Get())), |
| factory_(NULL, |
| base::Bind(&TestURLFetcherCallback::CreateURLFetcher, |
| @@ -342,6 +344,12 @@ class PrecacheFetcherTest : public testing::Test { |
| parallel_fetches_beyond_capacity_(false) {} |
| protected: |
| + void SetUp() override { |
| + ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir()); |
| + base::FilePath db_path = scoped_temp_dir_.path().Append( |
| + base::FilePath(FILE_PATH_LITERAL("precache_database"))); |
| + precache_database_.Init(db_path); |
| + } |
| void SetDefaultFlags() { |
| base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( |
| switches::kPrecacheConfigSettingsURL, kConfigURL); |
| @@ -355,25 +363,34 @@ class PrecacheFetcherTest : public testing::Test { |
| void CheckUntilParallelFetchesBeyondCapacity( |
| const PrecacheFetcher* precache_fetcher) { |
| if (!precache_fetcher->pool_.IsAvailable() && |
| - !precache_fetcher->resource_urls_to_fetch_.empty() && |
| - !precache_fetcher->manifest_urls_to_fetch_.empty()) { |
| + precache_fetcher->top_hosts_to_fetch_ && |
| + !precache_fetcher->top_hosts_to_fetch_->empty() && |
| + !precache_fetcher->resources_to_fetch_.empty()) { |
| parallel_fetches_beyond_capacity_ = true; |
| return; |
| } |
| // Check again after allowing the message loop to process some messages. |
| - loop_.task_runner()->PostTask( |
| + loop_.PostTask( |
| FROM_HERE, |
| base::Bind( |
| &PrecacheFetcherTest::CheckUntilParallelFetchesBeyondCapacity, |
| base::Unretained(this), precache_fetcher)); |
| } |
| + const scoped_refptr<base::SingleThreadTaskRunner>& task_runner() const { |
| + return task_runner_; |
| + } |
| + |
| + // Must be declared first so that it is destroyed last. |
| base::MessageLoopForUI loop_; |
| + scoped_refptr<base::SingleThreadTaskRunner> task_runner_; |
| scoped_refptr<net::TestURLRequestContextGetter> request_context_; |
| TestURLFetcherCallback url_callback_; |
| net::FakeURLFetcherFactory factory_; |
| TestPrecacheDelegate precache_delegate_; |
| + base::ScopedTempDir scoped_temp_dir_; |
| + PrecacheDatabase precache_database_; |
| int expected_total_response_bytes_; |
| // True if more parallel fetches were attempted beyond the fetcher pool |
| @@ -425,12 +442,13 @@ TEST_F(PrecacheFetcherTest, FullPrecache) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| // Destroy the PrecacheFetcher after it has finished, to record metrics. |
| } |
| @@ -493,12 +511,13 @@ TEST_F(PrecacheFetcherTest, PrecacheResourceSelection) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| // Destroy the PrecacheFetcher after it has finished, to record metrics. |
| } |
| @@ -565,12 +584,13 @@ TEST_F(PrecacheFetcherTest, PrecacheResourceSelectionMissingBitset) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| // Destroy the PrecacheFetcher after it has finished, to record metrics. |
| } |
| @@ -602,23 +622,25 @@ TEST_F(PrecacheFetcherTest, PrecachePauseResume) { |
| std::unique_ptr<PrecacheUnfinishedWork> initial_work( |
| new PrecacheUnfinishedWork()); |
| - initial_work->add_manifest()->set_url( |
| - "http://manifest-url-prefix.com/manifest1.com"); |
| - initial_work->add_manifest()->set_url( |
| - "http://manifest-url-prefix.com/manifest2.com"); |
| - initial_work->add_resource()->set_url(kGoodResourceURL); |
| + initial_work->add_top_host()->set_hostname("manifest1.com"); |
| + initial_work->add_top_host()->set_hostname("manifest2.com"); |
| initial_work->set_start_time( |
| (base::Time::Now() - base::TimeDelta::FromHours(1)).ToInternalValue()); |
| + // initial_work->add_resource()->set_url(kGoodResourceURL); |
| PrecacheFetcher first_fetcher(request_context_.get(), GURL(), std::string(), |
| std::move(initial_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), |
| &precache_delegate_); |
| - |
| - loop_.RunUntilIdle(); |
| + factory_.SetFakeResponse(GURL(kConfigURL), config.SerializeAsString(), |
| + net::HTTP_OK, net::URLRequestStatus::SUCCESS); |
| + first_fetcher.Start(); |
| + // base::RunLoop().RunUntilIdle(); |
| std::unique_ptr<PrecacheUnfinishedWork> unfinished_work = |
| first_fetcher.CancelPrecaching(); |
| std::multiset<GURL> expected_requested_urls; |
| + expected_requested_urls.insert(GURL(kConfigURL)); |
| EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls()); |
| factory_.SetFakeResponse(GURL(kConfigURL), config.SerializeAsString(), |
| @@ -633,19 +655,20 @@ TEST_F(PrecacheFetcherTest, PrecachePauseResume) { |
| net::URLRequestStatus::SUCCESS); |
| factory_.SetFakeResponse(GURL(kGoodResourceURL), "good", net::HTTP_OK, |
| net::URLRequestStatus::SUCCESS); |
| - // Starting hosts should not be fetched. |
| - unfinished_work->add_top_host()->set_hostname("bad-manifest.com"); |
| + |
| + // unfinished_work->add_top_host()->set_hostname("bad-manifest.com"); |
|
sclittle
2016/08/11 22:52:36
remove dead code, here and elsewhere
|
| + url_callback_.requested_urls().clear(); |
| PrecacheFetcher second_fetcher(request_context_.get(), GURL(), std::string(), |
| std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), |
| &precache_delegate_); |
| second_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| - expected_requested_urls.insert(GURL(kConfigURL)); |
| + base::RunLoop().RunUntilIdle(); |
| expected_requested_urls.insert( |
| GURL("http://manifest-url-prefix.com/manifest1.com")); |
| expected_requested_urls.insert( |
| GURL("http://manifest-url-prefix.com/manifest2.com")); |
| - expected_requested_urls.insert(GURL(kGoodResourceURL)); |
| + // expected_requested_urls.insert(GURL(kGoodResourceURL)); |
| EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls()); |
| EXPECT_TRUE(precache_delegate_.was_on_done_called()); |
| } |
| @@ -666,12 +689,13 @@ TEST_F(PrecacheFetcherTest, ResumeWithConfigOnly) { |
| factory_.SetFakeResponse(GURL(kGoodResourceURL), "good", net::HTTP_OK, |
| net::URLRequestStatus::SUCCESS); |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| } |
| std::multiset<GURL> expected_requested_urls; |
| @@ -706,10 +730,11 @@ TEST_F(PrecacheFetcherTest, CustomURLs) { |
| PrecacheFetcher precache_fetcher( |
| request_context_.get(), GURL(kCustomConfigURL), kCustomManifestURLPrefix, |
| - std::move(unfinished_work), kExperimentID, &precache_delegate_); |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| std::multiset<GURL> expected_requested_urls; |
| expected_requested_urls.insert(GURL(kCustomConfigURL)); |
| @@ -734,12 +759,13 @@ TEST_F(PrecacheFetcherTest, ConfigFetchFailure) { |
| factory_.SetFakeResponse(GURL(kGoodManifestURL), "", net::HTTP_OK, |
| net::URLRequestStatus::SUCCESS); |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), std::move(unfinished_work), |
| + kExperimentID, precache_database_.GetWeakPtr(), task_runner(), |
| + &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| std::multiset<GURL> expected_requested_urls; |
| expected_requested_urls.insert(GURL(kConfigURL)); |
| @@ -761,12 +787,13 @@ TEST_F(PrecacheFetcherTest, BadConfig) { |
| factory_.SetFakeResponse(GURL(kGoodManifestURL), "", net::HTTP_OK, |
| net::URLRequestStatus::SUCCESS); |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), std::move(unfinished_work), |
| + kExperimentID, precache_database_.GetWeakPtr(), task_runner(), |
| + &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| std::multiset<GURL> expected_requested_urls; |
| expected_requested_urls.insert(GURL(kConfigURL)); |
| @@ -792,9 +819,10 @@ TEST_F(PrecacheFetcherTest, Cancel) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| // Destroy the PrecacheFetcher, to cancel precaching. No metrics |
| @@ -802,7 +830,7 @@ TEST_F(PrecacheFetcherTest, Cancel) { |
| // called on the precache delegate. |
| } |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| std::multiset<GURL> expected_requested_urls; |
| expected_requested_urls.insert(GURL(kConfigURL)); |
| @@ -829,12 +857,13 @@ TEST_F(PrecacheFetcherTest, PrecacheUsingDefaultConfigSettingsURL) { |
| config.SerializeAsString(), net::HTTP_OK, |
| net::URLRequestStatus::SUCCESS); |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), std::move(unfinished_work), |
| + kExperimentID, precache_database_.GetWeakPtr(), task_runner(), |
| + &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| std::multiset<GURL> expected_requested_urls; |
| expected_requested_urls.insert(GURL(PRECACHE_CONFIG_SETTINGS_URL)); |
| @@ -867,12 +896,13 @@ TEST_F(PrecacheFetcherTest, PrecacheUsingDefaultManifestURLPrefix) { |
| factory_.SetFakeResponse(manifest_url, PrecacheManifest().SerializeAsString(), |
| net::HTTP_OK, net::URLRequestStatus::SUCCESS); |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), std::move(unfinished_work), |
| + kExperimentID, precache_database_.GetWeakPtr(), task_runner(), |
| + &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| std::multiset<GURL> expected_requested_urls; |
| expected_requested_urls.insert(GURL(kConfigURL)); |
| @@ -913,12 +943,13 @@ TEST_F(PrecacheFetcherTest, TopResourcesCount) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| // Destroy the PrecacheFetcher after it has finished, to record metrics. |
| } |
| @@ -990,12 +1021,13 @@ TEST_F(PrecacheFetcherTest, MaxBytesTotal) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| } |
| // We don't know which will make it and which won't due to the parallelism in |
| @@ -1052,15 +1084,16 @@ TEST_F(PrecacheFetcherTest, FetcherPoolMaxLimitReached) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| EXPECT_GT(kNumResources, precache_fetcher.pool_.max_size()); |
| CheckUntilParallelFetchesBeyondCapacity(&precache_fetcher); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| // Destroy the PrecacheFetcher after it has finished, to record metrics. |
| } |
| @@ -1093,12 +1126,13 @@ TEST_F(PrecacheFetcherTest, FilterInvalidManifestUrls) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| } |
| // The config is fetched, but not the invalid manifest URL. |
| @@ -1132,12 +1166,13 @@ TEST_F(PrecacheFetcherTest, FilterInvalidResourceUrls) { |
| base::HistogramTester histogram; |
| { |
| - PrecacheFetcher precache_fetcher(request_context_.get(), GURL(), |
| - std::string(), std::move(unfinished_work), |
| - kExperimentID, &precache_delegate_); |
| + PrecacheFetcher precache_fetcher( |
| + request_context_.get(), GURL(), std::string(), |
| + std::move(unfinished_work), kExperimentID, |
| + precache_database_.GetWeakPtr(), task_runner(), &precache_delegate_); |
| precache_fetcher.Start(); |
| - loop_.RunUntilIdle(); |
| + base::RunLoop().RunUntilIdle(); |
| } |
| // The config and manifest are fetched, but not the invalid resource URL. |