From 905e41bbddd72f4d19c294b430d1a7c282e22a5d Mon Sep 17 00:00:00 2001 From: Athul Iddya Date: Tue, 11 Jul 2023 01:21:11 -0700 Subject: [PATCH] fix: use StartUpdating method for PipeWire capturer (#38833) * fix: use StartUpdating method for PipeWire capturer Fixed a crash related to PipeWire capturer by adapting to Chromium's interface changes. Chromium expects a call to `NativeDesktopMediaList::StartUpdating` with an implementation of `DesktopMediaListObserver` for delegated capturers like PipeWire. This interface allows listening to user permission events and listing sources only after the user has made a choice on the permission dialog. The interface has been implemented by an inner class to allow listening to screen and window capture permissions concurrently using two instances of the class. A patch that was resetting the capturer on the first refresh has been changed to exclude PipeWire. PipeWire capturer object will follow the lifecycle of `NativeDesktopMediaList`, as is the case in Chromium. Fixes #37463 * fix: wait for thumbnails from PipeWire when necessary The PipeWire stream starts after the dialog is dismissed. If the sources are listed immediately afterwards, the thumbnail may not have been generated by that time. Explicitly wait for both thumbnail generation and a selection on the source dialog before listing sources. --- patches/chromium/desktop_media_list.patch | 11 ++- .../api/electron_api_desktop_capturer.cc | 92 +++++++++++++++---- .../api/electron_api_desktop_capturer.h | 31 ++++++- 3 files changed, 113 insertions(+), 21 deletions(-) diff --git a/patches/chromium/desktop_media_list.patch b/patches/chromium/desktop_media_list.patch index ba0b2842574a..e3aefc69cd99 100644 --- a/patches/chromium/desktop_media_list.patch +++ b/patches/chromium/desktop_media_list.patch @@ -82,7 +82,7 @@ index 33ca7a53dfb6d2c9e3a33f0065a3acd806e82e01..9fdf2e8ff0056ff407015b914c6b03eb const Source& GetSource(int index) const override; DesktopMediaList::Type GetMediaListType() const override; diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chrome/browser/media/webrtc/native_desktop_media_list.cc -index b548c9fbd3c0bf425447b29dcd866cd27e96b14c..f994ac6086c7b4cd3e8534f34691189d78a21601 100644 +index b548c9fbd3c0bf425447b29dcd866cd27e96b14c..4de719510eaeaaf77cdbd46560f3b4ab1869877a 100644 --- a/chrome/browser/media/webrtc/native_desktop_media_list.cc +++ b/chrome/browser/media/webrtc/native_desktop_media_list.cc @@ -147,7 +147,7 @@ BOOL CALLBACK AllHwndCollector(HWND hwnd, LPARAM param) { @@ -94,17 +94,20 @@ index b548c9fbd3c0bf425447b29dcd866cd27e96b14c..f994ac6086c7b4cd3e8534f34691189d #endif } // namespace -@@ -457,6 +457,9 @@ void NativeDesktopMediaList::Worker::RefreshNextThumbnail() { +@@ -457,6 +457,12 @@ void NativeDesktopMediaList::Worker::RefreshNextThumbnail() { FROM_HERE, base::BindOnce(&NativeDesktopMediaList::UpdateNativeThumbnailsFinished, media_list_)); + + // This call is necessary to release underlying OS screen capture mechanisms. -+ capturer_.reset(); ++ // Skip if the source list is delegated, as the source list window will be active. ++ if (!capturer_->GetDelegatedSourceListController()) { ++ capturer_.reset(); ++ } } void NativeDesktopMediaList::Worker::OnCaptureResult( -@@ -829,6 +832,11 @@ void NativeDesktopMediaList::RefreshForVizFrameSinkWindows( +@@ -829,6 +835,11 @@ void NativeDesktopMediaList::RefreshForVizFrameSinkWindows( FROM_HERE, base::BindOnce(&Worker::RefreshThumbnails, base::Unretained(worker_.get()), std::move(native_ids), thumbnail_size_)); diff --git a/shell/browser/api/electron_api_desktop_capturer.cc b/shell/browser/api/electron_api_desktop_capturer.cc index 650e4817c2ae..1e908db7c87c 100644 --- a/shell/browser/api/electron_api_desktop_capturer.cc +++ b/shell/browser/api/electron_api_desktop_capturer.cc @@ -181,6 +181,41 @@ 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)), + failure_callback_(std::move(failure_callback)), + have_thumbnail_(skip_thumbnails) {} + +DesktopCapturer::DesktopListListener::~DesktopListListener() = default; + +void DesktopCapturer::DesktopListListener::OnDelegatedSourceListSelection() { + if (have_thumbnail_) { + std::move(update_callback_).Run(); + } else { + have_selection_ = true; + } +} + +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. + std::move(update_callback_).Run(); + } else { + have_thumbnail_ = true; + } +} + +void DesktopCapturer::DesktopListListener::OnDelegatedSourceListDismissed() { + std::move(failure_callback_).Run(); +} + void DesktopCapturer::StartHandling(bool capture_window, bool capture_screen, const gfx::Size& thumbnail_size, @@ -212,11 +247,22 @@ void DesktopCapturer::StartHandling(bool capture_window, window_capturer_ = std::make_unique( DesktopMediaList::Type::kWindow, std::move(capturer)); window_capturer_->SetThumbnailSize(thumbnail_size); - window_capturer_->Update( - base::BindOnce(&DesktopCapturer::UpdateSourcesList, - weak_ptr_factory_.GetWeakPtr(), - window_capturer_.get()), - /* refresh_thumbnails = */ true); + + OnceCallback update_callback = base::BindOnce( + &DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(), + window_capturer_.get()); + + if (window_capturer_->IsSourceListDelegated()) { + OnceCallback failure_callback = base::BindOnce( + &DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr()); + window_listener_ = std::make_unique( + 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); + } } } @@ -226,11 +272,22 @@ void DesktopCapturer::StartHandling(bool capture_window, screen_capturer_ = std::make_unique( DesktopMediaList::Type::kScreen, std::move(capturer)); screen_capturer_->SetThumbnailSize(thumbnail_size); - screen_capturer_->Update( - base::BindOnce(&DesktopCapturer::UpdateSourcesList, - weak_ptr_factory_.GetWeakPtr(), - screen_capturer_.get()), - /* refresh_thumbnails = */ true); + + 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( + 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); + } } } } @@ -269,12 +326,7 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { // |media_list_sources|. if (!webrtc::DxgiDuplicatorController::Instance()->GetDeviceNames( &device_names)) { - v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); - v8::HandleScope scope(isolate); - gin_helper::CallMethod(this, "_onerror", "Failed to get sources."); - - Unpin(); - + HandleFailure(); return; } @@ -324,6 +376,14 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { } } +void DesktopCapturer::HandleFailure() { + v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); + v8::HandleScope scope(isolate); + gin_helper::CallMethod(this, "_onerror", "Failed to get sources."); + + Unpin(); +} + // static gin::Handle DesktopCapturer::Create(v8::Isolate* isolate) { auto handle = gin::CreateHandle(isolate, new DesktopCapturer(isolate)); diff --git a/shell/browser/api/electron_api_desktop_capturer.h b/shell/browser/api/electron_api_desktop_capturer.h index f06ce5faee28..65285086a158 100644 --- a/shell/browser/api/electron_api_desktop_capturer.h +++ b/shell/browser/api/electron_api_desktop_capturer.h @@ -62,8 +62,37 @@ class DesktopCapturer : public gin::Wrappable, void OnDelegatedSourceListDismissed() override {} private: - void UpdateSourcesList(DesktopMediaList* list); + using OnceCallback = base::OnceClosure; + class DesktopListListener : public DesktopMediaListObserver { + public: + DesktopListListener(OnceCallback update_callback, + OnceCallback failure_callback, + bool skip_thumbnails); + ~DesktopListListener() override; + + protected: + 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; + + private: + OnceCallback update_callback_; + OnceCallback failure_callback_; + bool have_selection_ = false; + bool have_thumbnail_ = false; + }; + + void UpdateSourcesList(DesktopMediaList* list); + void HandleFailure(); + + std::unique_ptr window_listener_; + std::unique_ptr screen_listener_; std::unique_ptr window_capturer_; std::unique_ptr screen_capturer_; std::vector captured_sources_;