[paint-timing] Fix test for non-automated contexts

[jugglinmike]

No description provided.

[foolip]

Can we change it to not change the DOM at all until the harness is complete?

[npm1]

Can you add a description on the problem? Is it the test harness producing text and making this test flaky?

[jugglinmike]

Can you add a description on the problem? Is it the test harness producing text and making this test flaky?

@npm1 That's correct. I've extended the inline comment to more completely describe what's going on.

Can we change it to not change the DOM at all until the harness is complete?

@foolip I assume you mean "complete" informally, as in "done changing the DOM," (versus "transitioned to the COMPLETE phase"). The trouble is the first-contentful-paint entry, and that will certainly be present if we explicitly pause for DOM operations to complete. I appreciate your thinking creatively about alternative solutions that aren't so complicated, and after some trial and error, I think I've found a better approach.

If we define this as a single-page test, the harness will not modify the DOM until after the test completes. Even though I'm generally opposed to the imprecision of single-page tests, I think it's preferable to the complexity of deeply nested iframes.

As far as I know, this testharness.js behavior is happenstance. There's risk in relying on it here because infrastructure maintainers could make a change that invalidates the test at any moment. It could be useful in other contexts, though, so we might be able to promote it as an explicit feature of the harness. That is to say: we could document it and test it. I'd be happy to do that, but first, do you folks agree that it's the direction we should take?

Here are some of the zany things I tried (sharing as a last-ditch attempt to legitimize the effort)

We could direct testharness.js to write to a dedicated iframe and then move its contents to the main document after testing is complete:

var iframe = document.createElement('iframe');
document.body.appendChild(iframe);
setup({output_document: iframe.contentWindow.document});
add_completion_callback(function() {
  var nodes = iframe.contentDocument.querySelectorAll('style, body > *');
  for (var idx = 0; idx < nodes.length; ++idx) {
    document.body.appendChild(nodes[idx]);
  }
  iframe.remove();
});

This is a little goofy, though, and it relies on the same semantics that this test is explicitly trying to verify (i.e. that the paint timing events don't propagate up from child windows).

We could intentionally trigger a "contentful" paint, capture the time with performance.now(), and finally verify that no newer entries are available at the end of the test. But I think that could hide failures. It changes the expectation from "no first-contentful-paint entries" to "no first-contentful-paint entries after a specific time," and browsers may have unrelated safeguards which prevent such entries of that type from appearing more than once.

[foolip]

@jugglinmike yes, I didn't have a precise condition in mind when I said this:

Can we change it to not change the DOM at all until the harness is complete?

Using a single-page test here seems for for the test itself, but it's a bit weird that it changes the outcome. Why does using a single-page test change when the DOM is modified? Could you add a test in infrastructure/ for what the precise thing this would depend on is? Even if we don't change it, that seems valuable to have.

[jugglinmike]

it's a bit weird that it changes the outcome. Why does using a single-page test change when the DOM is modified?

It's because for a single-page test, testharness.js has nothing to report prior to the final results. I think that's largely a vestige of the legacy "implicit opt-in" behavior, where the harness couldn't know whether it was waiting for subtests or already running a single-page test.

In master today:

• Event: initial parse, first test is being defined
  DOM:
  • test source (not "contentful")
• Event: message from iframe
  DOM:
  • test source (not "contentful")
  • iframe (from the test, not itself "contentful")
  • text: "Running, 0 complete, 1 remain" (from testharness.js, "contentful")

As a single-page test:

• Event: initial parse, first test is being defined
  DOM:
  • test source (not "contentful")
• Event: message from iframe
  DOM:
  • test source (not "contentful")
  • iframe (from the test, not itself "contentful")

Could you add a test in infrastructure/ for what the precise thing this would depend on is? Even if we don't change it, that seems valuable to have.

Agreed #20254

[npm1]

lgtm but is this problem unique to this test or do we also need to do this for any other test that checks FCP? If it is the latter then I'd say we want the testharness to stop meddling into the DOM of the test page before the test has finished running :)