From a1d28e6764ca011d10544a4f72efd28983512966 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Sun, 19 Apr 2026 13:48:38 -0500 Subject: [PATCH] fix: do not block indefinitely on thumbnails in desktopCapturer (#51128) * fix: do not block indefinitely on thumbnails in desktopCapturer fixes dad4ab6 regression * fix: build error * fixup! fix: do not block indefinitely on thumbnails in desktopCapturer chore: remove unnecessary code * Update shell/browser/api/electron_api_desktop_capturer.cc Co-authored-by: Niklas Wenzel --------- Co-authored-by: Niklas Wenzel --- lib/browser/api/desktop-capturer.ts | 65 ++++++++++-------- .../api/electron_api_desktop_capturer.cc | 68 +++++++++++++++++-- .../api/electron_api_desktop_capturer.h | 6 ++ 3 files changed, 102 insertions(+), 37 deletions(-) diff --git a/lib/browser/api/desktop-capturer.ts b/lib/browser/api/desktop-capturer.ts index df9f889eff..f9760dae18 100644 --- a/lib/browser/api/desktop-capturer.ts +++ b/lib/browser/api/desktop-capturer.ts @@ -53,45 +53,50 @@ export async function getSources(args: Electron.SourcesOptions) { } } + let resolveGetSources!: (value: ElectronInternal.GetSourcesResult[]) => void; + let rejectGetSources!: (reason?: any) => void; + const getSources = new Promise((resolve, reject) => { - let capturer: ElectronInternal.DesktopCapturer | null = createDesktopCapturer(); + resolveGetSources = resolve; + rejectGetSources = reject; + }); - const stopRunning = () => { - if (capturer) { - delete capturer._onerror; - delete capturer._onfinished; - capturer = null; + // Register in currentlyRunning BEFORE startHandling so that synchronous + // completion (e.g. when no capturers are created) can properly clean up. + currentlyRunning.push({ options, getSources }); - if (process.platform === 'darwin') { - for (const win of BrowserWindow.getAllWindows()) { - if (resizableValues.has(win.id)) { - win.resizable = resizableValues.get(win.id); - } + let capturer: ElectronInternal.DesktopCapturer | null = createDesktopCapturer(); + + const stopRunning = () => { + if (capturer) { + delete capturer._onerror; + delete capturer._onfinished; + capturer = null; + + if (process.platform === 'darwin') { + for (const win of BrowserWindow.getAllWindows()) { + if (resizableValues.has(win.id)) { + win.resizable = resizableValues.get(win.id); } } } - // Remove from currentlyRunning once we resolve or reject - currentlyRunning = currentlyRunning.filter((running) => running.options !== options); - }; + } + // Remove from currentlyRunning once we resolve or reject + currentlyRunning = currentlyRunning.filter((running) => running.options !== options); + }; - capturer._onerror = (error: string) => { - stopRunning(); - // eslint-disable-next-line prefer-promise-reject-errors - reject(error); - }; + capturer._onerror = (error: string) => { + stopRunning(); + // eslint-disable-next-line prefer-promise-reject-errors + rejectGetSources(error); + }; - capturer._onfinished = (sources: Electron.DesktopCapturerSource[]) => { - stopRunning(); - resolve(sources); - }; + capturer._onfinished = (sources: Electron.DesktopCapturerSource[]) => { + stopRunning(); + resolveGetSources(sources); + }; - capturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons); - }); - - currentlyRunning.push({ - options, - getSources - }); + capturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons); return getSources; } diff --git a/shell/browser/api/electron_api_desktop_capturer.cc b/shell/browser/api/electron_api_desktop_capturer.cc index 91748b2af0..2a15e25572 100644 --- a/shell/browser/api/electron_api_desktop_capturer.cc +++ b/shell/browser/api/electron_api_desktop_capturer.cc @@ -12,6 +12,7 @@ #include "base/memory/raw_ptr.h" #include "base/strings/utf_string_conversions.h" #include "base/task/sequenced_task_runner.h" +#include "base/time/time.h" #include "chrome/browser/media/webrtc/desktop_capturer_wrapper.h" #include "chrome/browser/media/webrtc/desktop_media_list.h" #include "chrome/browser/media/webrtc/desktop_media_list_observer.h" @@ -189,6 +190,12 @@ BOOL CALLBACK EnumDisplayMonitorsCallback(HMONITOR monitor, } #endif +// Give native capturers a few refresh/capture cycles to populate the list +// before returning the current snapshot. This preserves the old behavior of +// making progress even when some sources never produce thumbnails (e.g., a +// password manager window that's marked as uncapturable). +constexpr base::TimeDelta kDesktopCapturerReadyTimeout = base::Seconds(3); + } // namespace namespace gin { @@ -243,9 +250,7 @@ class DesktopCapturer::ListObserver : public DesktopMediaListObserver { if (!need_thumbnails_) return true; - // Every source needs a non-empty thumbnail. - // Thumbnails finish one-at-a-time via tasks posted by the worker thread, - // so wait until every thumbnail arrives. + // are all the tumbnails ready? for (int i = 0; i < list_->GetSourceCount(); ++i) { if (list_->GetSource(i).thumbnail.isNull()) return false; @@ -270,7 +275,7 @@ class DesktopCapturer::ListObserver : public DesktopMediaListObserver { has_sources_ = true; MaybeNotifyReady(); } - void OnSourceRemoved(int index) override {} + void OnSourceRemoved(int index) override { MaybeNotifyReady(); } void OnSourceMoved(int old_index, int new_index) override {} void OnSourceNameChanged(int index) override {} void OnSourceThumbnailChanged(int index) override { MaybeNotifyReady(); } @@ -298,6 +303,22 @@ DesktopCapturer::DesktopCapturer() = default; DesktopCapturer::~DesktopCapturer() = default; +void DesktopCapturer::FinalizeList(std::unique_ptr& observer, + std::unique_ptr& list) { + DCHECK(observer); + DCHECK(list); + + CollectSourcesFrom(list.get()); + if (finished_) + return; + + list.reset(); + observer.reset(); + + if (--pending_lists_ == 0) + HandleSuccess(); +} + void DesktopCapturer::StartHandling(bool capture_window, bool capture_screen, const gfx::Size& thumbnail_size, @@ -317,6 +338,7 @@ void DesktopCapturer::StartHandling(bool capture_window, captured_sources_.clear(); finished_ = false; pending_lists_ = 0; + deadline_.Stop(); #if BUILDFLAG(IS_MAC) if (!ui::TryPromptUserForScreenCapture()) { @@ -375,16 +397,46 @@ void DesktopCapturer::StartHandling(bool capture_window, pending_lists_++; } } + + // If no capturers were created (e.g. all factories returned nullptr), + // resolve immediately with an empty source list. + if (pending_lists_ == 0) { + HandleSuccess(); + return; + } + + deadline_.Start(FROM_HERE, kDesktopCapturerReadyTimeout, + base::BindOnce(&DesktopCapturer::OnReadyTimeout, + weak_ptr_factory_.GetWeakPtr())); } void DesktopCapturer::OnListReady(DesktopMediaList* list) { if (finished_) return; - CollectSourcesFrom(list); + if (list == window_capturer_.get()) { + FinalizeList(window_observer_, window_capturer_); + } else if (list == screen_capturer_.get()) { + FinalizeList(screen_observer_, screen_capturer_); + } else { + NOTREACHED(); + } +} - if (--pending_lists_ == 0) - HandleSuccess(); +void DesktopCapturer::OnReadyTimeout() { + if (finished_) + return; + + if (window_capturer_) + FinalizeList(window_observer_, window_capturer_); + + if (finished_) + return; + + if (screen_capturer_) + FinalizeList(screen_observer_, screen_capturer_); + + DCHECK(finished_ || pending_lists_ > 0); } void DesktopCapturer::CollectSourcesFrom(DesktopMediaList* list) { @@ -476,6 +528,7 @@ void DesktopCapturer::HandleSuccess() { if (finished_) return; finished_ = true; + deadline_.Stop(); v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::HandleScope scope(isolate); @@ -493,6 +546,7 @@ void DesktopCapturer::HandleFailure() { if (finished_) return; finished_ = true; + deadline_.Stop(); v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::HandleScope scope(isolate); diff --git a/shell/browser/api/electron_api_desktop_capturer.h b/shell/browser/api/electron_api_desktop_capturer.h index c36a41f8bd..4c3352702c 100644 --- a/shell/browser/api/electron_api_desktop_capturer.h +++ b/shell/browser/api/electron_api_desktop_capturer.h @@ -9,6 +9,8 @@ #include #include +#include "base/memory/weak_ptr.h" +#include "base/timer/timer.h" #include "chrome/browser/media/webrtc/desktop_media_list.h" #include "shell/common/gin_helper/pinnable.h" #include "shell/common/gin_helper/wrappable.h" @@ -59,7 +61,10 @@ class DesktopCapturer final private: class ListObserver; + void FinalizeList(std::unique_ptr& observer, + std::unique_ptr& list); void OnListReady(DesktopMediaList* list); + void OnReadyTimeout(); void CollectSourcesFrom(DesktopMediaList* list); void HandleFailure(); void HandleSuccess(); @@ -72,6 +77,7 @@ class DesktopCapturer final int pending_lists_ = 0; bool finished_ = false; bool fetch_window_icons_ = false; + base::OneShotTimer deadline_; #if BUILDFLAG(IS_WIN) bool using_directx_capturer_ = false; #endif // BUILDFLAG(IS_WIN)