From b0296fd7e97c1d47e10dece698482d8f02ee77d1 Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Thu, 10 Nov 2022 02:16:20 +0100 Subject: [PATCH] chore: cherry-pick ac4785387fff from chromium --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-ac4785387fff.patch | 286 ++++++++++++++++++ 2 files changed, 287 insertions(+) create mode 100644 patches/chromium/cherry-pick-ac4785387fff.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 6dcb26d5c7d86..de03b8eddb691 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -138,3 +138,4 @@ cherry-pick-d5ffb4dd4112.patch cherry-pick-933cc81c6bad.patch cherry-pick-06c87f9f42ff.patch cherry-pick-67c9cbc784d6.patch +cherry-pick-ac4785387fff.patch diff --git a/patches/chromium/cherry-pick-ac4785387fff.patch b/patches/chromium/cherry-pick-ac4785387fff.patch new file mode 100644 index 0000000000000..b333d967b07b4 --- /dev/null +++ b/patches/chromium/cherry-pick-ac4785387fff.patch @@ -0,0 +1,286 @@ +From ac4785387fff2efedef9fe784b5cb98bc1fecd04 Mon Sep 17 00:00:00 2001 +From: Dave Tapuska +Date: Thu, 03 Nov 2022 23:16:16 +0000 +Subject: [PATCH] [m106] Fix PauseOrFreezeOnWorkerThread with nested Worklets. + +Worklets can use the same backing thread which means we can have +nested WorkerThreads paused. If a parent WorkerThread gets deallocated +make sure we don't access anything after it is deallocated once the +nested event queue is released. + +BUG=1372695 + +(cherry picked from commit ff5696ba4bc0f8782e3de40e04685507d9f17fd2) + +Change-Id: I176b8f750da5a41d19d1b3a623944d9a2ed4a441 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3953152 +Commit-Queue: Dave Tapuska +Reviewed-by: Daniel Cheng +Cr-Original-Commit-Position: refs/heads/main@{#1059485} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004562 +Cr-Commit-Position: refs/branch-heads/5249@{#906} +Cr-Branched-From: 4f7bea5de862aaa52e6bde5920755a9ef9db120b-refs/heads/main@{#1036826} +--- + +diff --git a/third_party/blink/renderer/core/workers/threaded_worklet_test.cc b/third_party/blink/renderer/core/workers/threaded_worklet_test.cc +index c54d0a9..dbe40e69 100644 +--- a/third_party/blink/renderer/core/workers/threaded_worklet_test.cc ++++ b/third_party/blink/renderer/core/workers/threaded_worklet_test.cc +@@ -234,6 +234,7 @@ + + private: + friend class ThreadedWorkletTest; ++ FRIEND_TEST_ALL_PREFIXES(ThreadedWorkletTest, NestedRunLoopTermination); + + std::unique_ptr CreateWorkerThread() final { + return std::make_unique(WorkletObjectProxy()); +@@ -280,6 +281,16 @@ + } + Document& GetDocument() { return page_->GetDocument(); } + ++ void WaitForReady(WorkerThread* worker_thread) { ++ base::WaitableEvent child_waitable; ++ PostCrossThreadTask( ++ *worker_thread->GetTaskRunner(TaskType::kInternalTest), FROM_HERE, ++ CrossThreadBindOnce(&base::WaitableEvent::Signal, ++ CrossThreadUnretained(&child_waitable))); ++ ++ child_waitable.Wait(); ++ } ++ + private: + std::unique_ptr page_; + Persistent messaging_proxy_; +@@ -403,4 +414,40 @@ + test::EnterRunLoop(); + } + ++TEST_F(ThreadedWorkletTest, NestedRunLoopTermination) { ++ MessagingProxy()->Start(); ++ ++ ThreadedWorkletMessagingProxyForTest* second_messaging_proxy = ++ MakeGarbageCollected( ++ GetExecutionContext()); ++ ++ // Get a nested event loop where the first one is on the stack ++ // and the second is still alive. ++ second_messaging_proxy->Start(); ++ ++ // Wait until the workers are setup and ready to accept work before we ++ // pause them. ++ WaitForReady(GetWorkerThread()); ++ WaitForReady(second_messaging_proxy->GetWorkerThread()); ++ ++ // Pause the second worker, then the first. ++ second_messaging_proxy->GetWorkerThread()->Pause(); ++ GetWorkerThread()->Pause(); ++ ++ // Resume then terminate the second worker. ++ second_messaging_proxy->GetWorkerThread()->Resume(); ++ second_messaging_proxy->GetWorkerThread()->Terminate(); ++ second_messaging_proxy = nullptr; ++ ++ // Now resume the first worker. ++ GetWorkerThread()->Resume(); ++ ++ // Make sure execution still works without crashing. ++ PostCrossThreadTask( ++ *GetWorkerThread()->GetTaskRunner(TaskType::kInternalTest), FROM_HERE, ++ CrossThreadBindOnce(&ThreadedWorkletThreadForTest::TestTaskRunner, ++ CrossThreadUnretained(GetWorkerThread()))); ++ test::EnterRunLoop(); ++} ++ + } // namespace blink +diff --git a/third_party/blink/renderer/core/workers/worker_thread.cc b/third_party/blink/renderer/core/workers/worker_thread.cc +index 20d6741..156d8d8 100644 +--- a/third_party/blink/renderer/core/workers/worker_thread.cc ++++ b/third_party/blink/renderer/core/workers/worker_thread.cc +@@ -30,7 +30,6 @@ + #include + #include + +-#include "base/auto_reset.h" + #include "base/metrics/histogram_functions.h" + #include "base/synchronization/lock.h" + #include "base/synchronization/waitable_event.h" +@@ -597,6 +596,7 @@ + const absl::optional& thread_startup_data, + std::unique_ptr devtools_params) { + DCHECK(IsCurrentThread()); ++ backing_thread_weak_factory_.emplace(this); + worker_reporting_proxy_.WillInitializeWorkerContext(); + { + TRACE_EVENT0("blink.worker", "WorkerThread::InitializeWorkerContext"); +@@ -738,11 +738,13 @@ + SetThreadState(ThreadState::kReadyToShutdown); + } + ++ backing_thread_weak_factory_ = absl::nullopt; + if (pause_or_freeze_count_ > 0) { + DCHECK(nested_runner_); + pause_or_freeze_count_ = 0; + nested_runner_->QuitNow(); + } ++ pause_handle_.reset(); + + if (WorkerThreadDebugger* debugger = WorkerThreadDebugger::From(GetIsolate())) + debugger->WorkerThreadDestroyed(this); +@@ -887,8 +889,7 @@ + if (pause_or_freeze_count_ > 1) + return; + +- std::unique_ptr pause_handle = +- GetScheduler()->Pause(); ++ pause_handle_ = GetScheduler()->Pause(); + { + // Since the nested message loop runner needs to be created and destroyed on + // the same thread we allocate and destroy a new message loop runner each +@@ -896,13 +897,20 @@ + // the worker thread such that the resume/terminate can quit this runner. + std::unique_ptr nested_runner = + Platform::Current()->CreateNestedMessageLoopRunner(); +- base::AutoReset nested_runner_autoreset( +- &nested_runner_, nested_runner.get()); ++ auto weak_this = backing_thread_weak_factory_->GetWeakPtr(); ++ nested_runner_ = nested_runner.get(); + nested_runner->Run(); ++ ++ // Careful `this` may be destroyed. ++ if (!weak_this) { ++ return; ++ } ++ nested_runner_ = nullptr; + } + GlobalScope()->SetDefersLoadingForResourceFetchers(LoaderFreezeMode::kNone); + GlobalScope()->SetIsInBackForwardCache(false); + GlobalScope()->SetLifecycleState(mojom::blink::FrameLifecycleState::kRunning); ++ pause_handle_.reset(); + } + + void WorkerThread::ResumeOnWorkerThread() { +diff --git a/third_party/blink/renderer/core/workers/worker_thread.h b/third_party/blink/renderer/core/workers/worker_thread.h +index 7254460e..298c36e 100644 +--- a/third_party/blink/renderer/core/workers/worker_thread.h ++++ b/third_party/blink/renderer/core/workers/worker_thread.h +@@ -31,6 +31,7 @@ + + #include "base/gtest_prod_util.h" + #include "base/memory/scoped_refptr.h" ++#include "base/memory/weak_ptr.h" + #include "base/synchronization/lock.h" + #include "base/synchronization/waitable_event.h" + #include "base/task/single_thread_task_runner.h" +@@ -82,7 +83,7 @@ + // abstract class. Multiple WorkerThreads may share one WorkerBackingThread for + // worklets. + // +-// WorkerThread start and termination must be initiated on the main thread and ++// WorkerThread start and termination must be initiated on the parent thread and + // an actual task is executed on the worker thread. + // + // When termination starts, (debugger) tasks on WorkerThread are handled as +@@ -105,7 +106,7 @@ + ~WorkerThread() override; + + // Starts the underlying thread and creates the global scope. Called on the +- // main thread. ++ // parent thread. + // Startup data for WorkerBackingThread is absl::nullopt if |this| doesn't own + // the underlying WorkerBackingThread. + // TODO(nhiroki): We could separate WorkerBackingThread initialization from +@@ -117,14 +118,14 @@ + std::unique_ptr); + + // Posts a task to evaluate a top-level classic script on the worker thread. +- // Called on the main thread after Start(). ++ // Called on the parent thread after Start(). + void EvaluateClassicScript(const KURL& script_url, + const String& source_code, + std::unique_ptr> cached_meta_data, + const v8_inspector::V8StackTraceId& stack_id); + + // Posts a task to fetch and run a top-level classic script on the worker +- // thread. Called on the main thread after Start(). ++ // thread. Called on the parent thread after Start(). + void FetchAndRunClassicScript( + const KURL& script_url, + std::unique_ptr +@@ -136,7 +137,7 @@ + const v8_inspector::V8StackTraceId& stack_id); + + // Posts a task to fetch and run a top-level module script on the worker +- // thread. Called on the main thread after Start(). ++ // thread. Called on the parent thread after Start(). + void FetchAndRunModuleScript( + const KURL& script_url, + std::unique_ptr +@@ -158,7 +159,7 @@ + // Terminates the worker thread. Subclasses of WorkerThread can override this + // to do cleanup. The default behavior is to call Terminate() and + // synchronously call EnsureScriptExecutionTerminates() to ensure the thread +- // is quickly terminated. Called on the main thread. ++ // is quickly terminated. Called on the parent thread. + virtual void TerminateForTesting(); + + // Thread::TaskObserver. +@@ -185,7 +186,7 @@ + void DebuggerTaskStarted(); + void DebuggerTaskFinished(); + +- // Callable on both the main thread and the worker thread. ++ // Callable on both the parent thread and the worker thread. + const base::UnguessableToken& GetDevToolsWorkerToken() const { + return devtools_worker_token_; + } +@@ -330,7 +331,7 @@ + // already shutting down. Does not terminate if a debugger task is running, + // because the debugger task is guaranteed to finish and it heavily uses V8 + // API calls which would crash after forcible script termination. Called on +- // the main thread. ++ // the parent thread. + void EnsureScriptExecutionTerminates(ExitCode) LOCKS_EXCLUDED(lock_); + + // These are called in this order during worker thread startup. +@@ -417,7 +418,7 @@ + // A unique identifier among all WorkerThreads. + const int worker_thread_id_; + +- // Set on the main thread. ++ // Set on the parent thread. + bool requested_to_terminate_ GUARDED_BY(lock_) = false; + + ThreadState thread_state_ GUARDED_BY(lock_) = ThreadState::kNotStarted; +@@ -451,7 +452,7 @@ + TaskTypeTraits>; + TaskRunnerHashMap worker_task_runners_; + +- // This lock protects shared states between the main thread and the worker ++ // This lock protects shared states between the parent thread and the worker + // thread. See thread-safety annotations (e.g., GUARDED_BY) in this header + // file. + base::Lock lock_; +@@ -460,6 +461,10 @@ + // only on the worker thread. + int pause_or_freeze_count_ = 0; + ++ // The `PauseHandle` needs to be destroyed before the scheduler is destroyed ++ // otherwise we will hit a DCHECK. ++ std::unique_ptr pause_handle_; ++ + // A nested message loop for handling pausing. Pointer is not owned. Used only + // on the worker thread. + Platform::NestedMessageLoopRunner* nested_runner_ = nullptr; +@@ -485,6 +490,12 @@ + // a pointer to a member in this list. + HashSet> pending_interrupts_ GUARDED_BY(lock_); + ++ // Since the WorkerThread is allocated and deallocated on the parent thread, ++ // we need a WeakPtrFactory that is allocated and cleared on the backing ++ // thread. ++ absl::optional> ++ backing_thread_weak_factory_; ++ + THREAD_CHECKER(parent_thread_checker_); + }; +