chore: cherry-pick 2 changes from 1-M137 (#47368)

* chore: cherry-pick 45eb42cd398e from v8

* chore: cherry-pick f1e6422a355c from chromium
This commit is contained in:
Keeley Hammond 2025-06-05 10:00:41 +02:00 committed by GitHub
commit 62fbb1d67a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 307 additions and 0 deletions

View file

@ -149,3 +149,4 @@ mac_fix_check_on_ime_reconversion_due_to_invalid_replacement_range.patch
fix_osr_stutter_fix_backport_for_electron.patch fix_osr_stutter_fix_backport_for_electron.patch
do_not_check_the_order_of_display_id_order_on_windows.patch do_not_check_the_order_of_display_id_order_on_windows.patch
make_focus_methods_in_webcontentsviewchildframe_notimplemented.patch make_focus_methods_in_webcontentsviewchildframe_notimplemented.patch
cherry-pick-f1e6422a355c.patch

View file

@ -0,0 +1,273 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Wed, 21 May 2025 23:25:12 -0700
Subject: Enforce SharedWorker::Terminate() procedure order
During the investigation of crbug.com/409059706, we observed that
PerformShutdownOnWorkerThread() is called during the status is
running.
I suppose the root cause is race condition between `Terminate()`
procedure and a child process termination procedure in different
thread. WorkerThread can be terminated if two conditions are met;
`Terminate()` is called and all child worker threads have been
terminated. Both `Terminate()` and the child process termination
procedure may call `PerformShutdownOnWorkerThread()`, and former
is executed regardless of two conditions are met. The latter
is called if `Terminate()` is called and no child processes.
To be clear, "`Terminate()` is called" does not mean
`PrepareForShutdownOnWorkerThread()` is executed. `Terminate()`
queues it after the flag to tell `Terminate()` call. And, when
the issue happen, I am quite sure the flag is set but,
`PrepareForShutdownOnWorkerThread()` won't be executed yet.
The fix is that:
1. The "Terminate() is called" flag to be multi staged.
The flag is used for two purpose; a. avoid re-enter of
`Terminate()`, and b. `PrepareForShutdownOnWorkerThread()` is
in flight. The CL changed the flag to enum to represent
the stage properly.
2. `PerformShutdownOnWorkerThread()` is queued even if it is
called within the child process termination procedure.
It avoid the execution order flip between
`PrepareForShutdownOnWorkerThread()` and
`PerformShutdownOnWorkerThread()`.
In addition, this change ensures `PerformShutdownOnWorkerThread()`
is called once. While `PerformShutdownOnWorkerThread()` touches
fields inside, the fields must not be touched at some point within
the function, the function is actually not re-entrant when it reaches
to the end. Upon mikt@ suggestion, I made
`PerformShutdownOnWorkerThread()` is called only when two conditions
are fulfilled. i.e. `Terminate()` is called and the number of child
threads is 0. Also, the CL uses the enum to show
`PerformShutdownOnWorkerThread()` is in-flight to avoid re-entrance
in this level.
Bug: 409059706
Change-Id: I81a1c3b1a34e827fa75ec2d1a9b37023965dbe27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6543412
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1463892}
diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc
index 720c38313bd132a12b8465a4a2ca1bc32cf6f569..c2c8ee137a4b3a500954dcc9eaf3dea003613876 100644
--- a/third_party/blink/common/features.cc
+++ b/third_party/blink/common/features.cc
@@ -2828,6 +2828,13 @@ BASE_FEATURE(kWebviewAccelerateSmallCanvases,
"WebviewAccelerateSmallCanvases",
base::FEATURE_DISABLED_BY_DEFAULT);
+// WorkerThread termination procedure (prepare and shutdown) runs sequentially
+// in the same task without calling another cross thread post task.
+// Kill switch for crbug.com/409059706.
+BASE_FEATURE(kWorkerThreadSequentialShutdown,
+ "WorkerThreadSequentialShutdown",
+ base::FEATURE_ENABLED_BY_DEFAULT);
+
BASE_FEATURE(kNoReferrerForPreloadFromSubresource,
"NoReferrerForPreloadFromSubresource",
base::FEATURE_ENABLED_BY_DEFAULT);
diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h
index 9970347ef0ccb1801bbdd9e3ff904ae7591567c6..1cd31c80405bac33b11749587e5b05c0fd461856 100644
--- a/third_party/blink/public/common/features.h
+++ b/third_party/blink/public/common/features.h
@@ -1844,6 +1844,8 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWebUSBTransferSizeLimit);
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWebviewAccelerateSmallCanvases);
+BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kWorkerThreadSequentialShutdown);
+
// Kill switch for https://crbug.com/415810136.
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kNoReferrerForPreloadFromSubresource);
diff --git a/third_party/blink/renderer/core/workers/worker_thread.cc b/third_party/blink/renderer/core/workers/worker_thread.cc
index 9b2878bc23c78f092816524608776dd32fbde5a1..6d1f954733fbc574bcb1fda229fd30e7303fb9aa 100644
--- a/third_party/blink/renderer/core/workers/worker_thread.cc
+++ b/third_party/blink/renderer/core/workers/worker_thread.cc
@@ -264,9 +264,10 @@ void WorkerThread::Terminate() {
DCHECK_CALLED_ON_VALID_THREAD(parent_thread_checker_);
{
base::AutoLock locker(lock_);
- if (requested_to_terminate_)
+ if (termination_progress_ != TerminationProgress::kNotRequested) {
return;
- requested_to_terminate_ = true;
+ }
+ termination_progress_ = TerminationProgress::kRequested;
}
// Schedule a task to forcibly terminate the script execution in case that the
@@ -282,10 +283,33 @@ void WorkerThread::Terminate() {
*task_runner, FROM_HERE,
CrossThreadBindOnce(&WorkerThread::PrepareForShutdownOnWorkerThread,
CrossThreadUnretained(this)));
- PostCrossThreadTask(
- *task_runner, FROM_HERE,
- CrossThreadBindOnce(&WorkerThread::PerformShutdownOnWorkerThread,
- CrossThreadUnretained(this)));
+
+ if (!base::FeatureList::IsEnabled(
+ blink::features::kWorkerThreadSequentialShutdown)) {
+ PostCrossThreadTask(
+ *task_runner, FROM_HERE,
+ CrossThreadBindOnce(&WorkerThread::PerformShutdownOnWorkerThread,
+ CrossThreadUnretained(this)));
+ return;
+ }
+
+ bool perform_shutdown = false;
+ {
+ base::AutoLock locker(lock_);
+ CHECK_EQ(TerminationProgress::kRequested, termination_progress_);
+ termination_progress_ = TerminationProgress::kPrepared;
+ if (num_child_threads_ == 0) {
+ termination_progress_ = TerminationProgress::kPerforming;
+ perform_shutdown = true;
+ }
+ }
+
+ if (perform_shutdown) {
+ PostCrossThreadTask(
+ *task_runner, FROM_HERE,
+ CrossThreadBindOnce(&WorkerThread::PerformShutdownOnWorkerThread,
+ CrossThreadUnretained(this)));
+ }
}
void WorkerThread::TerminateForTesting() {
@@ -422,20 +446,48 @@ scoped_refptr<base::SingleThreadTaskRunner> WorkerThread::GetTaskRunner(
void WorkerThread::ChildThreadStartedOnWorkerThread(WorkerThread* child) {
DCHECK(IsCurrentThread());
-#if DCHECK_IS_ON()
+ child_threads_.insert(child);
{
base::AutoLock locker(lock_);
DCHECK_EQ(ThreadState::kRunning, thread_state_);
+ CHECK_EQ(TerminationProgress::kNotRequested, termination_progress_);
+ if (base::FeatureList::IsEnabled(
+ blink::features::kWorkerThreadSequentialShutdown)) {
+ ++num_child_threads_;
+ CHECK_EQ(child_threads_.size(), num_child_threads_);
+ }
}
-#endif
- child_threads_.insert(child);
}
void WorkerThread::ChildThreadTerminatedOnWorkerThread(WorkerThread* child) {
DCHECK(IsCurrentThread());
child_threads_.erase(child);
- if (child_threads_.empty() && CheckRequestedToTerminate())
- PerformShutdownOnWorkerThread();
+ if (!base::FeatureList::IsEnabled(
+ blink::features::kWorkerThreadSequentialShutdown)) {
+ if (child_threads_.empty() && CheckRequestedToTerminate()) {
+ PerformShutdownOnWorkerThread();
+ }
+ return;
+ }
+
+ bool perform_shutdown = false;
+ {
+ base::AutoLock locker(lock_);
+ --num_child_threads_;
+ CHECK_EQ(child_threads_.size(), num_child_threads_);
+ if (num_child_threads_ == 0 &&
+ termination_progress_ == TerminationProgress::kPrepared) {
+ termination_progress_ = TerminationProgress::kPerforming;
+ perform_shutdown = true;
+ }
+ }
+ if (perform_shutdown) {
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner =
+ GetWorkerBackingThread().BackingThread().GetTaskRunner();
+ GetWorkerBackingThread().BackingThread().GetTaskRunner()->PostTask(
+ FROM_HERE, WTF::BindOnce(&WorkerThread::PerformShutdownOnWorkerThread,
+ WTF::Unretained(this)));
+ }
}
WorkerThread::WorkerThread(WorkerReportingProxy& worker_reporting_proxy)
@@ -781,18 +833,32 @@ void WorkerThread::PerformShutdownOnWorkerThread() {
DCHECK(IsCurrentThread());
{
base::AutoLock locker(lock_);
- DCHECK(requested_to_terminate_);
+ if (!base::FeatureList::IsEnabled(
+ blink::features::kWorkerThreadSequentialShutdown)) {
+ DCHECK_NE(TerminationProgress::kNotRequested, termination_progress_);
+ } else {
+ DCHECK_EQ(TerminationProgress::kPerforming, termination_progress_);
+ }
DCHECK_EQ(ThreadState::kReadyToShutdown, thread_state_);
if (exit_code_ == ExitCode::kNotTerminated)
SetExitCode(ExitCode::kGracefullyTerminated);
}
- // When child workers are present, wait for them to shutdown before shutting
- // down this thread. ChildThreadTerminatedOnWorkerThread() is responsible
- // for completing shutdown on the worker thread after the last child shuts
- // down.
- if (!child_threads_.empty())
- return;
+ if (!base::FeatureList::IsEnabled(
+ blink::features::kWorkerThreadSequentialShutdown)) {
+ // When child workers are present, wait for them to shutdown before shutting
+ // down this thread. ChildThreadTerminatedOnWorkerThread() is responsible
+ // for completing shutdown on the worker thread after the last child shuts
+ // down.
+ if (!child_threads_.empty()) {
+ return;
+ }
+ } else {
+ // Child workers must not exist when `PerformShutdownOnWorkerThread()`
+ // is called because it has already been checked before calling the
+ // function.
+ CHECK(child_threads_.empty());
+ }
inspector_task_runner_->Dispose();
if (worker_inspector_controller_) {
@@ -848,7 +914,7 @@ void WorkerThread::SetExitCode(ExitCode exit_code) {
bool WorkerThread::CheckRequestedToTerminate() {
base::AutoLock locker(lock_);
- return requested_to_terminate_;
+ return termination_progress_ != TerminationProgress::kNotRequested;
}
void WorkerThread::PauseOrFreeze(mojom::blink::FrameLifecycleState state,
diff --git a/third_party/blink/renderer/core/workers/worker_thread.h b/third_party/blink/renderer/core/workers/worker_thread.h
index 48174fc364fd6d98f90b5a99a4ae403602691a52..e0423486b891d4d8638f455a01d07adc5b10b25b 100644
--- a/third_party/blink/renderer/core/workers/worker_thread.h
+++ b/third_party/blink/renderer/core/workers/worker_thread.h
@@ -321,6 +321,13 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
kTerminationUnnecessary,
};
+ enum class TerminationProgress {
+ kNotRequested,
+ kRequested,
+ kPrepared,
+ kPerforming,
+ };
+
// Returns true if we should synchronously terminate the script execution so
// that a shutdown task can be handled by the thread event loop.
TerminationState ShouldTerminateScriptExecution()
@@ -422,8 +429,10 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
// A unique identifier among all WorkerThreads.
const int worker_thread_id_;
- // Set on the parent thread.
- bool requested_to_terminate_ GUARDED_BY(lock_) = false;
+ // Represents progress after the Terminate() call.
+ TerminationProgress termination_progress_ GUARDED_BY(lock_) =
+ TerminationProgress::kNotRequested;
+ size_t num_child_threads_ GUARDED_BY(lock_) = 0;
ThreadState thread_state_ GUARDED_BY(lock_) = ThreadState::kNotStarted;
ExitCode exit_code_ GUARDED_BY(lock_) = ExitCode::kNotTerminated;

View file

@ -1,3 +1,4 @@
chore_allow_customizing_microtask_policy_per_context.patch chore_allow_customizing_microtask_policy_per_context.patch
enable_--perf-prof_flag_on_macos.patch enable_--perf-prof_flag_on_macos.patch
cherry-pick-7bc0a67ebfbf.patch cherry-pick-7bc0a67ebfbf.patch
cherry-pick-45eb42cd398e.patch

View file

@ -0,0 +1,32 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Igor Sheludko <ishell@chromium.org>
Date: Tue, 27 May 2025 21:34:45 +0200
Subject: Convert Smi to Word64 using zero extension
... when a known type range contains only positive values.
Bug: 420637585
Change-Id: I8d9bb3f2fe2e5268e1659bb4ea7bbf97bfb52288
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6594731
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100538}
diff --git a/src/compiler/representation-change.cc b/src/compiler/representation-change.cc
index f0d4f59c7625103a3fed789bfc1d08c407e00c08..8ce36e6a0f4a4a119a35c61dedcb22d2f55fa982 100644
--- a/src/compiler/representation-change.cc
+++ b/src/compiler/representation-change.cc
@@ -1323,7 +1323,12 @@ Node* RepresentationChanger::GetWord64RepresentationFor(
}
} else if (output_rep == MachineRepresentation::kTaggedSigned) {
if (output_type.Is(Type::SignedSmall())) {
- op = simplified()->ChangeTaggedSignedToInt64();
+ if (output_type.IsRange() && output_type.AsRange()->Min() >= 0) {
+ node = InsertChangeTaggedSignedToInt32(node);
+ op = machine()->ChangeUint32ToUint64();
+ } else {
+ op = simplified()->ChangeTaggedSignedToInt64();
+ }
} else {
return TypeError(node, output_rep, output_type,
MachineRepresentation::kWord64);