Skip to content

Commit 9c00fb1

Browse files
author
braveyao
committed
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 <[email protected]> Reviewed-by: Julien Isorce <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#588151}(cherry picked from commit 3b9ecca) Reviewed-on: https://chromium-review.googlesource.com/1207092 Reviewed-by: Weiyong Yao <[email protected]> Cr-Commit-Position: refs/branch-heads/3538@{#52} Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
1 parent fb71399 commit 9c00fb1

File tree

2 files changed

+45
-18
lines changed

2 files changed

+45
-18
lines changed

chrome/browser/media/webrtc/native_desktop_media_list.cc

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
#include "chrome/browser/media/webrtc/native_desktop_media_list.h"
66

77
#include "base/hash.h"
8+
#include "base/single_thread_task_runner.h"
89
#include "base/strings/utf_string_conversions.h"
910
#include "base/task/post_task.h"
11+
#include "base/threading/thread_restrictions.h"
12+
#include "build/build_config.h"
1013
#include "chrome/browser/media/webrtc/desktop_media_list_observer.h"
1114
#include "chrome/grit/generated_resources.h"
1215
#include "content/public/browser/browser_thread.h"
@@ -76,11 +79,13 @@ gfx::ImageSkia ScaleDesktopFrame(std::unique_ptr<webrtc::DesktopFrame> frame,
7679
class NativeDesktopMediaList::Worker
7780
: public webrtc::DesktopCapturer::Callback {
7881
public:
79-
Worker(base::WeakPtr<NativeDesktopMediaList> media_list,
82+
Worker(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
83+
base::WeakPtr<NativeDesktopMediaList> media_list,
8084
DesktopMediaID::Type type,
8185
std::unique_ptr<webrtc::DesktopCapturer> capturer);
8286
~Worker() override;
8387

88+
void Start();
8489
void Refresh(const DesktopMediaID::Id& view_dialog_id);
8590

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

101+
// Task runner used for capturing operations.
102+
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
103+
96104
base::WeakPtr<NativeDesktopMediaList> media_list_;
97105

98106
DesktopMediaID::Type type_;
@@ -106,17 +114,27 @@ class NativeDesktopMediaList::Worker
106114
};
107115

108116
NativeDesktopMediaList::Worker::Worker(
117+
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
109118
base::WeakPtr<NativeDesktopMediaList> media_list,
110119
DesktopMediaID::Type type,
111120
std::unique_ptr<webrtc::DesktopCapturer> capturer)
112-
: media_list_(media_list), type_(type), capturer_(std::move(capturer)) {
113-
capturer_->Start(this);
121+
: task_runner_(task_runner),
122+
media_list_(media_list),
123+
type_(type),
124+
capturer_(std::move(capturer)) {}
125+
126+
NativeDesktopMediaList::Worker::~Worker() {
127+
DCHECK(task_runner_->BelongsToCurrentThread());
114128
}
115129

116-
NativeDesktopMediaList::Worker::~Worker() {}
130+
void NativeDesktopMediaList::Worker::Start() {
131+
DCHECK(task_runner_->BelongsToCurrentThread());
132+
capturer_->Start(this);
133+
}
117134

118135
void NativeDesktopMediaList::Worker::Refresh(
119136
const DesktopMediaID::Id& view_dialog_id) {
137+
DCHECK(task_runner_->BelongsToCurrentThread());
120138
std::vector<SourceDescription> result;
121139

122140
webrtc::DesktopCapturer::SourceList sources;
@@ -163,6 +181,7 @@ void NativeDesktopMediaList::Worker::Refresh(
163181
void NativeDesktopMediaList::Worker::RefreshThumbnails(
164182
const std::vector<DesktopMediaID>& native_ids,
165183
const gfx::Size& thumbnail_size) {
184+
DCHECK(task_runner_->BelongsToCurrentThread());
166185
ImageHashesMap new_image_hashes;
167186

168187
// Get a thumbnail for each native source.
@@ -210,17 +229,30 @@ NativeDesktopMediaList::NativeDesktopMediaList(
210229
std::unique_ptr<webrtc::DesktopCapturer> capturer)
211230
: DesktopMediaListBase(base::TimeDelta::FromMilliseconds(
212231
kDefaultNativeDesktopMediaListUpdatePeriod)),
232+
thread_("DesktopMediaListCaptureThread"),
213233
weak_factory_(this) {
214234
type_ = type;
215-
capture_task_runner_ = base::CreateSequencedTaskRunnerWithTraits(
216-
{base::MayBlock(), base::TaskPriority::USER_VISIBLE});
217235

218-
worker_.reset(
219-
new Worker(weak_factory_.GetWeakPtr(), type, std::move(capturer)));
236+
#if defined(OS_WIN) || defined(OS_MACOSX)
237+
// On Windows/OSX the thread must be a UI thread.
238+
base::MessageLoop::Type thread_type = base::MessageLoop::TYPE_UI;
239+
#else
240+
base::MessageLoop::Type thread_type = base::MessageLoop::TYPE_DEFAULT;
241+
#endif
242+
thread_.StartWithOptions(base::Thread::Options(thread_type, 0));
243+
244+
worker_.reset(new Worker(thread_.task_runner(), weak_factory_.GetWeakPtr(),
245+
type, std::move(capturer)));
246+
247+
thread_.task_runner()->PostTask(
248+
FROM_HERE,
249+
base::BindOnce(&Worker::Start, base::Unretained(worker_.get())));
220250
}
221251

222252
NativeDesktopMediaList::~NativeDesktopMediaList() {
223-
capture_task_runner_->DeleteSoon(FROM_HERE, worker_.release());
253+
base::ThreadRestrictions::ScopedAllowIO allow_io;
254+
thread_.task_runner()->DeleteSoon(FROM_HERE, worker_.release());
255+
thread_.Stop();
224256
}
225257

226258
void NativeDesktopMediaList::Refresh() {
@@ -230,7 +262,7 @@ void NativeDesktopMediaList::Refresh() {
230262
new_aura_thumbnail_hashes_.clear();
231263
#endif
232264

233-
capture_task_runner_->PostTask(
265+
thread_.task_runner()->PostTask(
234266
FROM_HERE,
235267
base::BindOnce(&Worker::Refresh, base::Unretained(worker_.get()),
236268
view_dialog_id_.id));
@@ -280,7 +312,7 @@ void NativeDesktopMediaList::RefreshForAuraWindows(
280312
#if defined(USE_AURA)
281313
pending_native_thumbnail_capture_ = true;
282314
#endif
283-
capture_task_runner_->PostTask(
315+
thread_.task_runner()->PostTask(
284316
FROM_HERE, base::BindOnce(&Worker::RefreshThumbnails,
285317
base::Unretained(worker_.get()), native_ids,
286318
thumbnail_size_));

chrome/browser/media/webrtc/native_desktop_media_list.h

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include <memory>
99

1010
#include "base/memory/weak_ptr.h"
11-
#include "base/sequenced_task_runner.h"
11+
#include "base/threading/thread.h"
1212
#include "chrome/browser/media/webrtc/desktop_media_list_base.h"
1313
#include "content/public/browser/desktop_media_id.h"
1414
#include "ui/gfx/image/image.h"
@@ -44,12 +44,7 @@ class NativeDesktopMediaList : public DesktopMediaListBase {
4444
gfx::Image image);
4545
#endif
4646

47-
// Task runner used for the |worker_|.
48-
scoped_refptr<base::SequencedTaskRunner> capture_task_runner_;
49-
50-
// An object that does all the work of getting list of sources on a background
51-
// thread (see |capture_task_runner_|). Destroyed on |capture_task_runner_|
52-
// after the model is destroyed.
47+
base::Thread thread_;
5348
std::unique_ptr<Worker> worker_;
5449

5550
#if defined(USE_AURA)

0 commit comments

Comments
 (0)