From 986c1d5f240b1150b9c3fba8b14ddac99a185e1d Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Fri, 13 May 2022 19:16:26 +0200 Subject: [PATCH 1/2] chore: cherry-pick ec0cce63f47d from chromium --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-ec0cce63f47d.patch | 161 ++++++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 patches/chromium/cherry-pick-ec0cce63f47d.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index af1e2ae2e1b7c..06e21f83bde9e 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -135,3 +135,4 @@ cherry-pick-5be8e065f43e.patch cherry-pick-12ba78f3fa7a.patch reland_fix_noopener_case_for_user_activation_consumption.patch fsa_pass_file_ownership_to_worker_for_async_fsarfd_file_operations.patch +cherry-pick-ec0cce63f47d.patch diff --git a/patches/chromium/cherry-pick-ec0cce63f47d.patch b/patches/chromium/cherry-pick-ec0cce63f47d.patch new file mode 100644 index 0000000000000..8591e7b1a3a05 --- /dev/null +++ b/patches/chromium/cherry-pick-ec0cce63f47d.patch @@ -0,0 +1,161 @@ +From ec0cce63f47df40f35e27e668f321c75ff26e670 Mon Sep 17 00:00:00 2001 +From: Corentin Pescheloche +Date: Tue, 10 May 2022 14:05:53 +0000 +Subject: [PATCH] [M96-LTS][profiler] Cleanup profiler group detached profilers + +ProfilerGroup keeps track of detached profilers to be able to gracefully +stop leaked profilers when the corresponding ExecutionContext is +destroyed. + +(cherry picked from commit 9f9d5fd2f3085414fc8776bf556fb5c4fa2dac2c) + +Change-Id: I4fdbbc3a5208819397d742c9ecbff117f839691c +Bug: chromium:1297283 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3594570 +Commit-Queue: Corentin Pescheloche +Cr-Original-Commit-Position: refs/heads/main@{#994316} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3620956 +Reviewed-by: Oleh Lamzin +Owners-Override: Oleh Lamzin +Commit-Queue: Roger Felipe Zanoni da Silva +Auto-Submit: Roger Felipe Zanoni da Silva +Cr-Commit-Position: refs/branch-heads/4664@{#1629} +Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512} +--- + +diff --git a/third_party/blink/renderer/core/timing/profiler_group.cc b/third_party/blink/renderer/core/timing/profiler_group.cc +index 6638ee3..f498a12 100644 +--- a/third_party/blink/renderer/core/timing/profiler_group.cc ++++ b/third_party/blink/renderer/core/timing/profiler_group.cc +@@ -110,8 +110,7 @@ + : isolate_(isolate), + cpu_profiler_(nullptr), + next_profiler_id_(0), +- num_active_profilers_(0) { +-} ++ num_active_profilers_(0) {} + + void DiscardedSamplesDelegate::Notify() { + if (profiler_group_) { +@@ -234,6 +233,8 @@ + DCHECK(!profilers_.Contains(profiler)); + } + ++ StopDetachedProfilers(); ++ + if (cpu_profiler_) + TeardownV8Profiler(); + } +@@ -304,18 +305,49 @@ + + void ProfilerGroup::CancelProfilerAsync(ScriptState* script_state, + Profiler* profiler) { ++ DCHECK(IsMainThread()); + DCHECK(cpu_profiler_); + DCHECK(!profiler->stopped()); + profilers_.erase(profiler); + ++ // register the profiler to be cleaned up in case its associated context ++ // gets destroyed before the cleanup task is executed. ++ detached_profiler_ids_.push_back(profiler->ProfilerId()); ++ + // Since it's possible for the profiler to get destructed along with its + // associated context, dispatch a task to cleanup context-independent isolate + // resources (rather than use the context's task runner). + ThreadScheduler::Current()->V8TaskRunner()->PostTask( +- FROM_HERE, WTF::Bind(&ProfilerGroup::CancelProfilerImpl, ++ FROM_HERE, WTF::Bind(&ProfilerGroup::StopDetachedProfiler, + WrapPersistent(this), profiler->ProfilerId())); + } + ++void ProfilerGroup::StopDetachedProfiler(String profiler_id) { ++ DCHECK(IsMainThread()); ++ ++ // we use a vector instead of a map because the expected number of profiler ++ // is expected to be very small ++ auto* it = std::find(detached_profiler_ids_.begin(), ++ detached_profiler_ids_.end(), profiler_id); ++ ++ if (it == detached_profiler_ids_.end()) { ++ // Profiler already stopped ++ return; ++ } ++ ++ CancelProfilerImpl(profiler_id); ++ detached_profiler_ids_.erase(it); ++} ++ ++void ProfilerGroup::StopDetachedProfilers() { ++ DCHECK(IsMainThread()); ++ ++ for (auto& detached_profiler_id : detached_profiler_ids_) { ++ CancelProfilerImpl(detached_profiler_id); ++ } ++ detached_profiler_ids_.clear(); ++} ++ + void ProfilerGroup::CancelProfilerImpl(String profiler_id) { + if (!cpu_profiler_) + return; +diff --git a/third_party/blink/renderer/core/timing/profiler_group.h b/third_party/blink/renderer/core/timing/profiler_group.h +index d673d0d..8c27d41b 100644 +--- a/third_party/blink/renderer/core/timing/profiler_group.h ++++ b/third_party/blink/renderer/core/timing/profiler_group.h +@@ -81,6 +81,10 @@ + // Internal implementation of cancel. + void CancelProfilerImpl(String profiler_id); + ++ // Clean context independent resources for leaked profilers ++ void StopDetachedProfiler(String profiler_id); ++ void StopDetachedProfilers(); ++ + // Generates an unused string identifier to use for a new profiling session. + String NextProfilerId(); + +@@ -88,9 +92,11 @@ + v8::CpuProfiler* cpu_profiler_; + int next_profiler_id_; + int num_active_profilers_; +- + HeapHashSet> profilers_; + ++ // Store the ids of leaked collected profilers that needs to be stopped ++ Vector detached_profiler_ids_; ++ + // A set of observers, one for each ExecutionContext that has profiling + // enabled. + HeapHashSet> context_observers_; +diff --git a/third_party/blink/renderer/core/timing/profiler_group_test.cc b/third_party/blink/renderer/core/timing/profiler_group_test.cc +index 7f203a7..85f1742 100644 +--- a/third_party/blink/renderer/core/timing/profiler_group_test.cc ++++ b/third_party/blink/renderer/core/timing/profiler_group_test.cc +@@ -268,4 +268,29 @@ + test::RunPendingTasks(); + } + ++// Tests that a ProfilerGroup doesn't crash if the ProfilerGroup is destroyed ++// before a Profiler::Dispose is ran. ++TEST(ProfilerGroupTest, Bug1297283) { ++ { ++ V8TestingScope scope; ++ ProfilerGroup* profiler_group = ProfilerGroup::From(scope.GetIsolate()); ++ profiler_group->OnProfilingContextAdded(scope.GetExecutionContext()); ++ ++ ProfilerInitOptions* init_options = ProfilerInitOptions::Create(); ++ init_options->setSampleInterval(0); ++ init_options->setMaxBufferSize(0); ++ Profiler* profiler = profiler_group->CreateProfiler( ++ scope.GetScriptState(), *init_options, base::TimeTicks(), ++ scope.GetExceptionState()); ++ EXPECT_FALSE(profiler->stopped()); ++ ++ // Force a collection of the underlying Profiler ++ profiler = nullptr; ++ ThreadState::Current()->CollectAllGarbageForTesting(); ++ // Exit Scope deallocating Context triggering ProfilerGroup::WillBeDestroyed ++ // Ensure doesn't crash. ++ } ++ test::RunPendingTasks(); ++} ++ + } // namespace blink From 5a0b3928ee45bbd45d9665f2284201f84432a1ce Mon Sep 17 00:00:00 2001 From: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Date: Fri, 13 May 2022 17:29:02 +0000 Subject: [PATCH 2/2] chore: update patches --- .../chromium/cherry-pick-ec0cce63f47d.patch | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/patches/chromium/cherry-pick-ec0cce63f47d.patch b/patches/chromium/cherry-pick-ec0cce63f47d.patch index 8591e7b1a3a05..1da5b40a4893f 100644 --- a/patches/chromium/cherry-pick-ec0cce63f47d.patch +++ b/patches/chromium/cherry-pick-ec0cce63f47d.patch @@ -1,7 +1,7 @@ -From ec0cce63f47df40f35e27e668f321c75ff26e670 Mon Sep 17 00:00:00 2001 +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Corentin Pescheloche Date: Tue, 10 May 2022 14:05:53 +0000 -Subject: [PATCH] [M96-LTS][profiler] Cleanup profiler group detached profilers +Subject: Cleanup profiler group detached profilers ProfilerGroup keeps track of detached profilers to be able to gracefully stop leaked profilers when the corresponding ExecutionContext is @@ -21,13 +21,12 @@ Commit-Queue: Roger Felipe Zanoni da Silva Auto-Submit: Roger Felipe Zanoni da Silva Cr-Commit-Position: refs/branch-heads/4664@{#1629} Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512} ---- diff --git a/third_party/blink/renderer/core/timing/profiler_group.cc b/third_party/blink/renderer/core/timing/profiler_group.cc -index 6638ee3..f498a12 100644 +index 6638ee38519674036c6c45cf6cd1a4eff023289b..f498a122f5a491226720a36dc729364028148eaa 100644 --- a/third_party/blink/renderer/core/timing/profiler_group.cc +++ b/third_party/blink/renderer/core/timing/profiler_group.cc -@@ -110,8 +110,7 @@ +@@ -110,8 +110,7 @@ ProfilerGroup::ProfilerGroup(v8::Isolate* isolate) : isolate_(isolate), cpu_profiler_(nullptr), next_profiler_id_(0), @@ -37,7 +36,7 @@ index 6638ee3..f498a12 100644 void DiscardedSamplesDelegate::Notify() { if (profiler_group_) { -@@ -234,6 +233,8 @@ +@@ -234,6 +233,8 @@ void ProfilerGroup::WillBeDestroyed() { DCHECK(!profilers_.Contains(profiler)); } @@ -46,7 +45,7 @@ index 6638ee3..f498a12 100644 if (cpu_profiler_) TeardownV8Profiler(); } -@@ -304,18 +305,49 @@ +@@ -304,18 +305,49 @@ void ProfilerGroup::CancelProfiler(Profiler* profiler) { void ProfilerGroup::CancelProfilerAsync(ScriptState* script_state, Profiler* profiler) { @@ -98,10 +97,10 @@ index 6638ee3..f498a12 100644 if (!cpu_profiler_) return; diff --git a/third_party/blink/renderer/core/timing/profiler_group.h b/third_party/blink/renderer/core/timing/profiler_group.h -index d673d0d..8c27d41b 100644 +index 2cdbd795f4265086073b8c378ede4f7c6c60e3e3..ede25fa5656f32d1982077eda43585b70743fd63 100644 --- a/third_party/blink/renderer/core/timing/profiler_group.h +++ b/third_party/blink/renderer/core/timing/profiler_group.h -@@ -81,6 +81,10 @@ +@@ -81,6 +81,10 @@ class CORE_EXPORT ProfilerGroup // Internal implementation of cancel. void CancelProfilerImpl(String profiler_id); @@ -112,7 +111,7 @@ index d673d0d..8c27d41b 100644 // Generates an unused string identifier to use for a new profiling session. String NextProfilerId(); -@@ -88,9 +92,11 @@ +@@ -88,9 +92,11 @@ class CORE_EXPORT ProfilerGroup v8::CpuProfiler* cpu_profiler_; int next_profiler_id_; int num_active_profilers_; @@ -126,10 +125,10 @@ index d673d0d..8c27d41b 100644 // enabled. HeapHashSet> context_observers_; diff --git a/third_party/blink/renderer/core/timing/profiler_group_test.cc b/third_party/blink/renderer/core/timing/profiler_group_test.cc -index 7f203a7..85f1742 100644 +index adedff31f007ca2deff401ec93b39641f4ea516e..1c6dd092c36a27710085a03c74fa12e212adccb4 100644 --- a/third_party/blink/renderer/core/timing/profiler_group_test.cc +++ b/third_party/blink/renderer/core/timing/profiler_group_test.cc -@@ -268,4 +268,29 @@ +@@ -262,4 +262,29 @@ TEST(ProfilerGroupTest, LeakProfilerWithContext) { test::RunPendingTasks(); }