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 <dev@nikwen.de>

---------

Co-authored-by: Niklas Wenzel <dev@nikwen.de>

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<ElectronInternal.GetSourcesResult[]>((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;
 }

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<ListObserver>& observer,
+                                   std::unique_ptr<DesktopMediaList>& 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);

shell/browser/api/electron_api_desktop_capturer.h

@@ -9,6 +9,8 @@
 #include <string>
 #include <vector>
 
+#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<ListObserver>& observer,
+                    std::unique_ptr<DesktopMediaList>& 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)