mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
chore: cherry-pick fefd6198da31 from chromium (#35883)
* chore: [20-x-y] cherry-pick fefd6198da31 from chromium * chore: update patches * Update cherry-pick-fefd6198da31.patch * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: electron-patch-conflict-fixer[bot] <83340002+electron-patch-conflict-fixer[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
Jeremy Rose
@@ -125,6 +125,7 @@ cherry-pick-2083e894852c.patch
|
||||
cherry-pick-51daffbf5cd8.patch
|
||||
dpwa_enable_window_controls_overlay_by_default.patch
|
||||
create_browser_v8_snapshot_file_name_fuse.patch
|
||||
cherry-pick-fefd6198da31.patch
|
||||
cherry-pick-1eb1e18ad41d.patch
|
||||
cherry-pick-05a0d99c9715.patch
|
||||
cherry-pick-c83640db21b5.patch
|
||||
|
||||
305
patches/chromium/cherry-pick-fefd6198da31.patch
Normal file
305
patches/chromium/cherry-pick-fefd6198da31.patch
Normal file
@@ -0,0 +1,305 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Ben Wagner <benjaminwagner@google.com>
|
||||
Date: Tue, 20 Sep 2022 18:38:53 +0000
|
||||
Subject: Fix races in FrameQueueUnderlyingSource related to
|
||||
CrossThreadPersistent
|
||||
|
||||
The repro in bug 1323488 unfortunately does not reliably reproduce
|
||||
the races when run as a web test. I was also not able to repro the
|
||||
races in a unit test.
|
||||
|
||||
There are actually three fixes in this CL; it was easiest to fix them
|
||||
all together so that I can run the repro locally for an extended period
|
||||
without it being stopped by a crash.
|
||||
|
||||
The underlying cause for all three races is that CrossThreadPersistent
|
||||
can refer to an object whose heap has already been destroyed. When
|
||||
accessed on the thread corresponding to that heap, there is no race,
|
||||
but when accessed from a different thread, there is a period of time
|
||||
after the heap is destroyed before the CrossThreadPersistent is
|
||||
cleared.
|
||||
|
||||
1. The FrameQueueUnderlyingSource::transferred_source_ member's pointer
|
||||
is accessed in FrameQueueUnderlyingSource::Close. This CL adds a
|
||||
callback to clear the pointer in
|
||||
TransferredFrameQueueUnderlyingSource::ContextDestroyed.
|
||||
|
||||
2. The TransferredFrameQueueUnderlyingSource constructor takes a raw
|
||||
pointer to the original FrameQueueUnderlyingSource, which is
|
||||
associated with a different thread. GC won't be able to update this
|
||||
raw pointer since it's on the wrong stack. This CL changes the raw
|
||||
pointer to a CrossThreadPersistent which is visible to GC.
|
||||
|
||||
3. Same as 2, but for the callstack ConnectHostCallback,
|
||||
MediaStream(Audio|Video)TrackUnderlyingSource::OnSourceTransferStarted
|
||||
and FrameQueueUnderlyingSource::TransferSource.
|
||||
|
||||
(cherry picked from commit 63ce9c40e1a67395278dfc70ecfb545a818747bb)
|
||||
|
||||
Bug: 1323488
|
||||
Change-Id: Id63484eebefd2e003959b25bd752ac8263caab4f
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3865452
|
||||
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
|
||||
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
|
||||
Auto-Submit: Ben Wagner <benjaminwagner@google.com>
|
||||
Cr-Original-Commit-Position: refs/heads/main@{#1041434}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3908010
|
||||
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
|
||||
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
|
||||
Cr-Commit-Position: refs/branch-heads/5249@{#521}
|
||||
Cr-Branched-From: 4f7bea5de862aaa52e6bde5920755a9ef9db120b-refs/heads/main@{#1036826}
|
||||
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.cc b/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.cc
|
||||
index 2cc89e8cd32421bf80b262bac2e4a7cb685db62f..ab41fa086ad9eb8c1cd66db74ea1f05a66ac56a4 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.cc
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.cc
|
||||
@@ -19,10 +19,13 @@ FrameQueueTransferringOptimizer<NativeFrameType>::
|
||||
FrameQueueHost* host,
|
||||
scoped_refptr<base::SequencedTaskRunner> host_runner,
|
||||
wtf_size_t max_queue_size,
|
||||
- ConnectHostCallback connect_host_callback)
|
||||
+ ConnectHostCallback connect_host_callback,
|
||||
+ CrossThreadOnceClosure transferred_source_destroyed_callback)
|
||||
: host_(host),
|
||||
host_runner_(std::move(host_runner)),
|
||||
connect_host_callback_(std::move(connect_host_callback)),
|
||||
+ transferred_source_destroyed_callback_(
|
||||
+ std::move(transferred_source_destroyed_callback)),
|
||||
max_queue_size_(max_queue_size) {}
|
||||
|
||||
template <typename NativeFrameType>
|
||||
@@ -40,7 +43,8 @@ FrameQueueTransferringOptimizer<NativeFrameType>::PerformInProcessOptimization(
|
||||
|
||||
auto* source = MakeGarbageCollected<
|
||||
TransferredFrameQueueUnderlyingSource<NativeFrameType>>(
|
||||
- script_state, host, host_runner_);
|
||||
+ script_state, host, host_runner_,
|
||||
+ std::move(transferred_source_destroyed_callback_));
|
||||
|
||||
PostCrossThreadTask(
|
||||
*host_runner_, FROM_HERE,
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.h b/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.h
|
||||
index 6d33945b7c840e55ae73a1efd1f1cf33ccfaaa71..336073235ba7f6c05cf2cf19fccbfac0be9597c9 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.h
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/frame_queue_transferring_optimizer.h
|
||||
@@ -25,13 +25,15 @@ class FrameQueueTransferringOptimizer final
|
||||
|
||||
using ConnectHostCallback = CrossThreadOnceFunction<void(
|
||||
scoped_refptr<base::SequencedTaskRunner>,
|
||||
- TransferredFrameQueueUnderlyingSource<NativeFrameType>*)>;
|
||||
+ CrossThreadPersistent<
|
||||
+ TransferredFrameQueueUnderlyingSource<NativeFrameType>>)>;
|
||||
|
||||
FrameQueueTransferringOptimizer(
|
||||
FrameQueueHost*,
|
||||
scoped_refptr<base::SequencedTaskRunner> host_runner,
|
||||
wtf_size_t max_queue_size,
|
||||
- ConnectHostCallback callback);
|
||||
+ ConnectHostCallback connect_host_callback,
|
||||
+ CrossThreadOnceFunction<void()> transferred_source_destroyed_callback);
|
||||
~FrameQueueTransferringOptimizer() override = default;
|
||||
|
||||
UnderlyingSourceBase* PerformInProcessOptimization(
|
||||
@@ -41,6 +43,7 @@ class FrameQueueTransferringOptimizer final
|
||||
CrossThreadWeakPersistent<FrameQueueHost> host_;
|
||||
scoped_refptr<base::SequencedTaskRunner> host_runner_;
|
||||
ConnectHostCallback connect_host_callback_;
|
||||
+ CrossThreadOnceFunction<void()> transferred_source_destroyed_callback_;
|
||||
wtf_size_t max_queue_size_;
|
||||
};
|
||||
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.cc b/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.cc
|
||||
index b93b67f131e1e58118e32d2c9b29e472a28d4664..fcd676b3035e19c6e0b28ee1c44109c6737a1c35 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.cc
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.cc
|
||||
@@ -266,15 +266,22 @@ double FrameQueueUnderlyingSource<NativeFrameType>::DesiredSizeForTesting()
|
||||
|
||||
template <typename NativeFrameType>
|
||||
void FrameQueueUnderlyingSource<NativeFrameType>::TransferSource(
|
||||
- FrameQueueUnderlyingSource<NativeFrameType>* transferred_source) {
|
||||
+ CrossThreadPersistent<FrameQueueUnderlyingSource<NativeFrameType>>
|
||||
+ transferred_source) {
|
||||
DCHECK(realm_task_runner_->RunsTasksInCurrentSequence());
|
||||
MutexLocker locker(mutex_);
|
||||
DCHECK(!transferred_source_);
|
||||
- transferred_source_ = transferred_source;
|
||||
+ transferred_source_ = std::move(transferred_source);
|
||||
CloseController();
|
||||
frame_queue_handle_.Invalidate();
|
||||
}
|
||||
|
||||
+template <typename NativeFrameType>
|
||||
+void FrameQueueUnderlyingSource<NativeFrameType>::ClearTransferredSource() {
|
||||
+ MutexLocker locker(mutex_);
|
||||
+ transferred_source_.Clear();
|
||||
+}
|
||||
+
|
||||
template <typename NativeFrameType>
|
||||
void FrameQueueUnderlyingSource<NativeFrameType>::CloseController() {
|
||||
DCHECK(realm_task_runner_->RunsTasksInCurrentSequence());
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.h b/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.h
|
||||
index 7e097b4eb6e9085be73ceb7fd84a4588baf64471..3908909d04f5b6cf4a9be4a3a3e004c3775a3d42 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.h
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/frame_queue_underlying_source.h
|
||||
@@ -85,7 +85,13 @@ class FrameQueueUnderlyingSource
|
||||
// QueueFrame(). |transferred_source| will pull frames from the same circular
|
||||
// queue. Must be called on |realm_task_runner_|.
|
||||
void TransferSource(
|
||||
- FrameQueueUnderlyingSource<NativeFrameType>* transferred_source);
|
||||
+ CrossThreadPersistent<FrameQueueUnderlyingSource<NativeFrameType>>
|
||||
+ transferred_source);
|
||||
+
|
||||
+ // Due to a potential race condition between |transferred_source_|'s heap
|
||||
+ // being destroyed and the Close() method being called, we need to explicitly
|
||||
+ // clear |transferred_source_| when its context is being destroyed.
|
||||
+ void ClearTransferredSource();
|
||||
|
||||
protected:
|
||||
bool MustUseMonitor() const;
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.cc b/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.cc
|
||||
index bfc966452e4134e5133d559698497ce82f89aad4..da9a4bffc3a8c9ec5a1595a7cd78110bfadbfb5e 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.cc
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.cc
|
||||
@@ -93,14 +93,17 @@ MediaStreamAudioTrackUnderlyingSource::GetTransferringOptimizer() {
|
||||
this, GetRealmRunner(), MaxQueueSize(),
|
||||
CrossThreadBindOnce(
|
||||
&MediaStreamAudioTrackUnderlyingSource::OnSourceTransferStarted,
|
||||
+ WrapCrossThreadWeakPersistent(this)),
|
||||
+ CrossThreadBindOnce(
|
||||
+ &MediaStreamAudioTrackUnderlyingSource::ClearTransferredSource,
|
||||
WrapCrossThreadWeakPersistent(this)));
|
||||
}
|
||||
|
||||
void MediaStreamAudioTrackUnderlyingSource::OnSourceTransferStarted(
|
||||
scoped_refptr<base::SequencedTaskRunner> transferred_runner,
|
||||
- TransferredAudioDataQueueUnderlyingSource* source) {
|
||||
+ CrossThreadPersistent<TransferredAudioDataQueueUnderlyingSource> source) {
|
||||
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
|
||||
- TransferSource(source);
|
||||
+ TransferSource(std::move(source));
|
||||
RecordBreakoutBoxUsage(BreakoutBoxUsage::kReadableAudioWorker);
|
||||
}
|
||||
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.h b/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.h
|
||||
index 19a290ea6b83e4b56b8d96cb7632fca55bd65fd0..73c16dc5ff3855e258e1e70397def478485f1481 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.h
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/media_stream_audio_track_underlying_source.h
|
||||
@@ -56,7 +56,7 @@ class MODULES_EXPORT MediaStreamAudioTrackUnderlyingSource
|
||||
void DisconnectFromTrack();
|
||||
void OnSourceTransferStarted(
|
||||
scoped_refptr<base::SequencedTaskRunner> transferred_runner,
|
||||
- TransferredAudioDataQueueUnderlyingSource* source);
|
||||
+ CrossThreadPersistent<TransferredAudioDataQueueUnderlyingSource> source);
|
||||
|
||||
// Only used to prevent the gargabe collector from reclaiming the media
|
||||
// stream track processor that created |this|.
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.cc b/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.cc
|
||||
index bcb941aca4e4d0729dc1253e01ef477fdaae1877..e748bf6eaf8f9d1acf8519c38a5a8cb53b031d0e 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.cc
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.cc
|
||||
@@ -66,6 +66,9 @@ MediaStreamVideoTrackUnderlyingSource::GetStreamTransferOptimizer() {
|
||||
this, GetRealmRunner(), MaxQueueSize(),
|
||||
CrossThreadBindOnce(
|
||||
&MediaStreamVideoTrackUnderlyingSource::OnSourceTransferStarted,
|
||||
+ WrapCrossThreadWeakPersistent(this)),
|
||||
+ CrossThreadBindOnce(
|
||||
+ &MediaStreamVideoTrackUnderlyingSource::ClearTransferredSource,
|
||||
WrapCrossThreadWeakPersistent(this)));
|
||||
}
|
||||
|
||||
@@ -76,9 +79,9 @@ MediaStreamVideoTrackUnderlyingSource::GetIOTaskRunner() {
|
||||
|
||||
void MediaStreamVideoTrackUnderlyingSource::OnSourceTransferStarted(
|
||||
scoped_refptr<base::SequencedTaskRunner> transferred_runner,
|
||||
- TransferredVideoFrameQueueUnderlyingSource* source) {
|
||||
+ CrossThreadPersistent<TransferredVideoFrameQueueUnderlyingSource> source) {
|
||||
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
|
||||
- TransferSource(source);
|
||||
+ TransferSource(std::move(source));
|
||||
RecordBreakoutBoxUsage(BreakoutBoxUsage::kReadableVideoWorker);
|
||||
}
|
||||
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.h b/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.h
|
||||
index 1bbd4f6bbf20eaf168bf1319f1b35819dabe3cce..8964eb5eb83964c4bb2633092a12cb144da4e9b0 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.h
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/media_stream_video_track_underlying_source.h
|
||||
@@ -59,8 +59,9 @@ class MODULES_EXPORT MediaStreamVideoTrackUnderlyingSource
|
||||
bool StartFrameDelivery() override;
|
||||
void StopFrameDelivery() override;
|
||||
|
||||
- void OnSourceTransferStarted(scoped_refptr<base::SequencedTaskRunner>,
|
||||
- TransferredVideoFrameQueueUnderlyingSource*);
|
||||
+ void OnSourceTransferStarted(
|
||||
+ scoped_refptr<base::SequencedTaskRunner>,
|
||||
+ CrossThreadPersistent<TransferredVideoFrameQueueUnderlyingSource>);
|
||||
|
||||
void OnFrameFromTrack(
|
||||
scoped_refptr<media::VideoFrame> media_frame,
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.cc b/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.cc
|
||||
index 5c62b8f1b752c24f8b8d0048d646ea64dba0e49c..e38173482d455788861a1d831dd3422119d4d4d6 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.cc
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.cc
|
||||
@@ -15,11 +15,14 @@ template <typename NativeFrameType>
|
||||
TransferredFrameQueueUnderlyingSource<NativeFrameType>::
|
||||
TransferredFrameQueueUnderlyingSource(
|
||||
ScriptState* script_state,
|
||||
- FrameQueueHost* host,
|
||||
- scoped_refptr<base::SequencedTaskRunner> host_runner)
|
||||
+ CrossThreadPersistent<FrameQueueHost> host,
|
||||
+ scoped_refptr<base::SequencedTaskRunner> host_runner,
|
||||
+ CrossThreadOnceClosure transferred_source_destroyed_callback)
|
||||
: FrameQueueUnderlyingSource<NativeFrameType>(script_state, host),
|
||||
host_runner_(host_runner),
|
||||
- host_(host) {}
|
||||
+ host_(std::move(host)),
|
||||
+ transferred_source_destroyed_callback_(
|
||||
+ std::move(transferred_source_destroyed_callback)) {}
|
||||
|
||||
template <typename NativeFrameType>
|
||||
bool TransferredFrameQueueUnderlyingSource<
|
||||
@@ -44,6 +47,13 @@ void TransferredFrameQueueUnderlyingSource<
|
||||
CrossThreadBindOnce(&FrameQueueHost::Close, host_));
|
||||
}
|
||||
|
||||
+template <typename NativeFrameType>
|
||||
+void TransferredFrameQueueUnderlyingSource<
|
||||
+ NativeFrameType>::ContextDestroyed() {
|
||||
+ std::move(transferred_source_destroyed_callback_).Run();
|
||||
+ FrameQueueUnderlyingSource<NativeFrameType>::ContextDestroyed();
|
||||
+}
|
||||
+
|
||||
template <typename NativeFrameType>
|
||||
void TransferredFrameQueueUnderlyingSource<NativeFrameType>::Trace(
|
||||
Visitor* visitor) const {
|
||||
diff --git a/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.h b/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.h
|
||||
index 5f8a36719407a404756d672530c0b9fcff9d6924..7cd3270fbb3cf5b0e2c09b16000ea5c0e4247866 100644
|
||||
--- a/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.h
|
||||
+++ b/third_party/blink/renderer/modules/breakout_box/transferred_frame_queue_underlying_source.h
|
||||
@@ -19,8 +19,9 @@ class TransferredFrameQueueUnderlyingSource
|
||||
|
||||
TransferredFrameQueueUnderlyingSource(
|
||||
ScriptState*,
|
||||
- FrameQueueHost*,
|
||||
- scoped_refptr<base::SequencedTaskRunner> host_runner);
|
||||
+ CrossThreadPersistent<FrameQueueHost>,
|
||||
+ scoped_refptr<base::SequencedTaskRunner> host_runner,
|
||||
+ CrossThreadOnceClosure transferred_source_destroyed_callback);
|
||||
~TransferredFrameQueueUnderlyingSource() override = default;
|
||||
|
||||
TransferredFrameQueueUnderlyingSource(
|
||||
@@ -32,11 +33,15 @@ class TransferredFrameQueueUnderlyingSource
|
||||
bool StartFrameDelivery() override;
|
||||
void StopFrameDelivery() override;
|
||||
|
||||
+ // ExecutionLifecycleObserver
|
||||
+ void ContextDestroyed() override;
|
||||
+
|
||||
void Trace(Visitor*) const override;
|
||||
|
||||
private:
|
||||
scoped_refptr<base::SequencedTaskRunner> host_runner_;
|
||||
CrossThreadPersistent<FrameQueueHost> host_;
|
||||
+ CrossThreadOnceClosure transferred_source_destroyed_callback_;
|
||||
};
|
||||
|
||||
extern template class MODULES_EXTERN_TEMPLATE_EXPORT
|
||||
Reference in New Issue
Block a user