Revert "[bfcache] Add tests for outstanding network requests"

This reverts commit 267f7f3a.

Reason for revert: The new tests seem to be flaky. See crbug.com/1010658 for error messages.

Original change's description:
> [bfcache] Add tests for outstanding network requests
> 
> It turns out that fetch() and XMLHttpRequests are now tracked
> as network requests.
> 
> This patch adds tests to verify this behaviour.
> 
> This patch also slightly improves tracing for active feature
> detection.
> 
> R=​kouhei@chromium.org,arthursonzogni@chromium.org
> 
> Change-Id: I368f3b52ba8930269bef284f98bd1831944633f8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825419
> Commit-Queue: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
> Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#701708}

TBR=kouhei@chromium.org,altimin@chromium.org,arthursonzogni@chromium.org
BUG=1010658

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I62b5cb17c4afc5fac114c9b634d4696404f72e3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836155Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
Commit-Queue: Jeffrey Yasskin <jyasskin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702226}

content/browser/frame_host/back_forward_cache_metrics_browsertest.cc

@@ -14,7 +14,6 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "content/shell/browser/shell.h"
 #include "net/dns/mock_host_resolver.h"
-#include "net/test/embedded_test_server/controllable_http_response.h"
 #include "services/device/public/cpp/test/scoped_geolocation_overrider.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -35,9 +34,6 @@ constexpr uint64_t kRequestedGeolocationPermissionFeature =
     static_cast<uint64_t>(blink::scheduler::WebSchedulerTrackedFeature::
                               kRequestedGeolocationPermission);
 
-constexpr uint64_t kOutstandingNetworkRequest = static_cast<uint64_t>(
-    blink::scheduler::WebSchedulerTrackedFeature::kOutstandingNetworkRequest);
-
 ukm::SourceId ToSourceId(int64_t navigation_id) {
   return ukm::ConvertToSourceId(navigation_id,
                                 ukm::SourceIdType::NAVIGATION_ID);
@@ -95,18 +91,11 @@ class BackForwardCacheMetricsBrowserTest : public ContentBrowserTest,
         std::make_unique<device::ScopedGeolocationOverrider>(1.0, 1.0);
   }
 
-  WebContentsImpl* web_contents() const {
-    return static_cast<WebContentsImpl*>(shell()->web_contents());
-  }
-
-  RenderFrameHostImpl* current_frame_host() const {
-    return web_contents()->GetFrameTree()->root()->current_frame_host();
-  }
-
  protected:
   void SetUpOnMainThread() override {
     host_resolver()->AddRule("*", "127.0.0.1");
     content::SetupCrossSiteRedirector(embedded_test_server());
+    ASSERT_TRUE(embedded_test_server()->Start());
     WebContentsObserver::Observe(shell()->web_contents());
   }
 
@@ -121,7 +110,6 @@ class BackForwardCacheMetricsBrowserTest : public ContentBrowserTest,
 };
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, UKM) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(
@@ -200,7 +188,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, UKM) {
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        NavigatedToTheMostRecentEntry) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(
@@ -248,7 +235,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, CloneAndGoBack) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL("/title1.html"));
@@ -357,7 +343,6 @@ std::vector<FeatureUsage> GetFeatureUsageMetrics(
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, Features_MainFrame) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL(
@@ -385,7 +370,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, Features_MainFrame) {
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        Features_MainFrame_CrossOriginNavigation) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL(
@@ -413,7 +397,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        Features_SameOriginSubframes) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL(
@@ -442,7 +425,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        Features_SameOriginSubframes_CrossOriginNavigation) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL(
@@ -471,7 +453,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        Features_CrossOriginSubframes) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL(
@@ -500,7 +481,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, DedicatedWorker) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url(embedded_test_server()->GetURL(
@@ -524,8 +504,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, DedicatedWorker) {
 #define MAYBE_SharedWorker SharedWorker
 #endif
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, MAYBE_SharedWorker) {
-  ASSERT_TRUE(embedded_test_server()->Start());
-
   const GURL url(embedded_test_server()->GetURL(
       "/back_forward_cache/page_with_shared_worker.html"));
 
@@ -541,7 +519,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, MAYBE_SharedWorker) {
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        WindowOpen_SameOrigin) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL("/title1.html"));
@@ -576,7 +553,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        WindowOpen_CrossOrigin) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL("/title1.html"));
@@ -616,7 +592,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
                        WindowOpen_SameOrigin_Openee) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   ukm::TestAutoSetUkmRecorder recorder;
 
   const GURL url1(embedded_test_server()->GetURL("/title1.html"));
@@ -655,7 +630,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, Geolocation) {
-  ASSERT_TRUE(embedded_test_server()->Start());
   const GURL url1(embedded_test_server()->GetURL("/title1.html"));
   EXPECT_TRUE(NavigateToURL(shell(), url1));
 
@@ -672,70 +646,4 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, Geolocation) {
               (1 << kRequestedGeolocationPermissionFeature));
 }
 
-IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, Fetch) {
-  net::test_server::ControllableHttpResponse response(embedded_test_server(),
-                                                      "/fetch");
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  const GURL url(embedded_test_server()->GetURL("/title1.html"));
-  EXPECT_TRUE(NavigateToURL(shell(), url));
-
-  RenderFrameHostImpl* main_frame = current_frame_host();
-  // Ensure that there are no lingering requests from page load itself.
-  EXPECT_FALSE(main_frame->scheduler_tracked_features() &
-               (1ull << kOutstandingNetworkRequest));
-
-  EXPECT_TRUE(ExecJs(main_frame, "fetch('/fetch');"));
-  response.WaitForRequest();
-
-  // Ensure that we are tracking fetch() as a network request here.
-  EXPECT_TRUE(main_frame->scheduler_tracked_features() &
-              (1ull << kOutstandingNetworkRequest));
-}
-
-IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest, XHR) {
-  net::test_server::ControllableHttpResponse response(embedded_test_server(),
-                                                      "/xhr");
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  const GURL url(embedded_test_server()->GetURL("/title1.html"));
-  EXPECT_TRUE(NavigateToURL(shell(), url));
-
-  RenderFrameHostImpl* main_frame = current_frame_host();
-  // Ensure that there are no lingering requests from page load itself.
-  EXPECT_FALSE(main_frame->scheduler_tracked_features() &
-               (1ull << kOutstandingNetworkRequest));
-
-  EXPECT_TRUE(ExecJs(main_frame, R"(
-    var req = new XMLHttpRequest();
-    req.open("GET", "/xhr");
-    req.send();
-  )"));
-  response.WaitForRequest();
-
-  // Ensure that we are tracking XHR as a network request here.
-  EXPECT_TRUE(main_frame->scheduler_tracked_features() &
-              (1ull << kOutstandingNetworkRequest));
-}
-
-IN_PROC_BROWSER_TEST_F(BackForwardCacheMetricsBrowserTest,
-                       NetworkRequest_Script) {
-  net::test_server::ControllableHttpResponse response(
-      embedded_test_server(),
-      "/back_forward_cache/script-which-does-not-exist.js");
-  ASSERT_TRUE(embedded_test_server()->Start());
-
-  const GURL url(embedded_test_server()->GetURL(
-      "/back_forward_cache/page_with_nonexistent_script.html"));
-  shell()->LoadURL(url);
-
-  RenderFrameHostImpl* main_frame = current_frame_host();
-  response.WaitForRequest();
-
-  // Ensure that we are tracking subresource (in this case, a script as a
-  // network request).
-  EXPECT_TRUE(main_frame->scheduler_tracked_features() &
-              (1ull << kOutstandingNetworkRequest));
-}
-
 }  // namespace content

content/test/data/back_forward_cache/page_with_nonexistent_script.html

-<script src="script-which-does-not-exist.js"/>

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc

@@ -10,7 +10,6 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/trace_event/blame_context.h"
 #include "third_party/blink/public/common/features.h"
-#include "third_party/blink/public/common/scheduler/web_scheduler_tracked_feature.h"
 #include "third_party/blink/public/platform/blame_context.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
@@ -639,12 +638,6 @@ WebScopedVirtualTimePauser FrameSchedulerImpl::CreateWebScopedVirtualTimePauser(
 void FrameSchedulerImpl::ResetForNavigation() {
   document_bound_weak_factory_.InvalidateWeakPtrs();
 
-  for (const auto& it : back_forward_cache_opt_out_counts_) {
-    TRACE_EVENT_ASYNC_END0(
-        "renderer.scheduler", "ActiveSchedulerTrackedFeature",
-        reinterpret_cast<intptr_t>(this) ^ static_cast<int>(it.first));
-  }
-
   back_forward_cache_opt_out_counts_.clear();
   back_forward_cache_opt_outs_.reset();
   last_uploaded_active_features_ = 0;
@@ -657,19 +650,13 @@ void FrameSchedulerImpl::OnStartedUsingFeature(
 
   if (policy.disable_aggressive_throttling)
     OnAddedAggressiveThrottlingOptOut();
-  if (policy.disable_back_forward_cache) {
+  if (policy.disable_back_forward_cache)
     OnAddedBackForwardCacheOptOut(feature);
-  }
 
   uint64_t new_mask = GetActiveFeaturesTrackedForBackForwardCacheMetricsMask();
 
-  if (old_mask != new_mask) {
+  if (old_mask != new_mask)
     NotifyDelegateAboutFeaturesAfterCurrentTask();
-    TRACE_EVENT_ASYNC_BEGIN1(
-        "renderer.scheduler", "ActiveSchedulerTrackedFeature",
-        reinterpret_cast<intptr_t>(this) ^ static_cast<int>(feature), "feature",
-        FeatureToString(feature));
-  }
 }
 
 void FrameSchedulerImpl::OnStoppedUsingFeature(
@@ -684,12 +671,8 @@ void FrameSchedulerImpl::OnStoppedUsingFeature(
 
   uint64_t new_mask = GetActiveFeaturesTrackedForBackForwardCacheMetricsMask();
 
-  if (old_mask != new_mask) {
+  if (old_mask != new_mask)
     NotifyDelegateAboutFeaturesAfterCurrentTask();
-    TRACE_EVENT_ASYNC_END0(
-        "renderer.scheduler", "ActiveSchedulerTrackedFeature",
-        reinterpret_cast<intptr_t>(this) ^ static_cast<int>(feature));
-  }
 }
 
 void FrameSchedulerImpl::NotifyDelegateAboutFeaturesAfterCurrentTask() {