Bug 1682713 Comment 84 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I have managed to reproduce this after all. I need to send my DTK back soon but for now I've been able to do some debugging. My steps to reproduce are:

 1. Open a page that paints periodically but leaves large enough gaps between the individual paints that we stop listening for vsync in between, i.e. at least 100-200ms. For example this page: `data:text/html,<script>x=0;setInterval(() => document.body.style.backgroundColor = (x=1-x) ? "green" : "blue", 500)</script>`
 2. Put your machine to sleep using the menu.
 3. Wait until you don't hear any noise from it anymore.
 4. Wait for 20 seconds. If it starts making noise again, wait until it stops again and wait another 20 seconds.
 5. Wake up the machine.

I've found a fix. It's very similar to the other one, except that I have to destroy and recreate the CVDisplayLink rather than just stopping and starting it.

Technical description of what happens:

During wake-up, there's a "display double-swap" similar to what happens during user switching: The regular display is swapped for a temporary display, and then that temporary display is swapped again for the regular display. Example:

```
First swap:
[1] DisplayReconfigurationCallback for 1: BeginConfiguration
[2] DisplayReconfigurationCallback for 21: BeginConfiguration
[3] DisplayReconfigurationCallback for 1: Remove Disabled DesktopShapeChanged
[4] DisplayReconfigurationCallback for 21: Moved SetMain SetMode Add Enabled DesktopShapeChanged
Second swap:
[5] DisplayReconfigurationCallback for 21: BeginConfiguration
[6] DisplayReconfigurationCallback for 1: BeginConfiguration
[7] DisplayReconfigurationCallback for 21: Remove Disabled DesktopShapeChanged
[8] DisplayReconfigurationCallback for 1: Moved SetMain SetMode Add Enabled DesktopShapeChanged
```

Here 1 is the regular display and 21 is the temporary display. These two swaps happen within one second of each other, during wake-up.

In bug 1422855, we would create a CVDisplayLink before [1]. That CVDisplayLink object successfully picks the regular display, and stays assigned to the regular display throughout all of [1]-[8].
In this bug, we also try to create a CVDisplayLink before [1], but we do this just after sleep. The CVDisplayLink object created at that time does not assign itself to any display, and `CVDisplayLinkGetCurrentCGDisplay` returns 0. So we hit [this fallback code](https://searchfox.org/mozilla-central/rev/362676fcadac37f9f585141a244a9a640948794a/gfx/thebes/gfxPlatformMac.cpp#782-798) that keeps creating new display links every 100ms until it succeeds.

It doesn't succeed before [1]. But once the first swap happens, it does succeed; the timer can fire between [2] and [3], or between [4] and [5], and create the display link at that point. However, now that display link's current display is the temporary display. And once [5]-[8] are done, the display link object stays assigned to the temporary display even though that display has now been removed. And even the stop-start code from bug 1422855 isn't enough to get the display link object assigned to the regular display. And the display link does not fire any vsync notifications while it's assigned to the dead display.
But creating a new display link object at this point works, and it correctly uses the regular display. So that's what I'm going to do.

For a proper fix, we should stop using `CVDisplayLinkCreateWithActiveCGDisplays`. Instead, we should create per-display CVDisplayLink objects, and keep track of which displays are active and only have CVDisplayLinks for those.
I have managed to reproduce this after all. I need to send my DTK back soon but for now I've been able to do some debugging. My steps to reproduce are:

 1. Open a page that paints periodically but leaves large enough gaps between the individual paints that we stop listening for vsync in between, i.e. at least 100-200ms. For example this page: `data:text/html,<script>x=0;setInterval(() => document.body.style.backgroundColor = (x=1-x) ? "green" : "blue", 500)</script>`
 2. Put your machine to sleep using the menu.
 3. Wait until you don't hear any noise from it anymore.
 4. Wait for 20 seconds. If it starts making noise again, wait until it stops again and wait another 20 seconds.
 5. Wake up the machine.

I've found a fix. It's very similar to the other one, except that I have to destroy and recreate the CVDisplayLink rather than just stopping and starting it.

Technical description of what happens:

During wake-up, there's a "display double-swap" similar to what happens during user switching: The regular display is swapped for a temporary display, and then that temporary display is swapped again for the regular display. Example:

```
First swap:
[1] DisplayReconfigurationCallback for 1: BeginConfiguration
[2] DisplayReconfigurationCallback for 21: BeginConfiguration
[3] DisplayReconfigurationCallback for 1: Remove Disabled DesktopShapeChanged
[4] DisplayReconfigurationCallback for 21: Moved SetMain SetMode Add Enabled DesktopShapeChanged
Second swap:
[5] DisplayReconfigurationCallback for 21: BeginConfiguration
[6] DisplayReconfigurationCallback for 1: BeginConfiguration
[7] DisplayReconfigurationCallback for 21: Remove Disabled DesktopShapeChanged
[8] DisplayReconfigurationCallback for 1: Moved SetMain SetMode Add Enabled DesktopShapeChanged
```

Here 1 is the regular display and 21 is the temporary display. These two swaps happen within one second of each other, during wake-up.

(Edited for correctness.)
During user switching, the first swap happens when switching away, and the second swap happens when switching back. In bug 1422855, we would create a new display link between [4] and [5], and then that CVDisplayLink would stay assigned to the temporary display and get stuck after [8].
In this bug, we try to create a CVDisplayLink just after sleep, but before the swaps happen. The CVDisplayLink object created before [0] does not assign itself to any display, and `CVDisplayLinkGetCurrentCGDisplay` returns 0 at that time. So we hit [this fallback code](https://searchfox.org/mozilla-central/rev/362676fcadac37f9f585141a244a9a640948794a/gfx/thebes/gfxPlatformMac.cpp#782-798) that keeps creating new display links every 100ms until it succeeds.

It doesn't succeed before [1]. But once the first swap happens, it does succeed; the timer can fire between [2] and [3], or between [4] and [5], and create the display link at that point. However, now that display link's is assigned to the temporary display. So now we're in the same situation as in bug 1422855: The display link stays assigned to the temporary display, and is stuck with it even after [5]-[8] are done.

However, here's where the difference comes in: It seems that on Apple Silicon machines, stopping and starting the display link is not enough to get the it re-assigned to the regular display. So the display link stays assigned to the dead display and does not fire any vsync notifications. And, unlike on Intel machines, the stuck display link never recovers.
But creating a new display link object at this point works, and it correctly uses the regular display. So that's what I'm going to do.

For a proper fix, we should stop using `CVDisplayLinkCreateWithActiveCGDisplays`. Instead, we should create per-display CVDisplayLink objects, and keep track of which displays are active and only have CVDisplayLinks for those.

Back to Bug 1682713 Comment 84