[ResourceTiming] Prefer PerformanceObserver over getEntriesByType in …

…WPTs

Older versions of the Resource Timing spec don't specify a strong enough
ordering constraint between an element's "onload" and the
PerformanceResourceTiming entry being added to the Performance Timeline.
As such, our strategy of calling "performance.getEntriesByType()" after
the "onload" event fires isn't guaranteed to return the expected entry.

This CL modifies our approach to use, and block on, a
PerformanceObserver being notified of entries being added to the
timeline. This eliminates a class of flakiness we were seeing during
manual testing.

Bug: 1171767
Change-Id: Ifa4995683f5ef4ff6e53f9780f6d485e6cfb3d81
GithubIssue: w3c/resource-timing#254
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2826597
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Tom McKee <tommckee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#872849}

resource-timing/cross-origin-redirects.html

@@ -19,7 +19,7 @@
 const destUrl = `/common/redirect.py?location=${REMOTE_ORIGIN}/${blank_page}`;
 
 attribute_test(
-  () => load.iframe(destUrl),
+  load.iframe, destUrl,
   invariants.assert_cross_origin_redirected_resource,
   "Verify that cross origin resources' timings aren't exposed when even as " +
   "targets of HTTP redirects.");

resource-timing/entry-attributes.html

@@ -10,7 +10,7 @@
 <script src="resources/entry-invariants.js"></script>
 <script>
 attribute_test(
-  () => load.image("resources/fake_responses.py#hash=1"),
+  load.image, "resources/fake_responses.py#hash=1",
   entry => {
     assert_true(entry.name.includes('#hash=1'),
       "There should be a hash in the resource name");
@@ -19,12 +19,12 @@
   "Image resources should generate conformant entries");
 
 attribute_test(
-  () => load.font("/fonts/Ahem.ttf"),
+  load.font, "/fonts/Ahem.ttf",
   invariants.assert_http_resource,
   "Font resources should generate conformant entries");
 
 attribute_test(
-  () => load.image("/common/redirect.py?location=resources/fake_responses.py"),
+  load.image, "/common/redirect.py?location=resources/fake_responses.py",
   invariants.assert_same_origin_redirected_resource,
   "Same-origin redirects should populate redirectStart/redirectEnd");
 </script>

resource-timing/redirects.html

@@ -17,33 +17,33 @@
 const https_url_prefix = `${redirect_url}?location=${HTTPS_NOTSAMESITE_ORIGIN}/resource-timing/resources/`;
 
 attribute_test(
-  () => load.stylesheet(url_prefix + "resource_timing_test0.css"),
+  load.stylesheet, url_prefix + "resource_timing_test0.css",
   invariants.assert_same_origin_redirected_resource,
   "Verify attributes of a redirected stylesheet's PerformanceResourceTiming");
 
 attribute_test(
-  () => load.image(url_prefix + "blue.png"),
+  load.image, url_prefix + "blue.png",
   invariants.assert_same_origin_redirected_resource,
   "Verify attributes of a redirected image's PerformanceResourceTiming");
 
 attribute_test(
-  () => load.iframe(url_prefix + "blank_page_green.html"),
+  load.iframe, url_prefix + "blank_page_green.html",
   invariants.assert_same_origin_redirected_resource,
   "Verify attributes of a redirected iframe's PerformanceResourceTiming");
 
 attribute_test(
-  () => load.script(url_prefix + "empty_script.js"),
+  load.script, url_prefix + "empty_script.js",
   invariants.assert_same_origin_redirected_resource,
   "Verify attributes of a redirected script's PerformanceResourceTiming");
 
 attribute_test(
-  () => load.xhr_sync(url_prefix + "blank_page_green.htm?id=xhr"),
+  load.xhr_sync, url_prefix + "blank_page_green.htm?id=xhr",
   invariants.assert_same_origin_redirected_resource,
   "Verify attributes of a redirected synchronous XMLHttpRequest's " +
   "PerformanceResourceTiming");
 
 attribute_test(
-  () => load.xhr_sync(https_url_prefix + "blank_page_green.htm?id=xhr"),
+  load.xhr_sync, https_url_prefix + "blank_page_green.htm?id=xhr",
   invariants.assert_cross_origin_redirected_resource,
   "Verify attributes of a synchronous XMLHttpRequest's " +
   "PerformanceResourceTiming where the initial HTTP request is redirected " +

resource-timing/resources/entry-invariants.js

@@ -141,24 +141,30 @@ const invariants = {
   },
 };
 
-// Given a resource-loader and a PerformanceResourceTiming validator, loads a
-// resource and validates the resulting entry.
-const attribute_test = (load_resource, validate, test_label) => {
+// Given a resource-loader, a path (a relative path or absolute URL), and a
+// PerformanceResourceTiming validator, applies the loader to the resource path
+// and applies the validator to the resulting PerformanceResourceTiming entry.
+const attribute_test = (loader, path, validate, test_label) => {
   promise_test(
     async () => {
-      // Clear out everything that isn't the one ResourceTiming entry under test.
-      performance.clearResourceTimings();
-
-      await load_resource();
-
-      const entry_list = performance.getEntriesByType("resource");
-      if (entry_list.length != 1) {
-        const names = entry_list
-          .map(e => e.name)
-          .join(", ");
-        throw new Error(`There should be one entry for one resource (found ${entry_list.length}: ${names})`);
-      }
-
-      validate(entry_list[0]);
+      let loaded_entry = new Promise((resolve, reject) => {
+        new PerformanceObserver((entry_list, self) => {
+          try {
+            const name_matches = entry_list.getEntries().forEach(entry => {
+              if (entry.name.includes(path)) {
+                resolve(entry);
+              }
+            });
+          } catch(e) {
+            // By surfacing exceptions through the Promise interface, tests can
+            // fail fast with a useful message instead of timing out.
+            reject(e);
+          }
+        }).observe({"type": "resource"});
+      });
+
+      await loader(path);
+      const entry = await(loaded_entry);
+      validate(entry);
   }, test_label);
 }

resource-timing/tao-origin-SO-XO-SO-redirect-chain.https.html

@@ -22,7 +22,7 @@
 destUrl += '&tao_value=' + SAME_ORIGIN;
 
 attribute_test(
-  () => load.image(destUrl),
+  load.image, destUrl,
   invariants.assert_cross_origin_redirected_resource,
   "Verify that cross origin resources' timings are not exposed when " +
   "same-origin=>cross-origin=>same-origin redirects have " +

resource-timing/tao-star-SO-XO-SO-redirect-chain.https.html

@@ -21,7 +21,7 @@
 destUrl += '&tao_steps=3';
 
 attribute_test(
-  () => load.image(destUrl),
+  load.image, destUrl,
   invariants.assert_tao_enabled_cross_origin_redirected_resource,
   "Verify that cross origin resources' timings are exposed when cross-origin " +
   "redirects have `Timing-Allow-Origin: *` headers");