mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
chore: cherry-pick 1856470e25 and 9037876f53 from chromium (#29787)
* cherry-pick 1856470e25 and 9037876f53 from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
@@ -168,3 +168,5 @@ media_feeds_disable_media_feeds_and_related_features.patch
|
||||
remove_tabs_and_line_breaks_from_the_middle_of_app_names_when.patch
|
||||
autofill_fixed_refill_of_changed_form.patch
|
||||
x11_fix_window_enumeration_order_when_wm_doesn_t_set.patch
|
||||
m86-lts_longtaskdetector_remove_container_mutation_during.patch
|
||||
m86-lts_reduce_memory_consumption_on.patch
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Yutaka Hirano <yhirano@chromium.org>
|
||||
Date: Fri, 11 Jun 2021 08:07:55 +0000
|
||||
Subject: Remove container mutation during iteration
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
On LongTaskDetector, we call OnLongTaskDetected for all registered
|
||||
observers. Some observers call LongTaskDetector::UnregisterObserver
|
||||
in the callback, which is problematic because container mutation is
|
||||
not allowed during iteration.
|
||||
|
||||
Copy the observer set to avoid the violation.
|
||||
|
||||
(cherry picked from commit 702f4d4ddb963cafb0d133972282dfc803510b75)
|
||||
|
||||
(cherry picked from commit e88c656a9fb4a7bb1c66ddcedae8049a448ebef4)
|
||||
|
||||
Bug: 1210487
|
||||
Change-Id: Iccea748ac144def6884be8cf542cdc3572bed81a
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2909934
|
||||
Reviewed-by: Deep Roy <dproy@chromium.org>
|
||||
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
|
||||
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
|
||||
Cr-Original-Original-Commit-Position: refs/heads/master@{#885033}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2939704
|
||||
Auto-Submit: Yutaka Hirano <yhirano@chromium.org>
|
||||
Owners-Override: Prudhvi Kumar Bommana <pbommana@google.com>
|
||||
Reviewed-by: Prudhvi Kumar Bommana <pbommana@google.com>
|
||||
Cr-Original-Commit-Position: refs/branch-heads/4472@{#1443}
|
||||
Cr-Original-Branched-From: 3d60439cfb36485e76a1c5bb7f513d3721b20da1-refs/heads/master@{#870763}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2945787
|
||||
Owners-Override: Victor-Gabriel Savu <vsavu@google.com>
|
||||
Reviewed-by: Artem Sumaneev <asumaneev@google.com>
|
||||
Commit-Queue: Victor-Gabriel Savu <vsavu@google.com>
|
||||
Cr-Commit-Position: refs/branch-heads/4240@{#1669}
|
||||
Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
|
||||
|
||||
diff --git a/third_party/blink/renderer/core/loader/long_task_detector.cc b/third_party/blink/renderer/core/loader/long_task_detector.cc
|
||||
index 7e1499b1ddde30db2344db6fd9a9d3e7be574033..f040ae5053265fb136c629f106aeefa0b01130f1 100644
|
||||
--- a/third_party/blink/renderer/core/loader/long_task_detector.cc
|
||||
+++ b/third_party/blink/renderer/core/loader/long_task_detector.cc
|
||||
@@ -43,7 +43,10 @@ void LongTaskDetector::DidProcessTask(base::TimeTicks start_time,
|
||||
if ((end_time - start_time) < LongTaskDetector::kLongTaskThreshold)
|
||||
return;
|
||||
|
||||
- for (auto& observer : observers_) {
|
||||
+ // We copy `observers_` because it might be mutated in OnLongTaskDetected,
|
||||
+ // and container mutation is not allowed during iteration.
|
||||
+ const HeapHashSet<Member<LongTaskObserver>> observers = observers_;
|
||||
+ for (auto& observer : observers) {
|
||||
observer->OnLongTaskDetected(start_time, end_time);
|
||||
}
|
||||
}
|
||||
diff --git a/third_party/blink/renderer/core/loader/long_task_detector_test.cc b/third_party/blink/renderer/core/loader/long_task_detector_test.cc
|
||||
index 3384fa8ebfb0bd3ad1c408390db3fcb26edc4118..04959d3b682ddbf40577adc5799fe57a9ae9d500 100644
|
||||
--- a/third_party/blink/renderer/core/loader/long_task_detector_test.cc
|
||||
+++ b/third_party/blink/renderer/core/loader/long_task_detector_test.cc
|
||||
@@ -27,9 +27,24 @@ class TestLongTaskObserver :
|
||||
last_long_task_start = start_time;
|
||||
last_long_task_end = end_time;
|
||||
}
|
||||
-}; // Anonymous namespace
|
||||
+};
|
||||
+
|
||||
+class SelfUnregisteringObserver
|
||||
+ : public GarbageCollected<SelfUnregisteringObserver>,
|
||||
+ public LongTaskObserver {
|
||||
+ public:
|
||||
+ void OnLongTaskDetected(base::TimeTicks, base::TimeTicks) override {
|
||||
+ called_ = true;
|
||||
+ LongTaskDetector::Instance().UnregisterObserver(this);
|
||||
+ }
|
||||
+ bool IsCalled() const { return called_; }
|
||||
+
|
||||
+ private:
|
||||
+ bool called_ = false;
|
||||
+};
|
||||
|
||||
} // namespace
|
||||
+
|
||||
class LongTaskDetectorTest : public testing::Test {
|
||||
public:
|
||||
// Public because it's executed on a task queue.
|
||||
@@ -126,4 +141,13 @@ TEST_F(LongTaskDetectorTest, RegisterSameObserverTwice) {
|
||||
long_task_end_when_registered);
|
||||
}
|
||||
|
||||
+TEST_F(LongTaskDetectorTest, SelfUnregisteringObserver) {
|
||||
+ auto* observer = MakeGarbageCollected<SelfUnregisteringObserver>();
|
||||
+
|
||||
+ LongTaskDetector::Instance().RegisterObserver(observer);
|
||||
+ SimulateTask(LongTaskDetector::kLongTaskThreshold +
|
||||
+ base::TimeDelta::FromMilliseconds(10));
|
||||
+ EXPECT_TRUE(observer->IsCalled());
|
||||
+}
|
||||
+
|
||||
} // namespace blink
|
||||
118
patches/chromium/m86-lts_reduce_memory_consumption_on.patch
Normal file
118
patches/chromium/m86-lts_reduce_memory_consumption_on.patch
Normal file
@@ -0,0 +1,118 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Yutaka Hirano <yhirano@chromium.org>
|
||||
Date: Fri, 11 Jun 2021 08:42:36 +0000
|
||||
Subject: Reduce memory consumption on LongTaskObserver::DidProcessTask
|
||||
|
||||
https://crrev.com/c/2909934 fixed a security issue, but it introduced a
|
||||
copy operation for each DidProcessTask for a long task. We see a memory
|
||||
regression on the change, and this is an attempt to mitigate the
|
||||
regression.
|
||||
|
||||
(cherry picked from commit 8097e73295a88e64d8318d982847a5e4f2bcc4d2)
|
||||
|
||||
(cherry picked from commit 7be6a34fe2f01af881bb074bc616bf5b6b5f7c31)
|
||||
|
||||
Bug: 1210487, 1211539
|
||||
Change-Id: Ib9101e29d70fadb11b7967754e847bb5cc754feb
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2915153
|
||||
Reviewed-by: Benoit L <lizeb@chromium.org>
|
||||
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
|
||||
Cr-Original-Original-Commit-Position: refs/heads/master@{#886221}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2944320
|
||||
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
|
||||
Cr-Original-Commit-Position: refs/branch-heads/4472@{#1460}
|
||||
Cr-Original-Branched-From: 3d60439cfb36485e76a1c5bb7f513d3721b20da1-refs/heads/master@{#870763}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2952502
|
||||
Owners-Override: Victor-Gabriel Savu <vsavu@google.com>
|
||||
Reviewed-by: Artem Sumaneev <asumaneev@google.com>
|
||||
Commit-Queue: Victor-Gabriel Savu <vsavu@google.com>
|
||||
Cr-Commit-Position: refs/branch-heads/4240@{#1670}
|
||||
Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
|
||||
|
||||
diff --git a/third_party/blink/renderer/core/loader/long_task_detector.cc b/third_party/blink/renderer/core/loader/long_task_detector.cc
|
||||
index f040ae5053265fb136c629f106aeefa0b01130f1..3816779cfafaef06295734b4a8a2f033bf752691 100644
|
||||
--- a/third_party/blink/renderer/core/loader/long_task_detector.cc
|
||||
+++ b/third_party/blink/renderer/core/loader/long_task_detector.cc
|
||||
@@ -24,6 +24,7 @@ LongTaskDetector::LongTaskDetector() = default;
|
||||
void LongTaskDetector::RegisterObserver(LongTaskObserver* observer) {
|
||||
DCHECK(IsMainThread());
|
||||
DCHECK(observer);
|
||||
+ DCHECK(!iterating_);
|
||||
if (observers_.insert(observer).is_new_entry && observers_.size() == 1) {
|
||||
// Number of observers just became non-zero.
|
||||
Thread::Current()->AddTaskTimeObserver(this);
|
||||
@@ -32,6 +33,10 @@ void LongTaskDetector::RegisterObserver(LongTaskObserver* observer) {
|
||||
|
||||
void LongTaskDetector::UnregisterObserver(LongTaskObserver* observer) {
|
||||
DCHECK(IsMainThread());
|
||||
+ if (iterating_) {
|
||||
+ observers_to_be_removed_.push_back(observer);
|
||||
+ return;
|
||||
+ }
|
||||
observers_.erase(observer);
|
||||
if (observers_.size() == 0) {
|
||||
Thread::Current()->RemoveTaskTimeObserver(this);
|
||||
@@ -43,16 +48,21 @@ void LongTaskDetector::DidProcessTask(base::TimeTicks start_time,
|
||||
if ((end_time - start_time) < LongTaskDetector::kLongTaskThreshold)
|
||||
return;
|
||||
|
||||
- // We copy `observers_` because it might be mutated in OnLongTaskDetected,
|
||||
- // and container mutation is not allowed during iteration.
|
||||
- const HeapHashSet<Member<LongTaskObserver>> observers = observers_;
|
||||
- for (auto& observer : observers) {
|
||||
+ iterating_ = true;
|
||||
+ for (auto& observer : observers_) {
|
||||
observer->OnLongTaskDetected(start_time, end_time);
|
||||
}
|
||||
+ iterating_ = false;
|
||||
+
|
||||
+ for (const auto& observer : observers_to_be_removed_) {
|
||||
+ UnregisterObserver(observer);
|
||||
+ }
|
||||
+ observers_to_be_removed_.clear();
|
||||
}
|
||||
|
||||
void LongTaskDetector::Trace(Visitor* visitor) const {
|
||||
visitor->Trace(observers_);
|
||||
+ visitor->Trace(observers_to_be_removed_);
|
||||
}
|
||||
|
||||
} // namespace blink
|
||||
diff --git a/third_party/blink/renderer/core/loader/long_task_detector.h b/third_party/blink/renderer/core/loader/long_task_detector.h
|
||||
index dc6f0dbab5c059b83bfe4212f0126e9690ab1a7f..5fd4bb8d2abcc67dd4e47927daa260fa37bc4aca 100644
|
||||
--- a/third_party/blink/renderer/core/loader/long_task_detector.h
|
||||
+++ b/third_party/blink/renderer/core/loader/long_task_detector.h
|
||||
@@ -49,6 +49,8 @@ class CORE_EXPORT LongTaskDetector final
|
||||
base::TimeTicks end_time) override;
|
||||
|
||||
HeapHashSet<Member<LongTaskObserver>> observers_;
|
||||
+ HeapVector<Member<LongTaskObserver>> observers_to_be_removed_;
|
||||
+ bool iterating_ = false;
|
||||
|
||||
DISALLOW_COPY_AND_ASSIGN(LongTaskDetector);
|
||||
};
|
||||
diff --git a/third_party/blink/renderer/core/loader/long_task_detector_test.cc b/third_party/blink/renderer/core/loader/long_task_detector_test.cc
|
||||
index 04959d3b682ddbf40577adc5799fe57a9ae9d500..403ba452362a3fa2a6b24f238bad35d9eb1bac0c 100644
|
||||
--- a/third_party/blink/renderer/core/loader/long_task_detector_test.cc
|
||||
+++ b/third_party/blink/renderer/core/loader/long_task_detector_test.cc
|
||||
@@ -39,6 +39,8 @@ class SelfUnregisteringObserver
|
||||
}
|
||||
bool IsCalled() const { return called_; }
|
||||
|
||||
+ void Reset() { called_ = false; }
|
||||
+
|
||||
private:
|
||||
bool called_ = false;
|
||||
};
|
||||
@@ -148,6 +150,11 @@ TEST_F(LongTaskDetectorTest, SelfUnregisteringObserver) {
|
||||
SimulateTask(LongTaskDetector::kLongTaskThreshold +
|
||||
base::TimeDelta::FromMilliseconds(10));
|
||||
EXPECT_TRUE(observer->IsCalled());
|
||||
+ observer->Reset();
|
||||
+
|
||||
+ SimulateTask(LongTaskDetector::kLongTaskThreshold +
|
||||
+ base::TimeDelta::FromMilliseconds(10));
|
||||
+ EXPECT_FALSE(observer->IsCalled());
|
||||
}
|
||||
|
||||
} // namespace blink
|
||||
Reference in New Issue
Block a user