From 3b9ecca5b3a7290ea38e85539bf5173f68078af1 Mon Sep 17 00:00:00 2001 From: braveyao Date: Fri, 31 Aug 2018 20:30:55 +0000 Subject: [PATCH] NativeDesktopMediaList: do desktop capture on an exclusive thread webrtc::ScreenCapturerMac requires to be started and destroyed on same thread. The current SequencedTaskRunner can't satisfy this and will cause random crashes. This cl is to use an exclusive capture thread as same as the one in content/browser/media/capture/desktop_capture_device.cc. Bug: 877982,851883 Change-Id: I6508add3660f63e1d2539dd2175a131f869a836a Reviewed-on: https://chromium-review.googlesource.com/1199806 Commit-Queue: Weiyong Yao Reviewed-by: Julien Isorce Cr-Commit-Position: refs/heads/master@{#588151} --- .../media/webrtc/native_desktop_media_list.cc | 54 +++++++++++++++---- .../media/webrtc/native_desktop_media_list.h | 9 +--- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chrome/browser/media/webrtc/native_desktop_media_list.cc index 798b4d7b58d92..f5f62407f0d6d 100644 --- a/chrome/browser/media/webrtc/native_desktop_media_list.cc +++ b/chrome/browser/media/webrtc/native_desktop_media_list.cc @@ -5,8 +5,11 @@ #include "chrome/browser/media/webrtc/native_desktop_media_list.h" #include "base/hash.h" +#include "base/single_thread_task_runner.h" #include "base/strings/utf_string_conversions.h" #include "base/task/post_task.h" +#include "base/threading/thread_restrictions.h" +#include "build/build_config.h" #include "chrome/browser/media/webrtc/desktop_media_list_observer.h" #include "chrome/grit/generated_resources.h" #include "content/public/browser/browser_thread.h" @@ -76,11 +79,13 @@ gfx::ImageSkia ScaleDesktopFrame(std::unique_ptr frame, class NativeDesktopMediaList::Worker : public webrtc::DesktopCapturer::Callback { public: - Worker(base::WeakPtr media_list, + Worker(scoped_refptr task_runner, + base::WeakPtr media_list, DesktopMediaID::Type type, std::unique_ptr capturer); ~Worker() override; + void Start(); void Refresh(const DesktopMediaID::Id& view_dialog_id); void RefreshThumbnails(const std::vector& native_ids, @@ -93,6 +98,9 @@ class NativeDesktopMediaList::Worker void OnCaptureResult(webrtc::DesktopCapturer::Result result, std::unique_ptr frame) override; + // Task runner used for capturing operations. + scoped_refptr task_runner_; + base::WeakPtr media_list_; DesktopMediaID::Type type_; @@ -106,17 +114,27 @@ class NativeDesktopMediaList::Worker }; NativeDesktopMediaList::Worker::Worker( + scoped_refptr task_runner, base::WeakPtr media_list, DesktopMediaID::Type type, std::unique_ptr capturer) - : media_list_(media_list), type_(type), capturer_(std::move(capturer)) { - capturer_->Start(this); + : task_runner_(task_runner), + media_list_(media_list), + type_(type), + capturer_(std::move(capturer)) {} + +NativeDesktopMediaList::Worker::~Worker() { + DCHECK(task_runner_->BelongsToCurrentThread()); } -NativeDesktopMediaList::Worker::~Worker() {} +void NativeDesktopMediaList::Worker::Start() { + DCHECK(task_runner_->BelongsToCurrentThread()); + capturer_->Start(this); +} void NativeDesktopMediaList::Worker::Refresh( const DesktopMediaID::Id& view_dialog_id) { + DCHECK(task_runner_->BelongsToCurrentThread()); std::vector result; webrtc::DesktopCapturer::SourceList sources; @@ -163,6 +181,7 @@ void NativeDesktopMediaList::Worker::Refresh( void NativeDesktopMediaList::Worker::RefreshThumbnails( const std::vector& native_ids, const gfx::Size& thumbnail_size) { + DCHECK(task_runner_->BelongsToCurrentThread()); ImageHashesMap new_image_hashes; // Get a thumbnail for each native source. @@ -210,17 +229,30 @@ NativeDesktopMediaList::NativeDesktopMediaList( std::unique_ptr capturer) : DesktopMediaListBase(base::TimeDelta::FromMilliseconds( kDefaultNativeDesktopMediaListUpdatePeriod)), + thread_("DesktopMediaListCaptureThread"), weak_factory_(this) { type_ = type; - capture_task_runner_ = base::CreateSequencedTaskRunnerWithTraits( - {base::MayBlock(), base::TaskPriority::USER_VISIBLE}); - worker_.reset( - new Worker(weak_factory_.GetWeakPtr(), type, std::move(capturer))); +#if defined(OS_WIN) || defined(OS_MACOSX) + // On Windows/OSX the thread must be a UI thread. + base::MessageLoop::Type thread_type = base::MessageLoop::TYPE_UI; +#else + base::MessageLoop::Type thread_type = base::MessageLoop::TYPE_DEFAULT; +#endif + thread_.StartWithOptions(base::Thread::Options(thread_type, 0)); + + worker_.reset(new Worker(thread_.task_runner(), weak_factory_.GetWeakPtr(), + type, std::move(capturer))); + + thread_.task_runner()->PostTask( + FROM_HERE, + base::BindOnce(&Worker::Start, base::Unretained(worker_.get()))); } NativeDesktopMediaList::~NativeDesktopMediaList() { - capture_task_runner_->DeleteSoon(FROM_HERE, worker_.release()); + base::ThreadRestrictions::ScopedAllowIO allow_io; + thread_.task_runner()->DeleteSoon(FROM_HERE, worker_.release()); + thread_.Stop(); } void NativeDesktopMediaList::Refresh() { @@ -230,7 +262,7 @@ void NativeDesktopMediaList::Refresh() { new_aura_thumbnail_hashes_.clear(); #endif - capture_task_runner_->PostTask( + thread_.task_runner()->PostTask( FROM_HERE, base::BindOnce(&Worker::Refresh, base::Unretained(worker_.get()), view_dialog_id_.id)); @@ -280,7 +312,7 @@ void NativeDesktopMediaList::RefreshForAuraWindows( #if defined(USE_AURA) pending_native_thumbnail_capture_ = true; #endif - capture_task_runner_->PostTask( + thread_.task_runner()->PostTask( FROM_HERE, base::BindOnce(&Worker::RefreshThumbnails, base::Unretained(worker_.get()), native_ids, thumbnail_size_)); diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.h b/chrome/browser/media/webrtc/native_desktop_media_list.h index e6f0e17b05ee4..75c9ca04ce481 100644 --- a/chrome/browser/media/webrtc/native_desktop_media_list.h +++ b/chrome/browser/media/webrtc/native_desktop_media_list.h @@ -8,7 +8,7 @@ #include #include "base/memory/weak_ptr.h" -#include "base/sequenced_task_runner.h" +#include "base/threading/thread.h" #include "chrome/browser/media/webrtc/desktop_media_list_base.h" #include "content/public/browser/desktop_media_id.h" #include "ui/gfx/image/image.h" @@ -44,12 +44,7 @@ class NativeDesktopMediaList : public DesktopMediaListBase { gfx::Image image); #endif - // Task runner used for the |worker_|. - scoped_refptr capture_task_runner_; - - // An object that does all the work of getting list of sources on a background - // thread (see |capture_task_runner_|). Destroyed on |capture_task_runner_| - // after the model is destroyed. + base::Thread thread_; std::unique_ptr worker_; #if defined(USE_AURA)