Check http protocol when emitting resource timing entry

Currently when a resource load via file protocol fails, a resource
timing is emitted. We should only emit resource timing for resources
that are fetched over http(s) protocols or from web bundles.

This CL is to fix this issue. The  ResourceResponse::ShouldPopulateResourceTiming() method used for
checking protocol when load is successful checks if the
ResourceResponse::current_request_url_ is HTTP or the web bundle url
is valid.

However when the load fails, the ResourceResponse::current_request_url_
could be null, so we use ResourceRequest::url_, since we are only
checking for protocols.

Bug: 1472922
Change-Id: I674b2041f1911e9c6c04863dc38fc64a28be27af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4799574
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Hao Liu <haoliuk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1193611}

BUILD.gn

@@ -1364,6 +1364,7 @@ if (use_blink && !is_cronet_build) {
       "//third_party/blink/web_tests/overflow/",
       "//third_party/blink/web_tests/paint/",
       "//third_party/blink/web_tests/payments/",
+      "//third_party/blink/web_tests/performance_timeline/",
       "//third_party/blink/web_tests/permissionclient/",
       "//third_party/blink/web_tests/plugins/",
       "//third_party/blink/web_tests/pointer-lock/",

content/browser/performance_timeline_browsertest.cc

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/base_paths.h"
 #include "base/command_line.h"
+#include "base/path_service.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/content_navigation_policy.h"
 #include "content/public/common/content_switches.h"
@@ -45,6 +47,64 @@ class PerformanceTimelineBrowserTest : public ContentBrowserTest {
   }
 };
 
+IN_PROC_BROWSER_TEST_F(PerformanceTimelineBrowserTest,
+                       NoResourceTimingEntryForFileProtocol) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  base::FilePath file_path;
+  CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &file_path));
+
+  file_path = file_path.Append(GetTestDataFilePath())
+                  .AppendASCII(
+                      "performance_timeline/"
+                      "resource-timing-not-for-file-protocol.html");
+
+  EXPECT_TRUE(NavigateToURL(shell(), GetFileUrlWithQuery(file_path, "")));
+
+  // The test html page references 2 css file. One is present and would be
+  // loaded via file protocol and the other is not present and would have load
+  // failure. Both should not emit a resource timing entry.
+  EXPECT_EQ(
+      0, EvalJs(shell(),
+                "window.performance.getEntriesByType('resource').filter(e=>e."
+                "name.includes('css')).length;"));
+
+  std::string applied_style_color = "rgb(0, 128, 0)";
+
+  // Verify that style.css is fetched by verifying color green is applied.
+  EXPECT_EQ(applied_style_color, EvalJs(shell(), "getTextColor()"));
+
+  // If the same page is loaded via http protocol, both the successful load nad
+  // failure load should emit a resource timing entry.
+  const GURL url(embedded_test_server()->GetURL(
+      "a.com",
+      "/performance_timeline/"
+      "resource-timing-not-for-file-protocol.html"));
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  EXPECT_EQ(
+      2, EvalJs(shell(),
+                "window.performance.getEntriesByType('resource').filter(e=>e."
+                "name.includes('css')).length;"));
+
+  // Verify that style.css is fetched by verifying color green is applied.
+  EXPECT_EQ(applied_style_color, EvalJs(shell(), "getTextColor()"));
+
+  // Verify that style.css that is fetched has its resource timing entry.
+  EXPECT_EQ(
+      1, EvalJs(shell(),
+                "window.performance.getEntriesByType('resource').filter(e=>e."
+                "name.includes('resources/style.css')).length;"));
+
+  // Verify that non_exist.css that is not fetched has its resource timing
+  // entry.
+  EXPECT_EQ(
+      1, EvalJs(shell(),
+                "window.performance.getEntriesByType('resource').filter(e=>e."
+                "name.includes('resources/non_exist_style.css')).length;"));
+}
+
 class PerformanceTimelineLCPStartTimePrecisionBrowserTest
     : public PerformanceTimelineBrowserTest {
  protected:

content/test/content_test_bundle_data.filelist

@@ -6504,6 +6504,8 @@ data/payments/payment_app_window.html
 data/performance_timeline/cross-origin-non-tao-image.html
 data/performance_timeline/lcp-start-time-precision.html
 data/performance_timeline/prefetch.html
+data/performance_timeline/resource-timing-not-for-file-protocol.html
+data/performance_timeline/resources/style.css
 data/permissions-policy-main.html
 data/permissions-policy-main.html.mock-http-headers
 data/permissions-policy.xml

content/test/data/performance_timeline/resource-timing-not-for-file-protocol.html

@@ -0,0 +1,17 @@
+<!doctype html>
+
+<head>
+  <link rel="stylesheet" href="resources/style.css">
+  <link rel="stylesheet" href="resources/non_exist_style.css">
+</head>
+
+<body>
+  <p id="test">test</p>
+</body>
+<script>
+  const getTextColor = () => {
+    return getComputedStyle(
+      document.getElementById('test')).getPropertyValue('color');
+  }
+</script>
+</html>

content/test/data/performance_timeline/resources/style.css

@@ -0,0 +1,7 @@
+/* Copyright 2023 The Chromium Authors
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+p {
+  color: green;
+}

third_party/blink/renderer/core/loader/document_loader.cc

@@ -2710,6 +2710,9 @@ void DocumentLoader::CommitNavigation() {
       document_load_timing_.RedirectEnd();
 
   DCHECK(frame_->DomWindow());
+
+  // TODO(crbug.com/1476866): We should check for protocols and not emit
+  // performance timeline entries for file protocol navigations.
   DOMWindowPerformance::performance(*frame_->DomWindow())
       ->CreateNavigationTimingInstance(std::move(navigation_timing_info));
 

third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc

@@ -2351,7 +2351,13 @@ void ResourceFetcher::HandleLoaderError(Resource* resource,
   PendingResourceTimingInfo info = resource_timing_info_map_.Take(resource);
 
   if (!info.is_null()) {
-    PopulateAndAddResourceTimingInfo(resource, std::move(info), finish_time);
+    if (resource->GetResourceRequest().Url().ProtocolIsInHTTPFamily() ||
+        (resource->GetResourceRequest().GetWebBundleTokenParams() &&
+         resource->GetResourceRequest()
+             .GetWebBundleTokenParams()
+             ->bundle_url.IsValid())) {
+      PopulateAndAddResourceTimingInfo(resource, std::move(info), finish_time);
+    }
   }
 
   resource->VirtualTimePauser().UnpauseVirtualTime();

third_party/blink/web_tests/external/wpt/resource-timing/resource-timing-failed-fetch-web-bundle.tentative.html

@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<meta charset="utf-8" />
+<title>
+  Resource timing attributes are consistent for the same-origin subresources.
+</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/web-bundle/resources/test-helpers.js"></script>
+
+<body>
+  <script>
+    setup(() => {
+      assert_true(HTMLScriptElement.supports("webbundle"));
+    });
+
+    promise_test(async (t) => {
+      const script_url = 'dynamic1:resource1.js';
+      const non_exist_script_url = 'dynamic1:non_exist_resource.js';
+      // Using functions from /web-bundle/resources/test-helpers.js.
+      const element = createWebBundleElement(
+        "../web-bundle/resources/wbn/dynamic1.wbn",
+        /*resources=*/[script_url]
+      );
+
+      document.body.appendChild(element);
+
+      // Fetch the script that is in the web bundle.
+      const script = document.createElement("script");
+      script.type = "module";
+      script.src = script_url;
+      document.body.appendChild(script);
+
+      // Fetch the script that is not in the web bundle.
+      // Using functions from /web-bundle/resources/test-helpers.js.
+      await fetchAndWaitForReject(non_exist_script_url);
+
+      let resource1_entries = [];
+      let non_exist_resource_entries = [];
+
+      await new Promise((resolve) => {
+        new PerformanceObserver(list => {
+          resource1_entries.push(list.getEntries().filter(e => e.name.includes('resource1.js')));
+          non_exist_resource_entries.push(
+            list.getEntries().filter(e => e.name.includes('non_exist_resource.js')));
+          if (resource1_entries.length == 1 && non_exist_resource_entries.length == 1) {
+            resolve();
+          }
+        }
+        ).observe({ type: 'resource', buffered: true });
+      });
+    }, "Web bundle resources should have resource timing entries, even when the fetch failed.");
+  </script>
+</body>

third_party/blink/web_tests/external/wpt/resource-timing/resource-timing-failed-fetch.html

@@ -0,0 +1,34 @@
+<!doctype html>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<link rel="stylesheet" href="resources/empty_style.css">
+<link rel="stylesheet" href="resources/non_exist.css">
+
+<script>
+  promise_test(async () => {
+    const css_resource_timing_entries = await new Promise(resolve => {
+      new PerformanceObserver((list, observer) => {
+        let css_resource_timing_entries = list.getEntries().filter(e => e.name.includes('css'));
+        if (css_resource_timing_entries.length >= 2) {
+          resolve(css_resource_timing_entries);
+        }
+      }).observe({ 'type': 'resource', 'buffered': true });
+    });
+
+    assert_equals(css_resource_timing_entries.length, 2,
+      'There should be two resource timing entries for css resources');
+
+    assert_equals(css_resource_timing_entries.filter(
+      e => e.name.includes('empty_style.css')).length, 1,
+      'There should be one resource timing entry for successfully fetched resource.');
+
+    assert_equals(css_resource_timing_entries.filter(
+      e => e.name.includes('non_exist.css')).length, 1,
+      'There should be one resource timing entry for fetching failed resource.');
+
+  }, 'Resource fetched by HTTP protocol should have resource timing entry emitted, \
+  even when the fetch failed.');
+</script>
+
+</html>

third_party/blink/web_tests/performance_timeline/resource-timing-not-for-file-protocol.html

@@ -0,0 +1,43 @@
+<!doctype html>
+
+
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<link rel="stylesheet" href="resources/style.css">
+<link rel="stylesheet" href="resources/non_exist.css">
+
+<p id="test">text</p>
+<script>
+  promise_test(async () => {
+    // Wait until styles are applied so that the text should have color green
+    // as specified in the style.css file.
+    const checkStyle = async () => {
+      const element = document.getElementById('test');
+      return new Promise(resolve => {
+        const interval = setInterval(() => {
+          if (getComputedStyle(element).getPropertyValue('color')
+            == 'rgb(0, 128, 0)') {
+            clearInterval(interval);
+            resolve();
+          }
+        }, 10);
+      });
+    };
+
+    await checkStyle();
+
+    // At this point, we know the style.css file has been loaded as the styles are
+    // already applied. We then verify that no resource timing entries corresponding
+    // css files are emitted.
+    new PerformanceObserver((list) => {
+      if (list.getEntries().filter(e => e.name.includes('css')).length > 0) {
+        assert_unreached();
+      }
+    }).observe({ type: 'resource', buffered: true });
+
+    assert_equals(
+      performance.getEntriesByType('resource').filter(e => e.name.includes('css')).length, 0);
+
+  }, 'Resources fetched via file protocol should have no resource timing entries,\
+  regardless fetching success or failure');
+</script>

third_party/blink/web_tests/performance_timeline/resources/style.css

@@ -0,0 +1,7 @@
+/* Copyright 2023 The Chromium Authors
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+p {
+  color: green;
+}