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 27 matching lines @@
 const char kResourceFetchFailureURL[] = "http://resource-fetch-failure.com";
 const char kGoodResourceURL[] = "http://good-resource.com";
 const char kGoodResourceURLA[] = "http://good-resource.com/a";
 const char kGoodResourceURLB[] = "http://good-resource.com/b";
 const char kGoodResourceURLC[] = "http://good-resource.com/c";
 const char kGoodResourceURLD[] = "http://good-resource.com/d";
 const char kForcedStartingURLManifestURL[] =
     "http://manifest-url-prefix.com/forced-starting-url.com";
 const uint32_t kExperimentID = 123;
 
+}  // namespace
+
 class TestURLFetcherCallback {
  public:
   TestURLFetcherCallback() : total_response_bytes_(0) {}
 
   std::unique_ptr<net::FakeURLFetcher> CreateURLFetcher(
       const GURL& url,
       net::URLFetcherDelegate* delegate,
       const std::string& response_data,
       net::HttpStatusCode response_code,
       net::URLRequestStatus::Status status) {
@@ skipping 247 matching lines @@
 }
 
 class PrecacheFetcherTest : public testing::Test {
  public:
   PrecacheFetcherTest()
       : request_context_(new net::TestURLRequestContextGetter(
             base::ThreadTaskRunnerHandle::Get())),
         factory_(NULL,
                  base::Bind(&TestURLFetcherCallback::CreateURLFetcher,
                             base::Unretained(&url_callback_))),
-        expected_total_response_bytes_(0) {}
+        expected_total_response_bytes_(0),
+        parallel_fetches_beyond_capacity(false) {}
 
  protected:
   void SetDefaultFlags() {
     base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
         switches::kPrecacheConfigSettingsURL, kConfigURL);
     base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
         switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
   }
 
+  // Posts a task to check if more parallel fetches of precache manifest and
+  // resource URLs were attempted beyond the fetcher pool maximum defined
+  // capacity. The task will be posted repeatedly until such condition is met.
+  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()) {
+      parallel_fetches_beyond_capacity = true;
+      return;
+    }
+
+    // Check again after allowing the message loop to process some messages.
+    loop_.PostTask(
+        FROM_HERE,
+        base::Bind(
+            &PrecacheFetcherTest::CheckUntilParallelFetchesBeyondCapacity,
+            base::Unretained(this), precache_fetcher));
+  }
+
   base::MessageLoopForUI loop_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_;
   TestURLFetcherCallback url_callback_;
   net::FakeURLFetcherFactory factory_;
   TestPrecacheDelegate precache_delegate_;
   int expected_total_response_bytes_;
+
+  // True if more parallel fetches were attempted beyond the fetcher pool
+  // maximum capacity.
+  bool parallel_fetches_beyond_capacity;
 };
 
 TEST_F(PrecacheFetcherTest, FullPrecache) {
   SetDefaultFlags();
 
   std::unique_ptr<PrecacheUnfinishedWork> unfinished_work(
       new PrecacheUnfinishedWork());
   unfinished_work->set_start_time(base::Time::UnixEpoch().ToInternalValue());
   unfinished_work->add_top_host()->set_hostname("manifest-fetch-failure.com");
   unfinished_work->add_top_host()->set_hostname("bad-manifest.com");
@@ skipping 611 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);
 }
 
-}  // namespace
+// Tests the parallel fetch behaviour when more precache resource and manifest
+// requests are available than the maximum capacity of fetcher pool.
+TEST_F(PrecacheFetcherTest, FetcherPoolMaxLimitReached) {
+  SetDefaultFlags();
+
+  const size_t kNumTopHosts = 5;
+  const size_t kNumResources = GetDefaultMaxParallelFetches() + 1;
+
+  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();

[bengr, 2016/06/10 19:49:51]

I would change 1018 back to '= 15' and would make the pool capacity a member
method of PrecacheFetcher. Then here you'd do something like:
EXPECT_GT(kNumResources, precache_fetcher.GetMaximumParallelFetches());

size_t PrecacheFetcher::GetMaximumParallelFetches() const {
  return pool_.max_size();
}

[Raj, 2016/06/10 21:12:03]

Done.
Just added this test as a friend and accessing max_size_ directly.
Otherwise need to add two functions.
  GetMaximumParallelFetches() to PrecacheFetcher
  max_size() to FetcherPool

+
+    CheckUntilParallelFetchesBeyondCapacity(&precache_fetcher);
+
+    loop_.RunUntilIdle();
+
+    // Destroy the PrecacheFetcher after it has finished, to record metrics.
+  }
+
+  EXPECT_TRUE(parallel_fetches_beyond_capacity);
+
+  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 precache