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

components/precache/core/precache_fetcher_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/precache/core/precache_fetcher.h"
 
 #include <stdint.h>
 
 #include <cstring>
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ptr_util.h"
 #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"
 #include "components/precache/core/proto/precache.pb.h"
 #include "components/precache/core/proto/unfinished_work.pb.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_status.h"
@@ skipping 941 matching lines @@
   EXPECT_GT(kNumResources, url_callback_.requested_urls().size());
 
   EXPECT_TRUE(precache_delegate_.was_on_done_called());
 
   // good-manifest.com will not have been completed.
   EXPECT_THAT(histogram.GetAllSamples("Precache.Fetch.PercentCompleted"),
               ElementsAre(base::Bucket(0, 1)));
   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