Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Electron window may go blank after several minutes when hidden and disabling backgroundThrottling #42378

Open
3 tasks done
markh-discord opened this issue Jun 6, 2024 · 2 comments
Assignees
Labels
30-x-y 31-x-y 32-x-y bug 🪲 has-repro-gist Issue can be reproduced with code at https://gist.github.com/ platform/windows status/confirmed A maintainer reproduced the bug or agreed with the feature

Comments

@markh-discord
Copy link

Preflight Checklist

Electron Version

30.0.0

What operating system are you using?

Windows

Operating System Version

Windows 10 & 11

What arch are you using?

x64

Last Known Working Electron version

22.0.0

Expected Behavior

The Electron window contents should not disappear after several minutes when using the background throttling feature.

Actual Behavior

Reproduce steps:

  1. Run the Electron Fiddle project in the gist url provided
  2. Open the devtools and popout into separate window (this doesn't change the behavior but is needed to invoke the disabling of background throttling)
  3. Minimize the main electron window
  4. In the devtools console run electronApp.disableBackgroundThrottling();
  5. Restore the main electron window
  6. Wait 5-10 minutes (usually 10), window contents will incorrectly go blank
  7. Observe that moving the mouse around will still change the cursor, devtools will still show a valid DOM

This reproduces on Windows, have not tried on other OSes.

Note: Resizing the window or minimizing/restoring will temporarily fix the behavior and show the contents, but after the 5-10 minute window the contents will go blank again.

Testcase Gist URL

https://gist.github.com/markh-discord/4d828931b38d5917e5c8e73101981a54

Additional Information

This appears to be a bug with the disable_hidden.patch:

+ if (!host()->is_hidden() && !host()->disable_hidden_) {

Note: When background throttling is enabled (default), the disable_hidden_ state will be false.

From the reproduce steps, when the window is minimized (which calls RenderWidgetHostViewAura::HideImpl()) the condition above passes and eventually calls into:

delegated_frame_host_->WasHidden(cause);

https://chromium.googlesource.com/chromium/src.git/+/27caf432b8c27240dd42820a7c5e098374a91970/content/browser/renderer_host/render_widget_host_view_aura.cc#655

which calls:

frame_evictor_->SetVisible(false);

https://chromium.googlesource.com/chromium/src.git/+/27caf432b8c27240dd42820a7c5e098374a91970/content/browser/renderer_host/delegated_frame_host.cc#141

The next step is to disable the background throttling, effectively setting disable_hidden_ to true.
This triggers the RenderWidgetHostImpl::WasShown since it's currently hidden and sets is_hidden_ = true:


Note that at this point the window is still minimized and not visibly shown to the user.

The next step is then to restore the window, this then hits this code (linking to the patch here since that's effectively what it's running):

+ if (host_->is_hidden() || visibility_ == Visibility::VISIBLE)

This condition now fails because the host_->is_hidden() was toggled above to true, skipping the calls to:

  1. RenderWidgetHostViewBase::OnShowWithPageVisibility
  2. RenderWidgetHostViewAura::NotifyHostAndDelegateOnWasShown
  3. DelegatedFrameHost::WasShown
  4. FrameEvictor::SetVisible(true)

Since this entire runtime path is skipped, the FrameEvictor attached to the DelegatedFrameHost (which is attached to the RenderWidgetHost) believes it's still not visible.

The FrameEvictorManager has a 5 minute timer where it will begin evicting frames it believes aren't seen:

static constexpr base::TimeDelta kPeriodicCullingDelay = base::Minutes(5);

https://chromium.googlesource.com/chromium/src.git/+/27caf432b8c27240dd42820a7c5e098374a91970/components/viz/client/frame_eviction_manager.h#90
The timer gets setup here with the 5 minute delay: https://chromium.googlesource.com/chromium/src.git/+/27caf432b8c27240dd42820a7c5e098374a91970/components/viz/client/frame_eviction_manager.cc#88
Likely due to small timing differences after the timer runs, the 5 minutes hasn't quite been hit in the check here:
https://chromium.googlesource.com/chromium/src.git/+/27caf432b8c27240dd42820a7c5e098374a91970/components/viz/client/frame_eviction_manager.cc#183
So it must wait another 5 minutes before the timer ticks again, which usually results in the 10 minute wait needed to observe this problem.

We did not see this behavior in Electron 22, but noticed it in Electron 28. This doesn't seem to be a problem in Electron 22 since the FrameEvictor was disabled at that point in time and was enabled shortly after: https://chromium.googlesource.com/chromium/src.git/+/4fbc6d67f7ba71ed61bedac5a146d3d931aba4c1

@electron-issue-triage electron-issue-triage bot added 30-x-y has-repro-gist Issue can be reproduced with code at https://gist.github.com/ labels Jun 6, 2024
@codebytere codebytere added platform/windows status/confirmed A maintainer reproduced the bug or agreed with the feature 31-x-y labels Jun 7, 2024
markh-discord added a commit to discord/electron that referenced this issue Jun 14, 2024
electron#42378

Patch-Filename: fix_workaround_electron_bug_with_frame_eviction.patch
@Galkon
Copy link

Galkon commented Jun 14, 2024

We had this issue at Medal.tv. The way I solved it was:

  • Wait 2s after window hide event before setting background throttling to true
  • When the window emits focus event, set it back to false

This seems to have gotten around the issue for now. Worth noting, we saw this on electron 22.

markh-discord added a commit to discord/electron that referenced this issue Jun 14, 2024
electron#42378

Patch-Filename: fix_workaround_electron_bug_with_frame_eviction.patch
@markh-discord
Copy link
Author

markh-discord commented Jun 14, 2024

This is a workaround patch we're using: 699f048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
30-x-y 31-x-y 32-x-y bug 🪲 has-repro-gist Issue can be reproduced with code at https://gist.github.com/ platform/windows status/confirmed A maintainer reproduced the bug or agreed with the feature
Projects
Status: 👍 Does Not Block Stable
Status: 🛑 Blocks Stable
Status: 👀 Unsorted Items
Development

No branches or pull requests

6 participants
@Galkon @codebytere @VerteDinde @georgexu99 @markh-discord and others