An observer disconnected after a mark must receive the mark

[ferjm]

The following test from web-platform-tests/performance-timeline is failing:

async_test(function (t) {
    var observer = new PerformanceObserver(
        t.step_func(function(entryList, obs) {
          checkEntries(entryList.getEntries(),
            [{ entryType: "mark", name: "mark1"}]);
          t.done();
        })
      );
    observer.observe({entryTypes: ["mark"]});
    self.performance.mark("mark1");
    observer.disconnect();
    self.performance.mark("mark2");
  }, "An observer disconnected after a mark must receive the mark");

We should probably not remove the observer from the list while the pending_notification_observers_task flag is set.

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

[jdm]

Run the test with ./mach test-wpt tests/wpt/web-platform-tests/performance-timeline/po-disconnect.html

[pylbrecht]

I'd like to get my hands dirty on this. As I'm completely new to Servo and contributing at all, are there any resources where I can read up on performance observers, like what they are and what they have to do with the DOM? I guess this document from #18285 may be relevant?

[jdm]

That is the specification that defines the PerformanceObserver API, yes. https://developer.mozilla.org/en-US/docs/Web/API/Performance_Timeline may also be useful.

[ferjm]

Thank you for your interest on this task @pylbrecht :). You may want to work on top of #18283 as Performance.mark is required to test the fix for this issue.

[pylbrecht]

Alright, let's do this. Thanks for the hints!
@highfive: assign me

[highfive]

Hey @pylbrecht! Thanks for your interest in working on this issue. It's now assigned to you!

[pylbrecht]

Unfortunately I'm somehow unable to run the test.
I ran a debug build and executed the following commands (on top of #18283):

./mach test-wpt tests/wpt/web-platform-tests/performance-timeline/po-disconnect.html
This command confused me, because there is no po-disconnect.html file in that directory. But maybe I'm just not understanding how web-platform-tests are run.

./mach test-wpt tests/wpt/web-platform-tests/performance-timeline/
Here the related test is not failing.

Any ideas what I'm doing wrong?

[ferjm]

I am not sure how to run the test individually in this case. Running ./mach test-wpt tests/wpt/web-platform-tests/performance-timeline/po-disconnect.any.html gives me a similar output and it does not seem to run the test.

What I do is to run the entire suite for performance-timeline with the same command that you mentioned: ./mach test-wpt tests/wpt/web-platform-tests/performance-timeline/. Note that the output that you pasted isn't saying that the test didn't failed, what it is saying is that it run as expected. And in this case, the expectation is a timeout. What you can do is to s/TIMEOUT/PASS in line 3 and remove lines 4 and 5 from https://github.com/servo/servo/pull/18283/files#diff-385d3d27df317879daf4e2064605dd58 and then re-run the tests. You should see something like:

❯ ./mach test-wpt tests/wpt/web-platform-tests/performance-timeline/
Running 13 tests in web-platform-tests

  ▶ TIMEOUT [expected PASS] /performance-timeline/po-disconnect.any.html
  │
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  └ 3.3 (Core Profile) Mesa 17.2.0-devel

  ▶ Unexpected subtest result in /performance-timeline/po-disconnect.any.html:
  │ TIMEOUT [expected PASS] An observer disconnected after a mark must receive the mark
  └   → Test timed out

Ran 13 tests finished in 12.0 seconds.
  • 12 ran as expected. 0 tests skipped.
  • 1 tests timed out unexpectedly
  • 1 tests had unexpected subtest results

[pylbrecht]

Just to be clear: the test is expected to PASS after resolving this issue and is just expected to TIMEOUT right now, because the required module(s) are not fully implemented yet (i.e. test driven approach)?

[jdm]

Correct. The tests are shared between multiple web browsers, so other browsers end up writing tests for features that aren't yet implemented in Servo.