Upgrade WebKit Jan 4, 2026

[sosukesuzuki]

Upgrade WebKit Jan 4, 2026. Happy new year 🎉

---

WebKit Changes Summary

Merge Range: 1a1813b764ce → 694de82fa83d
Date Range: December 24, 2025 → January 3, 2026
Total Commits: 146

---

JavaScriptCore (JSC) Performance Improvements

Array Operations

• Fast path for Array.from(set) - Optimized conversion from Set to Array
• Fast path for Array.from(arguments) in strict mode - Improved performance for arguments object conversion
• Use hasSpecialProperties for checking iterable - More efficient iterable detection

Async/Generator Optimizations

• AsyncGenerator inline first queue item - Reduced memory allocation by embedding the first queue entry
• Inline async function body when there is no await - Skip unnecessary promise wrapping for sync async functions
• Move MicrotaskQueue driving AsyncGenerator code to C++ - Removed JS "WithoutPromise" variants
• Use InternalMicrotask in AsyncFromSyncIterator - More efficient async iteration
• Embed SuspendReason into State in JSAsyncGenerator - Reduced struct size

String Methods Moved to C++

• String#padStart and String#padEnd - Native implementation replacing JS builtins
• String#repeat - Native C++ implementation
• String.prototype.replace / String.prototype.replaceAll - Full C++ implementation
• RegExp.prototype[Symbol.replace] - Native C++ implementation
• HTML-annotation String methods (anchor, big, bold, etc.) - Moved to C++

Promise Improvements

• Track "then" property addition in Structure - Enables better optimization for promise-like objects
• Implement @InternalPromise.@internalAll in C++ - Native Promise.all for internal use

Memory Optimizations

• Shrink sizeof(QueuedTask) by embedding uint8_t payload into CompactRefPtrTuple
• Treat unused catch binding as optional catch binding - Reduced bytecode size
• Remove unused @setClone - Dead code removal

Bug Fixes

• Fix %TypedArray%.prototype.includes to align with ECMA-262
• Exception check error fixes

---

WTF (Web Template Framework)

Performance

• Optimize String::fromUTF8 using simdutf - SIMD-accelerated UTF-8 decoding
• Simplify SortedArrayMap / SortedArraySet - Removed requirement for separate std::array static variable
• PlainText(): avoid extra buffer allocation for empty ranges

New Features

• Introduce ThreadSafeWeakRef and adopt it where appropriate
• Enable CHECK_REF_COUNTED_LIFECYCLE - Better reference counting validation

Improvements

• Update Vector's uninitializedCopy() to use std::span
• Simplify strlenSpan() and compareSpansConstExpr()
• Fix subtle bug in SortedArrayMap's lessThanASCIICaseFolding()
• RefCountedWithInlineWeakPtr fixes for GCC Release builds
• Align ref and weakRef counting on uint32_t

Platform Fixes

• Use OS(WINDOWS) instead of PLATFORM(WIN) - Consistent platform detection
• Should not release RefPtr before using - Fixed use-after-free potential

---

Memory Safety (Safer C++)

Significant effort to reduce unsafe buffer usage across the codebase:

WTF

• Drop WTF_ALLOW_UNSAFE_BUFFER_USAGE from: DateMath.cpp, MemoryDump, dragonbox, FixedBitVector

WebCore

• Reduce unsafe member variables in:
  • GraphicsLayer, AudioMediaStreamTrackRenderer
  • AXSearchManager, FrameSnapshotting
  • WebCore/platform/graphics, scrolling code
  • MathMLRowElement, MediaSourcePrivateAVFObjC
  • mediastream code, WebAuthn code, mediasource/ code
  • User* files in WebCore/page

Platform-Specific

• GLIB: NetworkCacheDataGLib, WebSocketTaskSoup
• Cocoa: FaceDetectorImplementation.mm

Type Safety

• Remove static_cast from: InspectorCSSAgent, WebCore/bridge, WebCore/svg, Internals, ScriptModuleLoader, platform/graphics, Modules/mediastream

---

SVG Improvements

• Don't check 'visibility' in LegacyRenderSVGResourceMasker - Spec compliance fix
• Synthesize preserveAspectRatio='none' for SVGs embedded in <img> without viewBox
• stroke-dasharray applied to markers can now be overridden by ="0"
• SVG <stop> offset attribute correctly rejects invalid values with trailing garbage
• SVGLengthValue::setValueAsString avoids unnecessary String allocations
• Introduce decoratedBoundingBox to fix repaint rect and bounding box conflation in SVG resources
• Migrate from repaintRectInLocalCoordinates(Accurate) to direct computation
• Fix typo in SVGRadialGradientElement.h lengths

---

CSS/Style System

MathML

• Enable math-depth by default
• Make math-style and math-shift animate discretely
• <mpadded> treats percentage values as absent for width/height/depth attribute

Property Handling

• Convert 'caret-color' property to strong style types
• Generate logical property getter/setters via RenderStyleGen
• Generate setters for hasExplicitlySet flags*
• Remove custom style building code for fill and stroke properties
• Extract color/transform related computation from RenderStyle to UsedStyle

Layout Fixes

• -webkit-line-clamp supports inline-block children
• offsetHeight/offsetWidth of inline box wrapping block box now returns correct values (was 0)
• getClientRects now includes entry for inline boxes with block children
• Fix block-in-inline client rects
• Text layout consistency improvements for list-style-type

---

Core Image Filter Acceleration

New GPU-accelerated filter implementations for macOS/iOS:

• FEMerge in Core Image
• FEOffset in Core Image
• FEGaussianBlur in Core Image (with geometry fixes)
• GPU acceleration for morphology filters

---

AutoFill Debugging

• Automatically extract items from same-origin subframes
• Fix text extraction hanging indefinitely when invoked from system daemons
• Heuristically shorten/redact high-entropy URLs in extracted text
• Deduplicate shortened URLs and report replacements to client
• Add system trace points for text extraction performance analysis

---

Swift in WebKit

• Work towards modularizing PAL (part 3)
• Re-land modularization of WebGPU (part 2)
• Work towards having a proper JSC module (part 3)
• Fix Swift modules depending on WTF occasionally needing erroneous imports

---

Platform & Build Changes

Removals

• Remove ENABLE_TOUCH_EVENT_REGIONS definition
• Stop using LocalServiceAdditions.h and MockLocalServiceAdditions.h
• Remove unused WTF_MAKE_TZONE_OR_ISO_ALLOCATED macros

Build Fixes

• Fix watchOS and tvOS builds after 305052@main
• Fix WPE generated API header regression
• Add missing hyphenation dictionaries to GTK APT dependencies
• Fix LOG_CHANNEL_PREFIXFonts undeclared identifier in FontDescriptionCocoa.cpp
• Fix ASSERTION FAILED in WTF_OptionSet tests on GTK/WPE

---

Web Platform Features

• Expose operator lighter in SVGFECompositeElement IDL
• Fix HTMLImageElement.currentSrc for empty src attribute
• Resync web-animations from WPT upstream

---

Bug Fixes & Gardening

• Multiple test expectation updates for macOS, iOS, GTK, WPE, GStreamer
• Fix assertion failures in JSEventListener, PointerLockTests
• Fix GStreamerVideoFrameConverter weak pointer assertion
• Fix RecordMetaData::cacheStorageVersion uninitialized member
• Fix run-builtins-generator-tests --reset-results regression

[coderabbitai]

Walkthrough

This PR introduces extensive test coverage for Array.from operations, String prototype methods, and RegExp symbol.replace behavior, while refactoring core async generator and promise handling infrastructure. Changes span test additions across microbenchmarks and stress tests, layout test updates, and significant JavaScript and C++ runtime modifications for promise resolution and async function optimization.

Changes

Cohort / File(s)	Summary
Array.from microbenchmarks JSTests/microbenchmarks/array-from-cloned-arguments.js, JSTests/microbenchmarks/array-from-set-double-small.js, JSTests/microbenchmarks/array-from-set-double-large.js, JSTests/microbenchmarks/array-from-set-int-small.js, JSTests/microbenchmarks/array-from-set-int-large.js, JSTests/microbenchmarks/array-from-set-object-small.js, JSTests/microbenchmarks/array-from-set-object-large.js	Add microbenchmarks for Array.from with cloned arguments and Set conversion with various value types (doubles, integers, objects) and set sizes (small/large).
String padStart/padEnd microbenchmarks JSTests/microbenchmarks/string-prototype-padStart-single-char-resolved.js, JSTests/microbenchmarks/string-prototype-padStart-multiple-chars-resolved.js, JSTests/microbenchmarks/string-prototype-padEnd-single-char-resolved.js, JSTests/microbenchmarks/string-prototype-padEnd-multiple-chars-resolved.js	Add microbenchmarks exercising String.prototype.padStart and padEnd with single and multi-character fill strings over 1M iterations.
Promise microbenchmarks JSTests/microbenchmarks/internal-promise-internal-all.js	Add microbenchmark for internal Promise.all over 10K iterations with pre-resolved promises.
Array.from stress tests JSTests/stress/array-from-cloned-arguments.js, JSTests/stress/array-from-set.js	Add comprehensive stress tests validating Array.from(arguments) and Array.from(Set) with extensive type coverage, edge cases, and large-set handling.
Async function stress tests JSTests/stress/async-function-without-await.js, JSTests/stress/async-stack-trace-basic.js, JSTests/stress/async-stack-trace-nested.js, JSTests/stress/async-stack-trace-nested-deep.js	Add stress tests for async function behavior without await and async stack trace validation across nested calls.
RegExp Symbol.replace stress tests JSTests/stress/regexp-prototype-symbol-replace-basic.js, JSTests/stress/regexp-prototype-symbol-replace-capture-groups.js, JSTests/stress/regexp-prototype-symbol-replace-edge-cases.js, JSTests/stress/regexp-prototype-symbol-replace-functional.js, JSTests/stress/regexp-prototype-symbol-replace-named-groups.js, JSTests/stress/regexp-prototype-symbol-replace-slow-path.js, JSTests/stress/regexp-prototype-symbol-replace-unicode.js	Add comprehensive stress tests for RegExp.prototype[Symbol.replace] covering basic/global replacements, capture groups, backreferences, functional replacements, named groups, Unicode handling, and slow-path behaviors.
String method stress tests JSTests/stress/string-pad-start-end.js, JSTests/stress/string-repeat-complex.js	Add stress tests for String.prototype padStart/padEnd with Unicode variants and String.prototype repeat with edge cases and coercions.
TypedArray includes stress test JSTests/stress/typedarray-resize-includes.js, JSTests/stress/unused-catch-binding.js	Add stress test for includes with buffer resizing during index coercion and comprehensive catch binding behavior validation.
Internal Promise module test JSTests/modules/object-prototype-then-should-not-be-called-through-internal-promise.js, JSTests/modules/object-prototype-then-should-not-be-called-through-internal-promise/test.js	Add module test verifying Object.prototype.then is not invoked during internal Promise resolution.
Test262 expectations JSTests/test262/expectations.yaml	Remove two TypedArray includes test expectations relating to buffer shrinking.
Layout test expectations LayoutTests/TestExpectations, LayoutTests/fast/*/expected.txt, LayoutTests/platform/*/expected.txt	Update rendering expectations across platforms (glib, gtk, ios, mac, wpe, win) for inline widths and layout dimensions; add/remove CSS and layout test entries.
WebVTT and Web Platform Tests LayoutTests/imported/w3c/web-platform-tests/webvtt/rendering/cues-with-video/*.html, LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-trigger/*.html, LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/*	Add WebVTT processing tests, web-animations trigger/animation-types tests, and update expected outputs; rename AnimationTrigger to TimelineTrigger.
Text extraction tests LayoutTests/fast/text-extraction/, LayoutTests/http/tests/text-extraction/	Add new HTML test files and expected outputs for text extraction with URL shortening and subframe handling.
3D point-mapping test removals LayoutTests/transforms/3d/point-mapping/3d-point-mapping-*.html, LayoutTests/platform/*/transforms/3d/point-mapping/3d-point-mapping-*-expected.txt	Remove 3D point-mapping test files and corresponding expected outputs across multiple platforms.
Emoji deletion test removals LayoutTests/platform/*/editing/deleting/delete-emoji-*-expected.txt, LayoutTests/platform/win/editing/caret/emoji-expected.txt	Remove emoji deletion expected-output test fixtures across multiple platforms.
Async generator builtins Source/JavaScriptCore/builtins/AsyncGeneratorPrototype.js, Source/JavaScriptCore/builtins/AsyncFromSyncIteratorPrototype.js, Source/JavaScriptCore/builtins/AsyncIteratorPrototype.js	Refactor async generator queue handling from separate queue management functions to loop-based state machine using internal microtask system; replace promise callbacks with microtask continuations.
String prototype builtins removal Source/JavaScriptCore/builtins/StringPrototype.js	Remove string method implementations (repeat, padStart, padEnd, replace, replaceAll, createHTML, anchor, big, blink, bold, fixed, fontcolor, fontsize, italics, link, small, strike, sub, sup) relocating to C++.
RegExp prototype builtins removal Source/JavaScriptCore/builtins/RegExpPrototype.js	Remove getSubstitution helper and Symbol.replace implementation.
Module loader builtins Source/JavaScriptCore/builtins/ModuleLoader.js	Add new private recursive functions for async module evaluation, loading, and evaluation with await support.
Builtin names Source/JavaScriptCore/builtins/BuiltinNames.h	Replace promise-related without-promise macros with async/microtask-related entries (resolveWithInternalMicrotaskForAsyncAwait, asyncGeneratorQueueEnqueue/Dequeue variants).
Link-time constants Source/JavaScriptCore/bytecode/LinkTimeConstant.h, Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h, Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp	Update AsyncGenerator field mappings from suspend-reason/queue-first/queue-last to queue/resume-value/resume-mode/resume-promise; introduce InternalMicrotask constants.
Build configuration Source/JavaScriptCore/CMakeLists.txt, Source/JavaScriptCore/DerivedSources.make, Source/JavaScriptCore/DerivedSources-input.xcfilelist, Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj	Remove InternalPromiseConstructor.js from build sources.
Xcode configuration Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig	Update module verifier flags and per-SDK private module declarations.
DFG changes Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp, Source/JavaScriptCore/dfg/DFGFixupPhase.cpp, Source/JavaScriptCore/dfg/DFGGraph.cpp, Source/JavaScriptCore/dfg/DFGNode.h, Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp, Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h	Remove StringPrototypeReplace intrinsic inlining cases; update allocator macros and iterator protocol checks.
Bytecode generator Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp, Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h, Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp	Add async-function-without-await optimization path with inlining; update async generator field emitters and return node finally-scope handling.
Parser changes Source/JavaScriptCore/parser/Parser.cpp, Source/JavaScriptCore/parser/Parser.h, Source/JavaScriptCore/parser/ParserModes.h, Source/JavaScriptCore/parser/SourceProviderCacheItem.h	Add await-usage tracking, async-function-without-await feature flag, and body inlining path for async functions; propagate implementation visibility through cache.
Runtime async generator Source/JavaScriptCore/runtime/JSAsyncGenerator.h, Source/JavaScriptCore/runtime/JSAsyncGenerator.cpp	Introduce queue-based async generator with enqueue/dequeue, resume state management, and new field accessors (queue, resumeValue, resumeMode, resumePromise).
Runtime global object Source/JavaScriptCore/runtime/JSGlobalObject.cpp, Source/JavaScriptCore/runtime/JSGlobalObject.h, Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h	Replace resolveWithoutPromiseForAsyncAwait with resolveWithInternalMicrotaskForAsyncAwait; add asyncGeneratorQueueEnqueue/Dequeue functions; update queueMicrotask signature.
Runtime array construction Source/JavaScriptCore/runtime/ArrayConstructor.cpp	Add fast paths for Array.from(ClonedArguments) and Array.from(Set) with optimized butterfly allocation.
Runtime JSArray Source/JavaScriptCore/runtime/JSArray.cpp, Source/JavaScriptCore/runtime/JSArrayBufferView.cpp	Simplify isIteratorProtocolFastAndNonObservable to use hasSpecialProperties() check.
Runtime support classes Source/JavaScriptCore/runtime/CommonIdentifiers.h, Source/JavaScriptCore/runtime/ClassInfo.h, Source/JavaScriptCore/runtime/JSFunctionWithFields.h, Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h, Source/JavaScriptCore/runtime/JSCast.h, Source/JavaScriptCore/runtime/Intrinsic.h, Source/JavaScriptCore/runtime/ExecutionCounter.h	Add resumeMode identifier; update allocator macros; rename FunctionWithFields async field constants; remove string replace intrinsics.
Inspector scripts Source/JavaScriptCore/inspector/InjectedScriptManager.h, Source/JavaScriptCore/inspector/augmentable/AlternateDispatchableAgent.h, Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_type_conversions_header.py, Source/JavaScriptCore/inspector/scripts/tests/expected/*	Add isWebInjectedScriptManager virtual method; update allocator macros; refactor enum-string mapping to SortedArrayMap; add JSExportMacros and Platform includes to generated headers.
Builtin test expectations Source/JavaScriptCore/Scripts/tests/builtins/expected/JavaScriptCore-Builtin.Promise-*-result, Source/JavaScriptCore/Scripts/tests/builtins/expected/JavaScriptCore-BuiltinConstructor-*-result, Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-*-result, Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCoreJSBuiltins.h-result	Add forward declarations and #include to generated builtin headers.
JIT validation Source/JavaScriptCore/assembler/JITOperationValidation.h	Wrap WebKitAdditions include with diagnostic pragmas for module compilation.

Possibly related PRs

• Related to async generator refactoring: PRs modifying JSAsyncGenerator behavior, promise resolution patterns, and internal microtask infrastructure would align with the comprehensive async generator queue redesign.
• Related to Array.from optimizations: PRs implementing Set support or ClonedArguments fast-paths in ArrayConstructor directly complement the new fast-path handling.
• Related to builtin relocations: PRs moving string/regexp methods from JavaScript builtins to C++ implementations would correspond to the removal of StringPrototype and RegExpPrototype method implementations.

Pre-merge checks

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Upgrade WebKit Jan 4, 2026' is generic and does not clearly convey the main changes or objectives of this large multi-component pull request.	Consider a more specific title that highlights major changes such as 'JSC performance optimizations: Array.from fast paths, async inlining, string methods to C++' or similar to better reflect the primary objectives.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description is comprehensive and well-organized, covering JavaScriptCore optimizations, WTF improvements, memory safety work, SVG fixes, CSS/style changes, and platform updates with clear section headings and bullet points.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Source/JavaScriptCore/inspector/scripts/tests/expected/type-declaration-object-type.json-result (1)

1764-1773: LGTM: Refactor to SortedArrayMap for efficient enum parsing.

The switch to SortedArrayMap with ComparableASCIILiteral keys provides O(log n) lookup via binary search, which is an improvement over linear iteration. The keys are correctly sorted alphabetically ("Left", "Middle", "None", "Right").

Note: The indentation on line 1769 ( }) }; with 3 spaces) appears inconsistent with the rest of the block. Since this is auto-generated code, the fix should be applied to the generate-inspector-protocol-bindings.py script if this formatting is unintentional.

♻️ Duplicate comments (33)

LayoutTests/platform/win/fast/forms/input-number-click-expected.txt (1)

2-2: This typographical error was already flagged in a previous review.

The word "numnber" on line 2 should be corrected to "number" to match the test intent. This issue was previously flagged but remains unfixed in the current commit.

🔎 Proposed fix

-Test for the spin control behavior in a type=numnber input.
+Test for the spin control behavior in a type=number input.

Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_type_conversions_header.py (1)

169-172: Fix indentation to align with the opening statement.

The closing }) }; on line 172 uses 3 spaces, but should use 4 spaces to align with the opening static constexpr SortedArrayMap statement on line 169.

🔎 Proposed fix

-        lines.append('   }) };')
+        lines.append('    }) };')

LayoutTests/platform/wpe-legacy-api/TestExpectations (1)

87-87: Missing required bug reference for Failure expectation.

Line 87 marks the test as [ Failure ] without a bug reference, violating the policy stated at line 29: "ALWAYS WITH BUG ENTRIES for things like Timeout, Failure and Crash." This issue was already flagged in a previous review.

LayoutTests/platform/wpe/TestExpectations (3)

529-538: Add bug entries for crash expectations and fix comment grammar.

These Skip entries for datalist crashes still lack the required bug references. According to the file guidelines (line 115), crash expectations must have associated bug entries. Additionally, the comment on line 529 contains a grammatical error.

---

544-547: Add bug entries for failure expectations.

These datalist test failures still lack the required bug references per the file guidelines (line 115).

---

1011-1011: Add bug entry for the new failure expectation.

This new canvas toDataURL test failure still lacks the required bug reference per the file guidelines (line 115).

LayoutTests/platform/mac-wk2/TestExpectations (1)

2453-2455: Truncated assertion path in comment (previously flagged).

The comment on line 2453 has a truncated path ending with bindings/js/ without specifying the complete source file. This was previously noted in past reviews. Consider updating the comment to include the full file path when available to aid future debugging.

The Skip directives on lines 2454-2455 are properly formatted with the Debug configuration qualifier and appropriate bug reference.

LayoutTests/fast/lists/list-marker-outside-inside-flex-item.html (1)

1-24: Test is functionally correct.

The test structure and CSS properties are appropriate for validating list marker behavior with flex item children. The two test cases effectively cover baseline positioning and margin-left variants.

Note: Previous review comments identified minor style preferences (blank line at Line 5, expanding the test comment), but these don't affect the test's functionality.

LayoutTests/imported/w3c/web-platform-tests/web-animations/crashtests/set-timeline-undefined-progress.html (1)

17-18: Remove the empty script tag.

The empty <script> tag serves no purpose and should be removed.

LayoutTests/svg/as-image/svg-image-aspect-ration-par-none.html (1)

1-1: Fix typo in filename.

This issue was already identified in a previous review. The filename contains "aspect-ration-par-none" but should be "aspect-ratio-par-none".

JSTests/stress/async-stack-trace-nested.js (1)

175-178: Test expectations correctly updated for line number shift.

The test expectation updates accurately reflect the one-line shift caused by the insertion at line 155. However, if the unreachable code on line 155 is removed as suggested, these changes should also be reverted.

LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-trigger/event-trigger-alternate.tentative.html (1)

42-45: Incorrect currentTime calculation.

The calculation on line 44 is incorrect. animation.currentTime is relative to the animation's own timeline (starting from 0), not the document timeline. Adding animation.startTime (which is the document timeline time when the animation started) produces an incorrect value.

🔎 Proposed fix

         // Advance to middle of duration so that animation doesn't finish too
         // quickly after reversing.
-        animation.currentTime = animation.startTime + (ANIMATION_DURATION_MS/2);
+        animation.currentTime = ANIMATION_DURATION_MS / 2;
         await waitForAnimationFrames(1);

LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-trigger/event-trigger-repeat.tentative.html (1)

36-44: Previous timing concern already documented.

The potential race condition in the progress comparison at lines 40-44 has been identified in the past review. The rest of the test logic appears sound.

LayoutTests/http/tests/text-extraction/resources/subframe2.html (1)

17-21: The postMessage wildcard usage has already been flagged.

A previous review already identified that the "*" targetOrigin in the postMessage call should be replaced with a specific origin for better security practices in test code.

LayoutTests/fast/text-extraction/debug-text-extraction-shorten-urls.html (1)

34-34: Correct misleading link text.

The link text says "Email with params" but the mailto: URL contains no parameters (like ?subject= or &body=). Either add parameters to match the description or update the link text.

🔎 Proposed fix

Option 1 - Add parameters to match the description:

-        <a href="mailto:[email protected]">Email with params</a>
+        <a href="mailto:[email protected]?subject=Test&body=Hello">Email with params</a>

Option 2 - Update the link text to match the URL:

-        <a href="mailto:[email protected]">Email with params</a>
+        <a href="mailto:[email protected]">Email link</a>

LayoutTests/http/tests/text-extraction/resources/subframe1.html (2)

24-24: Fix invalid JavaScript URL.

The javascript:void link is missing parentheses and should be javascript:void(0) to be valid JavaScript. While this is a test fixture, using correct syntax ensures the test accurately reflects real-world scenarios.

🔎 Proposed fix

-            <a href="javascript:void">Forgot your email?</a>
+            <a href="javascript:void(0)">Forgot your email?</a>

---

36-45: Avoid wildcard origin in postMessage and validate message origin.

The script uses postMessage("loaded", "*") which broadcasts the message to any origin, and the message listener doesn't validate event.origin. While this is a test fixture, it demonstrates insecure messaging patterns that could be copied into production code.

🔎 Proposed fix

     addEventListener("load", () => {
         let subframe = document.querySelector("iframe");
         addEventListener("message", event => {
+            // Validate message origin if needed, or at minimum check source
+            if (event.source !== subframe.contentWindow)
+                return;
             if (event.data === "loaded")
-                window.parent.postMessage("loaded", "*");
+                window.parent.postMessage("loaded", window.location.origin);
         }, { once: true });
         subframe.src = "subframe2.html";
     });

Note: For same-origin frames, you can use window.location.origin. For cross-origin scenarios in tests, consider using a specific expected origin rather than "*".

LayoutTests/http/tests/text-extraction/debug-text-extraction-subframes.html (1)

36-44: Fix race condition and add origin validation in loadFrame.

The loadFrame function has two issues:

1. Race condition: When loading multiple frames sequentially, a "loaded" message from frame 2 could incorrectly resolve frame 1's promise, since the listener doesn't verify which frame sent the message.

2. Missing origin validation: The message listener doesn't validate event.source or event.origin, accepting "loaded" messages from any window or frame.

🔎 Proposed fix

 async function loadFrame(iframe, src) {
     await new Promise(resolve => {
-        addEventListener("message", event => {
+        const handler = (event) => {
+            // Verify the message came from the expected iframe
+            if (event.source !== iframe.contentWindow)
+                return;
             if (event.data === "loaded") {
+                removeEventListener("message", handler);
                 resolve();
+            }
-        }, { once: true });
+        };
+        addEventListener("message", handler);
         iframe.src = src;
     });
 }

Note: The fix removes { once: true } and manually calls removeEventListener only when the correct source is confirmed, preventing the listener from being removed by messages from unrelated frames.

LayoutTests/platform/ios/TestExpectations (1)

8185-8187: Add a tracking bug tag for the new WebVTT ImageOnlyFailure

The new expectation for line_1_wrapped_cue_grow_downwards.html mirrors the adjacent media_height_19.html test but lacks a webkit.org/b/... bug tag. For future triage, consider either:

• Prepending the same bug id as media_height_19.html if they share a root cause, e.g.:

Example adjustment

-imported/w3c/web-platform-tests/webvtt/rendering/cues-with-video/processing-model/line_1_wrapped_cue_grow_downwards.html [ ImageOnlyFailure ]
+webkit.org/b/296709 imported/w3c/web-platform-tests/webvtt/rendering/cues-with-video/processing-model/line_1_wrapped_cue_grow_downwards.html [ ImageOnlyFailure ]

• Or adding a new bug id specific to this test.

JSTests/stress/regexp-prototype-symbol-replace-named-groups.js (1)

52-54: Undefined variable testLoopCount will cause ReferenceError.

This issue was flagged in a previous review. The variable testLoopCount is referenced but never defined.

JSTests/stress/regexp-prototype-symbol-replace-capture-groups.js (1)

62-64: Undefined variable testLoopCount will cause ReferenceError.

This issue was flagged in a previous review. The variable testLoopCount is referenced but never defined.

JSTests/stress/regexp-prototype-symbol-replace-basic.js (1)

56-58: Undefined variable testLoopCount will cause ReferenceError.

This issue was flagged in a previous review. The variable testLoopCount is referenced but never defined.

JSTests/stress/regexp-prototype-symbol-replace-functional.js (1)

95-97: Undefined variable testLoopCount will cause ReferenceError.

This issue was flagged in a previous review. The variable testLoopCount is referenced but never defined.

LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/TimelineTrigger/constructor.html (1)

3-5: Update the help link to reference TimelineTrigger.

The title has been correctly updated to "TimelineTrigger constructor", but the help link on line 5 still references the old "animationtrigger-interface" fragment. This inconsistency should be resolved.

🔎 Proposed fix

 <title>TimelineTrigger constructor</title>
 <link rel="help"
-      href="https://drafts.csswg.org/web-animations-2/#the-animationtrigger-interface">
+      href="https://drafts.csswg.org/web-animations-2/#the-timelinetrigger-interface">
 <script src="/https/github.com/resources/testharness.js"></script>

LayoutTests/platform/mac-sonoma-wk1/transforms/3d/general/perspective-non-layer-expected.txt (1)

6-6: Platform-specific width variation requires verification.

Similar to the mac-wk2 file, the RenderInline {SPAN} width has been reduced to 148. The layer repositioning at line 11 (from 15,12 in mac-wk2 to 11,11 here) is an expected downstream effect of the width change.

This represents a different width than mac-wk2 (148 vs 156), which is typical for platform-specific rendering. However, the root cause verification requested in the previous comment applies here as well—ensure this change is intentional and caused by the intended code modifications in the PR.

Also applies to: 8-8, 11-11

LayoutTests/platform/mac-sequoia-wk2/transforms/3d/general/perspective-non-layer-expected.txt (1)

6-6: Consistent pattern across platforms confirms intentional change.

The RenderInline {SPAN} width reduction to 148 matches the mac-sonoma-wk1 variant, and the layer repositioning is consistent (line 11: 11,11). This three-way consistency across platform expectations strongly suggests an intentional, coordinated rendering behavior change rather than a test fluke.

However, the root cause verification remains critical—ensure that the underlying code change (from the CSS/layout fixes mentioned in the PR) is correct and that these files were regenerated via the test suite rather than manually edited.

Also applies to: 8-8, 11-11

LayoutTests/platform/win/TestExpectations (2)

4618-4622: Offscreen-canvas remote-read image skip is fine; optional metadata for consistency

Adding http/tests/security/offscreen-canvas-remote-read-remote-image.html [ Skip ] matches other OffscreenCanvas/remote-read skips and the earlier “OffscreenCanvas isn’t supported yet” block. Optionally, for consistency with nearby canvas-remote-read-remote-video-* entries, you could add a short # Failure / # Crash comment and/or reuse their bug reference if it’s the same root cause.

---

1960-1967: Add rationale/bug reference for new performance resource timing cross-origin media skips

The four performance-resource-timing-*-cross-origin-media*.html tests are now skipped on Win without any accompanying comment or bug link, unlike the adjacent webkit.org/b/226299 entries. Consider adding a brief comment (and bug ID if available) explaining whether these are flaky or blocked by a Win-specific limitation so future gardeners know when and why to revisit them.

JSTests/stress/array-from-cloned-arguments.js (1)

1-11: Strict mode directive is ineffective where it is placed

"use strict"; on Line 6 is after a function declaration, so it does not form a directive prologue and the file (and test) are not in strict mode. If the goal is to cover the strict‑mode Array.from(arguments) / ClonedArguments fast path, move the directive to the top of the file or make it the first statement inside test().

JSTests/stress/unused-catch-binding.js (1)

261-279: Async IIFEs still not awaited; assertion failures may not be surfaced

The async IIFEs for testAsyncUnusedCatch and testAsyncUsedCatch are invoked but their returned promises are ignored. Because shouldBe throws inside an async function, failures will become promise rejections rather than synchronous errors, and may not be seen by the harness.

To make failures reliably visible, either:

• Add a top‑level async wrapper that awaits these IIFEs, or
• Convert these tests to synchronous functions (dropping async) if you don’t need true async behavior.

LayoutTests/platform/glib/TestExpectations (1)

1120-1125: Please add bug IDs (or Bug(GLIB)) to new non‑PASS/Skip expectations

Per the file header, only PASSING/UNSUPPORTED entries should omit bug IDs, and an earlier review already flagged these same spots. These new or updated non‑PASS entries still have no bug prefix:

• Lines 1120–1125: compositing/blend-mode/non-separable-blend-modes.html and the four compositing/repaint/copy-forward-* tests (ImageOnlyFailure / ImageOnlyFailure Pass).
• Lines 2615–2617: webrtc/peerconnection-new-candidate-page-cache.html, fast/mediastream/RTCPeerConnection-iceconnectionstatechange-event.html, http/wpt/webrtc/rtcNetworkInterface.html are [ Skip ] under the LibWebRTC block but lack an explicit bug, unlike 2613–2614 which use webkit.org/b/235885.
• Line 3994 (and ideally the neighbor at 3995 while you are here): imported/blink/fast/backgrounds/root-background-with-page-scaled-out.html [ ImageOnlyFailure Pass ] in NEEDS TRIAGING with no bug.

Please either reuse the appropriate existing bug IDs (e.g. 235885 for the LibWebRTC tests) or file a new bug/Bug(GLIB) and prefix each line explicitly.

Example of how to make these expectations traceable

- compositing/blend-mode/non-separable-blend-modes.html [ ImageOnlyFailure ]
+ Bug(GLIB) compositing/blend-mode/non-separable-blend-modes.html [ ImageOnlyFailure ]

- compositing/repaint/copy-forward-dirty-region-purged-backbuffer.html [ ImageOnlyFailure Pass ]
+ Bug(GLIB) compositing/repaint/copy-forward-dirty-region-purged-backbuffer.html [ ImageOnlyFailure Pass ]

- compositing/repaint/copy-forward-clear-rect.html [ ImageOnlyFailure Pass ]
+ Bug(GLIB) compositing/repaint/copy-forward-clear-rect.html [ ImageOnlyFailure Pass ]

- compositing/repaint/copy-forward-dirty-region-purged.html [ ImageOnlyFailure Pass ]
+ Bug(GLIB) compositing/repaint/copy-forward-dirty-region-purged.html [ ImageOnlyFailure Pass ]

- compositing/repaint/not-self-painting-layer-stops-compositing.html [ ImageOnlyFailure Pass ]
+ Bug(GLIB) compositing/repaint/not-self-painting-layer-stops-compositing.html [ ImageOnlyFailure Pass ]

- webrtc/peerconnection-new-candidate-page-cache.html [ Skip ]
+ webkit.org/b/235885 webrtc/peerconnection-new-candidate-page-cache.html [ Skip ]

- fast/mediastream/RTCPeerConnection-iceconnectionstatechange-event.html [ Skip ]
+ webkit.org/b/235885 fast/mediastream/RTCPeerConnection-iceconnectionstatechange-event.html [ Skip ]

- http/wpt/webrtc/rtcNetworkInterface.html [ Skip ]
+ webkit.org/b/235885 http/wpt/webrtc/rtcNetworkInterface.html [ Skip ]

- imported/blink/fast/backgrounds/root-background-with-page-scaled-out.html [ ImageOnlyFailure Pass ]
+ Bug(GLIB) imported/blink/fast/backgrounds/root-background-with-page-scaled-out.html [ ImageOnlyFailure Pass ]

Adjust the specific bug IDs if these map to different issues than suggested above.

Also applies to: 2613-2617, 3994-3995

LayoutTests/TestExpectations (1)

609-609: Formatting inconsistency persists.

Line 609 still has a double space before [ Pass ], which is inconsistent with the other entries (lines 605-608, 610-613). This was previously flagged but remains unfixed.

🔎 Proposed fix

-imported/w3c/web-platform-tests/webvtt/rendering/cues-with-video/processing-model/dom_override_cue_line.html  [ Pass ]
+imported/w3c/web-platform-tests/webvtt/rendering/cues-with-video/processing-model/dom_override_cue_line.html [ Pass ]

Source/JavaScriptCore/parser/Parser.cpp (1)

2281-2286: ImplementationVisibility propagation and caching: semantics improved; avoid enum-order-dependent std::max (same concern as prior review)

The new visibility wiring does the right high-level things:

• In parseFunctionBody, async wrappers that actually use await now promote the implementation visibility and write it back to the current scope before building metadata (Lines 2281-2285), so the body scope’s view of visibility is authoritative.
• In parseFunctionInfo, for generator/async-wrapper modes you now:
  • Parse the body in a dedicated generatorBodyScope with an inner body parse mode (Lines 2713-2726),
  • Read generatorBodyScope->implementationVisibility() back into a local implementationVisibility (Line 2733),
  • Use that value for createFunctionMetadata(...) (through performParsingFunctionBody()), and
  • Store the same implementationVisibility into the cache parameters (Line 2777).
• On the cached path, you correctly reconstruct metadata using static_cast<ImplementationVisibility>(cachedInfo->implementationVisibility) (Line 2534), so cached and non-cached parses stay in sync.

Two minor robustness points remain, matching the earlier review on this area:

1. Avoid relying on enum ordering via std::max

   implementationVisibility = std::max(ImplementationVisibility::Private, implementationVisibility); assumes that:

   • The enum is totally ordered, and
   • “max” with Private expresses “at least Private”.

   That’s fragile if another state is added or the ordering changes. It would be safer (and clearer) to spell out the promotion explicitly, e.g.:

   if (implementationVisibility == ImplementationVisibility::Public)
       implementationVisibility = ImplementationVisibility::Private;

   or via a small helper like promoteToPrivateIfNeeded(ImplementationVisibility&) that you can reuse in both parseFunctionBody and any other place visibility promotion is needed.

2. Single helper for visibility computation

   You now compute/propagate implementationVisibility in multiple places (body parsing, wrapper handling, cache parameters, and async-body metadata in parseAsyncFunctionSourceElements). A tiny helper that encapsulates “given parser-level visibility and async-wrapper/body usage, what should the final visibility be?” would:

   • Make it harder for cached/non-cached paths to diverge, and
   • Document the intended visibility lattice in one place.

This comment repeats the spirit of the prior feedback on ImplementationVisibility handling in this file.

Also applies to: 2532-2537, 2711-2735, 2770-2778

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove unreachable code after throw statement.

The await 30; statement on line 155 is unreachable because line 154 throws an error and terminates execution. This dead code serves no purpose and should be removed.

🔎 Proposed fix

     throw new Error("error");
-    await 30;
 }

After removing the unreachable code, revert the test expectation updates on lines 175 and 178:

-            ["async test", "161:25"],
+            ["async test", "160:25"],
             ["drainMicrotasks", "[native code]"],
             ["shouldThrowAsync", "19:20"],
-            ["global code", "159:21"]
+            ["global code", "158:21"]

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In JSTests/stress/async-stack-trace-nested.js around lines 154-155, remove the
unreachable statement `await 30;` that follows the `throw new Error("error");`
on line 154; then restore the test expectation changes previously updated on
lines 175 and 178 back to their original values so the assertions match the code
flow after removing the dead await.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Undefined variable testLoopCount will cause ReferenceError.

The variable testLoopCount is referenced but never defined in this file, which will cause a ReferenceError when the test executes.

🔎 Proposed fix

Define testLoopCount at the top of the file:

+const testLoopCount = 10000;
+
 // Test 21: JIT warmup edge cases
 for (let i = 0; i < testLoopCount; i++) {
     shouldBe(/(?:)/g[Symbol.replace]("", "X"), "X");
     shouldBe(/(?:)/g[Symbol.replace]("abc", "-"), "-a-b-c-");
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In JSTests/stress/regexp-prototype-symbol-replace-edge-cases.js around lines 131
to 134, the loop uses testLoopCount which is not defined and will throw a
ReferenceError; fix by declaring and initializing testLoopCount near the top of
the file (e.g. const or let testLoopCount = <sane number>), or replace its usage
with a numeric literal, ensuring the chosen value is appropriate for stress
testing and consistent with other tests in this suite.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Undefined variable testLoopCount will cause ReferenceError.

The variable testLoopCount is referenced but never defined in this file, which will cause a ReferenceError when the test executes.

🔎 Proposed fix

Define testLoopCount at the top of the file:

+const testLoopCount = 10000;
+
 // Test 14: JIT warmup with unicode
 for (let i = 0; i < testLoopCount; i++) {
     shouldBe(/./gu[Symbol.replace]("\u{1F600}\u{1F601}", "X"), "XX");
 }

🤖 Prompt for AI Agents

In JSTests/stress/regexp-prototype-symbol-replace-unicode.js around lines 68 to
70, the loop references an undefined variable testLoopCount which will throw a
ReferenceError; declare and initialize testLoopCount near the top of the file
(e.g., const or let with an appropriate iteration count) before it is used, or
replace its usage with a numeric literal, ensuring the test harness uses a
consistent loop count variable accessible in this scope.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Note the quote style inconsistency.

The three new MathML properties use double quotes ("math-depth", "integer", "normal", "compact") while the rest of the file consistently uses single quotes (e.g., 'alignment-baseline', 'color', 'baseline').

Since this is an imported W3C web-platform-test, this inconsistency might reflect the upstream source. However, if this divergence was introduced locally, consider normalizing to single quotes for consistency.

🔎 Proposed fix for quote consistency

-  "math-depth": {
+  'math-depth': {
     // https://w3c.github.io/mathml-core/#propdef-math-depth
-    types: [ "integer" ],
+    types: [ 'integer' ],
   },
-  "math-shift": {
+  'math-shift': {
     // https://w3c.github.io/mathml-core/#propdef-math-shift
     types: [
-      { type: "discrete", options: [ [ "normal", "compact" ] ] },
+      { type: 'discrete', options: [ [ 'normal', 'compact' ] ] },
     ],
   },
-  "math-style": {
+  'math-style': {
     // https://w3c.github.io/mathml-core/#propdef-math-style
     types: [
-      { type: "discrete", options: [ [ "normal", "compact" ] ] },
+      { type: 'discrete', options: [ [ 'normal', 'compact' ] ] },
     ],
   },

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"math-depth": {
	// https://w3c.github.io/mathml-core/#propdef-math-depth
	types: [ "integer" ],
	},
	"math-shift": {
	// https://w3c.github.io/mathml-core/#propdef-math-shift
	types: [
	{ type: "discrete", options: [ [ "normal", "compact" ] ] },
	],
	},
	"math-style": {
	// https://w3c.github.io/mathml-core/#propdef-math-style
	types: [
	{ type: "discrete", options: [ [ "normal", "compact" ] ] },
	],
	},
	'math-depth': {
	// https://w3c.github.io/mathml-core/#propdef-math-depth
	types: [ 'integer' ],
	},
	'math-shift': {
	// https://w3c.github.io/mathml-core/#propdef-math-shift
	types: [
	{ type: 'discrete', options: [ [ 'normal', 'compact' ] ] },
	],
	},
	'math-style': {
	// https://w3c.github.io/mathml-core/#propdef-math-style
	types: [
	{ type: 'discrete', options: [ [ 'normal', 'compact' ] ] },
	],
	},

🤖 Prompt for AI Agents

LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/animation-types/property-list.js
around lines 908-923: the newly added MathML properties use double quotes while
the rest of the file uses single quotes; normalize the quote style by replacing
double quotes with single quotes for the property keys and all string literals
within those property definitions (e.g., 'math-depth', 'integer', 'math-shift',
'math-style', and the options 'normal' and 'compact') so the file follows the
established single-quote convention.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Remove unnecessary async keyword.

The function is declared as async but never uses await. Remove the async keyword for clarity.

🔎 Proposed fix

-  window.onload = async () => {
+  window.onload = () => {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	window.onload = async () => {
	window.onload = () => {

🤖 Prompt for AI Agents

In
LayoutTests/imported/w3c/web-platform-tests/web-animations/crashtests/set-timeline-undefined-progress.html
around line 23, the window.onload handler is declared as an async function but
does not use await; remove the unnecessary async keyword so the handler is a
plain function (e.g., change "window.onload = async () => {" to "window.onload =
() => {") and ensure no other code relies on a returned Promise.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

shouldEmitToThis helper is a good centralization of to_this emission and is used consistently.

The new lambda captures the existing heuristics (direct this use, eval, inner arrow this/super/eval, and sloppy super property) and is now reused for:

• Async functions without await (to avoid eagerly converting this when it’s never observed), and
• Non-constructor functions in the default parse-mode branch.

For async functions with await, you preserve prior behavior by still unconditionally emitting to_this (non-arrow), so there’s no behavior regression there. The lambda’s use of m_scopeNode->doAnyInnerArrowFunctionsUse* matches the existing arrow-context logic elsewhere in the file, and there are no new Bun-specific paths introduced.

If you later want to reduce unnecessary to_this conversions for async functions with await as well, you could switch those sites to also honor shouldEmitToThis(), but that would be a semantic change and probably belongs in its own patch. As per coding guidelines, this change itself keeps behavior compatible while simplifying the decision logic.

Also applies to: 819-843, 900-905

🤖 Prompt for AI Agents

In Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp around lines 785 to
805 (and also apply the same change at the similar sites around 819-843 and
900-905), consolidate the repeated heuristics for deciding when to emit to_this
into the new shouldEmitToThis lambda and replace the duplicated conditional
logic at the async/no-await site and the non-constructor/default parse-mode
branch with calls to shouldEmitToThis(); ensure that sites that must still
unconditionally emit to_this (e.g., async functions that contain await) are left
unchanged so behavior stays identical.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Async “no-await” fast path is structurally sound; consider a couple of invariants and control-flow details

The new isAsyncFunctionWithoutAwait() fast path that inlines the async body under a synthetic try/catch/finally and resolves/rejects the promise directly looks like the right high‑level shape and correctly reuses:

• resolvePromiseWithFirstResolvingFunctionCallCheck / rejectPromiseWithFirstResolvingFunctionCallCheck,
• FinallyContext and emitReturnViaFinallyIfNeeded via the pushed finally scope, and
• existing this handling for async arrows.

A few points worth explicitly validating:

1. AST shape vs. isAsyncFunctionWithoutAwait() assumption

   This path calls emitStatementsBytecode(generator, generator.ignoredResult()); on the current FunctionNode. That assumes that, when isAsyncFunctionWithoutAwait() is true, this node’s m_statements contain the original user body (not the “single FuncExprNode wrapper” shape used in the generator-based async path).

   If the front‑end guarantees that (i.e., it only builds the wrapper+inner FuncExprNode form when awaits are present), this is fine. If not, this would just emit the inner function expression and never execute the intended body.

   It might be worth adding a debug‑only assertion along the lines of “when isAsyncFunctionWithoutAwait() is true, singleStatement() is either null or not a single FuncExprNode expression statement” to guard that invariant.

2. Scope register restoration in the synthetic catch

   In TryNode::emitBytecode, a similar pushTry/emitOutOfLineCatchHandler pattern records localScopeCountBeforeTryBlock and conditionally calls restoreScopeRegister() in the catch path when the try body introduced new lexical scopes. Here, the outer try around emitStatementsBytecode doesn’t perform that check before running the synthetic catch body.

   If emitStatementsBytecode for the async body can introduce additional lexical scopes (typical for let/const blocks), consider mirroring the localScopeCountBeforeTryBlock / scopeRegisterMayBeClobbered logic from TryNode so that the scope register is in a known-good state at catchLabel, or confirm that emitOutOfLineCatchHandler alone is sufficient in this particular configuration.

3. Exceptions thrown in the synthetic catch / finally plumbing

   The second pushTry(..., HandlerType::Finally) around the rejectPromiseWithFirstResolvingFunctionCallCheck call plus the later emitOutOfLineFinallyHandler(finallyContext.completionValueRegister(), finallyContext.completionTypeRegister(), catchTryData); rely on the generic finally machinery, but this fast path never calls emitFinallyCompletion. That’s deliberate for the normal/return/throw flow you’re driving, but it changes how engine‑thrown exceptions from the reject path (or its helpers) are treated compared to the existing generator-based async path.

   It would be good to verify (with tests or inspection of BytecodeGenerator helpers) that:

   • exceptions from the catch‑path helpers either cannot happen in practice or are treated in a way you’re comfortable with (e.g., surfacing as unhandled exceptions rather than accidentally resolving the promise), and
   • there are no latent assumptions inside emitOutOfLineFinallyHandler that require a paired emitFinallyCompletion in this usage.

If these invariants hold, the fast path should preserve the observable async semantics while giving you the intended allocation and dispatch win for await‑free async functions.

🤖 Prompt for AI Agents

In Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp around lines 5476-5554,
add two fixes: 1) before inlining the async body when
generator.isAsyncFunctionWithoutAwait() is true, insert a debug-only assertion
that the function node’s singleStatement() is null or not a solitary
FuncExprNode expression (to ensure we’re emitting the real body, not a wrapper
FuncExprNode), and 2) mirror TryNode’s scope-restoration logic around the
try/catch: record localScopeCountBeforeTryBlock (and the
scopeRegisterMayBeClobbered condition) before emitStatementsBytecode, and in the
synthetic catch path restore the scope register if needed prior to calling
emitOutOfLineCatchHandler so lexical-scope registers are correct; also add a
short comment and/or test coverage to validate that exceptions thrown by the
reject-path helpers are acceptable (or adjust handling) and that
emitOutOfLineFinallyHandler does not depend on a paired emitFinallyCompletion in
this fast path.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Minor: Consider validating the enum cast.

The static_cast<ImplementationVisibility> during cache restoration is a common deserialization pattern, but the PR description mentions "conversion away from many static_cast usages." While this cast should be safe in practice (the cache should contain valid values), consider adding validation or documenting the assumption that info->implementationVisibility contains a valid enum value.

💡 Optional: Add validation for type safety

-        m_implementationVisibility = static_cast<ImplementationVisibility>(info->implementationVisibility);
+        auto visibility = static_cast<ImplementationVisibility>(info->implementationVisibility);
+        ASSERT(visibility == ImplementationVisibility::Public || visibility == ImplementationVisibility::Private || visibility == ImplementationVisibility::PrivateRecursive);
+        m_implementationVisibility = visibility;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	m_implementationVisibility = static_cast<ImplementationVisibility>(info->implementationVisibility);
	auto visibility = static_cast<ImplementationVisibility>(info->implementationVisibility);
	ASSERT(visibility == ImplementationVisibility::Public || visibility == ImplementationVisibility::Private || visibility == ImplementationVisibility::PrivateRecursive);
	m_implementationVisibility = visibility;

🤖 Prompt for AI Agents

In Source/JavaScriptCore/parser/Parser.h around line 863, the static_cast to
ImplementationVisibility from info->implementationVisibility lacks validation;
add a bounds check (or a small helper that maps/validates the integer) before
casting and handle out-of-range values (e.g., assert/log and assign a safe
default or return an error) and/or add a brief comment documenting the
assumption that the cached value must be valid.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

ClonedArguments length handling looks correct but relies on internal invariants

Using getArgumentsLength(ClonedArguments*) to read the length property via getDirect(clonedArgumentsLengthPropertyOffset) under isIteratorProtocolFastAndNonObservable() matches the intent to respect the visible .length while still using the generic tryCreateArrayFromArguments fast path. The two‑phase scan (indexing type then write) and reliance on getIndexQuickly are consistent with the existing Scoped/DirectArguments handling.

If you expect any future callers of tryCreateArrayFromClonedArguments outside this file, consider adding a debug ASSERT(length == arguments->internalLength()) (or similar) to catch mismatches early, but the current design is fine given the invariant checks already in place.

Based on learnings, this stays within the runtime fast‑path conventions for arguments objects.

Also applies to: 274-324, 336-339

🤖 Prompt for AI Agents

In Source/JavaScriptCore/runtime/ArrayConstructor.cpp around lines 250-266 (also
applies to 274-324 and 336-339), add a debug assertion after reading lengthValue
in getArgumentsLength(ClonedArguments*) that verifies the visible length matches
the internal length (i.e. assert lengthValue.asInt32() ==
arguments->internalLength()), placing it after the existing ASSERTs and before
returning the length; keep the check gated to debug builds so it does not affect
release behavior and retain the existing
isIteratorProtocolFastAndNonObservable() invariant.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Set fast path is sound; optional extra assertion for safety

The tryCreateArrayFromSet helper correctly:

• Derives length from set->size() and bails out on empty/sentinel.
• Runs a first pass to compute indexingType and allocate an appropriate Butterfly.
• Runs a second pass to copy entries into either the double or contiguous storage.
• Guards usage via set->isIteratorProtocolFastAndNonObservable() so no user code runs during iteration.

Given both passes rely on size() matching the number of iterated entries, you might optionally add a debug ASSERT(i == length); after the second pass to catch any internal mismatch between size() and iteration semantics, but behavior is otherwise consistent with existing array fast paths.

As per coding guidelines, this keeps the optimization entirely inside the JSC runtime without changing the public Array.from semantics.

🤖 Prompt for AI Agents

In Source/JavaScriptCore/runtime/ArrayConstructor.cpp around lines 341 to 436,
add a debug assertion to validate that the number of copied entries equals the
set size: after the second pass that fills resultButterfly (i.e., just before
Butterfly::clearRange(...)), insert ASSERT(i == length); to catch any internal
mismatch between set->size() and the actual iterated entries during
development/debug builds.

[github-actions]

⚠️ Preview build was cancelled

This may have been caused by a newer build being triggered for the same PR.

[sosukesuzuki]

/build-preview

[github-actions]

🔄 Preview build started

Release tag: autobuild-preview-pr-124-ae95679d
Workflow: View progress

[github-actions]

✅ Preview build completed

Release: autobuild-preview-pr-124-ae95679d

[sosukesuzuki]

/build-preview

[github-actions]

🔄 Preview build started

Release tag: autobuild-preview-pr-124-de829ef3
Workflow: View progress

[github-actions]

❌ Preview build failed

Check the workflow run for details.

[github-actions]

❌ Preview build failed

Check the workflow run for details.

[sosukesuzuki]

/build-preview

[github-actions]

🔄 Preview build started

Release tag: autobuild-preview-pr-124-4a684d45
Workflow: View progress

[github-actions]

✅ Preview build completed

Release: autobuild-preview-pr-124-4a684d45

[sosukesuzuki]

/build-preview

[github-actions]

🔄 Preview build started

Release tag: autobuild-preview-pr-124-4a684d45
Workflow: View progress

[github-actions]

✅ Preview build completed

Release: autobuild-preview-pr-124-4a684d45

[sosukesuzuki]

/build-preview

[github-actions]

🔄 Preview build started

Release tag: autobuild-preview-pr-124-710bb417
Workflow: View progress

[github-actions]

✅ Preview build completed

Release: autobuild-preview-pr-124-710bb417