Consolidate iframe & object resource timing code paths

So far some of the logic in resource timing for subframe navigations iframe/object/embed) was duplicated, e.g. both in blink and in content. This has led to race conditions, inconsistencies and sometimes XSS leaks. This patch attempts to improve the situation by consolidating the code paths: - NavigationRequest receives is_container_initiated, which ensures only container-initiated navigations are reported to the parent. This is a clarification of something that was ambiguous in the spec previously (whatwg/html#8846). It later uses ParentResourceTimingAccess to decide if a navigation should report to its parent with/without response details (status code and mime-type), or not report at all (TAO-fail, not an iframe, not container-initiated). - Both object fallbacks and cancelled navigations (204/205) report to the parent via RenderFrameImpl, and blink converts that to a ResourceTimingInfo object. This allows us to remove the duplicated resource timing creation code in //content. - We report fallback resource timing also for plugin error events and not only for load events. Bug: 1399862 Bug: 1410705 Change-Id: Id37d23cd02eee9e38f812e6f3da99caedafdee3d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4214695 Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org> Cr-Commit-Position: refs/heads/main@{#1110433}
web-platform-tests · Feb 27, 2023 · 6454474 · 6454474
1 parent b07ee81
commit 6454474
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 2 deletions.
diff --git a/resource-timing/entries-for-object-frame-options-deny.html b/resource-timing/entries-for-object-frame-options-deny.html
@@ -0,0 +1,35 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8" />
+<meta name="timeout" content="long">
+<link rel="author" title="Noam Rosenthal" href="noam@chromium.org">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/get-host-info.sub.js"></script>
+<script src="resources/entry-invariants.js"></script>
+</head>
+<body>
+<script>
+const {REMOTE_ORIGIN} = get_host_info();
+
+promise_test(async t => {
+  const success_url = new URL("/resource-timing/resources/object-frame-options-200.asis", REMOTE_ORIGIN).href;
+  const fail_url = new URL("/resource-timing/resources/object-frame-options-403.asis", REMOTE_ORIGIN).href;
+  const load_object = async url => {
+    const object = document.createElement("object");
+    object.data = url;
+    document.body.appendChild(object);
+    t.add_cleanup(() => object.remove());
+    await new Promise(resolve => {
+      object.onload = object.onerror = resolve;
+    });
+  };
+
+  await Promise.all([success_url, fail_url].map(load_object));
+  assert_equals(performance.getEntriesByName(success_url).length, 1);
+  assert_equals(performance.getEntriesByName(fail_url).length, 1);
+}, "Test that object elements with X-Frame-Options: Deny produce resource timing entries");
+</script>
+</body>
+</html>
diff --git a/resource-timing/iframe-sequence-of-events.html b/resource-timing/iframe-sequence-of-events.html
@@ -3,6 +3,7 @@
 <title>Test the sequence of events when reporting iframe timing.</title>
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
+<script src="resources/entry-invariants.js"></script>
 <script src="resources/frame-timing.js"></script>
 <script src="/common/utils.js"></script>
 <script src="/common/get-host-info.sub.js"></script>

diff --git a/resource-timing/resources/frame-timing.js b/resource-timing/resources/frame-timing.js
@@ -25,8 +25,8 @@ function test_frame_timing_before_load_event(type) {
 
 
 function test_frame_timing_change_src(type,
-                                      origin1 = document.origin,
-                                      origin2 = document.origin,
+                                      origin1 = location.origin,
+                                      origin2 = location.origin,
                                       tao = false, label = '') {
   const uid = token();
   promise_test(async t => {
@@ -59,5 +59,11 @@ function test_frame_timing_change_src(type,
 
     const entries = performance.getEntries().filter(e => e.name.includes(uid));
     assert_equals(entries.length, 2);
+    for (const entry of entries) {
+      if (tao || entry.name.startsWith(location.origin))
+        invariants.assert_tao_pass_no_redirect_http(entry);
+      else
+        invariants.assert_tao_failure_resource(entry);
+    }
   }, label || `A ${type} should report separate RT entries if its src changed from the outside`);
 }
diff --git a/resource-timing/resources/object-frame-options-200.asis b/resource-timing/resources/object-frame-options-200.asis
@@ -0,0 +1,6 @@
+HTTP/1.0 200 OK
+Content-Type: text/html
+X-Frame-Options: DENY
+Content-Security-Policy: frame-ancestors 'none'
+
+Hello
diff --git a/resource-timing/resources/object-frame-options-403.asis b/resource-timing/resources/object-frame-options-403.asis
@@ -0,0 +1,6 @@
+HTTP/1.0 403 OK
+Content-Type: text/html
+X-Frame-Options: DENY
+Content-Security-Policy: frame-ancestors 'none'
+
+Hello