Remove some prefetch experimentation code

It also looks like we can remove the ShouldBeginRequest from
RDHD, which I've done here.

TBR=pasko@chromium.org

Bug: 841473
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I238fe589cc192ccec47242350e6bc847f8da65cc
Reviewed-on: https://chromium-review.googlesource.com/1052889Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558333}

chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc

@@ -13,7 +13,6 @@
 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
-#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
@@ -365,32 +364,6 @@ ChromeResourceDispatcherHostDelegate::~ChromeResourceDispatcherHostDelegate() {
 #endif
 }
 
-bool ChromeResourceDispatcherHostDelegate::ShouldBeginRequest(
-    const std::string& method,
-    const GURL& url,
-    ResourceType resource_type,
-    content::ResourceContext* resource_context) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  // Handle a PREFETCH resource type. If prefetch is disabled, squelch the
-  // request.  Otherwise, do a normal request to warm the cache.
-  if (resource_type == content::RESOURCE_TYPE_PREFETCH) {
-    // All PREFETCH requests should be GETs, but be defensive about it.
-    if (method != "GET")
-      return false;
-
-    // If prefetch is disabled, kill the request.
-    std::string prefetch_experiment =
-        base::FieldTrialList::FindFullName("Prefetch");
-    if (base::StartsWith(prefetch_experiment, "ExperimentDisable",
-                         base::CompareCase::INSENSITIVE_ASCII)) {
-      return false;
-    }
-  }
-
-  return true;
-}
-
 void ChromeResourceDispatcherHostDelegate::RequestBeginning(
     net::URLRequest* request,
     content::ResourceContext* resource_context,

chrome/browser/loader/chrome_resource_dispatcher_host_delegate.h

@@ -49,10 +49,6 @@ class ChromeResourceDispatcherHostDelegate
   ~ChromeResourceDispatcherHostDelegate() override;
 
   // ResourceDispatcherHostDelegate implementation.
-  bool ShouldBeginRequest(const std::string& method,
-                          const GURL& url,
-                          content::ResourceType resource_type,
-                          content::ResourceContext* resource_context) override;
   void RequestBeginning(net::URLRequest* request,
                         content::ResourceContext* resource_context,
                         content::AppCacheService* appcache_service,

chrome/browser/prefetch/prefetch_browsertest.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "base/base_switches.h"
-#include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/net/prediction_options.h"
 #include "chrome/browser/profiles/profile.h"
@@ -40,17 +39,9 @@ class MockNetworkChangeNotifier4G : public NetworkChangeNotifier {
   }
 };
 
-class PrefetchBrowserTestBase : public InProcessBrowserTest {
+class PrefetchBrowserTest : public InProcessBrowserTest {
  public:
-  explicit PrefetchBrowserTestBase(bool disabled_via_field_trial)
-      : disabled_via_field_trial_(disabled_via_field_trial) {}
-
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    if (disabled_via_field_trial_) {
-      command_line->AppendSwitchASCII(switches::kForceFieldTrials,
-                                      "Prefetch/ExperimentDisabled/");
-    }
-  }
+  PrefetchBrowserTest() {}
 
   void SetUpOnMainThread() override {
     ASSERT_TRUE(embedded_test_server()->Start());
@@ -72,33 +63,11 @@ class PrefetchBrowserTestBase : public InProcessBrowserTest {
     ui_test_utils::NavigateToURL(browser, url);
     return expected_title == title_watcher.WaitAndGetTitle();
   }
-
- private:
-  bool disabled_via_field_trial_;
-};
-
-class PrefetchBrowserTestPrediction : public PrefetchBrowserTestBase {
- public:
-  PrefetchBrowserTestPrediction() : PrefetchBrowserTestBase(false) {}
 };
 
-class PrefetchBrowserTestPredictionDisabled : public PrefetchBrowserTestBase {
- public:
-  PrefetchBrowserTestPredictionDisabled() : PrefetchBrowserTestBase(true) {}
-};
-
-// Prefetch is disabled via field experiment.  Prefetch should be dropped.
-IN_PROC_BROWSER_TEST_F(PrefetchBrowserTestPredictionDisabled,
-                       ExperimentDisabled) {
-  EXPECT_TRUE(RunPrefetchExperiment(false, browser()));
-  // Should not prefetch even if preference is WIFI_ONLY.
-  SetPreference(NetworkPredictionOptions::NETWORK_PREDICTION_WIFI_ONLY);
-  EXPECT_TRUE(RunPrefetchExperiment(false, browser()));
-}
-
 // When initiated from the renderer, prefetch should be allowed regardless of
 // the network type.
-IN_PROC_BROWSER_TEST_F(PrefetchBrowserTestPrediction, PreferenceWorks) {
+IN_PROC_BROWSER_TEST_F(PrefetchBrowserTest, PreferenceWorks) {
   // Set real NetworkChangeNotifier singleton aside.
   std::unique_ptr<NetworkChangeNotifier::DisableForTest> disable_for_test(
       new NetworkChangeNotifier::DisableForTest);
@@ -131,7 +100,7 @@ IN_PROC_BROWSER_TEST_F(PrefetchBrowserTestPrediction, PreferenceWorks) {
 
 // Bug 339909: When in incognito mode the browser crashed due to an
 // uninitialized preference member. Verify that it no longer does.
-IN_PROC_BROWSER_TEST_F(PrefetchBrowserTestPrediction, IncognitoTest) {
+IN_PROC_BROWSER_TEST_F(PrefetchBrowserTest, IncognitoTest) {
   Profile* incognito_profile = browser()->profile()->GetOffTheRecordProfile();
   Browser* incognito_browser =
       new Browser(Browser::CreateParams(incognito_profile, true));

content/browser/browser_side_navigation_browsertest.cc

@@ -18,7 +18,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_types.h"
-#include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_features.h"
@@ -44,22 +43,6 @@
 
 namespace content {
 
-namespace {
-
-class RequestBlockingResourceDispatcherHostDelegate
-    : public ResourceDispatcherHostDelegate {
- public:
-  // ResourceDispatcherHostDelegate implementation:
-  bool ShouldBeginRequest(const std::string& method,
-                          const GURL& url,
-                          ResourceType resource_type,
-                          ResourceContext* resource_context) override {
-    return false;
-  }
-};
-
-}  // namespace
-
 // Test with BrowserSideNavigation enabled (aka PlzNavigate).
 // If you don't need a custom embedded test server, please use the next class
 // below (BrowserSideNavigationBrowserTest), it will automatically start the
@@ -719,41 +702,4 @@ IN_PROC_BROWSER_TEST_F(BrowserSideNavigationBrowserTest,
   }
 }
 
-// Requests not allowed by a ResourceDispatcherHostDelegate must be aborted.
-IN_PROC_BROWSER_TEST_F(BrowserSideNavigationBrowserTest,
-                       RequestBlockedByResourceDispatcherHostDelegate) {
-  // The Network Service doesn't use a ResourceDispatcherHost. A request can't
-  // be canceled by a ResourceDispatcherHostDelegate.
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
-    return;
-
-  // Add a ResourceDispatcherHost blocking every requests.
-  RequestBlockingResourceDispatcherHostDelegate delegate;
-  base::RunLoop loop;
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::BindOnce(
-          [](base::OnceClosure resume,
-             ResourceDispatcherHostDelegate* delegate) {
-            ResourceDispatcherHost::Get()->SetDelegate(delegate);
-            std::move(resume).Run();
-          },
-          loop.QuitClosure(), &delegate));
-  loop.Run();
-
-  // Navigate somewhere. The navigation will be aborted.
-  GURL simple_url(embedded_test_server()->GetURL("/simple_page.html"));
-  TestNavigationManager manager(shell()->web_contents(), simple_url);
-  NavigationHandleObserver handle_observer(shell()->web_contents(), simple_url);
-  shell()->LoadURL(simple_url);
-
-  EXPECT_TRUE(manager.WaitForRequestStart());
-  EXPECT_FALSE(manager.WaitForResponse());
-  manager.WaitForNavigationFinished();
-
-  EXPECT_FALSE(handle_observer.has_committed());
-  EXPECT_TRUE(handle_observer.is_error());
-  EXPECT_EQ(net::ERR_ABORTED, handle_observer.net_error_code());
-}
-
 }  // namespace content

content/browser/loader/mojo_async_resource_handler_unittest.cc

@@ -105,14 +105,6 @@ class TestResourceDispatcherHostDelegate final
   TestResourceDispatcherHostDelegate() = default;
   ~TestResourceDispatcherHostDelegate() override = default;
 
-  bool ShouldBeginRequest(const std::string& method,
-                          const GURL& url,
-                          ResourceType resource_type,
-                          ResourceContext* resource_context) override {
-    ADD_FAILURE() << "ShouldBeginRequest should not be called.";
-    return false;
-  }
-
   void RequestBeginning(
       net::URLRequest* request,
       ResourceContext* resource_context,

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -921,11 +921,9 @@ void ResourceDispatcherHostImpl::ContinuePendingBeginRequest(
       static_cast<ResourceType>(request_data.resource_type), &resource_context,
       &request_context);
 
-  // Allow the observer to block/handle the request.
-  if (delegate_ && !delegate_->ShouldBeginRequest(
-                       request_data.method, request_data.url,
-                       static_cast<ResourceType>(request_data.resource_type),
-                       resource_context)) {
+  // All PREFETCH requests should be GETs, but be defensive about it.
+  if (request_data.resource_type == RESOURCE_TYPE_PREFETCH &&
+      request_data.method != "GET") {
     AbortRequestBeforeItStarts(requester_info->filter(), request_id,
                                std::move(url_loader_client));
     return;
@@ -1590,10 +1588,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
       !policy->IsWebSafeScheme(info.common_params.url.scheme()) &&
       !is_external_protocol;
 
-  if (is_shutdown_ || non_web_url_in_guest ||
-      (delegate_ && !delegate_->ShouldBeginRequest(
-                        info.common_params.method, info.common_params.url,
-                        resource_type, resource_context))) {
+  if (is_shutdown_ || non_web_url_in_guest) {
     url_loader_client->OnComplete(
         network::URLLoaderCompletionStatus(net::ERR_ABORTED));
     return;

content/browser/loader/url_loader_factory_impl_unittest.cc

@@ -60,20 +60,6 @@ namespace {
 
 constexpr int kChildId = 99;
 
-class RejectingResourceDispatcherHostDelegate final
-    : public ResourceDispatcherHostDelegate {
- public:
-  RejectingResourceDispatcherHostDelegate() {}
-  bool ShouldBeginRequest(const std::string& method,
-                          const GURL& url,
-                          ResourceType resource_type,
-                          ResourceContext* resource_context) override {
-    return false;
-  }
-
-  DISALLOW_COPY_AND_ASSIGN(RejectingResourceDispatcherHostDelegate);
-};
-
 // The test parameter is the number of bytes allocated for the buffer in the
 // data pipe, for testing the case where the allocated size is smaller than the
 // size the mime sniffer *implicitly* requires.
@@ -309,11 +295,13 @@ TEST_P(URLLoaderFactoryImplTest, InvalidURL) {
 // This test tests a case where resource loading is cancelled before started.
 TEST_P(URLLoaderFactoryImplTest, ShouldNotRequestURL) {
   network::mojom::URLLoaderPtr loader;
-  RejectingResourceDispatcherHostDelegate rdh_delegate;
-  rdh_.SetDelegate(&rdh_delegate);
   network::ResourceRequest request;
   network::TestURLLoaderClient client;
-  request.url = GURL("http://localhost/");
+
+  // Child processes cannot request URLs with pseudo schemes like "about",
+  // except for about:blank. See ChildProcessSecurityPolicyImpl::CanRequestURL
+  // for details.
+  request.url = GURL("about:version");
   request.method = "GET";
   // |resource_type| can't be a frame type. It is because when PlzNavigate is
   // enabled, the url scheme of frame type requests from the renderer process
@@ -327,7 +315,6 @@ TEST_P(URLLoaderFactoryImplTest, ShouldNotRequestURL) {
       net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS));
 
   client.RunUntilComplete();
-  rdh_.SetDelegate(nullptr);
 
   ASSERT_FALSE(client.has_received_response());
   ASSERT_FALSE(client.response_body().is_valid());

content/public/browser/resource_dispatcher_host_delegate.cc

@@ -12,14 +12,6 @@ namespace content {
 
 ResourceDispatcherHostDelegate::~ResourceDispatcherHostDelegate() {}
 
-bool ResourceDispatcherHostDelegate::ShouldBeginRequest(
-    const std::string& method,
-    const GURL& url,
-    ResourceType resource_type,
-    ResourceContext* resource_context) {
-  return true;
-}
-
 void ResourceDispatcherHostDelegate::RequestBeginning(
     net::URLRequest* request,
     ResourceContext* resource_context,

content/public/browser/resource_dispatcher_host_delegate.h

@@ -37,12 +37,6 @@ class CONTENT_EXPORT ResourceDispatcherHostDelegate {
  public:
   virtual ~ResourceDispatcherHostDelegate();
 
-  // Called when a request begins. Return false to abort the request.
-  virtual bool ShouldBeginRequest(const std::string& method,
-                                  const GURL& url,
-                                  ResourceType resource_type,
-                                  ResourceContext* resource_context);
-
   // Called after ShouldBeginRequest to allow the embedder to add resource
   // throttles.
   virtual void RequestBeginning(

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -23,7 +23,6 @@
 -PKPModelClientTest.PKPEnforced/0
 -PKPModelClientTest.PKPEnforced/1
 -PolicyTest.DefaultCookiesSetting
--PrefetchBrowserTestPredictionDisabled.ExperimentDisabled
 -PreviewsOptimizationGuideBrowserTest.NoScriptPreviewsEnabledByWhitelist
 -ProcessManagerBrowserTest.NestedURLNavigationsToExtensionAllowed
 -ProfileWindowBrowserTest.GuestClearsCookies