Skip to content

Commit

Permalink
NativeDesktopMediaList: do desktop capture on an exclusive thread
Browse files Browse the repository at this point in the history
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 <braveyao@chromium.org>
Reviewed-by: Julien Isorce <julien.isorce@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588151}
  • Loading branch information
braveyao authored and Commit Bot committed Aug 31, 2018
1 parent f4aa8aa commit 3b9ecca
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 18 deletions.
54 changes: 43 additions & 11 deletions chrome/browser/media/webrtc/native_desktop_media_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -76,11 +79,13 @@ gfx::ImageSkia ScaleDesktopFrame(std::unique_ptr<webrtc::DesktopFrame> frame,
class NativeDesktopMediaList::Worker
: public webrtc::DesktopCapturer::Callback {
public:
Worker(base::WeakPtr<NativeDesktopMediaList> media_list,
Worker(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
base::WeakPtr<NativeDesktopMediaList> media_list,
DesktopMediaID::Type type,
std::unique_ptr<webrtc::DesktopCapturer> capturer);
~Worker() override;

void Start();
void Refresh(const DesktopMediaID::Id& view_dialog_id);

void RefreshThumbnails(const std::vector<DesktopMediaID>& native_ids,
Expand All @@ -93,6 +98,9 @@ class NativeDesktopMediaList::Worker
void OnCaptureResult(webrtc::DesktopCapturer::Result result,
std::unique_ptr<webrtc::DesktopFrame> frame) override;

// Task runner used for capturing operations.
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

base::WeakPtr<NativeDesktopMediaList> media_list_;

DesktopMediaID::Type type_;
Expand All @@ -106,17 +114,27 @@ class NativeDesktopMediaList::Worker
};

NativeDesktopMediaList::Worker::Worker(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
base::WeakPtr<NativeDesktopMediaList> media_list,
DesktopMediaID::Type type,
std::unique_ptr<webrtc::DesktopCapturer> 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<SourceDescription> result;

webrtc::DesktopCapturer::SourceList sources;
Expand Down Expand Up @@ -163,6 +181,7 @@ void NativeDesktopMediaList::Worker::Refresh(
void NativeDesktopMediaList::Worker::RefreshThumbnails(
const std::vector<DesktopMediaID>& native_ids,
const gfx::Size& thumbnail_size) {
DCHECK(task_runner_->BelongsToCurrentThread());
ImageHashesMap new_image_hashes;

// Get a thumbnail for each native source.
Expand Down Expand Up @@ -210,17 +229,30 @@ NativeDesktopMediaList::NativeDesktopMediaList(
std::unique_ptr<webrtc::DesktopCapturer> 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() {
Expand All @@ -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));
Expand Down Expand Up @@ -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_));
Expand Down
9 changes: 2 additions & 7 deletions chrome/browser/media/webrtc/native_desktop_media_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <memory>

#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"
Expand Down Expand Up @@ -44,12 +44,7 @@ class NativeDesktopMediaList : public DesktopMediaListBase {
gfx::Image image);
#endif

// Task runner used for the |worker_|.
scoped_refptr<base::SequencedTaskRunner> 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> worker_;

#if defined(USE_AURA)
Expand Down

0 comments on commit 3b9ecca

Please sign in to comment.