Browse Source

chore: cherry-pick 2 changes from 3-M133 (#46007)

chore: [33-x-y] cherry-pick 2 changes from 3-M133

* 0adceb6159fb from chromium
* 91343bb45c78 from v8
Pedro Pontes 1 month ago
parent
commit
5a8b62636d

+ 1 - 0
patches/chromium/.patches

@@ -145,3 +145,4 @@ fix_osr_stutter_in_both_cpu_and_gpu_capture_when_page_has_animation.patch
 reland_lzma_sdk_update_to_24_09.patch
 cherry-pick-521faebc8a7c.patch
 cherry-pick-9dacf5694dfd.patch
+cherry-pick-0adceb6159fb.patch

+ 62 - 0
patches/chromium/cherry-pick-0adceb6159fb.patch

@@ -0,0 +1,62 @@
+From 0adceb6159fb6cf8c7e66062964b07d522f32fd7 Mon Sep 17 00:00:00 2001
+From: Alexander Cooper <[email protected]>
+Date: Wed, 19 Feb 2025 14:50:50 -0800
+Subject: [PATCH] Speculative fix for cancelling current xr animation frame
+
+When cancelling an XrAnimationFrame we don't actually need to clear the
+list of async tasks, and in fact attempting to clear the current async
+task can cause errors. This list is only populated while callbacks are
+being executed, and is cleared once those callbacks are finished
+executing. Removing the callback id from
+`current_callback_frame_requests`, which is populated for the same
+life span as the async_tasks, is sufficient to ensure the cancelled
+callback does not run.
+
+Note: This is a speculative fix because even when
+external/wpt/webxr/xrSession_cancelAnimationFrame.https.html
+was modified to cancel the current frame, Issue 396481096 did not
+repro; however, from code analysis this should fix the issue.
+
+(cherry picked from commit 263f1bf5a386c1de1dfea09ca2d5e6abab287476)
+
+Fixed: 396481096
+Change-Id: Ic53895c4ab9cb39b8f9d2263749f5914f484a9f5
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6266810
+Commit-Queue: Alexander Cooper <[email protected]>
+Auto-Submit: Alexander Cooper <[email protected]>
+Reviewed-by: Brandon Jones <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1420734}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6282799
+Bot-Commit: Rubber Stamper <[email protected]>
+Commit-Queue: Rubber Stamper <[email protected]>
+Cr-Commit-Position: refs/branch-heads/6834@{#5493}
+Cr-Branched-From: 47a3549fac11ee8cb7be6606001ede605b302b9f-refs/heads/main@{#1381561}
+---
+
+diff --git a/third_party/blink/renderer/modules/xr/xr_frame_request_callback_collection.cc b/third_party/blink/renderer/modules/xr/xr_frame_request_callback_collection.cc
+index af51506..9a071c6 100644
+--- a/third_party/blink/renderer/modules/xr/xr_frame_request_callback_collection.cc
++++ b/third_party/blink/renderer/modules/xr/xr_frame_request_callback_collection.cc
+@@ -41,7 +41,13 @@
+     callback_frame_requests_.erase(id);
+     callback_async_tasks_.erase(id);
+     current_callback_frame_requests_.erase(id);
+-    current_callback_async_tasks_.erase(id);
++    // We intentionally do not erase from `current_callback_async_tasks_` here.
++    // If we are not actively processing a set of callbacks these will be empty.
++    // If we *are* actively processing callbacks, we cannot erase the task of
++    // the current callback, and these tasks will get cleaned up once the
++    // callbacks are finished processing. Removing the id from
++    // `current_callback_frame_requests_` is enough to ensure that the callback
++    // is not run.
+   }
+ }
+ 
+@@ -70,7 +76,6 @@
+     auto it_frame_request = current_callback_frame_requests_.find(id);
+     auto it_async_task = current_callback_async_tasks_.find(id);
+     if (it_frame_request == current_callback_frame_requests_.end()) {
+-      DCHECK_EQ(current_callback_async_tasks_.end(), it_async_task);
+       continue;
+     }
+     CHECK_NE(current_callback_async_tasks_.end(), it_async_task,

+ 1 - 0
patches/v8/.patches

@@ -13,3 +13,4 @@ cherry-pick-8131c09bc129.patch
 wasm_fix_freeing_of_identical_shared_wrappers.patch
 merged_wasm_replace_dead_code_set_with_is_dying_bit.patch
 merged_fix_out_of_bound_string_access.patch
+cherry-pick-91343bb45c78.patch

+ 48 - 0
patches/v8/cherry-pick-91343bb45c78.patch

@@ -0,0 +1,48 @@
+From 91343bb45c78ac5cf3d214f68161d8150d81fa8c Mon Sep 17 00:00:00 2001
+From: Darius Mercadier <[email protected]>
+Date: Tue, 18 Feb 2025 09:32:04 +0100
+Subject: [PATCH] [M132-LTS][turbofan] Disable escape analysis for TrustedHeapConstant
+
+More precisely: prevent eliding objects that contain
+TrustedHeapConstant, because it can lead to this constant flowing into
+a Phis where other inputs are regular HeapConstant, which confuses
+decompression optimization and leads to memory corruption.
+
+(cherry picked from commit b75e527fb521dca5e7621928846c0c7c6becc8dd)
+
+Fixed: chromium:390743124
+Change-Id: Ic60e4d7dd156367f7d4bb385d422591384c3033c
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6278358
+Reviewed-by: Nico Hartmann <[email protected]>
+Commit-Queue: Nico Hartmann <[email protected]>
+Auto-Submit: Darius Mercadier <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#98748}
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6317725
+Reviewed-by: Darius Mercadier <[email protected]>
+Commit-Queue: Gyuyoung Kim (xWF) <[email protected]>
+Cr-Commit-Position: refs/branch-heads/13.2@{#82}
+Cr-Branched-From: 24068c59cedad9ee976ddc05431f5f497b1ebd71-refs/heads/13.2.152@{#1}
+Cr-Branched-From: 6054ba94db0969220be4f94dc1677fc4696bdc4f-refs/heads/main@{#97085}
+---
+
+diff --git a/src/compiler/escape-analysis.cc b/src/compiler/escape-analysis.cc
+index eb223bc..c9a7bc9 100644
+--- a/src/compiler/escape-analysis.cc
++++ b/src/compiler/escape-analysis.cc
+@@ -622,6 +622,16 @@
+       Node* value = current->ValueInput(1);
+       const VirtualObject* vobject = current->GetVirtualObject(object);
+       Variable var;
++      if (value->opcode() == IrOpcode::kTrustedHeapConstant) {
++        // TODO(dmercadier): enable escaping objects containing
++        // TrustedHeapConstants. This is currently disabled because it leads to
++        // bugs when Trusted HeapConstant and regular HeapConstant flow into the
++        // same Phi, which can then be marked as Compressed, messing up the
++        // tagging of the Trusted HeapConstant.
++        current->SetEscaped(object);
++        current->SetEscaped(value);
++        break;
++      }
+       // BoundedSize fields cannot currently be materialized by the deoptimizer,
+       // so we must not dematerialze them.
+       if (vobject && !vobject->HasEscaped() &&