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

fix: skip the first two invalid updates when SCK is enabled #41344

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,4 @@ build_run_reclient_cfg_generator_after_chrome.patch
fix_suppress_clang_-wimplicit-const-int-float-conversion_in.patch
cherry-pick-e7ffe20ebfac.patch
fix_getcursorscreenpoint_wrongly_returns_0_0.patch
fix_add_support_for_skipping_first_no-op_refresh_in_thumb_cap.patch
fix_add_support_for_skipping_first_2_no-op_refreshes_in_thumb_cap.patch
Original file line number Diff line number Diff line change
@@ -1,35 +1,42 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <marshallofsound@electronjs.org>
Date: Tue, 13 Feb 2024 17:40:15 -0800
Subject: fix: add support for skipping first no-op refresh in thumb cap
Subject: fix: add support for skipping first 2 no-op refreshes in thumb cap

Fixes a bug in the SCK thumbnail capturer, will be reported upstream for a hopefully
less hacky fix.

The first refresh is "no windows yet, no thumbnails".
The second refresh is "we have windows, we queued the thumbnail requests"
The third refresh (the one we want) is "we have windows, and have thumbnail requests"

This really isn't ideal at all, we need to refactor desktopCapturer (read completely re-implement it)
to use StartUpdating and handle the events instead of using the "get the list once" method.

diff --git a/chrome/browser/media/webrtc/desktop_media_list.h b/chrome/browser/media/webrtc/desktop_media_list.h
index 0c6fccf16a11bbaff10115308e4b489490e5d3e6..3b541e6830ca902cf45483a3193376c0e559185e 100644
index 0c6fccf16a11bbaff10115308e4b489490e5d3e6..e5ec31054a43989e630115605de435399d36559b 100644
--- a/chrome/browser/media/webrtc/desktop_media_list.h
+++ b/chrome/browser/media/webrtc/desktop_media_list.h
@@ -143,6 +143,8 @@ class DesktopMediaList {
// important when IsSourceDelegated() returns true, as it helps to notify the
// delegated source list when it should be hidden.
virtual void HideList() = 0;
+
+ bool skip_next_refresh_ = false;
+ int skip_next_refresh_ = 0;
};

#endif // CHROME_BROWSER_MEDIA_WEBRTC_DESKTOP_MEDIA_LIST_H_
diff --git a/chrome/browser/media/webrtc/desktop_media_list_base.cc b/chrome/browser/media/webrtc/desktop_media_list_base.cc
index 780927301744ea7312f230cec76a24a33d71f767..321d3ff46cbb67e880d5414d83a199fb16457038 100644
index 780927301744ea7312f230cec76a24a33d71f767..d19b1cc9dedf839f12f4113db64293e5c8150f51 100644
--- a/chrome/browser/media/webrtc/desktop_media_list_base.cc
+++ b/chrome/browser/media/webrtc/desktop_media_list_base.cc
@@ -230,7 +230,11 @@ uint32_t DesktopMediaListBase::GetImageHash(const gfx::Image& image) {
void DesktopMediaListBase::OnRefreshComplete() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(refresh_callback_);
- std::move(refresh_callback_).Run();
+ if (skip_next_refresh_) {
+ skip_next_refresh_ = false;
+ if (skip_next_refresh_ > 0) {
+ skip_next_refresh_--;
+ } else {
+ std::move(refresh_callback_).Run();
+ }
Expand Down
6 changes: 4 additions & 2 deletions shell/browser/api/electron_api_desktop_capturer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ void DesktopCapturer::StartHandling(bool capture_window,
window_capturer_->SetThumbnailSize(thumbnail_size);
#if BUILDFLAG(IS_MAC)
window_capturer_->skip_next_refresh_ =
ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kWindow);
ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kWindow) ? 2
: 0;
#endif

OnceCallback update_callback = base::BindOnce(
Expand Down Expand Up @@ -334,7 +335,8 @@ void DesktopCapturer::StartHandling(bool capture_window,
screen_capturer_->SetThumbnailSize(thumbnail_size);
#if BUILDFLAG(IS_MAC)
screen_capturer_->skip_next_refresh_ =
ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kScreen);
ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kScreen) ? 2
: 0;
#endif

OnceCallback update_callback = base::BindOnce(
Expand Down