From c4b0ff499491813617dd7badc83953756dd462dd Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Thu, 29 Jun 2023 23:53:22 +0100 Subject: [PATCH] chore: cherry-pick 4 changes from Release-3-M114 (#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> --- patches/chromium/.patches | 2 + .../chromium/cherry-pick-85beff6fd302.patch | 215 ++++++++++++++++++ ...changing_temporal_layer_count_in_av1.patch | 99 ++++++++ patches/v8/.patches | 3 + ...ler_stackcheck_can_have_side_effects.patch | 31 +++ ...instance_prototypes_directly_on_maps.patch | 44 ++++ ...0fix_20using_20shared_20objects_20as.patch | 78 +++++++ patches/webrtc/.patches | 1 + ...eject_duplicate_ssrcs_in_ssrc-groups.patch | 79 +++++++ 9 files changed, 552 insertions(+) create mode 100644 patches/chromium/cherry-pick-85beff6fd302.patch create mode 100644 patches/chromium/m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch create mode 100644 patches/v8/merged_compiler_stackcheck_can_have_side_effects.patch create mode 100644 patches/v8/merged_runtime_set_instance_prototypes_directly_on_maps.patch create mode 100644 patches/v8/utf-8_q_shared-struct_20fix_20using_20shared_20objects_20as.patch create mode 100644 patches/webrtc/m114_sdp_reject_duplicate_ssrcs_in_ssrc-groups.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 49a591fd54f82..a53b0fc0bc74b 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -147,3 +147,5 @@ cherry-pick-ea1cd76358e0.patch m114_merge_fix_a_crash_caused_by_calling_trace_event.patch mojoipcz_copy_incoming_messages_early.patch base_do_not_use_va_args_twice_in_asprintf.patch +cherry-pick-85beff6fd302.patch +m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch diff --git a/patches/chromium/cherry-pick-85beff6fd302.patch b/patches/chromium/cherry-pick-85beff6fd302.patch new file mode 100644 index 0000000000000..bf19d8aae3152 --- /dev/null +++ b/patches/chromium/cherry-pick-85beff6fd302.patch @@ -0,0 +1,215 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Kevin McNee +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 +Auto-Submit: Kevin McNee +Reviewed-by: James Maclean +Cr-Original-Commit-Position: refs/heads/main@{#1153396} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4611152 +Commit-Queue: James Maclean +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 @@ ++ ++ ++ ++ ++ ++ +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> 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(); + 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. diff --git a/patches/chromium/m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch b/patches/chromium/m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch new file mode 100644 index 0000000000000..ff8c2f4d5911f --- /dev/null +++ b/patches/chromium/m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch @@ -0,0 +1,99 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Eugene Zemtsov +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 +Commit-Queue: Dale Curtis +Reviewed-by: Dale Curtis +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> frames_to_encode; ++ ++ std::vector 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 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); diff --git a/patches/v8/.patches b/patches/v8/.patches index 43c970e0f3432..2bac64e2ef55e 100644 --- a/patches/v8/.patches +++ b/patches/v8/.patches @@ -17,3 +17,6 @@ cherry-pick-73af1a19a901.patch cherry-pick-3b0607d14060.patch cherry-pick-9c6dfc733fce.patch cherry-pick-2e76270cf65e.patch +utf-8_q_shared-struct_20fix_20using_20shared_20objects_20as.patch +merged_runtime_set_instance_prototypes_directly_on_maps.patch +merged_compiler_stackcheck_can_have_side_effects.patch diff --git a/patches/v8/merged_compiler_stackcheck_can_have_side_effects.patch b/patches/v8/merged_compiler_stackcheck_can_have_side_effects.patch new file mode 100644 index 0000000000000..b257baafac0ec --- /dev/null +++ b/patches/v8/merged_compiler_stackcheck_can_have_side_effects.patch @@ -0,0 +1,31 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Tobias Tebbi +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 +Commit-Queue: Tobias Tebbi +Auto-Submit: Tobias Tebbi +Commit-Queue: Toon Verwaest +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>( // -- + IrOpcode::kJSStackCheck, // opcode +- Operator::kNoWrite, // properties ++ Operator::kNoProperties, // properties + "JSStackCheck", // name + 0, 1, 1, 0, 1, 2, // counts + kind); // parameter diff --git a/patches/v8/merged_runtime_set_instance_prototypes_directly_on_maps.patch b/patches/v8/merged_runtime_set_instance_prototypes_directly_on_maps.patch new file mode 100644 index 0000000000000..e3d4c341c0c75 --- /dev/null +++ b/patches/v8/merged_runtime_set_instance_prototypes_directly_on_maps.patch @@ -0,0 +1,44 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Toon Verwaest +Date: Fri, 16 Jun 2023 17:13:52 +0200 +Subject: Merged: [runtime] Set instance prototypes directly on maps + +Bug: chromium:1452137 +(cherry picked from commit c7c447735f762f6d6d0878e229371797845ef4ab) + +Change-Id: I611c41f942e2e51f3c4b4f1d119c18410617188e +Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4637888 +Commit-Queue: Igor Sheludko +Auto-Submit: Igor Sheludko +Commit-Queue: Toon Verwaest +Reviewed-by: Toon Verwaest +Cr-Commit-Position: refs/branch-heads/11.4@{#47} +Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1} +Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241} + +diff --git a/src/objects/js-function.cc b/src/objects/js-function.cc +index 54aec3861bbfdf956740a6c5d3bd5a6721a95a3f..2aa0710bc39d7bfbf55ae8ab2707d2be55cbb6b0 100644 +--- a/src/objects/js-function.cc ++++ b/src/objects/js-function.cc +@@ -673,6 +673,10 @@ void SetInstancePrototype(Isolate* isolate, Handle function, + // At that point, a new initial map is created and the prototype is put + // into the initial map where it belongs. + function->set_prototype_or_initial_map(*value, kReleaseStore); ++ if (value->IsJSObjectThatCanBeTrackedAsPrototype()) { ++ // Optimize as prototype to detach it from its transition tree. ++ JSObject::OptimizeAsPrototype(Handle::cast(value)); ++ } + } else { + Handle new_map = + Map::Copy(isolate, initial_map, "SetInstancePrototype"); +@@ -798,8 +802,10 @@ void JSFunction::EnsureHasInitialMap(Handle function) { + Handle prototype; + if (function->has_instance_prototype()) { + prototype = handle(function->instance_prototype(), isolate); ++ map->set_prototype(*prototype); + } else { + prototype = isolate->factory()->NewFunctionPrototype(function); ++ Map::SetPrototype(isolate, map, prototype); + } + DCHECK(map->has_fast_object_elements()); + diff --git a/patches/v8/utf-8_q_shared-struct_20fix_20using_20shared_20objects_20as.patch b/patches/v8/utf-8_q_shared-struct_20fix_20using_20shared_20objects_20as.patch new file mode 100644 index 0000000000000..a37f15a739e4b --- /dev/null +++ b/patches/v8/utf-8_q_shared-struct_20fix_20using_20shared_20objects_20as.patch @@ -0,0 +1,78 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Pedro Pontes +Date: Thu, 29 Jun 2023 01:14:35 -0700 +Subject: Fix using shared objects as prototypes more (Partial backport) + +The previous fix from +https://chromium-review.googlesource.com/c/v8/v8/+/4086127 was +insufficient. It prevented shared objects from being optimized as +prototypes, but callers of OptimizeAsPrototype also assume that all +JSObjects can track prototype users via prototype_info on the map. + +This CL attempts a broader fix where shared objects are not considered +optimizable as prototypes at all. When used as a prototype, shared +objects are treated like non-JSObjects (e.g. JSProxy, WasmObject). + +Bug: chromium:1401295, v8:12547 +Change-Id: I9886e9ccac9e597e7dd34a09083a096ff4e3bf16 +Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4112150 +Reviewed-by: Toon Verwaest +Commit-Queue: Shu-yu Guo +Cr-Commit-Position: refs/heads/main@{#84916} + +diff --git a/src/objects/heap-object.h b/src/objects/heap-object.h +index 2e4ad85766d95a081f6a59488c07971710aa082f..e1473ae473eda5ad5aeda075331ef0c549cba724 100644 +--- a/src/objects/heap-object.h ++++ b/src/objects/heap-object.h +@@ -119,6 +119,8 @@ class HeapObject : public Object { + STRUCT_LIST(DECL_STRUCT_PREDICATE) + #undef DECL_STRUCT_PREDICATE + ++ V8_INLINE bool IsJSObjectThatCanBeTrackedAsPrototype() const; ++ + // Converts an address to a HeapObject pointer. + static inline HeapObject FromAddress(Address address) { + DCHECK_TAG_ALIGNED(address); +diff --git a/src/objects/objects-inl.h b/src/objects/objects-inl.h +index 36ddbfb6a1e35ff142e0b81b6b182aa166ce6204..f2108a8a61792dc3c440a8aa507319277fc780c1 100644 +--- a/src/objects/objects-inl.h ++++ b/src/objects/objects-inl.h +@@ -84,6 +84,11 @@ bool Object::InSharedWritableHeap() const { + return IsHeapObject() && HeapObject::cast(*this).InSharedWritableHeap(); + } + ++bool Object::IsJSObjectThatCanBeTrackedAsPrototype() const { ++ return IsHeapObject() && ++ HeapObject::cast(*this).IsJSObjectThatCanBeTrackedAsPrototype(); ++} ++ + #define IS_TYPE_FUNCTION_DEF(type_) \ + bool Object::Is##type_() const { \ + return IsHeapObject() && HeapObject::cast(*this).Is##type_(); \ +@@ -190,6 +195,13 @@ bool HeapObject::InSharedWritableHeap() const { + return BasicMemoryChunk::FromHeapObject(*this)->InSharedHeap(); + } + ++bool HeapObject::IsJSObjectThatCanBeTrackedAsPrototype() const { ++ // Do not optimize objects in the shared heap because it is not ++ // threadsafe. Objects in the shared heap have fixed layouts and their maps ++ // never change. ++ return IsJSObject() && !InSharedWritableHeap(); ++} ++ + bool HeapObject::IsNullOrUndefined(Isolate* isolate) const { + return IsNullOrUndefined(ReadOnlyRoots(isolate)); + } +diff --git a/src/objects/objects.h b/src/objects/objects.h +index 2fa31a912c75a832cc0e051dfd54f4cd1ac50a79..fb02858b48507ef4c358a3b254c24ef95b366ee4 100644 +--- a/src/objects/objects.h ++++ b/src/objects/objects.h +@@ -341,6 +341,8 @@ class Object : public TaggedImpl { + bool IsWasmObject(Isolate* = nullptr) const { return false; } + #endif + ++ V8_INLINE bool IsJSObjectThatCanBeTrackedAsPrototype() const; ++ + enum class Conversion { kToNumber, kToNumeric }; + + #define DECL_STRUCT_PREDICATE(NAME, Name, name) \ diff --git a/patches/webrtc/.patches b/patches/webrtc/.patches index b8f4a6a81d1aa..883cd06a6f023 100644 --- a/patches/webrtc/.patches +++ b/patches/webrtc/.patches @@ -1,2 +1,3 @@ fix_fallback_to_x11_capturer_on_wayland.patch m114_move_transceiver_iteration_loop_over_to_the_signaling_thread.patch +m114_sdp_reject_duplicate_ssrcs_in_ssrc-groups.patch diff --git a/patches/webrtc/m114_sdp_reject_duplicate_ssrcs_in_ssrc-groups.patch b/patches/webrtc/m114_sdp_reject_duplicate_ssrcs_in_ssrc-groups.patch new file mode 100644 index 0000000000000..40b0b13495fec --- /dev/null +++ b/patches/webrtc/m114_sdp_reject_duplicate_ssrcs_in_ssrc-groups.patch @@ -0,0 +1,79 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Philipp Hancke +Date: Thu, 15 Jun 2023 07:21:56 +0200 +Subject: sdp: reject duplicate ssrcs in ssrc-groups + +while not really covered by + https://www.rfc-editor.org/rfc/rfc5576.html#section-4.2 +and using the same SSRC for RTX and primary payload may work +since payload type demuxing *could* be used is not a good idea. +This also applies to flexfec's FEC-FR. + +For the nonstandard SIM ssrc-group duplicates make no sense. +This rejects duplicates for unknown ssrc-groups as well. + +BUG=chromium:1454860 + +(cherry picked from commit 6a38a3eb38f732b89ca0d8e36c43a434670c4ef5) + +No-Try: true +Change-Id: I3e86101dbd5d6c4099f2fdb7b4a52d5cd0809c5f +Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308820 +Reviewed-by: Taylor Brandstetter +Reviewed-by: Harald Alvestrand +Commit-Queue: Philipp Hancke +Cr-Original-Commit-Position: refs/heads/main@{#40292} +Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/309601 +Cr-Commit-Position: refs/branch-heads/5735@{#4} +Cr-Branched-From: df7df199abd619e75b9f1d9a7e12fc3f3f748775-refs/heads/main@{#39949} + +diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc +index 69fa62ca37ff6e86524f707dcd8abe51688fea25..707e7cbadb2391eda609608b75ae50c9e30bac58 100644 +--- a/pc/webrtc_sdp.cc ++++ b/pc/webrtc_sdp.cc +@@ -3506,6 +3506,11 @@ bool ParseSsrcGroupAttribute(absl::string_view line, + if (!GetValueFromString(line, fields[i], &ssrc, error)) { + return false; + } ++ // Reject duplicates. While not forbidden by RFC 5576, ++ // they don't make sense. ++ if (absl::c_linear_search(ssrcs, ssrc)) { ++ return ParseFailed(line, "Duplicate SSRC in ssrc-group", error); ++ } + ssrcs.push_back(ssrc); + } + ssrc_groups->push_back(SsrcGroup(semantics, ssrcs)); +diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc +index 9f1cfc9c96aac958c0bec6a48c636b22ac319219..2e15fadb3b6e9784845993b162ee6fe058962b35 100644 +--- a/pc/webrtc_sdp_unittest.cc ++++ b/pc/webrtc_sdp_unittest.cc +@@ -4980,3 +4980,29 @@ TEST_F(WebRtcSdpTest, ParseIgnoreUnknownSsrcSpecificAttribute) { + SdpParseError error; + ASSERT_TRUE(webrtc::SdpDeserialize(sdp, &output, &error)); + } ++ ++TEST_F(WebRtcSdpTest, RejectDuplicateSsrcInSsrcGroup) { ++ std::string sdp = ++ "v=0\r\n" ++ "o=- 0 3 IN IP4 127.0.0.1\r\n" ++ "s=-\r\n" ++ "t=0 0\r\n" ++ "a=group:BUNDLE 0\r\n" ++ "a=fingerprint:sha-1 " ++ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n" ++ "a=setup:actpass\r\n" ++ "a=ice-ufrag:ETEn\r\n" ++ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n" ++ "m=video 9 UDP/TLS/RTP/SAVPF 96 97\r\n" ++ "c=IN IP4 0.0.0.0\r\n" ++ "a=rtcp-mux\r\n" ++ "a=sendonly\r\n" ++ "a=mid:0\r\n" ++ "a=rtpmap:96 VP8/90000\r\n" ++ "a=rtpmap:97 rtx/90000\r\n" ++ "a=fmtp:97 apt=96\r\n" ++ "a=ssrc-group:FID 1234 1234\r\n" ++ "a=ssrc:1234 cname:test\r\n"; ++ JsepSessionDescription jdesc(kDummyType); ++ EXPECT_FALSE(SdpDeserialize(sdp, &jdesc)); ++}