w3c / requestidlecallback

Cooperative Scheduling of Background Tasks
https://w3c.github.io/requestidlecallback/
Other
50 stars 19 forks source link

The order of idle callbacks across windows is inconsistent between spec & UAs #82

Closed rniwa closed 2 years ago

rniwa commented 4 years ago

Consider the following example where requestIdleCallback is requested in two different windows:

requestIdleCallbackIsCalled = false;
const iframe = document.createElement('iframe');
const logs = [];

iframe.onload = () => {
    requestIdleCallback(() => {
        requestIdleCallbackIsCalled = true;
        logs.push('1.A1');
    });

    iframe.contentWindow.requestIdleCallback(() => {
        requestIdleCallbackIsCalled = true;
        logs.push('2.B1');
    });

    iframe.contentWindow.requestIdleCallback(() => {
        requestIdleCallbackIsCalled = true;
        logs.push('3.B2');
    });

    requestIdleCallback(() => {
        requestIdleCallbackIsCalled = true;
        logs.push('4.A2');
    });
}
document.body.appendChild(iframe);

This would result in the log being 1.A1, 2.B1, 3.B2, 4.A2 in Chrome and sometimes in Firefox but my faithful implementation of the specification as well as Firefox would sometimes order them as 1.A1, 2.B1, 4.A2, 3.B2.

My statement assumes that step 3 in the start an idle period algorithm, which states "optionally, if the user agent determines the idle period should be delayed, return from this algorithm", doesn't allow UA to ignore the idle period for one window but not for the other.

My reading of the specification goes as follows:

  1. Adds a callback 1.A1 to the top-level window's idle request callbacks, then schedules a new task to start an idle period algorithm (SIPA-A1). Currently scheduled tasks are: [SIPA-A1].
  2. Adds a callback 2.B1 to iframe's content window's idle request callbacks, then schedules a new task to start an idle period algorithm (SIPA-B1). Now we have scheduled tasks: [SIPA-A1, SIPA-B1].
  3. Adds a callback 3.B2 to iframe's idle request callbacks. No new tasks scheduled.
  4. Adds a callback 4.A2 to iframe's idle request callbacks. No new tasks scheduled.
  5. SIPA-A1 runs. Callbacks 1.A1 and 4.A2 move from the list of idle request callbacks to the list of runnable idle callbacks. A new task to invoke idle callbacks (IIC-A1) is scheduled. Currently scheduled tasks are: [SIPA-B1, IIC-A1].
  6. SIPA-B1 runs. Callbacks 2.B1 and 3.B2 move from the list of idle request callbacks to the list of runnable idle callbacks. A new task to invoke idle callbacks (IIC-B1) is scheduled. Currently scheduled tasks are: [IIC-A1, IIC-B1].
  7. IIC-A1 runs. The callback 1.A1 is executed. A new task to invoke idle callbacks (IIC-A2) is scheduled. Currently scheduled tasks are: [IIC-B1, IIC-A2].
  8. IIC-B1 runs. The callback 2.B1 is executed. A new task to invoke idle callbacks (IIC-B2) is scheduled. Currently scheduled tasks are: [IIC-A2, IIC-B2].
  9. IIC-A2 runs. The callback 4.A2 is executed.
  10. IIC-B2 runs. The callback 3.B2 is executed.

What am I missing here? What allows Chrome and (sometimes) Firefox to execute callbacks in other orders? Perhaps step 3 in the start an idle period algorithm? If so, then 2.B1 should be the first to execute.

Perhaps step 1 in the invoke idle callbacks algorithm, which states, "if the user-agent believes it should end the idle period early due to newly scheduled high-priority work, skip to step 4." If this were to always happen at the beginning of (9) perhaps the observed order would make sense.

But then it seems problematic that the ordering of idle callbacks change across content windows. I understand the need to let UA delay work by arbitrary amount but the order of callbacks being completely dependent on UAs isn't ideal to say the least.

rniwa commented 4 years ago

@annevk @smaug----

annevk commented 4 years ago

I suspect like mutation observers this needs to use the similar-origin window agent for guaranteeing consistent ordering.

annevk commented 4 years ago

cc @farre

rniwa commented 4 years ago

Yeah, the fundamental problem here is that the queue is per window. It really ought to be per event loop.

rmcilroy commented 4 years ago

We used to associate the queue with the event loop and not the window (and that's how we implemented it in Chrome). We changed the spec later in 9f072ac4 to be associated with windows to address #57. I agree though, it should really be associated with the event loop if we can avoid the issues in #57.

annevk commented 4 years ago

I don't think there's anything there that would prevent specifying a consistent ordering. But also, "don't monkey patch" doesn't mean "don't change". You can (and should if needed) PR whatwg/html if you need a different infrastructure.

rmcilroy commented 4 years ago

I've created a WIP PR in https://github.com/rmcilroy/requestidlecallback/pull/1 that moves the idle callback lists to the event loop.

If this landed we would also need to change step 11 of the event loop processing model to only call the start an idle period algorithm once with the event loop as its argument. We might also want to move the definition of the idle period lists and deadline over to the html5 spec rather than defining them here.

Let me know if this seems reasonable?

rniwa commented 4 years ago

Seems like a reasonable approach.

noamr commented 2 years ago

I think this is solved by https://github.com/w3c/requestidlecallback/pull/95 and https://github.com/whatwg/html/pull/7166 once they are merged.

The event loop iterates through the windows of that event loop. Probably tests could reflect that better.

noamr commented 2 years ago

Closing, as the spec is explicity about this now. Feel free to reopen if something is missing.