Precache manifest should not be added to fetcher pool which is full

components/precache/core/precache_fetcher_unittest.cc

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 ccb1c7548a3ead4fa726aacfcca49080b2a0f730..7a2d1b2b085297bcd43fba5a6648f64b5c993bd6 100644
--- a/components/precache/core/precache_fetcher_unittest.cc
+++ b/components/precache/core/precache_fetcher_unittest.cc
@@ -20,6 +20,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
 #include "base/test/histogram_tester.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/precache/core/precache_switches.h"
@@ -981,6 +982,68 @@ TEST_F(PrecacheFetcherTest, MaxBytesTotal) {
   histogram.ExpectTotalCount("Precache.Fetch.TimeToComplete", 1);
 }
 
+// Tests the parallel fetch behaviour when more precache resource and manifest
+// requests are available than the maximum capacity of fetcher pool.
+TEST_F(PrecacheFetcherTest, ParallelFetches) {
+  SetDefaultFlags();
+
+  const size_t kNumTopHosts = 5;
+  const size_t kNumResources = 15;

[bengr, 2016/06/07 21:14:40]

This test should check the precondition that this exceeds the capacity of the
pool.

[Raj, 2016/06/08 03:10:42]

Done.
This test fails if the DCHECK was not removed.

https://cs.chromium.org/chromium/src/components/precache/core/precache_fetche...

[bengr, 2016/06/08 23:49:56]

That's not what I mean. If someone changes the pool capacity to be greater than
20, this test would pass even if the DCHECK were added back in, so you should
verify that the number of resources/hosts you are using in the test is greater
than the pool size.

[Raj, 2016/06/09 00:32:46]

Actually, this test will fail if pool capacity is increased to 20.
Since it checks for IsAvailable() to be false at some point during the fetches.

Though, I have added code to get the max pool size and using it for
kNumResources here.

+
+  PrecacheConfigurationSettings config;
+  PrecacheManifest top_host_manifest[kNumTopHosts];
+  std::multiset<GURL> expected_requested_urls;
+
+  config.set_top_sites_count(kNumTopHosts);
+  factory_.SetFakeResponse(GURL(kConfigURL), config.SerializeAsString(),
+                           net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+  expected_requested_urls.insert(GURL(kConfigURL));
+
+  std::unique_ptr<PrecacheUnfinishedWork> unfinished_work(
+      new PrecacheUnfinishedWork());
+  unfinished_work->set_start_time(base::Time::UnixEpoch().ToInternalValue());
+
+  for (size_t i = 0; i < kNumTopHosts; ++i) {
+    const std::string top_host_url = base::StringPrintf("top-host-%d.com", i);
+    unfinished_work->add_top_host()->set_hostname(top_host_url);
+
+    for (size_t j = 0; j < kNumResources; ++j) {
+      const std::string resource_url =
+          base::StringPrintf("http://top-host-%d.com/resource-%d", i, j);
+      top_host_manifest[i].add_resource()->set_url(resource_url);
+      factory_.SetFakeResponse(GURL(resource_url), "good", net::HTTP_OK,
+                               net::URLRequestStatus::SUCCESS);
+      expected_requested_urls.insert(GURL(resource_url));
+    }
+    factory_.SetFakeResponse(GURL(kManifestURLPrefix + top_host_url),
+                             top_host_manifest[i].SerializeAsString(),
+                             net::HTTP_OK, net::URLRequestStatus::SUCCESS);
+    expected_requested_urls.insert(GURL(kManifestURLPrefix + top_host_url));
+  }
+
+  base::HistogramTester histogram;
+
+  {
+    PrecacheFetcher precache_fetcher(request_context_.get(), GURL(),
+                                     std::string(), std::move(unfinished_work),
+                                     kExperimentID, &precache_delegate_);
+    precache_fetcher.Start();
+
+    loop_.RunUntilIdle();
+
+    // Destroy the PrecacheFetcher after it has finished, to record metrics.
+  }
+
+  EXPECT_EQ(expected_requested_urls, url_callback_.requested_urls());
+
+  EXPECT_TRUE(precache_delegate_.was_on_done_called());
+
+  histogram.ExpectUniqueSample("Precache.Fetch.PercentCompleted", 100, 1);
+  histogram.ExpectUniqueSample("Precache.Fetch.ResponseBytes.Total",
+                               url_callback_.total_response_bytes(), 1);
+  histogram.ExpectTotalCount("Precache.Fetch.TimeToComplete", 1);
+}
+
 }  // namespace
 
 }  // namespace precache