refactor: switch desktopCapturer to StartUpdating() with event-based observer

Replace the one-shot Update() approach with Chromium's intended
StartUpdating() pattern for all source types. This introduces a new
SourceListObserver that watches for sources and thumbnails to arrive
via DesktopMediaListObserver events, with a readiness heuristic that
naturally handles SCK's warmup cycles without the skip_next_refresh_ hack.

Key changes:
- Replace DesktopListListener with SourceListObserver
- DesktopCapturer no longer inherits DesktopMediaListObserver
- All capture paths (delegated and non-delegated) use StartUpdating()
- Remove skip_next_refresh_ assignments from electron code
- Add 5-second safety timeout to prevent indefinite hangs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

shell/browser/api/electron_api_desktop_capturer.cc

@@ -227,44 +227,54 @@ DesktopCapturer::DesktopCapturer(v8::Isolate* isolate) {}
 
 DesktopCapturer::~DesktopCapturer() = default;
 
-DesktopCapturer::DesktopListListener::DesktopListListener(
-    OnceCallback update_callback,
-    OnceCallback failure_callback,
-    bool skip_thumbnails)
-    : update_callback_(std::move(update_callback)),
+DesktopCapturer::SourceListObserver::SourceListObserver(
+    base::OnceClosure ready_callback,
+    base::OnceClosure failure_callback,
+    DesktopMediaList* list,
+    bool need_thumbnails)
+    : ready_callback_(std::move(ready_callback)),
       failure_callback_(std::move(failure_callback)),
-      have_thumbnail_(skip_thumbnails) {}
+      list_(list),
+      need_thumbnails_(need_thumbnails) {}
 
-DesktopCapturer::DesktopListListener::~DesktopListListener() = default;
+DesktopCapturer::SourceListObserver::~SourceListObserver() = default;
 
-void DesktopCapturer::DesktopListListener::OnDelegatedSourceListSelection() {
-  if (have_thumbnail_) {
-    content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
-                                                 std::move(update_callback_));
-  } else {
-    have_selection_ = true;
-  }
+void DesktopCapturer::SourceListObserver::OnSourceAdded(int index) {
+  CheckReady();
 }
 
-void DesktopCapturer::DesktopListListener::OnSourceThumbnailChanged(int index) {
-  if (have_selection_) {
-    // This is called every time a thumbnail is refreshed. Reset variable to
-    // ensure that the callback is not run again.
-    have_selection_ = false;
-
-    // PipeWire returns a single source, so index is not relevant.
-    content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
-                                                 std::move(update_callback_));
-  } else {
-    have_thumbnail_ = true;
-  }
+void DesktopCapturer::SourceListObserver::OnSourceThumbnailChanged(int index) {
+  CheckReady();
 }
 
-void DesktopCapturer::DesktopListListener::OnDelegatedSourceListDismissed() {
+void DesktopCapturer::SourceListObserver::OnDelegatedSourceListDismissed() {
+  if (finished_)
+    return;
+  finished_ = true;
   content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
                                                std::move(failure_callback_));
 }
 
+void DesktopCapturer::SourceListObserver::CheckReady() {
+  if (finished_)
+    return;
+
+  int count = list_->GetSourceCount();
+  if (count == 0)
+    return;
+
+  if (need_thumbnails_) {
+    for (int i = 0; i < count; i++) {
+      if (list_->GetSource(i).thumbnail.isNull())
+        return;
+    }
+  }
+
+  finished_ = true;
+  content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
+                                               std::move(ready_callback_));
+}
+
 void DesktopCapturer::StartHandling(bool capture_window,
                                     bool capture_screen,
                                     const gfx::Size& thumbnail_size,
@@ -283,6 +293,8 @@ void DesktopCapturer::StartHandling(bool capture_window,
   // clear any existing captured sources.
   captured_sources_.clear();
 
+  bool need_thumbnails = !thumbnail_size.IsEmpty();
+
   if (capture_window && capture_screen) {
     // Some capturers like PipeWire support a single capturer for both screens
     // and windows. Use it if possible, treating both as window capture
@@ -299,17 +311,20 @@ void DesktopCapturer::StartHandling(bool capture_window,
           DesktopMediaList::Type::kWindow, std::move(capturer), true, true);
       window_capturer_->SetThumbnailSize(thumbnail_size);
 
-      OnceCallback update_callback = base::BindOnce(
+      base::OnceClosure update_callback = base::BindOnce(
           &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
           window_capturer_.get());
-      OnceCallback failure_callback = base::BindOnce(
+      base::OnceClosure failure_callback = base::BindOnce(
           &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
 
-      window_listener_ = std::make_unique<DesktopListListener>(
+      window_observer_ = std::make_unique<SourceListObserver>(
           std::move(update_callback), std::move(failure_callback),
-          thumbnail_size.IsEmpty());
-      window_capturer_->StartUpdating(window_listener_.get());
+          window_capturer_.get(), need_thumbnails);
+      window_capturer_->StartUpdating(window_observer_.get());
 
+      safety_timer_.Start(FROM_HERE, base::Seconds(5),
+                          base::BindOnce(&DesktopCapturer::HandleSafetyTimeout,
+                                         weak_ptr_factory_.GetWeakPtr()));
       return;
     }
   }
@@ -325,71 +340,49 @@ void DesktopCapturer::StartHandling(bool capture_window,
   }
 #endif
 
-  {
-    // Initialize the source list.
-    // Apply the new thumbnail size and restart capture.
-    if (capture_window) {
-      auto capturer = MakeWindowCapturer();
-      if (capturer) {
-        window_capturer_ = std::make_unique<NativeDesktopMediaList>(
-            DesktopMediaList::Type::kWindow, std::move(capturer), true, true);
-        window_capturer_->SetThumbnailSize(thumbnail_size);
-#if BUILDFLAG(IS_MAC)
-        window_capturer_->skip_next_refresh_ =
-            ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kWindow)
-                ? thumbnail_size.IsEmpty() ? 1 : 2
-                : 0;
-#endif
+  if (capture_window) {
+    auto capturer = MakeWindowCapturer();
+    if (capturer) {
+      window_capturer_ = std::make_unique<NativeDesktopMediaList>(
+          DesktopMediaList::Type::kWindow, std::move(capturer), true, true);
+      window_capturer_->SetThumbnailSize(thumbnail_size);
 
-        OnceCallback update_callback = base::BindOnce(
-            &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
-            window_capturer_.get());
+      base::OnceClosure update_callback = base::BindOnce(
+          &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
+          window_capturer_.get());
+      base::OnceClosure failure_callback = base::BindOnce(
+          &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
 
-        if (window_capturer_->IsSourceListDelegated()) {
-          OnceCallback failure_callback = base::BindOnce(
-              &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
-          window_listener_ = std::make_unique<DesktopListListener>(
-              std::move(update_callback), std::move(failure_callback),
-              thumbnail_size.IsEmpty());
-          window_capturer_->StartUpdating(window_listener_.get());
-        } else {
-          window_capturer_->Update(std::move(update_callback),
-                                   /* refresh_thumbnails = */ true);
-        }
-      }
-    }
-
-    if (capture_screen) {
-      auto capturer = MakeScreenCapturer();
-      if (capturer) {
-        screen_capturer_ = std::make_unique<NativeDesktopMediaList>(
-            DesktopMediaList::Type::kScreen, std::move(capturer));
-        screen_capturer_->SetThumbnailSize(thumbnail_size);
-#if BUILDFLAG(IS_MAC)
-        screen_capturer_->skip_next_refresh_ =
-            ShouldUseThumbnailCapturerMac(DesktopMediaList::Type::kScreen)
-                ? thumbnail_size.IsEmpty() ? 1 : 2
-                : 0;
-#endif
-
-        OnceCallback update_callback = base::BindOnce(
-            &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
-            screen_capturer_.get());
-
-        if (screen_capturer_->IsSourceListDelegated()) {
-          OnceCallback failure_callback = base::BindOnce(
-              &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
-          screen_listener_ = std::make_unique<DesktopListListener>(
-              std::move(update_callback), std::move(failure_callback),
-              thumbnail_size.IsEmpty());
-          screen_capturer_->StartUpdating(screen_listener_.get());
-        } else {
-          screen_capturer_->Update(std::move(update_callback),
-                                   /* refresh_thumbnails = */ true);
-        }
-      }
+      window_observer_ = std::make_unique<SourceListObserver>(
+          std::move(update_callback), std::move(failure_callback),
+          window_capturer_.get(), need_thumbnails);
+      window_capturer_->StartUpdating(window_observer_.get());
     }
   }
+
+  if (capture_screen) {
+    auto capturer = MakeScreenCapturer();
+    if (capturer) {
+      screen_capturer_ = std::make_unique<NativeDesktopMediaList>(
+          DesktopMediaList::Type::kScreen, std::move(capturer));
+      screen_capturer_->SetThumbnailSize(thumbnail_size);
+
+      base::OnceClosure update_callback = base::BindOnce(
+          &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
+          screen_capturer_.get());
+      base::OnceClosure failure_callback = base::BindOnce(
+          &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
+
+      screen_observer_ = std::make_unique<SourceListObserver>(
+          std::move(update_callback), std::move(failure_callback),
+          screen_capturer_.get(), need_thumbnails);
+      screen_capturer_->StartUpdating(screen_observer_.get());
+    }
+  }
+
+  safety_timer_.Start(FROM_HERE, base::Seconds(5),
+                      base::BindOnce(&DesktopCapturer::HandleSafetyTimeout,
+                                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) {
@@ -483,10 +476,14 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) {
 }
 
 void DesktopCapturer::HandleSuccess() {
+  safety_timer_.Stop();
+
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::HandleScope scope(isolate);
   gin_helper::CallMethod(this, "_onfinished", captured_sources_);
 
+  window_observer_.reset();
+  screen_observer_.reset();
   screen_capturer_.reset();
   window_capturer_.reset();
 
@@ -494,17 +491,39 @@ void DesktopCapturer::HandleSuccess() {
 }
 
 void DesktopCapturer::HandleFailure() {
+  safety_timer_.Stop();
+
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::HandleScope scope(isolate);
 
   gin_helper::CallMethod(this, "_onerror", "Failed to get sources.");
 
+  window_observer_.reset();
+  screen_observer_.reset();
   screen_capturer_.reset();
   window_capturer_.reset();
 
   Unpin();
 }
 
+void DesktopCapturer::HandleSafetyTimeout() {
+  // Collect whatever sources are available from lists that haven't
+  // reported yet.
+  if (capture_window_ && window_capturer_) {
+    UpdateSourcesList(window_capturer_.get());
+  }
+  if (capture_screen_ && screen_capturer_) {
+    UpdateSourcesList(screen_capturer_.get());
+  }
+  // If UpdateSourcesList didn't trigger HandleSuccess (e.g., no capturers
+  // were created), force completion with whatever we have.
+  if (capture_window_ || capture_screen_) {
+    capture_window_ = false;
+    capture_screen_ = false;
+    HandleSuccess();
+  }
+}
+
 // static
 gin_helper::Handle<DesktopCapturer> DesktopCapturer::Create(
     v8::Isolate* isolate) {

shell/browser/api/electron_api_desktop_capturer.h

@@ -9,6 +9,8 @@
 #include <string>
 #include <vector>
 
+#include "base/memory/raw_ptr.h"
+#include "base/timer/timer.h"
 #include "chrome/browser/media/webrtc/desktop_media_list_observer.h"
 #include "chrome/browser/media/webrtc/native_desktop_media_list.h"
 #include "shell/common/gin_helper/pinnable.h"
@@ -23,8 +25,7 @@ namespace electron::api {
 
 class DesktopCapturer final
     : public gin_helper::DeprecatedWrappable<DesktopCapturer>,
-      public gin_helper::Pinnable<DesktopCapturer>,
-      private DesktopMediaListObserver {
+      public gin_helper::Pinnable<DesktopCapturer> {
  public:
   struct Source {
     DesktopMediaList::Source media_list_source;
@@ -59,54 +60,52 @@ class DesktopCapturer final
   ~DesktopCapturer() override;
 
  private:
-  // DesktopMediaListObserver:
-  void OnSourceAdded(int index) override {}
-  void OnSourceRemoved(int index) override {}
-  void OnSourceMoved(int old_index, int new_index) override {}
-  void OnSourceNameChanged(int index) override {}
-  void OnSourceThumbnailChanged(int index) override {}
-  void OnSourcePreviewChanged(size_t index) override {}
-  void OnDelegatedSourceListSelection() override {}
-  void OnDelegatedSourceListDismissed() override {}
-
-  using OnceCallback = base::OnceClosure;
-
-  class DesktopListListener : public DesktopMediaListObserver {
+  // Observes a DesktopMediaList and fires the ready callback when sources
+  // (and optionally thumbnails) are available. Replaces the old one-shot
+  // Update() approach with event-based StartUpdating().
+  class SourceListObserver : public DesktopMediaListObserver {
    public:
-    DesktopListListener(OnceCallback update_callback,
-                        OnceCallback failure_callback,
-                        bool skip_thumbnails);
-    ~DesktopListListener() override;
+    SourceListObserver(base::OnceClosure ready_callback,
+                       base::OnceClosure failure_callback,
+                       DesktopMediaList* list,
+                       bool need_thumbnails);
+    ~SourceListObserver() override;
 
    protected:
-    void OnSourceAdded(int index) override {}
+    // DesktopMediaListObserver:
+    void OnSourceAdded(int index) override;
     void OnSourceRemoved(int index) override {}
     void OnSourceMoved(int old_index, int new_index) override {}
     void OnSourceNameChanged(int index) override {}
     void OnSourceThumbnailChanged(int index) override;
     void OnSourcePreviewChanged(size_t index) override {}
-    void OnDelegatedSourceListSelection() override;
+    void OnDelegatedSourceListSelection() override {}
     void OnDelegatedSourceListDismissed() override;
 
    private:
-    OnceCallback update_callback_;
-    OnceCallback failure_callback_;
-    bool have_selection_ = false;
-    bool have_thumbnail_ = false;
+    void CheckReady();
+
+    base::OnceClosure ready_callback_;
+    base::OnceClosure failure_callback_;
+    raw_ptr<DesktopMediaList> list_;
+    bool need_thumbnails_;
+    bool finished_ = false;
   };
 
   void UpdateSourcesList(DesktopMediaList* list);
   void HandleFailure();
   void HandleSuccess();
+  void HandleSafetyTimeout();
 
-  std::unique_ptr<DesktopListListener> window_listener_;
-  std::unique_ptr<DesktopListListener> screen_listener_;
+  std::unique_ptr<SourceListObserver> window_observer_;
+  std::unique_ptr<SourceListObserver> screen_observer_;
   std::unique_ptr<DesktopMediaList> window_capturer_;
   std::unique_ptr<DesktopMediaList> screen_capturer_;
   std::vector<DesktopCapturer::Source> captured_sources_;
   bool capture_window_ = false;
   bool capture_screen_ = false;
   bool fetch_window_icons_ = false;
+  base::OneShotTimer safety_timer_;
 #if BUILDFLAG(IS_WIN)
   bool using_directx_capturer_ = false;
 #endif  // BUILDFLAG(IS_WIN)