Loading Predictor: Add Stop() to PrefetchManager.

Allows cancelling queued prefetch jobs, for parity with preconnect
manager.

Bug: 1092329
Change-Id: I1bb227fe9b0a0efe5abb7ec6279a5b8c91073818
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2255947
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781187}

chrome/browser/predictors/loading_predictor.cc

@@ -255,6 +255,8 @@ void LoadingPredictor::MaybeRemovePreconnect(const GURL& url) {
   DCHECK(!shutdown_);
   if (preconnect_manager_)
     preconnect_manager_->Stop(url);
+  if (prefetch_manager_)
+    prefetch_manager_->Stop(url);
 }
 
 void LoadingPredictor::HandleOmniboxHint(const GURL& url, bool preconnectable) {

chrome/browser/predictors/prefetch_manager.cc

@@ -132,6 +132,14 @@ void PrefetchManager::Start(const GURL& url,
   TryToLaunchPrefetchJobs();
 }
 
+void PrefetchManager::Stop(const GURL& url) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  auto it = prefetch_info_.find(url);
+  if (it == prefetch_info_.end())
+    return;
+  it->second->was_canceled = true;
+}
+
 void PrefetchManager::PrefetchUrl(std::unique_ptr<PrefetchJob> job,
                                   network::SharedURLLoaderFactory& factory) {
   DCHECK(job);
@@ -221,7 +229,7 @@ void PrefetchManager::TryToLaunchPrefetchJobs() {
     // |this| owns all infos.
     DCHECK(info);
 
-    if (job->url.is_valid() && factory)
+    if (job->url.is_valid() && factory && !info->was_canceled)
       PrefetchUrl(std::move(job), *factory);
   }
 }

chrome/browser/predictors/prefetch_manager.h

@@ -57,6 +57,7 @@ struct PrefetchInfo {
 
   GURL url;
   size_t job_count = 0;
+  bool was_canceled = false;
   std::unique_ptr<PrefetchStats> stats;
   // Owns |this|.
   PrefetchManager* const manager;
@@ -109,8 +110,9 @@ class PrefetchManager {
   // Starts prefetch jobs keyed by |url|.
   void Start(const GURL& url, std::vector<PrefetchRequest> requests);
 
-  // TODO(falken): Implement Stop().
-  // void Stop(const GURL& url);
+  // Stops further prefetch jobs keyed by |url|. Queued jobs will never start;
+  // started jobs will continue to completion.
+  void Stop(const GURL& url);
 
   base::WeakPtr<PrefetchManager> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();

chrome/browser/predictors/prefetch_manager_unittest.cc

@@ -260,4 +260,165 @@ TEST_F(PrefetchManagerTest, MultipleMainFrameUrlMultiplePrefetch) {
   fake_delegate_->WaitForPrefetchFinished(main_frame_url2);
 }
 
+TEST_F(PrefetchManagerTest, Stop) {
+  net::test_server::EmbeddedTestServer test_server;
+
+  // Set up prefetches (limit + 1 for URL1, and 1 for URL2)
+  size_t limit = PrefetchManager::kMaxInflightJobs;
+
+  GURL main_frame_url("https://abc.invalid");
+  std::vector<std::string> paths;
+  std::vector<PrefetchRequest> requests;
+  std::vector<std::unique_ptr<net::test_server::ControllableHttpResponse>>
+      responses;
+
+  GURL main_frame_url2("https://def.invalid");
+  std::string path2;
+  std::unique_ptr<net::test_server::ControllableHttpResponse> response2;
+
+  // The ControllableHttpResponses must be made before the test server
+  // is started.
+  for (size_t i = 0; i < limit; i++) {
+    std::string path = base::StringPrintf("/script%" PRIuS ".js", i);
+    paths.push_back(path);
+    responses.push_back(
+        std::make_unique<net::test_server::ControllableHttpResponse>(
+            &test_server, path));
+  }
+
+  path2 = base::StringPrintf("/script%" PRIuS ".js", limit);
+  response2 = std::make_unique<net::test_server::ControllableHttpResponse>(
+      &test_server, path2);
+
+  // Verify we don't see a request after Stop().
+  test_server.RegisterRequestMonitor(base::BindLambdaForTesting(
+      [&](const net::test_server::HttpRequest& request) {
+        EXPECT_NE(request.relative_url, "/should_be_cancelled");
+      }));
+
+  // Start the server.
+  auto test_server_handle = test_server.StartAndReturnHandle();
+  ASSERT_TRUE(test_server_handle);
+
+  // The request URLs can only be constructed after the server is started.
+  for (size_t i = 0; i < limit; i++) {
+    GURL url = test_server.GetURL(paths[i]);
+    requests.emplace_back(url, CreateNetworkIsolationKey(main_frame_url));
+  }
+  // This request should never be seen.
+  requests.emplace_back(test_server.GetURL("/should_be_cancelled"),
+                        CreateNetworkIsolationKey(main_frame_url));
+
+  // The request from the second navigation.
+  PrefetchRequest request2(test_server.GetURL(path2),
+                           CreateNetworkIsolationKey(main_frame_url2));
+
+  // Start URL1, URL2.
+  prefetch_manager_->Start(main_frame_url, requests);
+  prefetch_manager_->Start(main_frame_url2, {request2});
+
+  // Wait for |limit| requests from URL1.
+  for (auto& response : responses)
+    response->WaitForRequest();
+
+  // Call stop on URL1.
+  prefetch_manager_->Stop(main_frame_url);
+
+  // Let URL1 requests finish. This finishes URL1 without
+  // the limit + 1 request being sent.
+  for (auto& response : responses) {
+    response->Send("hi");
+    response->Done();
+  }
+  fake_delegate_->WaitForPrefetchFinished(main_frame_url);
+
+  // The request for URL2 should be requested.
+  response2->WaitForRequest();
+  response2->Send("hi");
+  response2->Done();
+
+  fake_delegate_->WaitForPrefetchFinished(main_frame_url2);
+}
+
+TEST_F(PrefetchManagerTest, StopAndStart) {
+  net::test_server::EmbeddedTestServer test_server;
+
+  // Set up prefetches (limit + 1).
+  size_t limit = PrefetchManager::kMaxInflightJobs;
+
+  GURL main_frame_url("https://abc.invalid");
+  std::vector<std::string> paths;
+  std::vector<PrefetchRequest> requests;
+  std::vector<std::unique_ptr<net::test_server::ControllableHttpResponse>>
+      responses;
+  std::vector<std::unique_ptr<net::test_server::ControllableHttpResponse>>
+      responses2;
+
+  // The ControllableHttpResponses must be made before the test server
+  // is started.
+  for (size_t i = 0; i < limit; i++) {
+    std::string path = base::StringPrintf("/script%" PRIuS ".js", i);
+    paths.push_back(path);
+    responses.push_back(
+        std::make_unique<net::test_server::ControllableHttpResponse>(
+            &test_server, path));
+    responses2.push_back(
+        std::make_unique<net::test_server::ControllableHttpResponse>(
+            &test_server, path));
+  }
+
+  // Verify we don't see a request after Stop().
+  test_server.RegisterRequestMonitor(base::BindLambdaForTesting(
+      [&](const net::test_server::HttpRequest& request) {
+        EXPECT_NE(request.relative_url, "/should_be_cancelled");
+      }));
+
+  // Start the server.
+  auto test_server_handle = test_server.StartAndReturnHandle();
+  ASSERT_TRUE(test_server_handle);
+
+  // The request URLs can only be constructed after the server is started.
+  for (size_t i = 0; i < limit; i++) {
+    GURL url = test_server.GetURL(paths[i]);
+    requests.emplace_back(url, CreateNetworkIsolationKey(main_frame_url));
+  }
+  // This request should never be seen.
+  requests.emplace_back(test_server.GetURL("/should_be_cancelled"),
+                        CreateNetworkIsolationKey(main_frame_url));
+
+  // Start.
+  prefetch_manager_->Start(main_frame_url, requests);
+
+  // Wait for |limit| requests from URL1.
+  for (auto& response : responses) {
+    response->WaitForRequest();
+  }
+
+  // Call stop.
+  prefetch_manager_->Stop(main_frame_url);
+
+  // Call start again. These requests will be coalesced
+  // with the stopped info, and will just be dropped.
+  prefetch_manager_->Start(main_frame_url, requests);
+
+  // Let the inflight requests finish. This finishes the
+  // info without the limit + 1 request or requests
+  // added after Stop() being sent.
+  for (auto& response : responses) {
+    response->Send("hi");
+    response->Done();
+  }
+  fake_delegate_->WaitForPrefetchFinished(main_frame_url);
+
+  // Restart requests. These requests will work as normal.
+  prefetch_manager_->Start(main_frame_url, requests);
+  for (auto& response : responses2) {
+    response->WaitForRequest();
+    response->Send("hi");
+    response->Done();
+  }
+
+  fake_delegate_->WaitForPrefetchFinished(main_frame_url);
+}
+
 }  // namespace predictors