forked from electron/electron
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 4 changes from Release-3-M114 (electron#38948)
* chore: [23-x-y] cherry-pick 4 changes from Release-3-M114 * 85beff6fd302 from chromium * 60b93798c991 from chromium * a1efa5343880 from v8 * d20849d07107 from webrtc * chore: update patches --------- Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
- Loading branch information
1 parent
abdb915
commit c4b0ff4
Showing
9 changed files
with
552 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,215 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Kevin McNee <[email protected]> | ||
Date: Wed, 14 Jun 2023 01:10:19 +0000 | ||
Subject: M114: Don't recursively destroy guests when clearing unattached | ||
guests | ||
|
||
Don't recursively destroy guests when clearing unattached guests | ||
|
||
When an embedder process is destroyed, we also destroy any unattached | ||
guests associated with that process. This is currently done with a | ||
single call to `owned_guests_.erase`. However, it's possible that two | ||
unattached guests could have an opener relationship, which causes the | ||
destruction of the opener guest to also destroy the other guest, during | ||
the call to `erase`, which is unsafe. | ||
|
||
We now separate the steps of erasing `owned_guests_` and destroying the | ||
guests, to avoid this recursive guest destruction. | ||
|
||
This also fixes the WaitForNumGuestsCreated test method to not | ||
return prematurely. | ||
|
||
(cherry picked from commit 6345e7871e8197af92f9c6158b06c6e197f87945) | ||
|
||
Bug: 1450397 | ||
Change-Id: Ifef5ec9ff3a1e6952ff56ec279e29e8522625ac0 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4589949 | ||
Commit-Queue: Kevin McNee <[email protected]> | ||
Auto-Submit: Kevin McNee <[email protected]> | ||
Reviewed-by: James Maclean <[email protected]> | ||
Cr-Original-Commit-Position: refs/heads/main@{#1153396} | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4611152 | ||
Commit-Queue: James Maclean <[email protected]> | ||
Cr-Commit-Position: refs/branch-heads/5735@{#1292} | ||
Cr-Branched-From: 2f562e4ddbaf79a3f3cb338b4d1bd4398d49eb67-refs/heads/main@{#1135570} | ||
|
||
diff --git a/chrome/browser/apps/guest_view/web_view_browsertest.cc b/chrome/browser/apps/guest_view/web_view_browsertest.cc | ||
index 3748f8119bf1d4f37635bdb4fb87d825881de7f0..00f0bcb19ae291e5389284f98ee922562f116496 100644 | ||
--- a/chrome/browser/apps/guest_view/web_view_browsertest.cc | ||
+++ b/chrome/browser/apps/guest_view/web_view_browsertest.cc | ||
@@ -2779,6 +2779,22 @@ IN_PROC_BROWSER_TEST_P(WebViewNewWindowTest, | ||
EXPECT_TRUE(content::NavigateToURLFromRenderer(guest2, coop_url)); | ||
} | ||
|
||
+// This test creates a situation where we have two unattached webviews which | ||
+// have an opener relationship, and ensures that we can shutdown safely. See | ||
+// https://crbug.com/1450397. | ||
+IN_PROC_BROWSER_TEST_P(WebViewNewWindowTest, DestroyOpenerBeforeAttachment) { | ||
+ TestHelper("testDestroyOpenerBeforeAttachment", "web_view/newwindow", | ||
+ NEEDS_TEST_SERVER); | ||
+ GetGuestViewManager()->WaitForNumGuestsCreated(2); | ||
+ | ||
+ content::RenderProcessHost* embedder_rph = | ||
+ GetEmbedderWebContents()->GetPrimaryMainFrame()->GetProcess(); | ||
+ content::RenderProcessHostWatcher kill_observer( | ||
+ embedder_rph, content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); | ||
+ EXPECT_TRUE(embedder_rph->Shutdown(content::RESULT_CODE_KILLED)); | ||
+ kill_observer.Wait(); | ||
+} | ||
+ | ||
IN_PROC_BROWSER_TEST_P(WebViewTest, ContextMenuInspectElement) { | ||
LoadAppWithGuest("web_view/context_menus/basic"); | ||
content::RenderFrameHost* guest_rfh = GetGuestRenderFrameHost(); | ||
diff --git a/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js b/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js | ||
index 900911f4963d23d74225868dce01326ba533f63a..4dd25d8849b0b13957ab7fa2912c0a158d3cd244 100644 | ||
--- a/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js | ||
+++ b/chrome/test/data/extensions/platform_apps/web_view/newwindow/embedder.js | ||
@@ -34,6 +34,9 @@ embedder.setUp_ = function(config) { | ||
embedder.guestWithLinkURL = embedder.baseGuestURL + | ||
'/extensions/platform_apps/web_view/newwindow' + | ||
'/guest_with_link.html'; | ||
+ embedder.guestOpenOnLoadURL = embedder.baseGuestURL + | ||
+ '/extensions/platform_apps/web_view/newwindow' + | ||
+ '/guest_opener_open_on_load.html'; | ||
}; | ||
|
||
/** @private */ | ||
@@ -652,6 +655,24 @@ function testNewWindowDeferredAttachmentIndefinitely() { | ||
embedder.setUpNewWindowRequest_(webview, 'guest.html', '', testName); | ||
} | ||
|
||
+// This is not a test in and of itself, but a means of creating a webview that | ||
+// is left in an unattached state while its opener webview is also in an | ||
+// unattached state, so that the C++ side can test it in that state. | ||
+function testDestroyOpenerBeforeAttachment() { | ||
+ embedder.test.succeed(); | ||
+ | ||
+ let webview = new WebView(); | ||
+ webview.src = embedder.guestOpenOnLoadURL; | ||
+ document.body.appendChild(webview); | ||
+ | ||
+ // By spinning forever here, we prevent `webview` from completing the | ||
+ // attachment process. But since the guest is still created and it calls | ||
+ // window.open, we have a situation where two unattached webviews have an | ||
+ // opener relationship. The C++ side will test that we can shutdown safely in | ||
+ // this case. | ||
+ while (true) {} | ||
+} | ||
+ | ||
embedder.test.testList = { | ||
'testNewWindowAttachAfterOpenerDestroyed': | ||
testNewWindowAttachAfterOpenerDestroyed, | ||
@@ -675,7 +696,9 @@ embedder.test.testList = { | ||
testNewWindowWebViewNameTakesPrecedence, | ||
'testNewWindowAndUpdateOpener': testNewWindowAndUpdateOpener, | ||
'testNewWindowDeferredAttachmentIndefinitely': | ||
- testNewWindowDeferredAttachmentIndefinitely | ||
+ testNewWindowDeferredAttachmentIndefinitely, | ||
+ 'testDestroyOpenerBeforeAttachment': | ||
+ testDestroyOpenerBeforeAttachment | ||
}; | ||
|
||
onload = function() { | ||
diff --git a/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest_opener_open_on_load.html b/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest_opener_open_on_load.html | ||
new file mode 100644 | ||
index 0000000000000000000000000000000000000000..e961feb3c6487066801adf414bf4a2746c50a3f6 | ||
--- /dev/null | ||
+++ b/chrome/test/data/extensions/platform_apps/web_view/newwindow/guest_opener_open_on_load.html | ||
@@ -0,0 +1,13 @@ | ||
+<!-- | ||
+Copyright 2023 The Chromium Authors | ||
+Use of this source code is governed by a BSD-style license that can be | ||
+found in the LICENSE file. | ||
+--> | ||
+<html> | ||
+<body> | ||
+<script> | ||
+ // A guest that opens a new window on load. | ||
+ window.open('guest.html'); | ||
+</script> | ||
+</body> | ||
+</html> | ||
diff --git a/components/guest_view/browser/guest_view_manager.cc b/components/guest_view/browser/guest_view_manager.cc | ||
index 94867bbd2ea58908b52474c3923354852c070aed..24199000741b29bcbe25a695090b2de3be894af2 100644 | ||
--- a/components/guest_view/browser/guest_view_manager.cc | ||
+++ b/components/guest_view/browser/guest_view_manager.cc | ||
@@ -324,7 +324,20 @@ void GuestViewManager::RemoveGuest(int guest_instance_id) { | ||
|
||
void GuestViewManager::EmbedderProcessDestroyed(int embedder_process_id) { | ||
embedders_observed_.erase(embedder_process_id); | ||
+ | ||
+ // We can't just call std::multimap::erase here because destroying a guest | ||
+ // could trigger the destruction of another guest which is also owned by | ||
+ // `owned_guests_`. Recursively calling std::multimap::erase is unsafe (see | ||
+ // https://crbug.com/1450397). So we take ownership of all of the guests that | ||
+ // will be destroyed before erasing the entries from the map. | ||
+ std::vector<std::unique_ptr<GuestViewBase>> guests_to_destroy; | ||
+ const auto destroy_range = owned_guests_.equal_range(embedder_process_id); | ||
+ for (auto it = destroy_range.first; it != destroy_range.second; ++it) { | ||
+ guests_to_destroy.push_back(std::move(it->second)); | ||
+ } | ||
owned_guests_.erase(embedder_process_id); | ||
+ guests_to_destroy.clear(); | ||
+ | ||
CallViewDestructionCallbacks(embedder_process_id); | ||
} | ||
|
||
diff --git a/components/guest_view/browser/test_guest_view_manager.cc b/components/guest_view/browser/test_guest_view_manager.cc | ||
index b1835691dfd8e7699575c26d4cb38b2247f9c9bc..69f4161746f756fdc022a20c23f44f593c09a6d6 100644 | ||
--- a/components/guest_view/browser/test_guest_view_manager.cc | ||
+++ b/components/guest_view/browser/test_guest_view_manager.cc | ||
@@ -36,7 +36,6 @@ TestGuestViewManager::TestGuestViewManager( | ||
num_guests_created_(0), | ||
expected_num_guests_created_(0), | ||
num_views_garbage_collected_(0), | ||
- waiting_for_guests_created_(false), | ||
waiting_for_attach_(nullptr) {} | ||
|
||
TestGuestViewManager::~TestGuestViewManager() = default; | ||
@@ -127,14 +126,15 @@ GuestViewBase* TestGuestViewManager::WaitForNextGuestViewCreated() { | ||
} | ||
|
||
void TestGuestViewManager::WaitForNumGuestsCreated(size_t count) { | ||
- if (count == num_guests_created_) | ||
+ if (count == num_guests_created_) { | ||
return; | ||
+ } | ||
|
||
- waiting_for_guests_created_ = true; | ||
expected_num_guests_created_ = count; | ||
|
||
num_created_run_loop_ = std::make_unique<base::RunLoop>(); | ||
num_created_run_loop_->Run(); | ||
+ num_created_run_loop_ = nullptr; | ||
} | ||
|
||
void TestGuestViewManager::WaitUntilAttached(GuestViewBase* guest_view) { | ||
@@ -179,13 +179,11 @@ void TestGuestViewManager::AddGuest(int guest_instance_id, | ||
created_run_loop_->Quit(); | ||
|
||
++num_guests_created_; | ||
- if (!waiting_for_guests_created_ && | ||
- num_guests_created_ != expected_num_guests_created_) { | ||
- return; | ||
- } | ||
|
||
- if (num_created_run_loop_) | ||
+ if (num_created_run_loop_ && | ||
+ num_guests_created_ == expected_num_guests_created_) { | ||
num_created_run_loop_->Quit(); | ||
+ } | ||
} | ||
|
||
void TestGuestViewManager::AttachGuest(int embedder_process_id, | ||
diff --git a/components/guest_view/browser/test_guest_view_manager.h b/components/guest_view/browser/test_guest_view_manager.h | ||
index dfb9f9001ed3324b507f5478391ad3640ebeeae9..f12db0cf32ef03c1e4dacc68b782e01e698cbcdb 100644 | ||
--- a/components/guest_view/browser/test_guest_view_manager.h | ||
+++ b/components/guest_view/browser/test_guest_view_manager.h | ||
@@ -119,7 +119,6 @@ class TestGuestViewManager : public GuestViewManager { | ||
size_t num_guests_created_; | ||
size_t expected_num_guests_created_; | ||
int num_views_garbage_collected_; | ||
- bool waiting_for_guests_created_; | ||
|
||
// Tracks the life time of the GuestView's main FrameTreeNode. The main FTN | ||
// has the same lifesspan as the GuestView. |
99 changes: 99 additions & 0 deletions
99
patches/chromium/m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Eugene Zemtsov <[email protected]> | ||
Date: Wed, 21 Jun 2023 17:57:52 +0000 | ||
Subject: webcodecs: Fix crash when changing temporal layer count in AV1 | ||
encoder | ||
|
||
(cherry picked from commit f312efac1b90117729e8961b58c643fc0eae1fbd) | ||
|
||
Bug: 1447568 | ||
Change-Id: I4ecb02ed956707571573a65ade17fdffe676b502 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4554300 | ||
Auto-Submit: Eugene Zemtsov <[email protected]> | ||
Commit-Queue: Dale Curtis <[email protected]> | ||
Reviewed-by: Dale Curtis <[email protected]> | ||
Cr-Original-Commit-Position: refs/heads/main@{#1148041} | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4610718 | ||
Cr-Commit-Position: refs/branch-heads/5735@{#1360} | ||
Cr-Branched-From: 2f562e4ddbaf79a3f3cb338b4d1bd4398d49eb67-refs/heads/main@{#1135570} | ||
|
||
diff --git a/media/video/av1_video_encoder.cc b/media/video/av1_video_encoder.cc | ||
index b5a075a8986cea0a07ce588330c4c0fa6e9ef593..73b072f05522c88248fc228f91fd61f18e23c352 100644 | ||
--- a/media/video/av1_video_encoder.cc | ||
+++ b/media/video/av1_video_encoder.cc | ||
@@ -118,6 +118,7 @@ EncoderStatus SetUpAomConfig(const VideoEncoder::Options& opts, | ||
svc_params = {}; | ||
svc_params.framerate_factor[0] = 1; | ||
svc_params.number_spatial_layers = 1; | ||
+ svc_params.number_temporal_layers = 1; | ||
if (opts.scalability_mode.has_value()) { | ||
switch (opts.scalability_mode.value()) { | ||
case SVCScalabilityMode::kL1T2: | ||
diff --git a/media/video/software_video_encoder_test.cc b/media/video/software_video_encoder_test.cc | ||
index 19d16a443e616303de0b7264eeaabaa9520b4c97..9fd17dbc726f949eccd3f4ed8a9fee4414623c0f 100644 | ||
--- a/media/video/software_video_encoder_test.cc | ||
+++ b/media/video/software_video_encoder_test.cc | ||
@@ -610,6 +610,63 @@ TEST_P(SVCVideoEncoderTest, EncodeClipTemporalSvc) { | ||
} | ||
} | ||
|
||
+TEST_P(SVCVideoEncoderTest, ChangeLayers) { | ||
+ VideoEncoder::Options options; | ||
+ options.frame_size = gfx::Size(640, 480); | ||
+ options.bitrate = Bitrate::ConstantBitrate(1000000u); // 1Mbps | ||
+ options.framerate = 25; | ||
+ options.scalability_mode = GetParam().scalability_mode; | ||
+ std::vector<scoped_refptr<VideoFrame>> frames_to_encode; | ||
+ | ||
+ std::vector<VideoEncoderOutput> chunks; | ||
+ size_t total_frames_count = 80; | ||
+ | ||
+ // Encoder all frames with 3 temporal layers and put all outputs in |chunks| | ||
+ auto frame_duration = base::Seconds(1.0 / options.framerate.value()); | ||
+ | ||
+ VideoEncoder::OutputCB encoder_output_cb = base::BindLambdaForTesting( | ||
+ [&](VideoEncoderOutput output, | ||
+ absl::optional<VideoEncoder::CodecDescription> desc) { | ||
+ chunks.push_back(std::move(output)); | ||
+ }); | ||
+ | ||
+ encoder_->Initialize(profile_, options, /*info_cb=*/base::DoNothing(), | ||
+ std::move(encoder_output_cb), | ||
+ ValidatingStatusCB(/* quit_run_loop_on_call */ true)); | ||
+ RunUntilQuit(); | ||
+ | ||
+ uint32_t color = 0x964050; | ||
+ for (auto frame_index = 0u; frame_index < total_frames_count; frame_index++) { | ||
+ auto timestamp = frame_index * frame_duration; | ||
+ | ||
+ const bool reconfigure = (frame_index == total_frames_count / 2); | ||
+ if (reconfigure) { | ||
+ encoder_->Flush(ValidatingStatusCB(/* quit_run_loop_on_call */ true)); | ||
+ RunUntilQuit(); | ||
+ | ||
+ // Ask encoder to change SVC mode, empty output callback | ||
+ // means the encoder should keep the old one. | ||
+ options.scalability_mode = SVCScalabilityMode::kL1T1; | ||
+ encoder_->ChangeOptions( | ||
+ options, VideoEncoder::OutputCB(), | ||
+ ValidatingStatusCB(/* quit_run_loop_on_call */ true)); | ||
+ RunUntilQuit(); | ||
+ } | ||
+ | ||
+ auto frame = | ||
+ CreateFrame(options.frame_size, pixel_format_, timestamp, color); | ||
+ color = (color << 1) + frame_index; | ||
+ frames_to_encode.push_back(frame); | ||
+ encoder_->Encode(frame, VideoEncoder::EncodeOptions(false), | ||
+ ValidatingStatusCB(/* quit_run_loop_on_call */ true)); | ||
+ RunUntilQuit(); | ||
+ } | ||
+ | ||
+ encoder_->Flush(ValidatingStatusCB(/* quit_run_loop_on_call */ true)); | ||
+ RunUntilQuit(); | ||
+ EXPECT_EQ(chunks.size(), total_frames_count); | ||
+} | ||
+ | ||
TEST_P(H264VideoEncoderTest, ReconfigureWithResize) { | ||
VideoEncoder::Options options; | ||
gfx::Size size1(320, 200), size2(400, 240); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31 changes: 31 additions & 0 deletions
31
patches/v8/merged_compiler_stackcheck_can_have_side_effects.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Tobias Tebbi <[email protected]> | ||
Date: Tue, 13 Jun 2023 17:08:59 +0200 | ||
Subject: Merged: [compiler] StackCheck can have side effects | ||
|
||
Bug: chromium:1452137 | ||
(cherry picked from commit e548943e473b020fdc1de6e5543ca31b24d8b7f9) | ||
|
||
Change-Id: Ibd7c9b02efd12341b452e4c34a635a58a817649f | ||
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4637129 | ||
Reviewed-by: Toon Verwaest <[email protected]> | ||
Commit-Queue: Tobias Tebbi <[email protected]> | ||
Auto-Submit: Tobias Tebbi <[email protected]> | ||
Commit-Queue: Toon Verwaest <[email protected]> | ||
Cr-Commit-Position: refs/branch-heads/11.4@{#49} | ||
Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1} | ||
Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241} | ||
|
||
diff --git a/src/compiler/js-operator.cc b/src/compiler/js-operator.cc | ||
index b72fe0a6669f6dfd7962f890017451d3fdc3f215..bace3a4647e14a8a3387fcffcd05195e9d11f926 100644 | ||
--- a/src/compiler/js-operator.cc | ||
+++ b/src/compiler/js-operator.cc | ||
@@ -1396,7 +1396,7 @@ const Operator* JSOperatorBuilder::CloneObject(FeedbackSource const& feedback, | ||
const Operator* JSOperatorBuilder::StackCheck(StackCheckKind kind) { | ||
return zone()->New<Operator1<StackCheckKind>>( // -- | ||
IrOpcode::kJSStackCheck, // opcode | ||
- Operator::kNoWrite, // properties | ||
+ Operator::kNoProperties, // properties | ||
"JSStackCheck", // name | ||
0, 1, 1, 0, 1, 2, // counts | ||
kind); // parameter |
Oops, something went wrong.