refactor: desktop capturer module (#14835)

* Revert "post all desktop capturer apis to worker threads"

This reverts commit 5a28759fea.

* refactor: desktop capturer module

* Creates the screen and window capturer for the liftime of the app
* Fixes incorrect usage of weak ptr

* build: add //ui/snapshot to chromium_src deps

* fix: handle scenarios when there are no captured sources
This commit is contained in:
Robo 2018-10-03 17:56:42 +05:30 committed by Cheng Zhao
parent e06bc311a9
commit 596ae2c2df
13 changed files with 465 additions and 829 deletions

View file

@ -469,3 +469,11 @@ patches:
* Fixes a main_application_delegate SDK change
* Fixes a non-null SDK change in a net unittest.
This is needed for Electron to compile with XCode 10.0.
-
author: deepak1556 <hop2deep@gmail.com>
file: desktop_media_list.patch
description: |
* Adds a new observer method to DesktopMediaListObserver for
desktop capture api.
* Backports https://chromium-review.googlesource.com/c/chromium/src/+/1199806
that fixes crash with screen capturer, can be removed in 71.0.3539.0

View file

@ -0,0 +1,304 @@
diff --git a/chrome/browser/media/webrtc/desktop_media_list.h b/chrome/browser/media/webrtc/desktop_media_list.h
index 8e02a8a95eb0..3497b85428a5 100644
--- a/chrome/browser/media/webrtc/desktop_media_list.h
+++ b/chrome/browser/media/webrtc/desktop_media_list.h
@@ -32,6 +32,9 @@ class DesktopMediaList {
virtual ~DesktopMediaList() {}
+ // Allows listening to notifications generated by the model.
+ virtual void AddObserver(DesktopMediaListObserver* observer) = 0;
+
// Sets time interval between updates. By default list of sources and their
// thumbnail are updated once per second. If called after StartUpdating() then
// it will take effect only after the next update.
@@ -51,10 +54,11 @@ class DesktopMediaList {
// enumerated. After the initial enumeration the model will be refreshed based
// on the update period, and notifications generated only for changes in the
// model.
- virtual void StartUpdating(DesktopMediaListObserver* observer) = 0;
+ virtual void StartUpdating() = 0;
virtual int GetSourceCount() const = 0;
virtual const Source& GetSource(int index) const = 0;
+ virtual const std::vector<Source>& GetSources() const = 0;
virtual content::DesktopMediaID::Type GetMediaListType() const = 0;
};
diff --git a/chrome/browser/media/webrtc/desktop_media_list_base.cc b/chrome/browser/media/webrtc/desktop_media_list_base.cc
index 43dd95ef72f5..d4662708f649 100644
--- a/chrome/browser/media/webrtc/desktop_media_list_base.cc
+++ b/chrome/browser/media/webrtc/desktop_media_list_base.cc
@@ -18,6 +18,11 @@ DesktopMediaListBase::DesktopMediaListBase(base::TimeDelta update_period)
DesktopMediaListBase::~DesktopMediaListBase() {}
+void DesktopMediaListBase::AddObserver(DesktopMediaListObserver* observer) {
+ DCHECK(!observer_);
+ observer_ = observer;
+}
+
void DesktopMediaListBase::SetUpdatePeriod(base::TimeDelta period) {
DCHECK(!observer_);
update_period_ = period;
@@ -31,10 +36,7 @@ void DesktopMediaListBase::SetViewDialogWindowId(DesktopMediaID dialog_id) {
view_dialog_id_ = dialog_id;
}
-void DesktopMediaListBase::StartUpdating(DesktopMediaListObserver* observer) {
- DCHECK(!observer_);
-
- observer_ = observer;
+void DesktopMediaListBase::StartUpdating() {
Refresh();
}
@@ -49,6 +51,11 @@ const DesktopMediaList::Source& DesktopMediaListBase::GetSource(
return sources_[index];
}
+const std::vector<DesktopMediaList::Source>& DesktopMediaListBase::GetSources()
+ const {
+ return sources_;
+}
+
DesktopMediaID::Type DesktopMediaListBase::GetMediaListType() const {
return type_;
}
@@ -60,6 +67,12 @@ DesktopMediaListBase::SourceDescription::SourceDescription(
void DesktopMediaListBase::UpdateSourcesList(
const std::vector<SourceDescription>& new_sources) {
+ // Notify observer when there was no new source captured.
+ if (new_sources.empty()) {
+ observer_->OnSourceUnchanged(this);
+ return;
+ }
+
typedef std::set<DesktopMediaID> SourceSet;
SourceSet new_source_set;
for (size_t i = 0; i < new_sources.size(); ++i) {
@@ -132,6 +145,8 @@ void DesktopMediaListBase::UpdateSourceThumbnail(DesktopMediaID id,
}
void DesktopMediaListBase::ScheduleNextRefresh() {
+ if (!observer_->ShouldScheduleNextRefresh(this))
+ return;
BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
base::BindOnce(&DesktopMediaListBase::Refresh,
weak_factory_.GetWeakPtr()),
diff --git a/chrome/browser/media/webrtc/desktop_media_list_base.h b/chrome/browser/media/webrtc/desktop_media_list_base.h
index 746df1210aa9..461e0edf8509 100644
--- a/chrome/browser/media/webrtc/desktop_media_list_base.h
+++ b/chrome/browser/media/webrtc/desktop_media_list_base.h
@@ -24,12 +24,14 @@ class DesktopMediaListBase : public DesktopMediaList {
~DesktopMediaListBase() override;
// DesktopMediaList interface.
+ void AddObserver(DesktopMediaListObserver* observer) override;
void SetUpdatePeriod(base::TimeDelta period) override;
void SetThumbnailSize(const gfx::Size& thumbnail_size) override;
void SetViewDialogWindowId(content::DesktopMediaID dialog_id) override;
- void StartUpdating(DesktopMediaListObserver* observer) override;
+ void StartUpdating() override;
int GetSourceCount() const override;
const Source& GetSource(int index) const override;
+ const std::vector<Source>& GetSources() const override;
content::DesktopMediaID::Type GetMediaListType() const override;
static uint32_t GetImageHash(const gfx::Image& image);
diff --git a/chrome/browser/media/webrtc/desktop_media_list_observer.h b/chrome/browser/media/webrtc/desktop_media_list_observer.h
index 47401abc984e..ca6a527ffac8 100644
--- a/chrome/browser/media/webrtc/desktop_media_list_observer.h
+++ b/chrome/browser/media/webrtc/desktop_media_list_observer.h
@@ -18,6 +18,10 @@ class DesktopMediaListObserver {
int new_index) = 0;
virtual void OnSourceNameChanged(DesktopMediaList* list, int index) = 0;
virtual void OnSourceThumbnailChanged(DesktopMediaList* list, int index) = 0;
+ virtual void OnSourceUnchanged(DesktopMediaList* list) = 0;
+ // Return value indicates whether the observer should continue listening
+ // for capture updates.
+ virtual bool ShouldScheduleNextRefresh(DesktopMediaList* list) = 0;
protected:
virtual ~DesktopMediaListObserver() {}
diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chrome/browser/media/webrtc/native_desktop_media_list.cc
index 0c9ba953cb3c..5a2d853aeeac 100644
--- a/chrome/browser/media/webrtc/native_desktop_media_list.cc
+++ b/chrome/browser/media/webrtc/native_desktop_media_list.cc
@@ -5,10 +5,16 @@
#include "chrome/browser/media/webrtc/native_desktop_media_list.h"
#include "base/hash.h"
+#include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task_scheduler/post_task.h"
+#include "base/threading/thread_restrictions.h"
+#include "build/build_config.h"
#include "chrome/browser/media/webrtc/desktop_media_list_observer.h"
+#if 0
#include "chrome/grit/generated_resources.h"
+#endif
#include "content/public/browser/browser_thread.h"
#include "media/base/video_util.h"
#include "third_party/libyuv/include/libyuv/scale_argb.h"
@@ -76,11 +82,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,
@@ -93,6 +101,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_;
@@ -106,17 +117,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;
@@ -133,11 +154,8 @@ void NativeDesktopMediaList::Worker::Refresh(
// Just in case 'Screen' is inflected depending on the screen number,
// use plural formatter.
title = mutiple_sources
- ? l10n_util::GetPluralStringFUTF16(
- IDS_DESKTOP_MEDIA_PICKER_MULTIPLE_SCREEN_NAME,
- static_cast<int>(i + 1))
- : l10n_util::GetStringUTF16(
- IDS_DESKTOP_MEDIA_PICKER_SINGLE_SCREEN_NAME);
+ ? base::UTF8ToUTF16("Screen " + base::IntToString(i + 1))
+ : base::UTF8ToUTF16("Entire screen");
break;
case DesktopMediaID::TYPE_WINDOW:
@@ -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.
@@ -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() {
@@ -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 e6f0e17b05ee..75c9ca04ce48 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 <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"
@@ -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)

View file

@ -1,13 +1,5 @@
repo: src/third_party/webrtc
patches:
-
author: null
file: webrtc-desktop_capturer_mac.patch
description: null
-
author: null
file: webrtc-rwlock_null.patch
description: null
-
author: Nitish Sakhawalkar <nitsakh@icloud.com>
file: disable-warning-win.patch

View file

@ -1,117 +0,0 @@
diff --git a/modules/desktop_capture/mac/screen_capturer_mac.mm b/modules/desktop_capture/mac/screen_capturer_mac.mm
index df18777226..6a1e3da6ab 100644
--- a/modules/desktop_capture/mac/screen_capturer_mac.mm
+++ b/modules/desktop_capture/mac/screen_capturer_mac.mm
@@ -16,6 +16,7 @@
#include "rtc_base/checks.h"
#include "rtc_base/constructormagic.h"
#include "rtc_base/logging.h"
+#include "rtc_base/synchronization/rw_lock_wrapper.h"
#include "rtc_base/timeutils.h"
#include "sdk/objc/Framework/Classes/Common/scoped_cftyperef.h"
@@ -28,18 +29,31 @@ namespace webrtc {
// destroy itself once it's done.
class DisplayStreamManager {
public:
- int GetUniqueId() { return ++unique_id_generator_; }
- void DestroyStream(int unique_id) {
- auto it = display_stream_wrappers_.find(unique_id);
- RTC_CHECK(it != display_stream_wrappers_.end());
- RTC_CHECK(!it->second.active);
- CFRelease(it->second.stream);
- display_stream_wrappers_.erase(it);
+ DisplayStreamManager() : rw_lock_(RWLockWrapper::CreateRWLock()) {}
+ RWLockWrapper* GetLock() {return rw_lock_.get();};
- if (ready_for_self_destruction_ && display_stream_wrappers_.empty()) delete this;
+ int GetUniqueId() {
+ WriteLockScoped scoped_display_stream_manager_lock(*rw_lock_);
+ return ++unique_id_generator_;
+ }
+ void DestroyStream(int unique_id) {
+ bool finalize;
+ {
+ WriteLockScoped scoped_display_stream_manager_lock(*rw_lock_);
+ auto it = display_stream_wrappers_.find(unique_id);
+ RTC_CHECK(it != display_stream_wrappers_.end());
+ RTC_CHECK(!it->second.active);
+ CFRelease(it->second.stream);
+ display_stream_wrappers_.erase(it);
+ finalize = ready_for_self_destruction_ && display_stream_wrappers_.empty();
+ }
+ if (finalize) {
+ delete this;
+ }
}
void SaveStream(int unique_id, CGDisplayStreamRef stream) {
+ WriteLockScoped scoped_display_stream_manager_lock(*rw_lock_);
RTC_CHECK(unique_id <= unique_id_generator_);
DisplayStreamWrapper wrapper;
wrapper.stream = stream;
@@ -47,6 +61,7 @@ class DisplayStreamManager {
}
void UnregisterActiveStreams() {
+ WriteLockScoped scoped_display_stream_manager_lock(*rw_lock_);
for (auto& pair : display_stream_wrappers_) {
DisplayStreamWrapper& wrapper = pair.second;
if (wrapper.active) {
@@ -61,12 +76,23 @@ class DisplayStreamManager {
void PrepareForSelfDestruction() {
ready_for_self_destruction_ = true;
- if (display_stream_wrappers_.empty()) delete this;
+ bool finalize;
+ {
+ WriteLockScoped scoped_display_stream_manager_lock(*rw_lock_);
+ ready_for_self_destruction_ = true;
+ finalize = display_stream_wrappers_.empty();
+ }
+ if (finalize) {
+ delete this;
+ }
}
// Once the DisplayStreamManager is ready for destruction, the
// ScreenCapturerMac is no longer present. Any updates should be ignored.
- bool ShouldIgnoreUpdates() { return ready_for_self_destruction_; }
+ // Note: not thread-safe! Acquire and release a lock manually.
+ bool ShouldIgnoreUpdates() {
+ return ready_for_self_destruction_;
+ }
private:
struct DisplayStreamWrapper {
@@ -81,6 +107,7 @@ class DisplayStreamManager {
std::map<int, DisplayStreamWrapper> display_stream_wrappers_;
int unique_id_generator_ = 0;
bool ready_for_self_destruction_ = false;
+ std::unique_ptr<RWLockWrapper> rw_lock_;
};
namespace {
@@ -507,8 +534,6 @@ bool ScreenCapturerMac::RegisterRefreshAndMoveHandlers() {
return;
}
- if (manager->ShouldIgnoreUpdates()) return;
-
// Only pay attention to frame updates.
if (status != kCGDisplayStreamFrameStatusFrameComplete) return;
@@ -518,7 +543,12 @@ bool ScreenCapturerMac::RegisterRefreshAndMoveHandlers() {
if (count != 0) {
// According to CGDisplayStream.h, it's safe to call
// CGDisplayStreamStop() from within the callback.
- ScreenRefresh(count, rects, display_origin);
+ manager->GetLock()->AcquireLockShared();
+ bool screen_capturer_mac_invalidated = manager->ShouldIgnoreUpdates();
+ if (!screen_capturer_mac_invalidated) {
+ ScreenRefresh(count, rects, display_origin);
+ }
+ manager->GetLock()->ReleaseLockShared();
}
};

View file

@ -1,36 +0,0 @@
diff --git a/rtc_base/synchronization/rw_lock_wrapper.cc b/rtc_base/synchronization/rw_lock_wrapper.cc
index c8cd17edb8..50c6e25ad9 100644
--- a/rtc_base/synchronization/rw_lock_wrapper.cc
+++ b/rtc_base/synchronization/rw_lock_wrapper.cc
@@ -11,6 +11,9 @@
#include "rtc_base/synchronization/rw_lock_wrapper.h"
#include <assert.h>
+#include <stdlib.h>
+
+#include "system_wrappers/include/sleep.h"
#if defined(_WIN32)
#include "rtc_base/synchronization/rw_lock_win.h"
@@ -21,11 +23,19 @@
namespace webrtc {
RWLockWrapper* RWLockWrapper::CreateRWLock() {
+ RWLockWrapper* rw_lock_ptr;
#ifdef _WIN32
- return RWLockWin::Create();
+ rw_lock_ptr = RWLockWin::Create();
#else
- return RWLockPosix::Create();
+ rw_lock_ptr = RWLockPosix::Create();
#endif
+ if (rw_lock_ptr != NULL) {
+ return rw_lock_ptr;
+ } else {
+ int msec_wait = 10 + (rand() % 90);
+ SleepMs(msec_wait);
+ return CreateRWLock();
+ }
}
} // namespace webrtc