Upgrade to upstream WebKit 24362e675175#251
Conversation
https://bugs.webkit.org/show_bug.cgi?id=316519 Reviewed by Adrian Perez de Castro. Make sure the User-Agent used for HBO Max is the one matching Firefox, otherwise if the application uses a non-default (like Android) User-Agent WebSetting we might be redirected to a page asking the user to install the HBO Android app. Test: Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp * Source/WebCore/platform/glib/UserAgentQuirks.cpp: (WebCore::urlRequiresFirefoxBrowser): * Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp: (TestWebKitAPI::TEST(UserAgentTest, Quirks)): Canonical link: https://commits.webkit.org/314746@main
https://bugs.webkit.org/show_bug.cgi?id=295350 rdar://155454170 Reviewed by Elliott Williams. Add a --delete-existing/--no-delete-existing flag, similar to the one git-webkit already supports, to control whether the exporter may overwrite existing local branches or force-push to existing remote branches. Unlike git-webkit, which defaults to rebasing an existing branch, the exporter defaults to --no-delete-existing and refuses to proceed when it finds pre-existing state. As part of this, stop unconditionally force-pushing: push without -f first, parse the --porcelain output to detect a non-fast-forward rejection, and only force-push if --delete-existing was given. * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/local/git.py: (Git): Add push_error property for injecting push failures in tests. Add show-ref --verify --quiet and push --porcelain routes, along with show_ref_verify and push_porcelain methods. * Tools/Scripts/webkitpy/w3c/test_exporter.py: (WebPlatformTestExporter.create_branch_with_patch): Check whether the local branch exists with show-ref --verify --quiet; refuse to proceed without --delete-existing. Use checkout -b when creating, -B when overwriting. (WebPlatformTestExporter.push_to_wpt_fork): Replace unconditional push -f with push --porcelain; parse output to distinguish non-fast-forward rejections from fatal errors, and only force-push on rejection when --delete-existing is set. (parse_args): Add --delete-existing/--no-delete-existing via arguments.NoAction. * Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py: (TestExporterTest.test_export_local_branch_exists): Pass --delete-existing. (TestExporterTest.test_export_remote_branch_exists_no_pr): Ditto. (TestExporterTest.test_export_updates_existing_open_pr): Ditto. (TestExporterTest.test_export_local_and_remote_exist_with_open_pr): Ditto. (TestExporterTest.test_export_creates_new_pr_when_existing_is_closed): Ditto. (TestExporterTest.test_export_local_branch_exists_errors_without_flag): (TestExporterTest.test_export_remote_branch_exists_errors_without_flag): (TestExporterTest.test_export_push_fatal_error): (TestExporterTest.test_export_push_non_rejected_error): (TestExporterTest.test_export_open_pr_errors_without_flag): (TestExporterTest.test_export_local_and_open_pr_errors_without_flag): (TestExporterTest.test_export_closed_pr_succeeds_without_flag): Canonical link: https://commits.webkit.org/314747@main
https://bugs.webkit.org/show_bug.cgi?id=316493 rdar://178919004 Reviewed by Mike Wyrzykowski. Saves about 5s in a clean build. (Worth 120 CPUs on a Mac Studio, since configuration is single-core and blocks the rest of the build). * CMakeLists.txt: Moved header map generation to be a single Python invocation at the end of configuration, instead of one invocation per target, since each invocation costs like 150ms. * Source/ThirdParty/ANGLE/PlatformCocoa.cmake: Only search for ZLib if needed. * Source/WTF/Scripts/generate-unified-source-bundles.py: Run --print-bundled-sources during --generate-bundles so we can run this script once instead of twice. * Source/WTF/wtf/PlatformCocoa.cmake: Use absolute paths when specifying dependencies because absolute paths get the O(1) hash table treatment in depdendency resolution while relative paths get the O(N^2) linear scan treatment. * Source/WebCore/PlatformCocoa.cmake: Only search for SQLite and ZLib if needed. * Source/WebCore/WebCoreMacros.cmake: Use absolute paths. (See above.) * Source/cmake/OptionsCocoa.cmake: No need to search for things that we know are in the SDK. They have no other location. * Source/cmake/OptionsMac.cmake: Stop erroring when using a newer SDK to target an older OS. This is a supported configuration (for downlevels), and the warning was crufting up my output. * Source/cmake/WebKitCommon.cmake: Stop launching Ruby 16 times to evaluate all its features; we just need a version number. * Source/cmake/WebKitHeaderMap.cmake: Put all header map manifests in one place so they're easy to find at the end of configuration, for a single pass of header map generation. * Source/cmake/WebKitMacros.cmake: generate-unified-source-bundles.py only needs to run once now. Use absolutely paths to avoid the O(N^2) behavior. (See above.) * Tools/Scripts/generate-header-map: Search for all header map manifests in a single folder and generate them all. (Used to avoid lauching Python once per target. There are many targets.) * Tools/Scripts/hmaptool: Factored write_headermap into a helper function so we can call it more than once for more than one manifest. Canonical link: https://commits.webkit.org/314748@main
https://bugs.webkit.org/show_bug.cgi?id=316232 rdar://178656517 Reviewed by Mike Wyrzykowski. Note: Relanding 314554@main after correcting build error. Enables ENABLE_USER_SCRIPT_SANDBOXING=YES for six projects (bmalloc, PAL, WebGPU, DumpRenderTree, TestWebKitAPI, WebKitTestRunner), bringing them in line with the framework targets that already sandbox their script phases. Trims dead and now-sandbox-safe entries from EXCLUDED_USER_SCRIPT_SANDBOXING_PHASE_NAMES in WebKit, WebCore, JavaScriptCore, WebKitLegacy, and WTF. Removes alwaysOutOfDate = 1 from "Check For Inappropriate Files In Framework" in WebKit/WebCore/JavaScriptCore/WebKitLegacy in favor of declared inputs ($(TARGET_BUILD_DIR)/$(EXECUTABLE_PATH) plus the checker script) and a timestamp output that the shell script now materializes on every code path; the phase is now dependency-analyzed and will skip on incremental builds. Adds explicit name = "..." fields to five previously-unnamed script phases in DumpRenderTree, TestWebKitAPI, and WebKitTestRunner so that the exclusion list can match them (Xcode matches the explicit name field, not the "ShellScript" display fallback). These changes should improve the likelihood that the Xcodebuild scheduler will decide to run more of these steps in parallell. * Source/JavaScriptCore/Configurations/Base.xcconfig: * Source/WTF/Configurations/Base.xcconfig: * Source/WebCore/Configurations/Base.xcconfig: * Source/WebCore/PAL/Configurations/Base.xcconfig: * Source/WebGPU/Configurations/Base.xcconfig: * Source/WebKit/Configurations/Base.xcconfig: * Source/WebKit/Scripts/copy-webcontent-resources-to-private-headers.sh: * Source/WebKit/WebKit.xcodeproj/project.pbxproj: * Source/WebKitLegacy/mac/Configurations/Base.xcconfig: * Source/bmalloc/Configurations/Base.xcconfig: * Tools/DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj: * Tools/DumpRenderTree/mac/Configurations/Base.xcconfig: * Tools/TestWebKitAPI/Configurations/Base.xcconfig: * Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: * Tools/WebKitTestRunner/Configurations/Base.xcconfig: * Tools/WebKitTestRunner/WebKitTestRunner.xcodeproj/project.pbxproj: Canonical link: https://commits.webkit.org/314749@main
https://bugs.webkit.org/show_bug.cgi?id=315363 rdar://177719743 Reviewed by Etienne Segonzac. Add runtime switch for changing default dynamic-range-limit for model. Fixes a number of bugs where switching from standard to high didn't update the appearance of the model immedietly and a page reload was required. But CSS can set this property at any time so the feature was not functioning as expected. Also update tonemapping appearance to more closely match RealityCoreRenderer's tone mapping policy while ensuring 1.0 matches page white. * Source/WebCore/platform/graphics/cg/UTIRegistry.mm: (WebCore::allowableDefaultSupportedImageTypes): * Source/WebCore/rendering/RenderLayerBacking.cpp: (WebCore::RenderLayerBacking::updateConfiguration): * Source/WebKit/GPUProcess/graphics/Model/ModelRenderer.swift: (Renderer.tonemapEnabled): (Renderer.rasterSampleCount): (ColorAdjustTilePipelineKey.colorAdjustTilePipelineStates): (ImplicitFragmentInPlace.colorAdjustTilePipelineState(for:rasterSampleCount:)): (Renderer.createMaterialCompiler(_:)): Deleted. (Renderer.createRenderer(_:)): Deleted. (Renderer.newCommandBuffer): Deleted. (Renderer.setFOV(_:)): Deleted. (Renderer.setBackgroundColor(_:)): (Renderer.setCameraTransformForModelTransform(_:)): Deleted. * Source/WebKit/GPUProcess/graphics/Model/ModelTypes.h: * Source/WebKit/GPUProcess/graphics/Model/RemoteMesh.cpp: (WebKit::RemoteMesh::RemoteMesh): (WebKit::RemoteMesh::updateRenderBuffers): * Source/WebKit/GPUProcess/graphics/Model/RemoteMesh.h: * Source/WebKit/GPUProcess/graphics/Model/USDModel.swift: (WKBridgeUSDConfiguration.standardDynamicRange): (WKBridgeUSDConfiguration.makeRendererResources): (WKBridgeUSDConfiguration.createRenderer): * Source/WebKit/GPUProcess/graphics/Model/WebKitMesh.h: * Source/WebKit/GPUProcess/graphics/Model/WebKitMesh.mm: (WebKit::createMetalTextures): (WebKit::pixelFormatForDynamicRange): (WebKit::WebMesh::WebMesh): (WebKit::WebMesh::updateRenderBuffers): * Source/WebKit/GPUProcess/graphics/WebGPU/RemoteGPU.cpp: (WebKit::RemoteGPU::createRenderBuffers): (WebKit::createModelBackingInternal): (WebKit::RemoteGPU::createModelBacking): * Source/WebKit/GPUProcess/graphics/WebGPU/RemoteGPU.h: * Source/WebKit/GPUProcess/graphics/WebGPU/RemoteGPU.messages.in: * Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteGPUProxy.cpp: (WebKit::RemoteGPUProxy::createModelBacking): * Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteGPUProxy.h: * Source/WebKit/WebProcess/Model/WebModelPlayer.h: * Source/WebKit/WebProcess/Model/WebModelPlayer.mm: (WebKit::m_page): (WebKit::contentsFormatForDynamicRange): (WebKit::WebModelPlayer::load): (WebKit::WebModelPlayer::sizeDidChange): (WebKit::WebModelPlayer::contentsDisplayDelegate): (WebKit::WebModelPlayer::render): (WebKit::WebModelPlayer::visibilityStateDidChange): (WebKit::WebModelPlayer::updateContentsHeadroom): (WebKit::WebModelPlayer::updateScreenHeadroomFromPage): (WebKit::WebModelPlayer::updateScreenHeadroom): (WebKit::WebModelPlayer::setDynamicRangeLimit): (WebKit::WebModelPlayer::dynamicRangeLimitDidChange): * Tools/TestWebKitAPI/Tests/WebKit/WKWebView/mac/ScrollbarTests.mm: (-[ContainerView setCustomCornerRadius:]): Canonical link: https://commits.webkit.org/314750@main
…der of the Networking process https://bugs.webkit.org/show_bug.cgi?id=316124 rdar://178551436 Reviewed by Sihui Liu. To achieve this, we can move these directories to its container. We have previously done the same for the WebContent and GPU process. * Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.Networking.sb.in: * Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm: (WebKit::XPCServiceEventHandler): * Source/WebKit/UIProcess/Launcher/cocoa/ProcessLauncherCocoa.mm: (WebKit::ProcessLauncher::tryFinishLaunchingProcess): Canonical link: https://commits.webkit.org/314751@main
https://bugs.webkit.org/show_bug.cgi?id=316461 rdar://178858593 Reviewed by Brandon Stewart and Tim Nguyen. This also rebaselines web-platform-tests/fetch/local-network-access/. Upstream: web-platform-tests/wpt@6a45392 Canonical link: https://commits.webkit.org/314752@main
…sence of any other attributes https://bugs.webkit.org/show_bug.cgi?id=316489 rdar://178909853 Reviewed by Abrar Rahman Protyasha. Opportunistically surface class names and ID attributes in the html, textTree, and JSON output formats when an element would otherwise show up as nothing more than `uid=…`. However, only do this if the class name or ID is likely to convey any semantic meaning; to achieve this… 1. In the web process, we apply a cheap entropy filter (the existing isCandidateClassOrId predicate) to populate two new fields on `TextExtraction::Item` — `Vector<String> classNames`, capped at 5, and String `idAttribute`. 2. In the UI process, the new `TextExtractionTokenizer` helper class further filters those candidates against `NLEmbedding` word vocabularies for `{en, de, es, fr, it, pt}` — but only when the element carries no other semantic signal. The gate suppresses the hint when any of the following are present: `accessibilityRole`, `title`, `ariaAttributes`, `clientAttributes`, an editable `label`/`placeholder`/`name`/`pattern`, `altText` on images, `name`/`autocomplete` on forms, `completedURL` on links, `origin` on iframes, or text children with trimmed length > 2. The tokenizer splits on camelCase boundaries and HTML-attribute-safe symbols (-, _, :, ., /, whitespace), drops pure-digit segments, and accepts the input when recognized tokens cover strictly more than half of the total non-digit token characters across all six embeddings (English first, early-exit per language). This "mostly recognized" rule lets common abbreviations like `btn` ride along inside identifiers like `userMenuBtn` while still rejecting hashed/random class names. When both id and class survive, id wins. * LayoutTests/fast/text-extraction/debug-text-extraction-basic-expected.txt: * LayoutTests/fast/text-extraction/debug-text-extraction-basic.html: Cover both the surfacing paths (basic class hint, id beats class, half-recognized class) and the gate suppression paths (text > 2 chars, aria-label, input placeholder). * Source/WebCore/PAL/pal/cocoa/NaturalLanguageSoftLink.h: * Source/WebCore/PAL/pal/cocoa/NaturalLanguageSoftLink.mm: * Source/WebCore/page/text-extraction/TextExtraction.cpp: (WebCore::TextExtraction::extractRecursive): Populates classNames (capped at 5) and idAttribute on the per-item `TextExtraction::Item`, gated on the existing `isCandidateClassOrId` entropy heuristic. * Source/WebCore/page/text-extraction/TextExtractionTypes.h: * Source/WebKit/Platform/classifier/cocoa/TextExtractionTokenizer.h: Added. * Source/WebKit/Platform/classifier/cocoa/TextExtractionTokenizer.mm: Added. (WebKit::TextExtractionTokenizer::isMostlyRecognized): Implements the "more than half of token characters are recognized" rule. * Source/WebKit/Shared/TextExtractionToStringConversion.cpp: (WebKit::recognizedClassesAndIdForItem): Implements the suppression gate and id-over-class priority. Returns empty when the gate suppresses the hint, in which case neither classes nor id are surfaced. * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in: * Source/WebKit/SourcesCocoa.txt: * Source/WebKit/WebKit.xcodeproj/project.pbxproj: Canonical link: https://commits.webkit.org/314753@main
…able size https://bugs.webkit.org/show_bug.cgi?id=316548 rdar://179020924 Reviewed by Marcus Plutowski. Add vector-hint when creating an array for Promise combinators (e.g. Promise.all) based on length of iterable. So this avoids repeated extension of JSArray. * Source/JavaScriptCore/runtime/JSPromiseConstructor.cpp: (JSC::vectorLengthHintForCombinator): (JSC::promiseAllSlow): (JSC::JSC_DEFINE_HOST_FUNCTION): (JSC::promiseAllSettledSlow): (JSC::promiseAnySlow): Canonical link: https://commits.webkit.org/314754@main
https://bugs.webkit.org/show_bug.cgi?id=316434 rdar://178838754 Reviewed by Sihui Liu. When USE(EXTENSIONKIT) == 1, there is no need to create sandbox extensions for the temp and cache folder for the Networking process, since these are located in its container. * Source/WebKit/NetworkProcess/NetworkProcess.cpp: (WebKit::NetworkProcess::addWebsiteDataStore): * Source/WebKit/Shared/AuxiliaryProcess.cpp: (WebKit::AuxiliaryProcess::grantAccessToContainerTempDirectory): Deleted. * Source/WebKit/Shared/AuxiliaryProcess.h: * Source/WebKit/Shared/WebsiteDataStoreParameters.h: * Source/WebKit/Shared/WebsiteDataStoreParameters.serialization.in: * Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp: (WebKit::WebsiteDataStore::parameters): Canonical link: https://commits.webkit.org/314755@main
https://bugs.webkit.org/show_bug.cgi?id=316553 rdar://179025550 Reviewed by Yijia Huang. JSPromiseCombinatorsGlobalContext was used for normal combinator's context, or a bit random cell holders. Let's replace this random cell holder to JSSlimPromiseReaction instead. And then making JSPromiseCombinatorsGlobalContext cleaner and efficient: using uint64_t for m_remainingElementsCount and making access around it efficient. * Source/JavaScriptCore/runtime/JSMicrotask.cpp: (JSC::promiseAllResolveJob): (JSC::promiseAllSettledResolveJob): (JSC::promiseAnyResolveJob): (JSC::promiseFinallyAwaitJob): (JSC::promiseFinallyReactionJob): (JSC::dynamicImportLoadSettled): (JSC::dynamicImportDeferDependencySettled): (JSC::runInternalMicrotask): * Source/JavaScriptCore/runtime/JSPromise.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/JSPromiseCombinatorsGlobalContext.cpp: (JSC::JSPromiseCombinatorsGlobalContext::create): (JSC::JSPromiseCombinatorsGlobalContext::visitChildrenImpl): * Source/JavaScriptCore/runtime/JSPromiseCombinatorsGlobalContext.h: * Source/JavaScriptCore/runtime/JSPromiseConstructor.cpp: (JSC::promiseAllSlow): (JSC::JSC_DEFINE_HOST_FUNCTION): (JSC::promiseAllSettledSlow): (JSC::promiseAnySlow): * Source/JavaScriptCore/runtime/JSPromisePrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): Canonical link: https://commits.webkit.org/314756@main
…nested https://bugs.webkit.org/show_bug.cgi?id=310888 rdar://174010503 Reviewed by Simon Fraser. Because the transform we use to compensate for scrolling affects descendants (including out-of-flow descendants), a fixedpos anchored box inside another fixedpos box currently double-compensates for scrolling. This patch introduces an unwind() method to the AnchorScrollAdjuster so that it can avoid duplicating the scroll compensation of its parent. It also tags such boxes by setting a zeroed anchorScrollAdjustment on them even if their AnchorScrollAdjuster is fully empty, so that the compositing code knows that (unlike regular fixedpos elements) they can be scrolled into view. * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/anchor-scroll-fixedpos-nested-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/anchor-scroll-fixedpos-nested-001.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/anchor-scroll-fixedpos-nested-002-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/anchor-scroll-fixedpos-nested-002.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/reference/anchor-scroll-fixedpos-nested-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/reference/anchor-scroll-fixedpos-nested-002-ref.html: Added. * Source/WebCore/page/LocalFrameViewLayoutContext.cpp: (WebCore::LocalFrameViewLayoutContext::unregisterAnchorScrollAdjusterFor): * Source/WebCore/page/LocalFrameViewLayoutContext.h: * Source/WebCore/rendering/RenderLayer.h: Allow maintaining an anchorScrollAdjustment even if there is no anchorScrollAdjuster (to identify cases where the scroll adjustment is managed by an ancestor). * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::requiresCompositingForPosition const): Maintain compositing of scroll-adjusted fixedpos boxes, even if they're offscreen. * Source/WebCore/style/AnchorPositionEvaluator.cpp: (WebCore::AnchorScrollAdjuster::removeMatchingSnapshots): (WebCore::Style::clearAnchorScrollSnapshots): (WebCore::Style::AnchorPositionEvaluator::captureScrollSnapshots): * Source/WebCore/style/AnchorPositionEvaluator.h: Unwind scroll adjustments already performed by our ancestor. Canonical link: https://commits.webkit.org/314757@main
rdar://173245853 https://bugs.webkit.org/show_bug.cgi?id=310638 Reviewed by Sihui Liu. The steps for "set the application badge": https://www.w3.org/TR/badging/#dfn-set-the-application-badge ...clearly point out that only script from an origin matching the top origin can set the badge. Otherwise a security error should result. So let's implement that. Test: Tools/TestWebKitAPI/Tests/WebKitCocoa/Badging.mm LayoutTests/imported/w3c/web-platform-tests/badging/setAppBadge_cross_origin.sub.https * LayoutTests/imported/w3c/web-platform-tests/badging/setAppBadge_cross_origin.sub.https-expected.txt: * Source/WebCore/Modules/badge/NavigatorBadge.idl: * Source/WebCore/page/Navigator.cpp: (WebCore::Navigator::setAppBadge): (WebCore::Navigator::clearAppBadge): * Source/WebCore/page/Navigator.h: * Source/WebCore/page/WorkerNavigator.cpp: (WebCore::WorkerNavigator::setAppBadge): (WebCore::WorkerNavigator::clearAppBadge): * Source/WebCore/page/WorkerNavigator.h: * Tools/TestWebKitAPI/Tests/WebKitCocoa/Badging.mm: ((Badging, CrossOrigin)): ((Badging, SameOrigin)): ((Badging, DISABLED_Origin)): Deleted. Originally-landed-as: 305413.581@rapid/safari-7624.2.5.110-branch (f0c6cc4). rdar://176061670 Canonical link: https://commits.webkit.org/314758@main
https://bugs.webkit.org/show_bug.cgi?id=316581 Unreviewed gardening. * LayoutTests/platform/glib/TestExpectations: * LayoutTests/platform/gtk/TestExpectations: * LayoutTests/platform/wpe/TestExpectations: Canonical link: https://commits.webkit.org/314759@main
https://bugs.webkit.org/show_bug.cgi?id=316587 Unreviewed gardening. * LayoutTests/platform/wpe/TestExpectations: Canonical link: https://commits.webkit.org/314760@main
https://bugs.webkit.org/show_bug.cgi?id=316555 rdar://179025965 Reviewed by Yijia Huang. This patch optimizes save / restore ParenContext in YarrJIT. We carefully reorder the load / store so that we remove unnecessary loads, which is critical for performance in particular in RegExp which tends to have small JIT code. 1. matchAmount load is duplicate. We should hoist it and remove duplicate loads. 2. Skip return-address storing when we know it is not necessary (there are only 1 (or less) alternative). * Source/JavaScriptCore/yarr/YarrJIT.cpp: Canonical link: https://commits.webkit.org/314761@main
…-source-real-webm-currenttime-progression.html is a flaky text diff https://bugs.webkit.org/show_bug.cgi?id=316578 rdar://179039564 Unreviewed test gardening. * LayoutTests/platform/mac-wk2/TestExpectations: Canonical link: https://commits.webkit.org/314762@main
https://bugs.webkit.org/show_bug.cgi?id=316516 Reviewed by Per Arne Vollan. * Source/WebKit/Shared/Cocoa/XPCEndpointClient.mm: (WebKit::XPCEndpointClient::setEndpoint): Canonical link: https://commits.webkit.org/314763@main
…hared() https://bugs.webkit.org/show_bug.cgi?id=316514 Reviewed by Rupin Mittal. decidePolicyForResponseShared() consumed `frameInfo` by passing WTF::move(frameInfo) to API::FrameInfo::create(), and then immediately read `frameInfo.securityOrigin.securityOrigin()` for the COOP browsing-context-group-switch check on the next line. API::FrameInfo's constructor stores the FrameInfoData by move (m_data(WTF::move(data))), so the SecurityOriginData's String members were left empty and the COOP comparison saw an empty origin — typically a false negative, meaning we failed to disown the opener and clear m_openedMainFrameName when COOP required it. The same moved-from `frameInfo` was then captured by WTF::move into the policy listener lambda, where it was further used (e.g. by FrameInfoData { frameInfo } on the safe-browsing failure path). Reorder the function so the COOP check runs first against a live `frameInfo`, pass a copy to API::FrameInfo::create(), and let the lambda's capture be the sole move. * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::decidePolicyForResponseShared): Canonical link: https://commits.webkit.org/314764@main
…obDataTasksForAPI https://bugs.webkit.org/show_bug.cgi?id=316524 Reviewed by Alex Christensen. NetworkSessionCocoa::BlobDataTaskClient takes a NetworkSessionCocoa& in its constructor but never assigned it to its m_session member (declared as WeakPtr<NetworkSessionCocoa>). As a result, the cleanup in didCompleteWithError(): ``` if (CheckedPtr session = m_session.get()) session->removeBlobDataTask(m_identifier); ``` never ran, because m_session.get() was always null. Each successful blob: data task started via NetworkSessionCocoa::dataTaskWithRequest() left its Ref<BlobDataTaskClient> (and the wrapped NetworkDataTaskBlob) retained in m_blobDataTasksForAPI for the lifetime of the session unless the caller explicitly invoked cancelDataTask(). Initialize m_session in the constructor's member initializer list so the entry is removed when the load completes. * Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm: Canonical link: https://commits.webkit.org/314765@main
https://bugs.webkit.org/show_bug.cgi?id=316552 rdar://179024722 Reviewed by Simon Fraser. RenderFrameSet and its nested GridAxis spread member initialization across hand-written constructor init lists. GridAxis::GridAxis() only initialized m_splitBeingResized and omitted m_splitResizeOffset, leaving it uninitialized; it happens to be assigned in startResizing() before its read in continueResizing(), so there is no observable change, but the member was not safely default-initialized. Move the data members to default member initializers in the header so every constructor stays in sync: - GridAxis::m_splitBeingResized defaults to noSplit and m_splitResizeOffset to 0, letting GridAxis() become = default. - RenderFrameSet::m_isResizing defaults to false. The GridAxis constructor definition and the RenderFrameSet m_isResizing init list entry are dropped. No change in behavior. * Source/WebCore/rendering/RenderFrameSet.cpp: (WebCore::RenderFrameSet::RenderFrameSet): (WebCore::RenderFrameSet::GridAxis::GridAxis): Deleted. * Source/WebCore/rendering/RenderFrameSet.h: Canonical link: https://commits.webkit.org/314766@main
https://bugs.webkit.org/show_bug.cgi?id=316510 Reviewed by Darin Adler. * Source/WTF/wtf/LazyRef.h: * Source/WTF/wtf/LazyUniqueRef.h: Canonical link: https://commits.webkit.org/314767@main
https://bugs.webkit.org/show_bug.cgi?id=316511 Reviewed by Darin Adler. clampToInteger had three overloads. The float and double versions were thin wrappers that delegated to clampTo<int>. The integral version was its own implementation that only clamped the upper bound — for values below INT_MIN it fell through to a bare static_cast<int>(x), which is implementation-defined for out-of-range conversions. In practice, clampToInteger<int64_t>(INT64_MIN) returned 0 and clampToInteger<int64_t>(INT_MIN - 1) returned INT_MAX instead of clamping to INT_MIN. The integral version was also miscompiled for narrow signed/unsigned T (int8_t, int16_t, uint8_t, uint16_t) because `static_cast<unsigned>(INT_MAX)` truncates when assigned to `T intMax` — caught by -Werror=constant-conversion, so those instantiations were unreachable from production code. Rather than fix the integral overload in place, remove the API entirely and migrate callers to clampTo<int>(value), whose existing overload set already handles every integral and floating-point source type with correct two-sided clamping. Test: Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp * Source/WTF/wtf/MathExtras.h: (clampToInteger): Deleted. * Source/WebCore/dom/Element.cpp: (WebCore::Element::scrollTo): (WebCore::Element::setScrollLeft): (WebCore::Element::setScrollTop): * Source/WebCore/platform/LayoutUnit.h: (WebCore::LayoutUnit::LayoutUnit): (WebCore::LayoutUnit::fromFloatCeil): (WebCore::LayoutUnit::fromFloatFloor): * Source/WebCore/platform/graphics/DoublePoint.h: (WebCore::flooredIntPoint): (WebCore::roundedIntPoint): * Source/WebCore/platform/graphics/FloatPoint.h: (WebCore::flooredIntSize): (WebCore::roundedIntPoint): (WebCore::flooredIntPoint): (WebCore::ceiledIntPoint): * Source/WebCore/platform/graphics/FloatSize.h: (WebCore::roundedIntSize): (WebCore::flooredIntSize): (WebCore::expandedIntSize): (WebCore::flooredIntPoint): * Source/WebCore/platform/graphics/IntPoint.cpp: (WebCore::IntPoint::IntPoint): * Source/WebCore/platform/graphics/IntRect.cpp: (WebCore::IntRect::IntRect): * Source/WebCore/platform/graphics/IntSize.cpp: (WebCore::IntSize::IntSize): * Source/WebCore/platform/graphics/cocoa/IOSurface.mm: (WebCore::computeMaximumSurfaceSize): * Source/WebCore/rendering/AttachmentLayout.mm: (WebCore::AttachmentLayout::AttachmentLayout): * Source/WebCore/style/StyleAdjuster.cpp: (WebCore::Style::addIntrinsicMargins): * Source/WebKit/Shared/wpe/WebEventFactoryWPE.cpp: (WebKit::positionFromEvent): * Source/WebKit/UIProcess/API/gtk/DropTargetGtk4.cpp: (WebKit::DropTarget::DropTarget): * Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp: (webkitWebViewBaseScrollEvent): * Source/WebKit/UIProcess/Cocoa/UserMediaPermissionRequestManagerProxy.mm: (-[WKRotationCoordinatorObserver observeValueForKeyPath:ofObject:change:context:]): (-[WKRotationCoordinatorObserver start:layer:]): * Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp: (TestWebKitAPI::TEST(WTF, clampToIntFloat)): (TestWebKitAPI::TEST(WTF, clampToIntDouble)): (TestWebKitAPI::TEST(WTF, clampToIntIntegral)): (TestWebKitAPI::TEST(WTF, clampToIntegerFloat)): Deleted. (TestWebKitAPI::TEST(WTF, clampToIntegerDouble)): Deleted. Canonical link: https://commits.webkit.org/314768@main
https://bugs.webkit.org/show_bug.cgi?id=316566 rdar://179033258 Reviewed by Sam Weinig. Verified on M3 Max CMake build. (Other CPUs and Xcode build measured a regression but I did not measure the fix on them individually because we do not have a build speed A/B bot.) * Source/WebCore/dom/ElementInlines.h: Avoid including StyleComputedStyle+GettersInlines.h because parsing costs 6s per translation unit and ElementInlines.h is included in hundreds of translation units. Clients who end up using the StyleComputedStyle+GettersInlines.h aspect of this header can #include StyleComputedStyle+GettersInlines.h individually. Canonical link: https://commits.webkit.org/314769@main
https://bugs.webkit.org/show_bug.cgi?id=316490 Reviewed by Yusuke Suzuki. http/tests/inspector/network/resource-certificate-subject-format.html expects the certificate subject shown in Web Inspector to be a human-readable summary. The layout test certificate has no CN attribute (its subject is "C=AU, ST=Some-State, O=WebKit Layout Tests"), so the curl port fell back to printing the whole distinguished name ("WebKit Layout Tests, Some-State, AU"). Fall back to the organization name (O) before the full subject string, matching the behavior of SecCertificateCopySubjectSummary on macOS and the soup port's subjectSummaryFromDN() (314545@main). * Source/WebCore/platform/network/curl/OpenSSLHelper.cpp: (WebCore::OpenSSL::getSubjectEntry): Added, generalized from getCommonName. (WebCore::OpenSSL::getCommonName): (WebCore::OpenSSL::getOrganizationName): Added. (WebCore::OpenSSL::createSummaryInfo): Prefer CN, then O, then the full subject. Canonical link: https://commits.webkit.org/314770@main
…taParser emits AAC CMSampleBuffers without a sample-size array https://bugs.webkit.org/show_bug.cgi?id=316425 rdar://178041154 Reviewed by Jer Noble. For some AAC content, AVStreamDataParser emits multi-sample audio CMSampleBuffers whose CMSampleBufferGetSampleSizeArray returns kCMSampleBufferError_BufferHasNoSampleSizes; per-packet sizes are carried in the audio stream packet description array instead. MediaSampleAVFObjC::divide() subdivided buffers using CMSampleBufferCallBlockForEachSample, which cannot operate on that shape and returns with kCMSampleBufferError_CannotSubdivide before invoking its callback. divide() therefore returned an empty Vector. With MediaContainmentEnabled the IPC encode path (samplesBlockFromCMSampleBuffer -> divide() -> per-packet MediaSampleItems) shipped an empty MediaSamplesBlock to the GPU process; the rebuilt CMSampleBuffer carried no decodable packets, so the audio renderer played silence while the synchronizer's timebase still advanced -- currentTime progressed but no audio was heard. Fixed by detecting the BufferHasNoSampleSizes shape in MediaSampleAVFObjC::divide() and walking the audio stream packet description array directly, building per-packet sub-CMSampleBuffers whose data buffers reference the source block buffer (zero-copy via CMBlockBufferAppendBufferReference) instead of relying on CMSampleBufferCallBlockForEachSample. Those MediaSample aren't correctly handled by other related methods in MediaSampleAVFObjC and will cause incorrect handling when removed or seeked into. We will handle those in webkit.org/b/316467 * Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm: (WebCore::MediaSampleAVFObjC::divide): When the source CMSampleBuffer reports BufferHasNoSampleSizes, walk getPacketDescriptions() and build per-packet sub-CMSampleBuffers referencing the source block buffer. * LayoutTests/media/media-source/content/test-aac-shared-format.mp4: Added. * LayoutTests/media/media-source/content/test-adts.aac: Added. * LayoutTests/media/media-source/media-managedmse-aac-shared-format.html: Added. * LayoutTests/media/media-source/media-managedmse-adts-aac.html: Added. Canonical link: https://commits.webkit.org/314771@main
…ing(this)` and `ToUint32(limit)` https://bugs.webkit.org/show_bug.cgi?id=316508 Reviewed by Yusuke Suzuki. stringProtoFuncSplit ran ToString(this) and ToUint32(limit) after checking isSymbolSplitFastAndNonObservable(). Both conversions can run arbitrary JS that invalidates the checked conditions (e.g. overriding RegExp.prototype.exec or installing an own .constructor on the separator), which RegExpSplit observes after those conversions. operationStringSplitRegExp had the same problem for ToUint32(limit). Only take the fast path when both conversions are non-observable: `this` is a string primitive and limit is a number or undefined. Otherwise fall through to the generic @@split call, as the old JS-builtin implementation did (isLimitNumberOrUndefined, bmo#1287525). let execCalled = false; let evil = { valueOf() { RegExp.prototype.exec = () => { execCalled = true; return null; }; return 4; } }; "a,b,c".split(/,/, evil); // Before: ["a","b","c"], execCalled === false // After (spec, V8): ["a,b,c"], execCalled === true Tests: JSTests/stress/string-split-regexp-fast-path-limit-side-effects.js JSTests/stress/string-split-regexp-fast-path-tostring-side-effects.js * JSTests/stress/string-split-regexp-fast-path-limit-side-effects.js: Added. (shouldBe): (let.evil.valueOf.RegExp.prototype.exec): (let.evil.valueOf): * JSTests/stress/string-split-regexp-fast-path-tostring-side-effects.js: Added. (shouldBe): (throw.new.Error.MyRegExp): (throw.new.Error): (shouldBe.let.obj.toString.RegExp.prototype.exec): * Source/JavaScriptCore/dfg/DFGOperations.cpp: (JSC::DFG::JSC_DEFINE_JIT_OPERATION): * Source/JavaScriptCore/runtime/StringPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): Canonical link: https://commits.webkit.org/314772@main
https://bugs.webkit.org/show_bug.cgi?id=316520 Reviewed by Darin Adler. * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: * Source/JavaScriptCore/SaferCPPExpectations/NoUncountedMemberCheckerExpectations: * Source/JavaScriptCore/dfg/DFGGraph.h: * Source/JavaScriptCore/dfg/DFGPropertyTypeKey.h: Removed. * Source/WTF/WTF.xcodeproj/project.pbxproj: * Source/WTF/wtf/CMakeLists.txt: * Source/WTF/wtf/MainThreadData.h: Removed. * Source/WTF/wtf/StatisticsManager.cpp: Removed. * Source/WTF/wtf/StatisticsManager.h: Removed. * Source/WebCore/editing/SurroundingText.h: Removed. * Source/WebCore/page/AudioOutputProvider.h: Removed. * Source/WebCore/page/RenderingUpdateThrottleState.h: Removed. * Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp: * Source/WebKit/NetworkProcess/NetworkStorageSessionProvider.h: Removed. * Source/WebKit/NetworkProcess/mac/SecItemShim.mm: * Source/WebKit/Shared/BlockingResponseMap.h: Removed. * Source/WebKit/SourcesCocoa.txt: * Source/WebKit/UIProcess/ios/WKWebEvent.h: Removed. * Source/WebKit/UIProcess/ios/WKWebEvent.mm: Removed. * Source/WebKit/WebKit.xcodeproj/project.pbxproj: * Source/WebKit/WebProcess/WebCoreSupport/WebCaptionDisplaySettingsOptions.h: Removed. * Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp: * Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/FrameDamageForTesting.h: Removed. Canonical link: https://commits.webkit.org/314773@main
https://bugs.webkit.org/show_bug.cgi?id=316513 Reviewed by Abrar Rahman Protyasha and Wenson Hsieh. WebPageProxy::viewScaleFactorDidChange() is the IPC handler that the WebProcess uses to notify the UIProcess after auto-scaling the view to fit the document (the path enabled by setShouldScaleViewToFitDocument(), exposed via WKView's _WKLayoutModeDynamicSizeComputedFromMinimumDocumentSize layout strategy). Unlike its sibling pageScaleFactorDidChange(), it forgot to update the cached m_viewScaleFactor member. As a result, [WKWebView _viewScale] returned a stale value after the auto-scale, and the stale value was also propagated to newly launched WebContent processes via WebPageCreationParameters::viewScaleFactor on process swap. Test: WebKit.ViewScaleFactorAfterShrinkToFit * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::viewScaleFactorDidChange): Cache the new scale factor, mirroring pageScaleFactorDidChange(). * Tools/TestWebKitAPI/Tests/WebKit/WKWebView/ShrinkToFit.mm: (TEST(WebKit, ViewScaleFactorAfterShrinkToFit)): Add a regression test that uses the dynamic-size layout mode to drive WebProcess-side auto-scale and verifies [WKWebView _viewScale] reflects the new value. Canonical link: https://commits.webkit.org/314774@main
… prototype belongs to another realm https://bugs.webkit.org/show_bug.cgi?id=316506 Reviewed by Yusuke Suzuki. A cached NonThenable is guarded by structure->realm()'s promiseThenWatchpointSet, but the cacheability check compared the prototype against the caller's objectPrototype(). Once a realm-A structure holds realm B's Object.prototype and the first resolution happens in realm B, the cache survives `then` being added to realm B's Object.prototype. That fires only realm B's watchpoint set. So realm A treats a genuine thenable as a plain value: const other = createGlobalObject(); function make() { const o = { a: 1 }; Object.setPrototypeOf(o, other.Object.prototype); return o; } other.Promise.resolve(make()); // caches NonThenable on the realm-A structure drainMicrotasks(); other.Object.prototype.then = function (resolve) { resolve(42); }; const v = await Promise.resolve(make()); // v is the object, should be 42 Compare against structure->realm()->objectPrototype() instead; mixed-realm chains become Uncacheable and take the per-call walk. Test: JSTests/stress/promise-resolve-non-thenable-structure-cache-cross-realm-proto.js * JSTests/stress/promise-resolve-non-thenable-structure-cache-cross-realm-proto.js: Added. (shouldBe): (asyncTest.async localStructureForeignProto.make): (asyncTest.async localStructureForeignProto.other.Function): (asyncTest.async localStructureForeignProto): (asyncTest.async foreignStructureLocalProto.Object.prototype.then): (asyncTest.async foreignStructureLocalProto): * Source/JavaScriptCore/runtime/JSPromise.cpp: (JSC::isDefinitelyNonThenable): Canonical link: https://commits.webkit.org/314775@main
https://bugs.webkit.org/show_bug.cgi?id=316562 rdar://165842824 Reviewed by Abrar Rahman Protyasha and Wenson Hsieh. Moving from temporary SPI to shipping API. Not removing the old SPI calls and allowances yet, as this is very new and people need time to update their systems and tools. Will completely remove SPI in a later step. * Source/WebKit/Configurations/AllowedSPI.toml: * Source/WebKit/Platform/spi/ios/UIKitSPI.h: * Source/WebKit/UIProcess/ios/TextCheckerIOS.mm: (WebKit::TextChecker::requestExtendedCheckingOfString): Canonical link: https://commits.webkit.org/314976@main
https://bugs.webkit.org/show_bug.cgi?id=316610 Reviewed by Yusuke Suzuki. Per the import defer proposal, IsSymbolLikeNamespaceKey(P, O) treats "then" as symbol-like on a deferred namespace, so [[GetOwnProperty]]("then") returns OrdinaryGetOwnProperty(O, "then") = undefined. EnumerableOwnProperties filters each key through [[GetOwnProperty]] and skips keys whose descriptor is undefined, so Object.keys / Object.values / Object.entries / spread / for-in must not list "then". [[OwnPropertyKeys]] returns [[Exports]] unfiltered, so Reflect.ownKeys and Object.getOwnPropertyNames still include it. JSModuleNamespaceObject::getOwnPropertyNames in DontEnumPropertiesMode::Exclude already performed [[GetOwnProperty]] on each export name (for the ReferenceError side effect on uninitialized bindings) but ignored its result and added every key unconditionally, so "then" leaked into Object.keys: // dep.js: export let foo = 1; export function then(cb) { cb(); } import defer * as ns from "./dep.js"; Object.keys(ns); // ["foo", "then"], must be ["foo"] Use the result of the [[GetOwnProperty]] lookup and skip keys with no descriptor, the same way ProxyObject::performGetOwnEnumerablePropertyNames implements this filter. The only key that can lack a descriptor is the symbol-like "then" of a deferred namespace, so non-deferred namespaces are unaffected. Test: JSTests/modules/import-defer-then-not-enumerable.js * JSTests/modules/import-defer-then-not-enumerable.js: Added. (shouldBe.JSON.stringify): * Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp: (JSC::JSModuleNamespaceObject::getOwnPropertyNames): Canonical link: https://commits.webkit.org/314977@main
…while concurrent marking is active https://bugs.webkit.org/show_bug.cgi?id=316635 Reviewed by Keith Miller. The between-GC clearing path added in 314730@main guards against JS executing, a live GCOwnedDataScope, and in-flight JIT compilations, but not against concurrent marking. Marker threads load a JSString's StringImpl via fiberConcurrently() and then dereference it without taking a ref (JSString::visitChildrenImpl / JSString::estimatedSize). The retained list is exactly what keeps those racily-loaded impls alive, so clearing it while marking is in flight can free an impl a marker is about to read, which is a use-after-free on the collector thread. ASAN confirms: the read is in WTF::StringImpl::costDuringGC under SlotVisitor::drain on the collector thread, and the free is in clearConcurrentRetainedDataIfPossible under the IncrementalSweeper timer on the mutator thread. Skip the clear while mutatorShouldBeFenced() is true. That flag is set exactly while markers can be running (the same invariant the write barriers rely on), and the read is properly ordered because the timer callback acquires the API lock first. The clear is only deferred, not lost: the sweep timer re-fires every 100ms, so the list is cleared on the first fire after marking finishes, and GC-finalize pruning still bounds the list across collection cycles. * Source/JavaScriptCore/heap/Heap.cpp: (JSC::Heap::clearConcurrentRetainedDataIfPossible): Canonical link: https://commits.webkit.org/314978@main
https://bugs.webkit.org/show_bug.cgi?id=316812 rdar://179261686 Reviewed by Anne van Kesteren. NavigationAPIMethodTracker exposed all of its state through getters and setters, and the spec algorithms that operate on it ("notify about the committed-to entry", "resolve/reject the finished promise") lived in Navigation, probing committedToEntry() and a finishedBeforeCommit flag from the outside. Several call sites also settled the tracker's promises by hand, each with a slightly different recipe (the rate limiter path, the navigate() path for loads that never reach the navigate event, the intercept-handler failure path, and the traversal cancellation callback). Move those algorithms into the tracker as commitTo(), resolveFinished() and rejectFinished(), driven by an internal state enum (Pending / FinishedBeforeCommit / Committed / Settled) that replaces the finishedBeforeCommit bool. The tracker now guarantees its own invariants: each promise settles at most once, a finish signal that arrives before commit is held until commitTo() runs, and an uncommitted navigation rejects both promises while a committed one only rejects the finished promise. This also removes the redundant re-resolution of the committed promise that resolveFinishedPromise used to perform, which relied on DeferredPromise ignoring a second resolve. Navigation's notifyCommittedToEntry / resolveFinishedPromise / rejectFinishedPromise become thin wrappers that delegate to the tracker and handle tracker cleanup, and all remaining settlement paths are routed through them. One behavior fix: when NavigationScheduler cancelled a scheduled key traversal, the completion callback rejected the tracker's promises but never removed the tracker from m_upcomingTraverseMethodTrackers. A later traverseTo(), back() or forward() to the same key would then be deduplicated against the stale tracker and receive the already-rejected promises instead of starting a new traversal. The callback now goes through rejectFinishedPromise, which also cleans up the upcoming map. This is preparation for implementing NavigationInterceptOptions' precommitHandler, which adds another commit-timing state and would otherwise have to thread more flags through Navigation. No new tests. Covered by existing imported/w3c/web-platform-tests/ navigation-api tests, which pass in both release and debug. * Source/WebCore/page/Navigation.h: (WebCore::NavigationAPIMethodTracker::hasCommitted const): (WebCore::NavigationAPIMethodTracker::isSettled const): (WebCore::NavigationAPIMethodTracker::committedToEntry const): Deleted. (WebCore::NavigationAPIMethodTracker::setCommittedToEntry): Deleted. (WebCore::NavigationAPIMethodTracker::finishedBeforeCommit const): Deleted. (WebCore::NavigationAPIMethodTracker::setFinishedBeforeCommit): Deleted. (WebCore::NavigationAPIMethodTracker::setSerializedState): Deleted. (WebCore::NavigationAPIMethodTracker::takeSerializedState): Deleted. * Source/WebCore/page/Navigation.cpp: (WebCore::NavigationAPIMethodTracker::NavigationAPIMethodTracker): (WebCore::NavigationAPIMethodTracker::commitTo): (WebCore::NavigationAPIMethodTracker::resolveFinished): (WebCore::NavigationAPIMethodTracker::rejectFinished): (WebCore::Navigation::maybeSetUpcomingNonTraversalTracker): (WebCore::Navigation::addUpcomingTraverseAPIMethodTracker): (WebCore::Navigation::navigate): (WebCore::Navigation::performTraversal): (WebCore::Navigation::notifyCommittedToEntry): (WebCore::Navigation::resolveFinishedPromise): (WebCore::Navigation::rejectFinishedPromise): (WebCore::Navigation::handleSameDocumentNavigation): (WebCore::Navigation::innerDispatchNavigateEvent): Canonical link: https://commits.webkit.org/314979@main
…ompletion handler on early return https://bugs.webkit.org/show_bug.cgi?id=316736 Reviewed by Pascoe. * Source/WebKit/WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp: (WebKit::WebAuthenticatorCoordinator::makeCredential): (WebKit::WebAuthenticatorCoordinator::getAssertion): Canonical link: https://commits.webkit.org/314980@main
https://bugs.webkit.org/show_bug.cgi?id=316568 Reviewed by Matthew Finkel. m_scriptTrackingPrivacyDataUpdateObserver is reused for 2 observers: - WebProcessPool::observeConsistentQueryParameterFilteringQuirkUpdatesIfNeeded() - WebProcessPool::observeScriptTrackingPrivacyUpdatesIfNeeded() * Source/WebKit/UIProcess/WebProcessPool.cpp: (WebKit::WebProcessPool::observeConsistentQueryParameterFilteringQuirkUpdatesIfNeeded): * Source/WebKit/UIProcess/WebProcessPool.h: Canonical link: https://commits.webkit.org/314981@main
https://bugs.webkit.org/show_bug.cgi?id=316729 Reviewed by Per Arne Vollan. Avoid some ref-counting churn in OriginStorageManager. Stopping calling ThreadSafeWeakPtr::get() several times in a row. * Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp: (WebKit::OriginStorageManager::cacheStorageManager): (WebKit::OriginStorageManager::backgroundFetchManager): Canonical link: https://commits.webkit.org/314982@main
…hout DOM or node id support https://bugs.webkit.org/show_bug.cgi?id=314147 Reviewed by BJ Burg. Implement the style-sheet-related commands of the CSS domain for frame targets: getAllStyleSheets, getStyleSheet, getStyleSheetText, setStyleSheetText, createStyleSheet, and getSupportedCSSProperties. These operate on stylesheets, which don't require DOM nodeId binding and FrameDOMAgent integration. FrameCSSAgent now also manages its own style sheet lifecycle: it fires styleSheetAdded, styleSheetRemoved, and styleSheetChanged events. - setStyleSheetText bypasses InspectorHistory and does not have undo/redo support yet. The undo infrastructure is currently tightly coupled to InspectorDOMAgent's page-level history. Address this later separately (https://webkit.org/b/314244). - FrameCSSAgent keeps the document-to-data mappings like in InspectorCSSAgent, m_documentToInspectorStyleSheet and m_documentToKnownCSSStyleSheets but for a different reason. InspectorCSSAgent needs them because a page hosts many documents simultaneously: the main frame plus every subframe. FrameCSSAgent inspects only one frame, but the frame (and this agent attached to it) outlives any single document there. Document-keyed state is still required to scope commands to the right document across time. The production frontend does not yet consume these frame-target CSS events or route commands to frame targets: CSSObserver and CSSManager explicitly ignore frame targets for now. The plan is to build out more of FrameCSSAgent's remaining commands (node-dependent queries and rule editing commands), then integrate the frontend in a single switchover patch once the agent is mature enough to fully replace InspectorCSSAgent's role under site isolation. (https://webkit.org/b/314148) Test: http/tests/site-isolation/inspector/css/css-stylesheet-commands-frame-target.html Add test cases for these new commands despite the frame target's CSS agent is not yet used by the frontend. * LayoutTests/http/tests/site-isolation/inspector/css/css-stylesheet-commands-frame-target-expected.txt: * LayoutTests/http/tests/site-isolation/inspector/css/css-stylesheet-commands-frame-target.html: * Source/JavaScriptCore/inspector/protocol/CSS.json: * Source/WebCore/inspector/InspectorInstrumentation.cpp: (WebCore::InspectorInstrumentation::documentDetachedImpl): (WebCore::InspectorInstrumentation::mediaQueryResultChangedImpl): (WebCore::InspectorInstrumentation::activeStyleSheetsUpdatedImpl): * Source/WebCore/inspector/InspectorInstrumentation.h: (WebCore::InspectorInstrumentation::mediaQueryResultChanged): * Source/WebCore/inspector/agents/frame/FrameCSSAgent.cpp: (WebCore::FrameCSSAgent::enable): (WebCore::FrameCSSAgent::disable): (WebCore::FrameCSSAgent::getAllStyleSheets): (WebCore::FrameCSSAgent::getStyleSheet): (WebCore::FrameCSSAgent::getStyleSheetText): (WebCore::FrameCSSAgent::setStyleSheetText): (WebCore::FrameCSSAgent::createStyleSheet): (WebCore::FrameCSSAgent::getSupportedCSSProperties): (WebCore::FrameCSSAgent::styleSheetChanged): (WebCore::FrameCSSAgent::documentDetached): (WebCore::FrameCSSAgent::mediaQueryResultChanged): (WebCore::FrameCSSAgent::activeStyleSheetsUpdated): (WebCore::FrameCSSAgent::reset): (WebCore::FrameCSSAgent::bindStyleSheet): (WebCore::FrameCSSAgent::assertStyleSheetForId): (WebCore::FrameCSSAgent::collectAllDocumentStyleSheets): (WebCore::FrameCSSAgent::collectStyleSheets): (WebCore::FrameCSSAgent::setActiveStyleSheetsForDocument): (WebCore::FrameCSSAgent::createInspectorStyleSheetForDocument): (WebCore::FrameCSSAgent::detectOrigin): * Source/WebCore/inspector/agents/frame/FrameCSSAgent.h: * Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js: (WI.CSSManager.prototype.set layoutContextTypeChangedMode): (WI.CSSManager.prototype.styleSheetAdded): * Source/WebInspectorUI/UserInterface/Protocol/CSSObserver.js: (WI.CSSObserver.prototype.mediaQueryResultChanged): (WI.CSSObserver.prototype.styleSheetChanged): (WI.CSSObserver.prototype.styleSheetAdded): (WI.CSSObserver.prototype.styleSheetRemoved): Canonical link: https://commits.webkit.org/314983@main
https://bugs.webkit.org/show_bug.cgi?id=316746 rdar://179191475 Reviewed by Ryosuke Niwa. dispatchPlayPauseEventsIfNeedsQuirks() dispatches synthesized `playing` and `pause` events to satisfy sites whose JS player state machines rely on these events to detect that autoplay was prevented (SynthesizedPauseEvents), or to recover after being restored from the back/forward cache (rdar://68938833). The function was not re-entrant-safe because a common JS pattern is to call video.play() from a `pause` event handler to retry playback. When autoplay was blocked, the sequence was: 1. setAutoplayEventPlaybackState(PreventedAutoplay) => dispatchPlayPauseEventsIfNeedsQuirks() // schedule playing + pause events 2. `pause` event fires, JS handler calls video.play() // autoplay blocked again 3. setAutoplayEventPlaybackState(PreventedAutoplay) => dispatchPlayPauseEventsIfNeedsQuirks() // schedule playing + pause events again, and cause infinite loop This can produce hundreds of log entries per millisecond and could trigger logd's per-process quarantine on affected pages. To fix this, this patch adds a bool guard m_isDispatchingAutoplayPlayPauseQuirkEvents and consolidates two scheduleEvent() calls into a single queued task that ispatches both events synchronously via dispatchEvent(). A SetForScope sets the guard for the duration of the task, covering the entire JS execution window. Any re-entrant call to dispatchPlayPauseEventsIfNeedsQuirks() during that window — whether from the `playing` or the `pause` handler — returns immediately. API Test: WebpagePreferences.WebsitePoliciesSynthesizedPauseEventFiresOnce * Source/WebCore/html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::dispatchPlayPauseEventsIfNeedsQuirks): * Source/WebCore/html/HTMLMediaElement.h: * Tools/TestWebKitAPI/Resources/cocoa/autoplay-quirk-pause-fires-once.html: Added. * Tools/TestWebKitAPI/Tests/WebKit/WKWebView/WebsitePolicies.mm: (TEST(WebpagePreferences, WebsitePoliciesSynthesizedPauseEventFiresOnce)): Canonical link: https://commits.webkit.org/314984@main
…n up stress tests https://bugs.webkit.org/show_bug.cgi?id=316793 rdar://179110003 Reviewed by Yusuke Suzuki. toIntegerWithTruncation() can throw (e.g. when an argument's valueOf triggers JS execution) and return NaN. PlainDate, PlainYearMonth, and PlainMonthDay constructors placed RETURN_IF_EXCEPTION after the !isfinite() guard, so when toIntegerWithTruncation() threw and returned NaN, the isfinite check triggered throwVMRangeError on top of a pending exception, crashing via verifyExceptionCheckNeedIsSatisfied. Fix by moving RETURN_IF_EXCEPTION immediately after each toIntegerWithTruncation() call. Also remove stale //@ skip from temporal-calendar.js, temporal-duration.js, and temporal-timezone.js now that Temporal is complete, and enable the previously-FIXMEd ZonedDateTime.timeZoneId and offset tests in temporal-timezone.js. Test: JSTests/stress/temporal-plainyearmonth-constructor-exception-check.js Canonical link: https://commits.webkit.org/314985@main
…rge string input https://bugs.webkit.org/show_bug.cgi?id=316805 rdar://179110736 Reviewed by Marcus Plutowski. fromMonthDayString used makeString to include the user-supplied string in the RangeError message. makeString calls WTFCrash on allocation failure, so passing a very large string (e.g. ~2 GB) caused a crash instead of a clean RangeError. Fix by using tryMakeString with a static fallback message, matching the pattern already used in fromYearMonthString. Test: JSTests/stress/temporal-plainmonthday-from-large-string.js Canonical link: https://commits.webkit.org/314986@main
…RegistryProxy_ClientIsGoingAway' https://bugs.webkit.org/show_bug.cgi?id=315738 rdar://178065152 Reviewed by Charlie Wolfe. The message check added to requestLock() and clientIsGoingAway(): MESSAGE_CHECK(m_process->hasCommittedClientOrigin(clientOrigin)); may still fail after the fix in rdar://177020691. The issue is that the ClientOrigins that the UI process stores in the map may be different than the ones sent by web process to requestLock(). The ClientOrigins that the web process sends are computed in WebLockManager::clientOriginFromContext(). In cases where the document's topOrigin or securityOrigin is inherited from the parent or owner document, this function will indeed use the inherited origins. But the UI process side doesn't account for inherited security origins. WebFrameProxy::didCommitLoad() computes the topOrigin and securityOrigin straight from the URL. So in the case of an about:blank URL, it will compute an opaque securityOrigin, whereas the web process will find and send over the inherited securityOrigin. The new test WebLocks.CrossSiteIframeUsingLocksInsideAboutBlankPopup demonstrates such a scenario. In this case, there is a main frame hosted on server1. It creates an about:blank popup and embeds in it an iframe that is hosted on server2. The UI process will store the following (topOrigin, securityOrigin): main frame: { server1, server1 } popup: { opaque1, opaque2 } iframe: { opaque3, server2 } The web process accounts for inherited origins and will have: main frame: { server1, server1 } popup: { server1, server1 } iframe: { server1, server2 } So when the iframe sends the requestLock() or clientIsGoingAway() IPCs, its ClientOrigin isn't found in the map, and the message check kills the web process. We need to replicate the inheritance logic of the web process in the UI process. This already exists in WebFrameProxy::updateDocumentSecurityOrigin(). So we fix the issue by ensuring that we pass the creator (parent or opener if there's no parent) to this function so that it correctly computes and sets the documentSecurityOrigin and then using these computed securityOrigins. * Source/WebKit/UIProcess/WebFrameProxy.cpp: (WebKit::WebFrameProxy::didCommitLoad): * Tools/TestWebKitAPI/Tests/WebKit/WKWebView/WebLocks.mm: (TestWebKitAPI::TEST(WebLocks, CrossSiteIframeUsingLocksInsideAboutBlankPopup)): Canonical link: https://commits.webkit.org/314987@main
…mpl by a raw reference https://bugs.webkit.org/show_bug.cgi?id=316747 Reviewed by Abrar Rahman Protyasha and Aditya Keerthi. * Source/WebKit/UIProcess/mac/CorrectionPanel.mm: (WebKit::CorrectionPanel::show): Canonical link: https://commits.webkit.org/314988@main
…an overflow the stack on deep async module graphs https://bugs.webkit.org/show_bug.cgi?id=316615 Reviewed by Yusuke Suzuki. GatherAvailableAncestors[1] and AsyncModuleExecutionRejected[2] walk [[AsyncParentModules]] with unguarded native recursion, so when the top-level-await leaf of a long module chain settles, a deep enough chain crashes with a hard stack overflow. These operations are infallible and run from microtasks, so they cannot throw a RangeError like InnerModuleEvaluation does. Instead, flatten both recursions into explicit worklists. AsyncModuleExecutionRejected pushes parents in reverse to preserve the spec's depth-first order, which is observable through the rejection order of [[TopLevelCapability]] promises. [1]: https://tc39.es/ecma262/#sec-gather-available-ancestors [2]: https://tc39.es/ecma262/#sec-async-module-execution-rejected * Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp: (JSC::gatherAvailableAncestors): (JSC::CyclicModuleRecord::asyncExecutionRejected): Canonical link: https://commits.webkit.org/314989@main
…est::visitAdditionalChildren() rdar://172462937 Reviewed by Chris Dumez. m_controller can be nullified while being used in GC thread. We remove usage of m_controller in the GC thread. Instead, request will store its stream as a JSValueInWrappedObject, and we will use this JSValueInWrappedObject in the GC thread. We make sure to clear the JSValueInWrappedObject when the request gets invalidated. Originally-landed-as: 305413.480@rapid/safari-7624.2.5.110-branch (5d62bc6). rdar://176062477 Canonical link: https://commits.webkit.org/314990@main
…rrectly https://bugs.webkit.org/show_bug.cgi?id=309120 rdar://171292767 Reviewed by Dan Glastonbury. Fix by removing the explicit static cast. The range type is uint32 and can be converted to GLint64 implicitly and correctly. If the type is changed to incompatible type (size_t), the implicit cast will warn. * Source/ThirdParty/ANGLE/src/libANGLE/validationES.h: (gl::ValidateDrawElementsCommon): * Source/ThirdParty/ANGLE/src/tests/gl_tests/WebGLCompatibilityTest.cpp: Originally-landed-as: 305413.413@safari-7624-branch (a3d82cd). rdar://176066815 Canonical link: https://commits.webkit.org/314991@main
https://bugs.webkit.org/show_bug.cgi?id=309538 rdar://172053974 Reviewed by Yusuke Suzuki. There is a race when parsing the wasm "name" custom section. Compiler threads and the main thread may race on access of the name section. This PR fixes the race by making the name section go through a rel/acq accessor. Lifetime safety is upkept by holding onto the retired name section (the initial empty one) when the parser finishes parsing a name section. For simplicity, all name sections after the first one are ignored, as there is no normative requirement on custom sections. No test added as manual sleeping is required to widen the window to reproduce. * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp: (JSC::Wasm::BBQJITImpl::BBQJIT::addTopLevel): * Source/JavaScriptCore/wasm/WasmBBQPlan.cpp: (JSC::Wasm::BBQPlan::work): * Source/JavaScriptCore/wasm/WasmCallee.cpp: (JSC::Wasm::OptimizingJITCallee::addCodeOrigin): * Source/JavaScriptCore/wasm/WasmIPIntPlan.cpp: (JSC::Wasm::IPIntPlan::compileFunction): * Source/JavaScriptCore/wasm/WasmModuleInformation.cpp: (JSC::Wasm::ModuleInformation::ModuleInformation): (JSC::Wasm::ModuleInformation::setNameSection): * Source/JavaScriptCore/wasm/WasmModuleInformation.h: * Source/JavaScriptCore/wasm/WasmOMGPlan.cpp: (JSC::Wasm::OMGPlan::work): * Source/JavaScriptCore/wasm/WasmOSREntryPlan.cpp: (JSC::Wasm::OSREntryPlan::work): * Source/JavaScriptCore/wasm/WasmPlan.cpp: (JSC::Wasm::Plan::signpostMessage const): * Source/JavaScriptCore/wasm/WasmSectionParser.cpp: (JSC::Wasm::SectionParser::parseCustom): * Source/JavaScriptCore/wasm/WasmStreamingParser.cpp: (JSC::Wasm::StreamingParser::finalize): Originally-landed-as: 305413.432@rapid/safari-7624.2.5.110-branch (935ddf6). rdar://176067717 Canonical link: https://commits.webkit.org/314992@main
…2e675175 # Conflicts: # Source/JavaScriptCore/heap/FreeListInlines.h # Source/JavaScriptCore/heap/GCOwnedDataScope.h # Source/JavaScriptCore/runtime/CodeCache.cpp # Source/JavaScriptCore/runtime/GetPutInfo.h # Source/JavaScriptCore/runtime/JSFunction.cpp # Source/JavaScriptCore/runtime/JSMicrotask.cpp # Source/JavaScriptCore/runtime/JSPromisePrototype.cpp
WalkthroughThe pull request updates build configuration and versioning, adds broad JavaScript, Temporal, Intl, and WebAssembly test coverage, reduces test262 skips and expectations, and adds or modernizes many LayoutTests and expected outputs. ChangesBuild, engine tests, and layout test updates
Possibly related PRs
|
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html (1)
25-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnbalanced async lifecycle can hang this test on unsupported platforms.
Line 25 enables async mode, but Lines 32-39 return early without calling
finishJSTest(). That can leave the test waiting indefinitely.Suggested fix
- window.jsTestIsAsync = true; + window.jsTestIsAsync = true; @@ if (!window.eventSender) { debug('This test requires DRT.'); + finishJSTest(); return; } if (!eventSender.gestureTapDown) { debug('GestureTapDown is not supported by this platform'); + finishJSTest(); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html` around lines 25 - 39, The test enables async mode via window.jsTestIsAsync but returns early in runTests when window.eventSender is missing or when eventSender.gestureTapDown is unsupported, which can hang the harness; update runTests to call finishJSTest() before each early return (i.e., in the branches checking window.eventSender and eventSender.gestureTapDown) so the async lifecycle is completed and the test never waits indefinitely.LayoutTests/fast/events/mouse-cursor-update-during-raf.html (1)
21-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConsolidated root cause: async harness completion is not guaranteed on unsupported/dependency-missing paths.
Both tests switched to
js-test.jsasync mode but leave at least one branch where async is enabled andfinishJSTest()is not reached. This creates timeout-prone tests and flaky infrastructure behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/mouse-cursor-update-during-raf.html` around lines 21 - 23, The test enables js-test.js async mode (window.jsTestIsAsync = true) without guaranteeing a corresponding finishJSTest() on all execution paths, causing timeouts; update the test to either only enable async when you will definitely call finishJSTest() (wrap the assignment and any async-only logic in the existing window.testRunner guard) or ensure every branch that can exit early calls finishJSTest() (add calls to finishJSTest() in unsupported/dependency-missing branches). Refer to window.testRunner, window.jsTestIsAsync, finishJSTest(), and js-test.js when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@JSTests/microbenchmarks/regexp-prototype-match-no-strength-reduction.js`:
- Around line 6-9: The benchmark's hot loop is calling Math.random() each
iteration, adding nondeterministic overhead; in the for-loop that invokes
shouldBe and RegExp.prototype[Symbol.match], remove Math.random() from the
string concatenation (the second shouldBe call) so the input is deterministic —
e.g. replace "hello" + "world" + Math.random() with a constant like "helloworld"
or compute a single random suffix once outside the loop if variability is
required; update the call site that uses /hello/[Symbol.match] so it no longer
invokes Math.random() on every iteration.
In `@JSTests/stress/async-generator-reentrant-resume-during-resolve.js`:
- Around line 24-32: The test replaces Object.prototype.then without restoring
it; save the original descriptor for Object.prototype.then (e.g., origThenDesc =
Object.getOwnPropertyDescriptor(Object.prototype, "then")) before calling
Object.defineProperty, and after the test body (or in a finally/cleanup block)
restore it: if origThenDesc is undefined, delete Object.prototype.then,
otherwise call Object.defineProperty(Object.prototype, "then", origThenDesc).
Ensure the cleanup runs even if the test throws so thenGets/iter.next behavior
does not leak into other tests.
In `@JSTests/stress/temporal-gc-jit-stress.js`:
- Around line 77-82: The loop currently asserts monotonicity by comparing prev
and curr using Temporal.Instant.compare and throwing on cmp > 0; remove that
monotonicity check and the throw, and instead assert API validity by checking
each call returns a Temporal.Instant (e.g., verify typeof/instance or that
Temporal.Instant.compare can be called with the value without expecting
ordering), replacing uses of prev/curr comparison with a simple validation of
the result of Temporal.Now.instant() inside the for loop (referencing
Temporal.Now.instant, Temporal.Instant.compare only for validation if needed,
and the loop variables prev and curr).
In `@JSTests/stress/temporal-plainyearmonth-constructor-exception-check.js`:
- Around line 5-7: The test currently triggers a TDZ ReferenceError because it
declares let a4 and immediately uses a4.withPlainTime(a3); replace that
self-referential initialization by calling withPlainTime on the constructed
PlainYearMonth instance (use a2.withPlainTime(a3)) and keep a3 as the local time
object used for the call so the intended Temporal.PlainYearMonth path is
executed (refer to variables a1, a2, a3, a4 and the withPlainTime call).
In `@JSTests/stress/temporal-zdt-large-fold-disambiguation.js`:
- Around line 3-6: The assertion helper shouldBe currently uses JSON.stringify
on actual/expected which throws for BigInt; update shouldBe to format values
using JSON.stringify with a replacer that converts BigInt to a string (e.g. v =>
typeof v === 'bigint' ? v.toString() + 'n' : v) so mismatches produce the
intended "expected … but got …" message; modify the call sites inside the
shouldBe function to use this safe serializer for both actual and expected.
In `@JSTests/stress/temporal-zoned-datetime-dst-fold.js`:
- Around line 7-10: The helper shouldThrow currently swallows unexpected
exceptions; update the catch in shouldThrow(fn, msg) so it only treats errors
whose message starts with msg as the expected case and rethrows any other thrown
value (including non-Error values) — e.g. in the catch block, if the caught
value’s message startsWith(msg) return/allow, otherwise throw the caught value
(or rethrow non-Error as is) so unrelated exceptions do not produce
false-positive passes.
In `@JSTests/stress/wasm-table-constructor-wrapper-function-default.js`:
- Around line 87-90: The comment and test disagree: the block intends to
validate that non-function default values throw for new WebAssembly.Table but
the code passes a function (() => 1). Either replace the default argument with a
non-function (e.g., use 1 instead of () => 1) so the test actually exercises
non-function defaults when calling new WebAssembly.Table({ element: "funcref",
initial: 1 }, 1), or change the comment to state that the test is verifying
function defaults (keep () => 1) so the intent matches the code; update the
comment near the WebAssembly.Table invocation accordingly.
In
`@JSTests/test262/test/intl402/Temporal/PlainMonthDay/prototype/toLocaleString/basic.js`:
- Line 33: Remove the duplicated frontmatter entry "features: [Temporal]" so
only a single "features: [Temporal]" remains in the test file; locate the
repeated frontmatter key and delete the second occurrence, leaving the original
declaration intact to avoid metadata parsing ambiguity.
In
`@LayoutTests/editing/selection/ios/tap-focused-input-clears-outside-selection.html`:
- Around line 22-53: The test currently assumes UIHelper exists and may hang
because jsTestIsAsync = true; wrap the load handler logic with a guard like if
(typeof UIHelper === "undefined") { /* report skip and call finishJSTest() */
return; } so that when UIHelper/WTR is not available the test logs/skips and
calls finishJSTest() instead of awaiting UIHelper methods; ensure all calls to
UIHelper.activateAndWaitForInputSessionAt, UIHelper.longPressElement, and
UIHelper.activateElementAndWaitForInputSession are only executed when the guard
passes and that finishJSTest() is always invoked on the early-exit path to avoid
timeouts.
In `@LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html`:
- Around line 21-24: The test marks success when the OES_element_index_uint
extension is missing but then continues executing the UNSIGNED_INT draw path;
modify the branch that checks contextType == "webgl" and the call to
context.getExtension("OES_element_index_uint") so that when the extension is
absent you not only call testPassed("no extension") but also return/exit the
test immediately to avoid running the UNSIGNED_INT draw code and producing
incorrect GL errors; locate the check around contextType and getExtension in the
test and add an early return (or skip) after testPassed to stop further
execution.
In `@LayoutTests/fast/canvas/webgl/gl-teximage-imagebitmap.html`:
- Around line 133-137: The rejection handler for the test promise must complete
the async JS test teardown by calling finishJSTest() in addition to reporting
the failure; update the rejection branch (the promise second argument or the
.catch handler that currently calls testFailed(String(e))) to also call
finishJSTest() after reporting the error (or use a .finally that calls
finishJSTest()), ensuring test(), testFailed(String(e)), and finishJSTest() are
invoked so window.jsTestIsAsync teardown completes on failures.
In `@LayoutTests/fast/dom/HTMLLinkElement/subresource.html`:
- Line 6: The test sets window.jsTestIsAsync = true but the onerror handler only
calls testFailed(...), so failures never call finishJSTest() and the test can
hang; update the onerror handler (the function assigned to the element's
onerror) to call finishJSTest() after reporting the failure (e.g., call
finishJSTest() immediately after testFailed(...)) so both success and failure
paths terminate the async test; ensure similar fixes are applied to the other
occurrences referenced by the review (the other onerror handlers at the
indicated locations).
In `@LayoutTests/fast/events/pointerleave-after-ancestor-removal.html`:
- Around line 30-43: When window.eventSender is not present the async waiter
never resolves; add a no-eventSender completion path so the test won't hang:
detect if (!window.eventSender) before using eventSender and immediately set
containerLeaveReceived = true (or call shouldBeTrue/finishJSTest directly) so
UIHelper.waitForCondition(() => containerLeaveReceived) can proceed; update the
block around eventSender.mouseMoveTo and the subsequent
UIHelper.waitForCondition usage (referencing eventSender,
containerLeaveReceived, UIHelper.waitForCondition, and finishJSTest) to ensure
the test completes synchronously when eventSender is unavailable.
In `@LayoutTests/fast/events/popup-blocking-timers2.html`:
- Line 5: The test enables async mode unconditionally via window.jsTestIsAsync
which can hang when window.eventSender automation is absent; change the harness
so async mode is only enabled when an automated click path exists (e.g., guard
setting window.jsTestIsAsync = true behind a check for window.eventSender or
equivalent), and ensure the code paths around the eventSender-driven clicks (the
logic referenced in the block handling the automated click) either call
finishJSTest() or avoid requiring async mode when window.eventSender is
undefined; update references in this file to gate setting window.jsTestIsAsync
and the eventSender click flow accordingly.
In `@LayoutTests/fast/events/touch/gesture/gesture-scroll.html`:
- Line 122: The test sets window.jsTestIsAsync = true but some
unsupported-environment or early-exit branches return/throw without calling
finishJSTest(), causing timeouts; update every early-exit/fallback branch in
this file (any feature-detection or "unsupported" code paths) to call
finishJSTest() before returning or exiting so the async harness is always
terminated (ensure finishJSTest() is invoked in the same branches that currently
call return/throw or call testFailed/closeEarly).
In
`@LayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-nested-iframes-triggers-drag.html`:
- Around line 12-17: The test sets window.jsTestIsAsync = true but only calls
finishJSTest() conditionally inside onDragStart, which can leave non-TestRunner
environments hanging; modify onDragStart to always call finishJSTest() when
running in async mode by invoking finishJSTest() unconditionally (or guarded
only by the global jsTestIsAsync flag) after setting the result element,
referencing the onDragStart function and the finishJSTest call so the test
finishes in all environments.
---
Outside diff comments:
In `@LayoutTests/fast/events/mouse-cursor-update-during-raf.html`:
- Around line 21-23: The test enables js-test.js async mode
(window.jsTestIsAsync = true) without guaranteeing a corresponding
finishJSTest() on all execution paths, causing timeouts; update the test to
either only enable async when you will definitely call finishJSTest() (wrap the
assignment and any async-only logic in the existing window.testRunner guard) or
ensure every branch that can exit early calls finishJSTest() (add calls to
finishJSTest() in unsupported/dependency-missing branches). Refer to
window.testRunner, window.jsTestIsAsync, finishJSTest(), and js-test.js when
making the change.
In `@LayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.html`:
- Around line 25-39: The test enables async mode via window.jsTestIsAsync but
returns early in runTests when window.eventSender is missing or when
eventSender.gestureTapDown is unsupported, which can hang the harness; update
runTests to call finishJSTest() before each early return (i.e., in the branches
checking window.eventSender and eventSender.gestureTapDown) so the async
lifecycle is completed and the test never waits indefinitely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a4ef0777-0e3f-4f5e-abd2-34b37dcc81fc
📒 Files selected for processing (300)
.gitignoreCMakeLists.txtConfigurations/CommonBase.xcconfigConfigurations/Version.xcconfigJSTests/microbenchmarks/class-fields-anonymous-arrow-function-fields.jsJSTests/microbenchmarks/regexp-empty-body-parens.jsJSTests/microbenchmarks/regexp-fixed-count-capturing-parens.jsJSTests/microbenchmarks/regexp-prototype-match-anchor.jsJSTests/microbenchmarks/regexp-prototype-match-basic.jsJSTests/microbenchmarks/regexp-prototype-match-complex-pattern.jsJSTests/microbenchmarks/regexp-prototype-match-no-strength-reduction.jsJSTests/microbenchmarks/regexp-prototype-match-observable-side-effects.jsJSTests/microbenchmarks/regexp-prototype-match-short-string.jsJSTests/modules/import-defer-then-not-enumerable.jsJSTests/stress/array-sort-nested-comparator-call-ignore-result.jsJSTests/stress/array-sort-nested-comparator-tail-call.jsJSTests/stress/async-generator-next-on-completed-reentrant.jsJSTests/stress/async-generator-reentrant-resume-during-resolve.jsJSTests/stress/class-fields-anonymous-function-name-edge-cases.jsJSTests/stress/class-fields-anonymous-function-name-inference.jsJSTests/stress/intl-canonical-iana-time-zone.jsJSTests/stress/intl-datetimeformat-temporal-era-width.jsJSTests/stress/intl-datetimeformat-temporal-non-default-calendar.jsJSTests/stress/iterator-close-map-set-fast-path-mid-iteration-return.jsJSTests/stress/multi-get-by-val-mixed-dfg.jsJSTests/stress/multi-get-by-val-mixed-oob-dfg.jsJSTests/stress/multi-get-by-val-sane-chain-oob-dfg.jsJSTests/stress/multi-put-by-val-mixed-dfg.jsJSTests/stress/multi-put-by-val-oob-dfg.jsJSTests/stress/promise-constructor-length.jsJSTests/stress/promise-resolve-non-thenable-structure-cache-cross-realm-proto.jsJSTests/stress/promise-resolving-functions-name-and-length.jsJSTests/stress/regexp-greedy-varcount-reextend-after-content-backtrack.jsJSTests/stress/regexp-prototype-symbol-match-edge-cases.jsJSTests/stress/regexp-prototype-symbol-match-watchpoint-invalidation.jsJSTests/stress/regexp-string-list-empty-alternative.jsJSTests/stress/string-split-regexp-fast-path-limit-side-effects.jsJSTests/stress/string-split-regexp-fast-path-tostring-side-effects.jsJSTests/stress/stringProtoFuncAt-GCOwnedDataScope-atomstring-swap.jsJSTests/stress/stringProtoFuncEndsWith-GCOwnedDataScope-atomstring-swap.jsJSTests/stress/stringProtoFuncLocaleCompare-GCOwnedDataScope-atomstring-swap.jsJSTests/stress/stringProtoFuncStartsWith-GCOwnedDataScope-atomstring-swap.jsJSTests/stress/temporal-boundary-values.jsJSTests/stress/temporal-calendar-daymath-weeks.jsJSTests/stress/temporal-calendar-month-boundary.jsJSTests/stress/temporal-calendar.jsJSTests/stress/temporal-cross-type.jsJSTests/stress/temporal-duration-relative-to-era-calendar.jsJSTests/stress/temporal-duration-relativeto.jsJSTests/stress/temporal-duration.jsJSTests/stress/temporal-gc-jit-stress.jsJSTests/stress/temporal-int32-overflow-duration.jsJSTests/stress/temporal-iso8601-parse-bounds.jsJSTests/stress/temporal-japanese-ce-bce-era-validation.jsJSTests/stress/temporal-month-boundary-arithmetic.jsJSTests/stress/temporal-month-overflow-truncation.jsJSTests/stress/temporal-monthday-parsing-timezone.jsJSTests/stress/temporal-monthday-refyear.jsJSTests/stress/temporal-nan-fields.jsJSTests/stress/temporal-non-iso-calendar.jsJSTests/stress/temporal-plain-date-time-nan-fields.jsJSTests/stress/temporal-plain-date-time-with-era-calendar.jsJSTests/stress/temporal-plain-date-with-era-calendar.jsJSTests/stress/temporal-plain-month-day-with-era-calendar.jsJSTests/stress/temporal-plain-year-month-with-era-calendar.jsJSTests/stress/temporal-plaindate-add-huge-days-out-of-range.jsJSTests/stress/temporal-plaindate-to-yearmonth-monthday-calendar.jsJSTests/stress/temporal-plaindatetime-calendar-propagation.jsJSTests/stress/temporal-plainmonthday-from-large-string.jsJSTests/stress/temporal-plainmonthday-to-plaindate-calendar.jsJSTests/stress/temporal-plainyearmonth-constructor-exception-check.jsJSTests/stress/temporal-pre-lmt-timezone.jsJSTests/stress/temporal-rounding-modes.jsJSTests/stress/temporal-string-parsing-edge.jsJSTests/stress/temporal-timezone-diversity.jsJSTests/stress/temporal-timezone-offset-identity.jsJSTests/stress/temporal-timezone.jsJSTests/stress/temporal-zdt-large-fold-disambiguation.jsJSTests/stress/temporal-zdt-round-dst-midnight-crossing.jsJSTests/stress/temporal-zdt-rounding-boundary.jsJSTests/stress/temporal-zdt-rounding-halfeven.jsJSTests/stress/temporal-zdt-timezone-canonicalization.jsJSTests/stress/temporal-zdt-to-locale-string-inherited-options.jsJSTests/stress/temporal-zdt-with-boundary.jsJSTests/stress/temporal-zoned-date-time-with-era-calendar.jsJSTests/stress/temporal-zoned-datetime-difference-day-correction.jsJSTests/stress/temporal-zoned-datetime-dst-arithmetic.jsJSTests/stress/temporal-zoned-datetime-dst-fold.jsJSTests/stress/temporal-zoned-datetime-dst-gap.jsJSTests/stress/temporal-zoned-datetime-timezone-identifier.jsJSTests/stress/wasm-table-constructor-wrapper-function-default.jsJSTests/stress/yarr-auto-possessification.jsJSTests/stress/yarr-quantified-empty-parentheses.jsJSTests/stress/yarr-quantified-parentheses-unified-backtracking.jsJSTests/test262/config.yamlJSTests/test262/expectations.yamlJSTests/test262/test/intl402/Temporal/PlainMonthDay/prototype/toLocaleString/basic.jsJSTests/wasm/libwabt.jsJSTests/wasm/stress/memory32-atomics-pointer-upper-bits.jsJSTests/wasm/stress/memory32-grow-upper-bits.jsJSTests/wasm/stress/omg-indirect-tail-call-late-input-clobber.jsLayoutTests/TestExpectationsLayoutTests/accessibility/aria-grid-with-presentational-sections-expected.txtLayoutTests/accessibility/aria-grid-with-presentational-sections.htmlLayoutTests/accessibility/aria-owns-crash-after-subtree-update-expected.txtLayoutTests/accessibility/aria-owns-crash-after-subtree-update.htmlLayoutTests/accessibility/iframe-content-remains-accessible-after-navigation-expected.txtLayoutTests/accessibility/iframe-content-remains-accessible-after-navigation.htmlLayoutTests/accessibility/mac/input-type-change-without-renderer-expected.txtLayoutTests/accessibility/mac/input-type-change-without-renderer.htmlLayoutTests/accessibility/mac/selection-element-tabbing-to-link.htmlLayoutTests/accessibility/resources/iframe-with-buttons.htmlLayoutTests/editing/input/delete-text-in-composition.htmlLayoutTests/editing/pasteboard/paste-contents-with-side-effects.htmlLayoutTests/editing/selection/5497643.htmlLayoutTests/editing/selection/ios/select-text-by-long-press-with-focused-element-expected.txtLayoutTests/editing/selection/ios/select-text-by-long-press-with-focused-element.htmlLayoutTests/editing/selection/ios/select-text-by-long-press-with-hardware-keyboard-expected.txtLayoutTests/editing/selection/ios/select-text-by-long-press-with-hardware-keyboard.htmlLayoutTests/editing/selection/ios/tap-focused-input-clears-outside-selection-expected.txtLayoutTests/editing/selection/ios/tap-focused-input-clears-outside-selection.htmlLayoutTests/editing/spelling/document-markers-remain-after-removing-text-expected.txtLayoutTests/editing/spelling/document-markers-remain-after-removing-text.htmlLayoutTests/fast/animation/request-animation-frame-during-modal.htmlLayoutTests/fast/animation/request-animation-frame-iframe.htmlLayoutTests/fast/animation/request-animation-frame-iframe2.htmlLayoutTests/fast/animation/request-animation-frame-prefix-expected.txtLayoutTests/fast/animation/request-animation-frame-prefix.htmlLayoutTests/fast/animation/request-animation-frame-too-rapid-expected.txtLayoutTests/fast/animation/request-animation-frame-too-rapid.htmlLayoutTests/fast/attachment/cocoa/wide-attachment-save-event-expected.txtLayoutTests/fast/attachment/cocoa/wide-attachment-save-event.htmlLayoutTests/fast/canvas/2d.context.expando.htmlLayoutTests/fast/canvas/canvas-blending-color-over-image.htmlLayoutTests/fast/canvas/canvas-blending-color-over-pattern.htmlLayoutTests/fast/canvas/canvas-blending-gradient-over-image.htmlLayoutTests/fast/canvas/canvas-blending-gradient-over-pattern.htmlLayoutTests/fast/canvas/canvas-blending-image-over-color.htmlLayoutTests/fast/canvas/canvas-blending-image-over-gradient.htmlLayoutTests/fast/canvas/canvas-line-batch-flush-expected.txtLayoutTests/fast/canvas/canvas-line-batch-flush.htmlLayoutTests/fast/canvas/neutered-imagedata.htmlLayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index-expected.txtLayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.htmlLayoutTests/fast/canvas/webgl/gl-teximage-imagebitmap.htmlLayoutTests/fast/canvas/webgl/shader-mix-with-bool-expected.txtLayoutTests/fast/canvas/webgl/shader-mix-with-bool.htmlLayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video-rgb565.htmlLayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video-rgba4444.htmlLayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video-rgba5551.htmlLayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.htmlLayoutTests/fast/canvas/webgl/webgl-clear-composited-notshowing.htmlLayoutTests/fast/canvas/webgl/webgl-compressed-texture-astc-expected.txtLayoutTests/fast/canvas/webgl/webgl-compressed-texture-astc.htmlLayoutTests/fast/css-generated-content/pseudo-element-events.htmlLayoutTests/fast/css-generated-content/pseudo-transition-event.htmlLayoutTests/fast/css-grid-layout/masonry-span-exceeds-grid-axis-tracks-crash-expected.txtLayoutTests/fast/css-grid-layout/masonry-span-exceeds-grid-axis-tracks-crash.htmlLayoutTests/fast/css-grid-layout/masonry-subgrid-excluded-legend-crash-expected.txtLayoutTests/fast/css-grid-layout/masonry-subgrid-excluded-legend-crash.htmlLayoutTests/fast/css3-text/css3-text-wrap/text-wrap-balance-line-clamp-1-expected.htmlLayoutTests/fast/css3-text/css3-text-wrap/text-wrap-balance-line-clamp-1.htmlLayoutTests/fast/dom/DeviceMotion/no-page-cache.htmlLayoutTests/fast/dom/DeviceOrientation/no-page-cache.htmlLayoutTests/fast/dom/Geolocation/window-close-crash.htmlLayoutTests/fast/dom/HTMLButtonElement/change-type-expected.txtLayoutTests/fast/dom/HTMLButtonElement/change-type.htmlLayoutTests/fast/dom/HTMLImageElement/sizes/image-sizes-js-change-expected.txtLayoutTests/fast/dom/HTMLImageElement/sizes/image-sizes-js-change.htmlLayoutTests/fast/dom/HTMLImageElement/sizes/image-sizes-js-innerhtml-expected.txtLayoutTests/fast/dom/HTMLImageElement/sizes/image-sizes-js-innerhtml.htmlLayoutTests/fast/dom/HTMLLinkElement/link-and-subresource-test-nonexistent.htmlLayoutTests/fast/dom/HTMLLinkElement/link-and-subresource-test.htmlLayoutTests/fast/dom/HTMLLinkElement/prefetch-onerror.htmlLayoutTests/fast/dom/HTMLLinkElement/prefetch-too-many-clients-expected.txtLayoutTests/fast/dom/HTMLLinkElement/prefetch-too-many-clients.htmlLayoutTests/fast/dom/HTMLLinkElement/prefetch.htmlLayoutTests/fast/dom/HTMLLinkElement/subresource.htmlLayoutTests/fast/dom/HTMLScriptElement/dont-load-unknown-type-expected.txtLayoutTests/fast/dom/HTMLScriptElement/dont-load-unknown-type.htmlLayoutTests/fast/dom/Window/Location/window-shadow-location-using-js-object-with-toString-expected.txtLayoutTests/fast/dom/Window/Location/window-shadow-location-using-js-object-with-toString.htmlLayoutTests/fast/dom/Window/Location/window-shadow-location-using-string-expected.txtLayoutTests/fast/dom/Window/Location/window-shadow-location-using-string.htmlLayoutTests/fast/dom/Window/new-window-opener.htmlLayoutTests/fast/dom/Window/open-window-min-size.htmlLayoutTests/fast/dom/Window/window-property-invalid-characters-ignored.htmlLayoutTests/fast/dom/crash-with-bad-url.htmlLayoutTests/fast/dom/gc-image-element-2-expected.txtLayoutTests/fast/dom/gc-image-element-2.htmlLayoutTests/fast/dom/gc-image-element-expected.txtLayoutTests/fast/dom/gc-image-element.htmlLayoutTests/fast/dom/location-new-window-no-crash.htmlLayoutTests/fast/dom/node-filter-detached-iframe-crash-expected.txtLayoutTests/fast/dom/node-filter-detached-iframe-crash.htmlLayoutTests/fast/dom/node-move-to-new-document-crash-main.htmlLayoutTests/fast/dom/non-reified-event-isTrusted-ic-crash.htmlLayoutTests/fast/dom/pseudo-element-hidePopover-then-crash-expected.txtLayoutTests/fast/dom/pseudo-element-hidePopover-then-crash.htmlLayoutTests/fast/dom/window-inner-size-scaling-expected.txtLayoutTests/fast/dom/window-inner-size-scaling.htmlLayoutTests/fast/dom/window-scroll-scaling-expected.txtLayoutTests/fast/dom/window-scroll-scaling.htmlLayoutTests/fast/dom/wrapper-classes.htmlLayoutTests/fast/encoding/char-encoding-mac.htmlLayoutTests/fast/encoding/char-encoding.htmlLayoutTests/fast/encoding/resources/char-encoding-utils.jsLayoutTests/fast/events/autoscroll-in-iframe-body-expected.txtLayoutTests/fast/events/autoscroll-in-iframe-body.htmlLayoutTests/fast/events/autoscroll-in-iframe-expected.txtLayoutTests/fast/events/autoscroll-in-iframe.htmlLayoutTests/fast/events/autoscroll-in-overflow-hidden-html-expected.txtLayoutTests/fast/events/autoscroll-in-overflow-hidden-html.htmlLayoutTests/fast/events/click-focus-control-expected.txtLayoutTests/fast/events/click-focus-control.htmlLayoutTests/fast/events/click-range-slider-expected.txtLayoutTests/fast/events/click-range-slider.htmlLayoutTests/fast/events/clientXY-in-zoom-and-scroll-expected.txtLayoutTests/fast/events/clientXY-in-zoom-and-scroll.htmlLayoutTests/fast/events/data-transfer-files-attribute-identity-expected.txtLayoutTests/fast/events/data-transfer-files-attribute-identity.htmlLayoutTests/fast/events/file-input-hidden-in-ondrop-expected.txtLayoutTests/fast/events/file-input-hidden-in-ondrop.htmlLayoutTests/fast/events/ios/viewport-no-width-value-allows-double-tap.htmlLayoutTests/fast/events/mouse-cursor-change.htmlLayoutTests/fast/events/mouse-cursor-hidden-when-out-of-bounds.htmlLayoutTests/fast/events/mouse-cursor-image-set.htmlLayoutTests/fast/events/mouse-cursor-light-dark.htmlLayoutTests/fast/events/mouse-cursor-multiframecur.htmlLayoutTests/fast/events/mouse-cursor-no-mousemove.htmlLayoutTests/fast/events/mouse-cursor-pseudo-elements.htmlLayoutTests/fast/events/mouse-cursor-update-during-raf.htmlLayoutTests/fast/events/mouse-cursor.htmlLayoutTests/fast/events/mouseover-button-expected.txtLayoutTests/fast/events/mouseover-button.htmlLayoutTests/fast/events/onerror-img-after-gc-expected.txtLayoutTests/fast/events/onerror-img-after-gc.htmlLayoutTests/fast/events/page-visibility-iframe-move-test.htmlLayoutTests/fast/events/pointerleave-after-ancestor-removal-expected.txtLayoutTests/fast/events/pointerleave-after-ancestor-removal.htmlLayoutTests/fast/events/popup-allowed-from-gesture-initiated-event-expected.txtLayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.htmlLayoutTests/fast/events/popup-blocked-from-fake-button-click-expected.txtLayoutTests/fast/events/popup-blocked-from-fake-button-click.htmlLayoutTests/fast/events/popup-blocked-from-fake-focus-expected.txtLayoutTests/fast/events/popup-blocked-from-fake-focus.htmlLayoutTests/fast/events/popup-blocked-from-fake-user-gesture-expected.txtLayoutTests/fast/events/popup-blocked-from-fake-user-gesture.htmlLayoutTests/fast/events/popup-blocked-from-untrusted-click-event-on-anchor-expected.txtLayoutTests/fast/events/popup-blocked-from-untrusted-click-event-on-anchor.htmlLayoutTests/fast/events/popup-blocking-timers1-expected.txtLayoutTests/fast/events/popup-blocking-timers1.htmlLayoutTests/fast/events/popup-blocking-timers2-expected.txtLayoutTests/fast/events/popup-blocking-timers2.htmlLayoutTests/fast/events/popup-blocking-timers3-expected.txtLayoutTests/fast/events/popup-blocking-timers3.htmlLayoutTests/fast/events/popup-blocking-timers5-expected.txtLayoutTests/fast/events/popup-blocking-timers5.htmlLayoutTests/fast/events/popup-blocking-timers6-expected.txtLayoutTests/fast/events/popup-blocking-timers6.htmlLayoutTests/fast/events/site-isolation/event-timing-back-forward-cache-duration-expected.txtLayoutTests/fast/events/site-isolation/event-timing-back-forward-cache-duration.htmlLayoutTests/fast/events/touch/basic-single-touch-events.htmlLayoutTests/fast/events/touch/emulated-touch-iframe-expected.txtLayoutTests/fast/events/touch/emulated-touch-iframe.htmlLayoutTests/fast/events/touch/gesture/gesture-click.htmlLayoutTests/fast/events/touch/gesture/gesture-dblclick.htmlLayoutTests/fast/events/touch/gesture/gesture-scroll.htmlLayoutTests/fast/events/touch/gesture/gesture-tap-active-state-iframe.htmlLayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-iframe-triggers-drag.htmlLayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-nested-iframes-triggers-drag.htmlLayoutTests/fast/events/touch/gesture/resources/gesture-helpers.jsLayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body-xhidden.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body-yhidden.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-div.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-iframe.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-div-not-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-div-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-div-scaled.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-div-twice-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-div.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-iframe-editable.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-iframe-not-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-iframe-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-iframe.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-page-not-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-page-propagated.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-page.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-shy-target.htmlLayoutTests/fast/events/touch/gesture/touch-gesture-scroll-sideways.htmlLayoutTests/fast/events/touch/ios/gesture-node-move-between-documents-expected.txtLayoutTests/fast/events/touch/ios/gesture-node-move-between-documents.htmlLayoutTests/fast/events/touch/ios/netflix-volume-slider-touch-action-quirk-expected.txtLayoutTests/fast/events/touch/ios/netflix-volume-slider-touch-action-quirk.htmlLayoutTests/fast/events/touch/ios/overflow-node-move-between-documents-expected.txtLayoutTests/fast/events/touch/ios/overflow-node-move-between-documents.htmlLayoutTests/fast/events/touch/ios/slider-node-move-between-documents-expected.txtLayoutTests/fast/events/touch/ios/slider-node-move-between-documents.html
💤 Files with no reviewable changes (14)
- LayoutTests/fast/canvas/webgl/webgl-compressed-texture-astc-expected.txt
- JSTests/stress/temporal-calendar.js
- LayoutTests/accessibility/mac/selection-element-tabbing-to-link.html
- LayoutTests/fast/events/mouse-cursor.html
- LayoutTests/fast/events/mouse-cursor-pseudo-elements.html
- LayoutTests/fast/events/mouse-cursor-hidden-when-out-of-bounds.html
- JSTests/stress/temporal-duration.js
- LayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
- LayoutTests/fast/events/mouse-cursor-light-dark.html
- LayoutTests/fast/canvas/neutered-imagedata.html
- LayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video-rgba4444.html
- JSTests/test262/expectations.yaml
- LayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video-rgb565.html
- LayoutTests/fast/canvas/webgl/tex-image-and-sub-image-2d-with-video-rgba5551.html
| for (let i = 0; i < 1e5; i++) { | ||
| shouldBe(new RegExp("wor" + "ld")[Symbol.match]("hello world")[0], "world"); | ||
| shouldBe(/hello/[Symbol.match]("hello" + "world" + Math.random())[0], "hello"); | ||
| } |
There was a problem hiding this comment.
Remove Math.random() from the benchmark hot path.
Using RNG each iteration introduces unrelated and nondeterministic overhead, which can skew the RegExp.prototype[Symbol.match] measurement.
Proposed change
for (let i = 0; i < 1e5; i++) {
shouldBe(new RegExp("wor" + "ld")[Symbol.match]("hello world")[0], "world");
- shouldBe(/hello/[Symbol.match]("hello" + "world" + Math.random())[0], "hello");
+ const input = "hello" + "world" + (i & 1);
+ shouldBe(/hello/[Symbol.match](input)[0], "hello");
}📝 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.
| for (let i = 0; i < 1e5; i++) { | |
| shouldBe(new RegExp("wor" + "ld")[Symbol.match]("hello world")[0], "world"); | |
| shouldBe(/hello/[Symbol.match]("hello" + "world" + Math.random())[0], "hello"); | |
| } | |
| for (let i = 0; i < 1e5; i++) { | |
| shouldBe(new RegExp("wor" + "ld")[Symbol.match]("hello world")[0], "world"); | |
| const input = "hello" + "world" + (i & 1); | |
| shouldBe(/hello/[Symbol.match](input)[0], "hello"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/microbenchmarks/regexp-prototype-match-no-strength-reduction.js`
around lines 6 - 9, The benchmark's hot loop is calling Math.random() each
iteration, adding nondeterministic overhead; in the for-loop that invokes
shouldBe and RegExp.prototype[Symbol.match], remove Math.random() from the
string concatenation (the second shouldBe call) so the input is deterministic —
e.g. replace "hello" + "world" + Math.random() with a constant like "helloworld"
or compute a single random suffix once outside the loop if variability is
required; update the call site that uses /hello/[Symbol.match] so it no longer
invokes Math.random() on every iteration.
| Object.defineProperty(Object.prototype, "then", { | ||
| get() { | ||
| log.push("then:" + thenGets); | ||
| if (thenGets++ === 0) | ||
| iter.next(); | ||
| return undefined; | ||
| }, | ||
| configurable: true, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Restore Object.prototype.then after the test body.
This test mutates a global prototype and never tears it down. Add cleanup to prevent cross-test/harness leakage.
Proposed fix
Object.defineProperty(Object.prototype, "then", {
get() {
log.push("then:" + thenGets);
if (thenGets++ === 0)
iter.next();
return undefined;
},
configurable: true,
});
-let error = null;
-let resolvedWith = null;
-
-(async function main() {
- iter = gen();
- await iter.next();
- log.push("await-first");
- await 0;
- await 0;
- return 10;
-})().then(result => { resolvedWith = result; }, e => { error = e; });
-
-drainMicrotasks();
-
-if (error)
- throw error;
-shouldBe(log.join("|"), "start|then:0|resume|await-first|then:1");
-shouldBe(resolvedWith, 10);
+let error = null;
+let resolvedWith = null;
+try {
+ (async function main() {
+ iter = gen();
+ await iter.next();
+ log.push("await-first");
+ await 0;
+ await 0;
+ return 10;
+ })().then(result => { resolvedWith = result; }, e => { error = e; });
+
+ drainMicrotasks();
+
+ if (error)
+ throw error;
+ shouldBe(log.join("|"), "start|then:0|resume|await-first|then:1");
+ shouldBe(resolvedWith, 10);
+} finally {
+ delete Object.prototype.then;
+}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/async-generator-reentrant-resume-during-resolve.js` around
lines 24 - 32, The test replaces Object.prototype.then without restoring it;
save the original descriptor for Object.prototype.then (e.g., origThenDesc =
Object.getOwnPropertyDescriptor(Object.prototype, "then")) before calling
Object.defineProperty, and after the test body (or in a finally/cleanup block)
restore it: if origThenDesc is undefined, delete Object.prototype.then,
otherwise call Object.defineProperty(Object.prototype, "then", origThenDesc).
Ensure the cleanup runs even if the test throws so thenGets/iter.next behavior
does not leak into other tests.
| let prev = Temporal.Now.instant(); | ||
| for (let i = 0; i < 100; i++) { | ||
| const curr = Temporal.Now.instant(); | ||
| const cmp = Temporal.Instant.compare(prev, curr); | ||
| if (cmp > 0) throw new Error(`Now.instant() not monotonic at iteration ${i}`); | ||
| prev = curr; |
There was a problem hiding this comment.
❓ Verification inconclusive
🌐 Web query:
Does the ECMAScript Temporal specification guarantee that Temporal.Now.instant() is monotonic (non-decreasing) across calls?
💡 Result:
No, the ECMAScript Temporal specification does not guarantee that Temporal.Now.instant is monotonic [1]. Subsequent calls to Temporal.Now.instant may return the same value or, in some cases, even a smaller value if the system clock is adjusted (e.g., via NTP) between calls [2][1]. The Temporal specification defines Temporal.Now.instant to return the current system time [3][4]. Because it relies on the underlying system clock, it does not provide the monotonic properties required for measuring elapsed time without potential drift or stagnation [2]. TC39 champions have explicitly declined to mandate monotonicity for this method, noting that limiting the accuracy of the system clock is the domain of the host environment (such as the browser or runtime) rather than the ECMAScript specification itself [1]. For monotonic time measurements, developers should use platform-specific APIs, such as performance.now in web browsers [2].
Citations:
- 1: API/Documentation risk with Temporal.Now tc39/proposal-temporal#2713
- 2: Does temporal support for query monotonic nanosecond current time? tc39/proposal-temporal#2048
- 3: https://tc39.es/proposal-temporal/docs/now.html
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/Now/instant
Monotonic wall-clock assertion can make this stress test flaky.
Lines 77–82 assume Temporal.Now.instant() is non-decreasing across calls, but the Temporal spec/TC39 does not guarantee monotonicity because it’s based on the underlying system clock and can move backwards if the host adjusts time (e.g., NTP). Replace the monotonic check with an API-validity assertion (e.g., that each call returns a Temporal.Instant) to avoid unrelated failures.
Deterministic alternative focused on API validity (not host clock monotonicity)
- let prev = Temporal.Now.instant();
- for (let i = 0; i < 100; i++) {
- const curr = Temporal.Now.instant();
- const cmp = Temporal.Instant.compare(prev, curr);
- if (cmp > 0) throw new Error(`Now.instant() not monotonic at iteration ${i}`);
- prev = curr;
- }
+ for (let i = 0; i < 100; i++) {
+ const curr = Temporal.Now.instant();
+ shouldBe(curr instanceof Temporal.Instant, true, `Now.instant() returns Instant at iteration ${i}`);
+ }📝 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.
| let prev = Temporal.Now.instant(); | |
| for (let i = 0; i < 100; i++) { | |
| const curr = Temporal.Now.instant(); | |
| const cmp = Temporal.Instant.compare(prev, curr); | |
| if (cmp > 0) throw new Error(`Now.instant() not monotonic at iteration ${i}`); | |
| prev = curr; | |
| for (let i = 0; i < 100; i++) { | |
| const curr = Temporal.Now.instant(); | |
| shouldBe(curr instanceof Temporal.Instant, true, `Now.instant() returns Instant at iteration ${i}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/temporal-gc-jit-stress.js` around lines 77 - 82, The loop
currently asserts monotonicity by comparing prev and curr using
Temporal.Instant.compare and throwing on cmp > 0; remove that monotonicity check
and the throw, and instead assert API validity by checking each call returns a
Temporal.Instant (e.g., verify typeof/instance or that Temporal.Instant.compare
can be called with the value without expecting ordering), replacing uses of
prev/curr comparison with a simple validation of the result of
Temporal.Now.instant() inside the for loop (referencing Temporal.Now.instant,
Temporal.Instant.compare only for validation if needed, and the loop variables
prev and curr).
| a3 = a1; | ||
| let a4 = a4.withPlainTime(a3); | ||
| throw a1; |
There was a problem hiding this comment.
Fix self-referential a4 TDZ that masks the intended Temporal check.
Line 6 (let a4 = a4.withPlainTime(a3);) always throws ReferenceError first, so this test stops validating the intended Temporal.PlainYearMonth flow. Use the constructed a2 object instead, and keep a3 local.
Proposed fix
function opt(a1) {
var a2 = new Temporal.PlainYearMonth(((NaN * a1 <= 0).__proto__ = "\\u{1F4A9}ba"), 1, 'chinese', 1);
- a3 = a1;
- let a4 = a4.withPlainTime(a3);
+ let a3 = a1;
+ a2.withPlainTime(a3);
throw a1;
}📝 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.
| a3 = a1; | |
| let a4 = a4.withPlainTime(a3); | |
| throw a1; | |
| function opt(a1) { | |
| var a2 = new Temporal.PlainYearMonth(((NaN * a1 <= 0).__proto__ = "\\u{1F4A9}ba"), 1, 'chinese', 1); | |
| let a3 = a1; | |
| a2.withPlainTime(a3); | |
| throw a1; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/temporal-plainyearmonth-constructor-exception-check.js` around
lines 5 - 7, The test currently triggers a TDZ ReferenceError because it
declares let a4 and immediately uses a4.withPlainTime(a3); replace that
self-referential initialization by calling withPlainTime on the constructed
PlainYearMonth instance (use a2.withPlainTime(a3)) and keep a3 as the local time
object used for the call so the intended Temporal.PlainYearMonth path is
executed (refer to variables a1, a2, a3, a4 and the withPlainTime call).
| function shouldBe(actual, expected) { | ||
| if (actual !== expected) | ||
| throw new Error(`expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does JSON.stringify throw on JavaScript BigInt values, and what error is specified?
💡 Result:
Yes, JSON.stringify throws a TypeError when it encounters a JavaScript BigInt value [1][2][3]. This occurs because BigInt values are not supported by the JSON standard [1][4]. The specific error message can vary slightly depending on the JavaScript engine: - Firefox typically throws: TypeError: BigInt value can't be serialized in JSON [1]. - V8-based engines (such as Chrome and Node.js) typically throw: TypeError: Do not know how to serialize a BigInt [1][5][6]. - Safari typically throws: TypeError: JSON.stringify cannot serialize BigInt [1]. To resolve this, you can provide a replacer function to JSON.stringify to convert BigInts to strings or numbers, or define a toJSON method on BigInt.prototype [1][5][7].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/BigInt_not_serializable
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global%5FObjects/JSON/stringify
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
- 4: ECMAScript specification documentation needs to be updated about JSON.stringify(24.5.2.1)? tc39/ecma262#1840
- 5: https://stackoverflow.com/questions/79529978/typeerror-do-not-know-how-to-serialize-a-bigint-when-json-stringifyobject
- 6: JSON.stringify should disallow BigInt value microsoft/TypeScript#63333
- 7: https://stackoverflow.com/questions/65152373/typescript-serialize-bigint-in-json
Fix BigInt assertion error formatting in shouldBe().
shouldBe() formats mismatches via JSON.stringify(...), but JSON.stringify(BigInt) throws a TypeError (“Do not know how to serialize a BigInt” / similar), so a failing BigInt comparison reports that TypeError instead of the intended “expected … but got …” message.
function shouldBe(actual, expected) {
+ const format = (value) => typeof value === "bigint" ? `${value}n` : JSON.stringify(value);
if (actual !== expected)
- throw new Error(`expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`);
+ throw new Error(`expected ${format(expected)} but got ${format(actual)}`);
}📝 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.
| function shouldBe(actual, expected) { | |
| if (actual !== expected) | |
| throw new Error(`expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); | |
| } | |
| function shouldBe(actual, expected) { | |
| const format = (value) => typeof value === "bigint" ? `${value}n` : JSON.stringify(value); | |
| if (actual !== expected) | |
| throw new Error(`expected ${format(expected)} but got ${format(actual)}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/temporal-zdt-large-fold-disambiguation.js` around lines 3 - 6,
The assertion helper shouldBe currently uses JSON.stringify on actual/expected
which throws for BigInt; update shouldBe to format values using JSON.stringify
with a replacer that converts BigInt to a string (e.g. v => typeof v ===
'bigint' ? v.toString() + 'n' : v) so mismatches produce the intended "expected
… but got …" message; modify the call sites inside the shouldBe function to use
this safe serializer for both actual and expected.
| <script src="../../../resources/js-test-pre.js"></script> | ||
| <script src="../../../resources/js-test.js"></script> | ||
| <script> | ||
| window.jsTestIsAsync = true; |
There was a problem hiding this comment.
Missing async completion on failure path causes timeout
Line 15 handles onerror with testFailed(...) only. With Line 6 setting window.jsTestIsAsync = true, the error path never calls finishJSTest(), so this test can hang instead of terminating cleanly.
Suggested fix
<script>
window.jsTestIsAsync = true;
function prefetch_onload() {
testPassed('onload called!');
finishJSTest();
}
+function prefetch_onerror() {
+ testFailed('onerror');
+ finishJSTest();
+}
</script>
@@
-<link href="prefetch.link" rel="prefetch" onload="prefetch_onload()" onerror="testFailed('onerror')" />
+<link href="prefetch.link" rel="prefetch" onload="prefetch_onload()" onerror="prefetch_onerror()" />Also applies to: 9-9, 15-15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/dom/HTMLLinkElement/subresource.html` at line 6, The test
sets window.jsTestIsAsync = true but the onerror handler only calls
testFailed(...), so failures never call finishJSTest() and the test can hang;
update the onerror handler (the function assigned to the element's onerror) to
call finishJSTest() after reporting the failure (e.g., call finishJSTest()
immediately after testFailed(...)) so both success and failure paths terminate
the async test; ensure similar fixes are applied to the other occurrences
referenced by the review (the other onerror handlers at the indicated
locations).
| if (window.eventSender) { | ||
| var r = btn.getBoundingClientRect(); | ||
| eventSender.mouseMoveTo(r.left + r.width / 2, r.top + r.height / 2); | ||
|
|
||
| container.removeChild(wrapper); | ||
|
|
||
| var r2 = other.getBoundingClientRect(); | ||
| eventSender.mouseMoveTo(r2.left + r2.width / 2, r2.top + r2.height / 2); | ||
| } | ||
|
|
||
| UIHelper.waitForCondition(() => containerLeaveReceived).then(() => { | ||
| shouldBeTrue("containerLeaveReceived"); | ||
| finishJSTest(); | ||
| }); |
There was a problem hiding this comment.
Add a no-eventSender completion path to avoid async hang.
window.jsTestIsAsync is always true, but when window.eventSender is unavailable the condition at Line 40 never becomes true, so the test can stall until timeout.
Suggested fix
- if (window.eventSender) {
+ if (window.eventSender) {
var r = btn.getBoundingClientRect();
eventSender.mouseMoveTo(r.left + r.width / 2, r.top + r.height / 2);
container.removeChild(wrapper);
var r2 = other.getBoundingClientRect();
eventSender.mouseMoveTo(r2.left + r2.width / 2, r2.top + r2.height / 2);
+ } else {
+ testFailed("This test requires eventSender");
+ finishJSTest();
+ return;
}
UIHelper.waitForCondition(() => containerLeaveReceived).then(() => {
shouldBeTrue("containerLeaveReceived");
finishJSTest();📝 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.
| if (window.eventSender) { | |
| var r = btn.getBoundingClientRect(); | |
| eventSender.mouseMoveTo(r.left + r.width / 2, r.top + r.height / 2); | |
| container.removeChild(wrapper); | |
| var r2 = other.getBoundingClientRect(); | |
| eventSender.mouseMoveTo(r2.left + r2.width / 2, r2.top + r2.height / 2); | |
| } | |
| UIHelper.waitForCondition(() => containerLeaveReceived).then(() => { | |
| shouldBeTrue("containerLeaveReceived"); | |
| finishJSTest(); | |
| }); | |
| if (window.eventSender) { | |
| var r = btn.getBoundingClientRect(); | |
| eventSender.mouseMoveTo(r.left + r.width / 2, r.top + r.height / 2); | |
| container.removeChild(wrapper); | |
| var r2 = other.getBoundingClientRect(); | |
| eventSender.mouseMoveTo(r2.left + r2.width / 2, r2.top + r2.height / 2); | |
| } else { | |
| testFailed("This test requires eventSender"); | |
| finishJSTest(); | |
| return; | |
| } | |
| UIHelper.waitForCondition(() => containerLeaveReceived).then(() => { | |
| shouldBeTrue("containerLeaveReceived"); | |
| finishJSTest(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/events/pointerleave-after-ancestor-removal.html` around
lines 30 - 43, When window.eventSender is not present the async waiter never
resolves; add a no-eventSender completion path so the test won't hang: detect if
(!window.eventSender) before using eventSender and immediately set
containerLeaveReceived = true (or call shouldBeTrue/finishJSTest directly) so
UIHelper.waitForCondition(() => containerLeaveReceived) can proceed; update the
block around eventSender.mouseMoveTo and the subsequent
UIHelper.waitForCondition usage (referencing eventSender,
containerLeaveReceived, UIHelper.waitForCondition, and finishJSTest) to ensure
the test completes synchronously when eventSender is unavailable.
| <script src="../../resources/js-test-pre.js"></script> | ||
| <script src="../../resources/js-test.js"></script> | ||
| <script> | ||
| window.jsTestIsAsync = true; |
There was a problem hiding this comment.
Async mode is enabled even when no automated click path exists.
Line 5 forces async mode, but Lines 19–27 do nothing when window.eventSender is unavailable. That combination can hang this test (no finishJSTest() path triggered automatically).
Suggested fix (gate async mode to automation path)
- window.jsTestIsAsync = true;
+ window.jsTestIsAsync = !!window.eventSender;Also applies to: 19-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/events/popup-blocking-timers2.html` at line 5, The test
enables async mode unconditionally via window.jsTestIsAsync which can hang when
window.eventSender automation is absent; change the harness so async mode is
only enabled when an automated click path exists (e.g., guard setting
window.jsTestIsAsync = true behind a check for window.eventSender or
equivalent), and ensure the code paths around the eventSender-driven clicks (the
logic referenced in the block handling the automated click) either call
finishJSTest() or avoid requiring async mode when window.eventSender is
undefined; update references in this file to gate setting window.jsTestIsAsync
and the eventSender click flow accordingly.
|
|
||
| if (window.testRunner) | ||
| testRunner.waitUntilDone(); | ||
| window.jsTestIsAsync = true; |
There was a problem hiding this comment.
Root cause: async harness migration missed fallback termination paths.
These tests now opt into async (jsTestIsAsync = true) but at least one unsupported-environment branch exits/fails without finishJSTest(), creating timeout risk. Ensure every early-exit/fallback path explicitly finishes the test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/events/touch/gesture/gesture-scroll.html` at line 122, The
test sets window.jsTestIsAsync = true but some unsupported-environment or
early-exit branches return/throw without calling finishJSTest(), causing
timeouts; update every early-exit/fallback branch in this file (any
feature-detection or "unsupported" code paths) to call finishJSTest() before
returning or exiting so the async harness is always terminated (ensure
finishJSTest() is invoked in the same branches that currently call return/throw
or call testFailed/closeEarly).
| window.jsTestIsAsync = true; | ||
| function onDragStart() { | ||
| document.getElementById("result").innerHTML = "PASS"; | ||
| if (window.testRunner) { | ||
| testRunner.notifyDone(); | ||
| finishJSTest(); | ||
| } |
There was a problem hiding this comment.
Call finishJSTest() unconditionally in async mode.
With window.jsTestIsAsync = true on Line 12, gating completion on Line 16 can leave non-TestRunner runs pending indefinitely.
Proposed fix
function onDragStart() {
document.getElementById("result").innerHTML = "PASS";
- if (window.testRunner) {
- finishJSTest();
- }
+ finishJSTest();
}📝 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.
| window.jsTestIsAsync = true; | |
| function onDragStart() { | |
| document.getElementById("result").innerHTML = "PASS"; | |
| if (window.testRunner) { | |
| testRunner.notifyDone(); | |
| finishJSTest(); | |
| } | |
| window.jsTestIsAsync = true; | |
| function onDragStart() { | |
| document.getElementById("result").innerHTML = "PASS"; | |
| finishJSTest(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@LayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-nested-iframes-triggers-drag.html`
around lines 12 - 17, The test sets window.jsTestIsAsync = true but only calls
finishJSTest() conditionally inside onDragStart, which can leave non-TestRunner
environments hanging; modify onDragStart to always call finishJSTest() when
running in async mode by invoking finishJSTest() unconditionally (or guarded
only by the global jsTestIsAsync flag) after setting the result element,
referencing the onDragStart function and the finishJSTest call so the test
finishes in all environments.
- PlatformJSCOnly.cmake: Mig_EXECUTABLE is only set by WebKitXcodeSDK.cmake, which is included only on Darwin hosts. Fall back to find_program so the Linux-hosted macOS cross build finds the in-container mig wrapper. - InlineCacheHandler.h: skip the offsetOfUid() == 40 layout-drift assert on Windows. The MSVC ABI ignores [[no_unique_address]], so RefCountedBase's empty RefCountDebugger occupies storage and m_uid lands at 48.
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
LayoutTests/fast/events/popup-blocked-from-fake-user-gesture.html (1)
29-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
finishJSTest()whentestRunneris unavailable.The entire test logic is wrapped in an
if (window.testRunner)guard (lines 29-44) with no else branch. WhentestRunneris unavailable,finishJSTest()is never called, causing a timeout in async mode (jsTestIsAsync = trueat line 9).🔧 Suggested fix
if (window.testRunner) { var button = document.getElementById("test"); if (window.eventSender) { eventSender.mouseMoveTo(button.offsetLeft + button.offsetWidth / 2, button.offsetTop + button.offsetHeight / 2); eventSender.mouseDown(); eventSender.mouseUp(); } clickHandler = clickHandler2; window.event = stolenEvent; button.click(); finishJSTest(); + } else { + debug("This test requires testRunner."); + finishJSTest(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/popup-blocked-from-fake-user-gesture.html` around lines 29 - 44, The test currently only calls finishJSTest() inside the if (window.testRunner) block, so when window.testRunner is falsy the async test never finishes; update the logic around the if (window.testRunner) guard (the block that manipulates button, eventSender, clickHandler, window.event and calls button.click()) to ensure finishJSTest() is always invoked regardless of window.testRunner presence — either by adding an else that calls finishJSTest() or by moving the finishJSTest() call out of the conditional so finishJSTest() runs in all cases.LayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body-yhidden.html (1)
142-143:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
finishJSTest()in the no-eventSender fallback path.When
window.eventSenderis unavailable, this else branch (lines 142-143) only logs a debug message without callingfinishJSTest(). Since the test enables async mode (jsTestIsAsync = trueat line 127), this causes a timeout.🔧 Suggested fix
} else { debug("This test requires DumpRenderTree. Touch scroll the red rect to log."); + finishJSTest(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body-yhidden.html` around lines 142 - 143, The else branch that runs when window.eventSender is missing only calls debug("This test requires DumpRenderTree. Touch scroll the red rect to log."); but does not call finishJSTest(), causing an async test timeout (jsTestIsAsync is true). Update that fallback else block to call finishJSTest() after the debug() call so the test completes; locate the branch that checks window.eventSender and add a finishJSTest() invocation to it.LayoutTests/fast/events/popup-blocking-timers5.html (1)
22-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
finishJSTest()wheneventSenderis unavailable.
clickButton()is called unconditionally on page load (line 34). Whenwindow.eventSenderis missing, the conditional block (lines 26-30) prevents the click from firing, soclickHandler()never runs andfinishJSTest()is never called, causing a timeout in async test mode.🔧 Suggested fix
function clickButton() { var button = document.getElementById("test"); var buttonX = button.offsetLeft + button.offsetWidth / 2; var buttonY = button.offsetTop + button.offsetHeight / 2; if (window.eventSender) { eventSender.mouseMoveTo(buttonX, buttonY); eventSender.mouseDown(); eventSender.mouseUp(); + } else { + debug("This test requires DumpRenderTree."); + finishJSTest(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/popup-blocking-timers5.html` around lines 22 - 31, The clickButton function currently only triggers a click when window.eventSender exists, so when eventSender is missing clickHandler and finishJSTest never run; update clickButton to handle the fallback case by either invoking button.click() or calling clickHandler() directly and then calling finishJSTest() when eventSender is unavailable (referencing clickButton, clickHandler, finishJSTest, eventSender and the DOM element with id "test").LayoutTests/fast/events/touch/gesture/gesture-dblclick.html (1)
76-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
finishJSTest()in the no-eventSender fallback path.When
window.eventSenderis unavailable, this else branch (lines 76-78) only logs a debug message without callingfinishJSTest(). Since the test enables async mode (jsTestIsAsync = trueat line 70), this causes a timeout.🔧 Suggested fix
} else { debug("This test requires DumpRenderTree.") + finishJSTest(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/touch/gesture/gesture-dblclick.html` around lines 76 - 78, The else branch that runs when window.eventSender is unavailable only calls debug("This test requires DumpRenderTree.") and never completes the async test; update that branch to call finishJSTest() after logging so the async test ends correctly (jsTestIsAsync is set to true); locate the else block in gesture-dblclick.html that checks for window.eventSender and add a finishJSTest() call there to avoid timeouts.
♻️ Duplicate comments (14)
JSTests/test262/test/intl402/Temporal/PlainMonthDay/prototype/toLocaleString/basic.js (1)
33-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicated
featuresfrontmatter entry.Line 33 repeats
features: [Temporal]already declared at Line 31. The duplicate entry should be removed to avoid metadata parsing ambiguity.🧹 Proposed fix
locale: [en-US, de-AT] -features: [Temporal] ---*/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@JSTests/test262/test/intl402/Temporal/PlainMonthDay/prototype/toLocaleString/basic.js` at line 33, Remove the duplicated frontmatter entry by deleting the second "features: [Temporal]" occurrence so only a single "features: [Temporal]" remains in the file; search for the duplicate string "features: [Temporal]" and remove the later instance to avoid metadata ambiguity.LayoutTests/editing/selection/ios/tap-focused-input-clears-outside-selection.html (1)
25-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a guard for missing UIHelper to prevent async test hangs.
With
jsTestIsAsync = true(line 22), ifUIHelperis unavailable, the load handler will throw an error at line 31 before reachingfinishJSTest()(line 52), causing an indefinite timeout. The past review comment correctly identified this issue.🛡️ Proposed guard
addEventListener("load", async () => { + if (!window.UIHelper) { + testFailed("This test requires WebKitTestRunner UIHelper."); + finishJSTest(); + return; + } const input = document.querySelector("input");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/editing/selection/ios/tap-focused-input-clears-outside-selection.html` around lines 25 - 53, The load handler assumes UIHelper exists and will throw before finishJSTest(), causing the async test to hang; add an early guard at the top of the addEventListener("load", ...) callback that checks for window.UIHelper (or typeof UIHelper !== "undefined") and if missing logs a clear failure and calls finishJSTest() (or otherwise terminates the test) so the test never times out; reference the UIHelper usage (activateAndWaitForInputSessionAt, longPressElement, activateElementAndWaitForInputSession) and ensure finishJSTest() is invoked on the guard path.LayoutTests/fast/events/pointerleave-after-ancestor-removal.html (1)
30-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd fallback when
eventSenderis unavailable to prevent test hang.When
window.eventSenderis not present,containerLeaveReceivedremainsfalseindefinitely, causingUIHelper.waitForCondition()at line 40 to wait forever and the async test to hang.🔧 Suggested fix
if (window.eventSender) { var r = btn.getBoundingClientRect(); eventSender.mouseMoveTo(r.left + r.width / 2, r.top + r.height / 2); container.removeChild(wrapper); var r2 = other.getBoundingClientRect(); eventSender.mouseMoveTo(r2.left + r2.width / 2, r2.top + r2.height / 2); + } else { + testFailed("This test requires eventSender"); + finishJSTest(); + return; } UIHelper.waitForCondition(() => containerLeaveReceived).then(() => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/pointerleave-after-ancestor-removal.html` around lines 30 - 43, The test can hang if window.eventSender is missing because containerLeaveReceived never becomes true; update the logic around the eventSender check (the block that calls eventSender.mouseMoveTo) to provide a fallback when window.eventSender is falsy: either synthesize the pointer/leave outcome by setting containerLeaveReceived = true (or dispatching the equivalent DOM event) before calling UIHelper.waitForCondition(), so UIHelper.waitForCondition(() => containerLeaveReceived) will not wait indefinitely; adjust the code near eventSender, containerLeaveReceived, and UIHelper.waitForCondition to implement this fallback.JSTests/stress/temporal-plainyearmonth-constructor-exception-check.js (1)
5-7:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix self-referential
a4TDZ that masks the intended Temporal check.Line 6 (
let a4 = a4.withPlainTime(a3);) always throwsReferenceErrordue to temporal dead zone before the intendedTemporal.PlainYearMonthvalidation can execute. Use the constructeda2instance or fix the initialization chain.🐛 Proposed fix
function opt(a1) { var a2 = new Temporal.PlainYearMonth(((NaN * a1 <= 0).__proto__ = "\\u{1F4A9}ba"), 1, 'chinese', 1); - a3 = a1; - let a4 = a4.withPlainTime(a3); + let a3 = a1; + a2.withPlainTime(a3); throw a1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@JSTests/stress/temporal-plainyearmonth-constructor-exception-check.js` around lines 5 - 7, The current initialization uses a self-referential let binding: "let a4 = a4.withPlainTime(a3);" which triggers a ReferenceError from the TDZ and prevents the intended Temporal.PlainYearMonth validation; change the initialization to call withPlainTime on the already-constructed instance (use "a2.withPlainTime(a3)" or otherwise use the constructed object instead of a4) so the Temporal check occurs instead of the TDZ error, and ensure subsequent throws still reference the correct variables (a1, a2, a3, a4).LayoutTests/fast/dom/HTMLLinkElement/subresource.html (1)
6-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winError path still missing finishJSTest().
The success path (L9) correctly calls
finishJSTest(), but the error handler at L15 (onerror="testFailed('onerror')") does not callfinishJSTest(). Withwindow.jsTestIsAsync = trueset at L6, the test will hang on failure instead of terminating.🛡️ Proposed fix
+function prefetch_onerror() { + testFailed('onerror'); + finishJSTest(); +} </script> <html> <p>This test will only print "PASS" or "FAIL" if link prefetches are enabled, otherwise it will show nothing below.</p> -<link href="prefetch.link" rel="prefetch" onload="prefetch_onload()" onerror="testFailed('onerror')" /> +<link href="prefetch.link" rel="prefetch" onload="prefetch_onload()" onerror="prefetch_onerror()" />Also applies to: 9-9, 15-15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/dom/HTMLLinkElement/subresource.html` at line 6, The error handler for the image link does not call finishJSTest(), so when window.jsTestIsAsync = true the test hangs on failure; update the onerror path to ensure finishJSTest() is invoked (either by calling finishJSTest() directly in the onerror="..." attribute alongside testFailed('onerror') or by making testFailed(...) itself call finishJSTest()), referencing the existing onerror handler and the finishJSTest function to ensure the test always terminates on both success and failure.LayoutTests/fast/canvas/webgl/gl-teximage-imagebitmap.html (1)
135-137:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRejection handler must call
finishJSTest()to complete async test.With
window.jsTestIsAsync = true, the rejection path must also callfinishJSTest()aftertestFailed(). Without it, test failures will time out instead of completing.🔧 Proposed fix
test().then(() => { finishJSTest(); }, e => { testFailed(String(e)); + finishJSTest(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/canvas/webgl/gl-teximage-imagebitmap.html` around lines 135 - 137, The rejection handler for the promise currently only calls testFailed(String(e)) and must also call finishJSTest() so the async JS test completes; update the arrow function (the rejection callback passed after the success handler) to call testFailed(String(e)) followed by finishJSTest(), ensuring window.jsTestIsAsync is respected.LayoutTests/fast/events/touch/gesture/gesture-scroll.html (1)
137-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
finishJSTest()in the no-eventSender fallback path.The test sets
jsTestIsAsync = true(line 122), but whenwindow.eventSenderis unavailable, this branch only callsdebug()without callingfinishJSTest(), causing a timeout.🔧 Suggested fix
} else { debug("This test requires DumpRenderTree."); + finishJSTest(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/touch/gesture/gesture-scroll.html` around lines 137 - 138, The no-eventSender fallback branch only calls debug("This test requires DumpRenderTree.") while jsTestIsAsync is set, causing a timeout; update that branch so after the debug() call it also calls finishJSTest() to end the async test. Locate the check for window.eventSender and add the finishJSTest() invocation after the debug(...) call to ensure the test completes when eventSender is unavailable.JSTests/stress/temporal-zoned-datetime-dst-fold.js (1)
7-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
shouldThrowsilently swallows unexpected exceptions, causing false-positive test passes.The catch block only rethrows when
e.message.startsWith(msg). Iffn()throws an unrelated exception (e.g., a TypeError or a RangeError with a different message), the test will silently pass instead of failing.🐛 Proposed fix
function shouldThrow(fn, msg) { - try { fn(); throw new Error(`${msg}: should have thrown`); } - catch (e) { if (e.message.startsWith(msg)) throw e; } + let didThrow = false; + try { + fn(); + } catch (e) { + didThrow = true; + } + if (!didThrow) + throw new Error(`${msg}: should have thrown`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@JSTests/stress/temporal-zoned-datetime-dst-fold.js` around lines 7 - 10, The helper shouldThrow currently swallows unexpected exceptions; update the function shouldThrow(fn, msg) so that it throws the "should have thrown" error when fn() does not throw, and in the catch block rethrow any exception that does not match the expected message (i.e., if !e.message.startsWith(msg) then throw e), otherwise treat the expected exception as success. Locate shouldThrow and adjust the try/catch accordingly to avoid silently swallowing unrelated errors.LayoutTests/fast/events/popup-blocking-timers2.html (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAsync mode is enabled even when no automated click path exists.
Line 5 forces async mode, but Lines 19–27 do nothing when
window.eventSenderis unavailable. That combination can hang this test (nofinishJSTest()path triggered automatically).Suggested fix (gate async mode to automation path)
- window.jsTestIsAsync = true; + window.jsTestIsAsync = !!window.eventSender;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/popup-blocking-timers2.html` at line 5, Gate enabling async mode to the automated click path: only set window.jsTestIsAsync = true when the automation eventSender is present (e.g., wrap the assignment in if (window.eventSender) { window.jsTestIsAsync = true; }). Ensure the existing code paths that use window.eventSender (and the automatic finishJSTest() trigger) remain unchanged so tests without eventSender do not enter async mode and hang.LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html (1)
21-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn early when
OES_element_index_uintis unavailable in WebGL1.Line 23 marks pass (
"no extension") but continues into theUNSIGNED_INTdraw path. Without the extension,drawElementswithUNSIGNED_INTgeneratesINVALID_ENUM, not the expectedINVALID_OPERATIONat line 40, causing the test to fail incorrectly.🐛 Proposed fix to add early return
if (contextType == "webgl") { - if (!context.getExtension("OES_element_index_uint")) + if (!context.getExtension("OES_element_index_uint")) { testPassed("no extension"); + return; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html` around lines 21 - 24, In the webgl branch where you check for OES_element_index_uint (the if (contextType == "webgl") block that calls context.getExtension("OES_element_index_uint") and testPassed("no extension")), return immediately after logging the pass so execution does not fall through into the UNSIGNED_INT drawElements path; specifically, add an early return right after testPassed("no extension") to avoid calling drawElements with UNSIGNED_INT when the extension is missing.JSTests/stress/async-generator-reentrant-resume-during-resolve.js (1)
24-32: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRestore
Object.prototype.thenafter the test body.This test mutates a global prototype and never tears it down. Add cleanup to prevent cross-test/harness leakage.
♻️ Proposed fix to add cleanup
Object.defineProperty(Object.prototype, "then", { get() { log.push("then:" + thenGets); if (thenGets++ === 0) iter.next(); return undefined; }, configurable: true, }); let error = null; let resolvedWith = null; - -(async function main() { - iter = gen(); - await iter.next(); - log.push("await-first"); - await 0; - await 0; - return 10; -})().then(result => { resolvedWith = result; }, e => { error = e; }); - -drainMicrotasks(); - -if (error) - throw error; -shouldBe(log.join("|"), "start|then:0|resume|await-first|then:1"); -shouldBe(resolvedWith, 10); +try { + (async function main() { + iter = gen(); + await iter.next(); + log.push("await-first"); + await 0; + await 0; + return 10; + })().then(result => { resolvedWith = result; }, e => { error = e; }); + + drainMicrotasks(); + + if (error) + throw error; + shouldBe(log.join("|"), "start|then:0|resume|await-first|then:1"); + shouldBe(resolvedWith, 10); +} finally { + delete Object.prototype.then; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@JSTests/stress/async-generator-reentrant-resume-during-resolve.js` around lines 24 - 32, The test adds a getter for Object.prototype.then and never restores it; save the original descriptor for "then" before calling Object.defineProperty (or detect if it was undefined), then ensure you restore the original state in a finally/cleanup block after the test body (either by re-defining the saved descriptor or deleting Object.prototype.then if it didn’t exist). Locate the existing Object.defineProperty(Object.prototype, "then", { get() { ... } }) usage (and related symbols thenGets, iter, log) and wrap the test mutation so the original descriptor is restored regardless of test success or failure to avoid leaking into other tests.JSTests/stress/wasm-table-constructor-wrapper-function-default.js (1)
87-95: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winClarify comment or fix test input to match intent.
The comment on Line 87 states "Non-function default values keep throwing," but Line 90 passes a function (
() => 1). If the intent is to test rejection of plain JS functions (not WebAssembly wrapper functions), the comment should be reworded. If the intent is to test actual non-function values (numbers, strings, objects), then replace() => 1with a non-function value like1.Option 1: Test actual non-function values
-// Non-function default values keep throwing. +// Non-function default values throw TypeError. let threw = false; try { - new WebAssembly.Table({ element: "funcref", initial: 1 }, () => 1); + new WebAssembly.Table({ element: "funcref", initial: 1 }, 1); } catch (error) {Option 2: Clarify comment for plain JS function rejection
-// Non-function default values keep throwing. +// Plain JS functions (not WebAssembly wrapper functions) throw TypeError. let threw = false; try { new WebAssembly.Table({ element: "funcref", initial: 1 }, () => 1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@JSTests/stress/wasm-table-constructor-wrapper-function-default.js` around lines 87 - 95, The test comment and input disagree: the comment "Non-function default values keep throwing" but the code passes a function in the WebAssembly.Table constructor call new WebAssembly.Table({ element: "funcref", initial: 1 }, () => 1); — either update the comment to state "Plain JS functions are rejected as defaults" if you intend to assert rejection of regular JS functions, or change the second argument to a non-function value (e.g., replace () => 1 with 1 or "foo") to test actual non-function defaults; modify the comment or the constructor call in the test so the intent and input match (refer to the WebAssembly.Table constructor call in this file).JSTests/stress/temporal-zdt-large-fold-disambiguation.js (1)
3-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix BigInt serialization in shouldBe error messages.
The
shouldBehelper usesJSON.stringify()which throwsTypeErrorwhen encountering BigInt values. Since this test comparesepochNanoseconds(BigInt), any assertion failure would produce a serialization error instead of the intended "expected … but got …" message.🛠️ Proposed fix
function shouldBe(actual, expected) { + const format = (value) => typeof value === "bigint" ? `${value}n` : JSON.stringify(value); if (actual !== expected) - throw new Error(`expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); + throw new Error(`expected ${format(expected)} but got ${format(actual)}`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@JSTests/stress/temporal-zdt-large-fold-disambiguation.js` around lines 3 - 6, The shouldBe helper throws a TypeError when JSON.stringify encounters BigInt (so assertion failures on epochNanoseconds hide the real message); update the shouldBe function to safely serialize values (e.g., use a replacer that converts BigInt to a string or detect typeof value === 'bigint' and call toString()) before constructing the error message so the thrown Error contains "expected … but got …" for BigInt comparisons; keep the function name shouldBe and only change how actual and expected are stringified for the error text.LayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-nested-iframes-triggers-drag.html (1)
15-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCall
finishJSTest()unconditionally in async mode.With
window.jsTestIsAsync = trueon line 12, the conditional guard on line 15 can prevent proper test completion. When using the async harness pattern,finishJSTest()should be called unconditionally to properly close the test.🔧 Proposed fix
function onDragStart() { document.getElementById("result").innerHTML = "PASS"; - if (window.testRunner) { - finishJSTest(); - } + finishJSTest(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-nested-iframes-triggers-drag.html` around lines 15 - 17, The test currently calls finishJSTest() only when window.testRunner exists, which can prevent completion in async mode (window.jsTestIsAsync = true); remove the conditional guard around finishJSTest() so that finishJSTest() is invoked unconditionally at the end of the script (replace the if (window.testRunner) { finishJSTest(); } block with a direct call to finishJSTest()), ensuring the async test harness always closes the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@JSTests/microbenchmarks/regexp-prototype-match-observable-side-effects.js`:
- Around line 26-28: Change the loose equality checks that rely on coercion
(assert(accesses == "")) to explicit empty-array checks: use
assert(accesses.length === 0, "...") (or assert(Array.isArray(accesses) &&
accesses.length === 0, "...") if you want to be defensive) for each occurrence
where the test verifies no accesses (the assert calls around the
RegExp.prototype[Symbol.match].call(obj, ...) checks referencing the variable
accesses). Replace all instances of accesses == "" with the explicit length (or
Array.isArray + length) check so the intent is clear and not dependent on JS
coercion.
- Line 109: Replace the loose equality check in the assertion with a strict
equality check: in the assertion that compares accesses.toString() to
"flags,exec" (the assert call referencing the accesses variable), change the
operator from == to === so the test uses strict string comparison and avoids
coercion.
In `@JSTests/stress/async-generator-next-on-completed-reentrant.js`:
- Around line 17-30: The test modifies Object.prototype by defining a "then"
accessor; record the original descriptor (or presence) before defining it and
restore it after the test completes (after the it.next() invocation and any
awaited microtasks), or at minimum delete Object.prototype.then at the end of
the test; update the code around the getter definition for "then" and the final
it.next() call to ensure the original state is restored (use the saved
originalDescriptor or delete if none) so global prototype pollution is cleaned
up.
In `@JSTests/stress/multi-put-by-val-mixed-dfg.js`:
- Line 20: The for-loop uses an undefined variable testLoopCount which causes a
ReferenceError; define a local iteration constant (e.g. const testLoopCount =
1e4 or another large number appropriate for DFG stress) above the loop that
contains for (let i = 0; i < testLoopCount; ++i) so the loop can run—add the
constant near the top of the file or immediately before the loop to ensure the
test executes and triggers DFG compilation.
In `@JSTests/stress/promise-resolving-functions-name-and-length.js`:
- Line 87: The for-loop references an undefined variable testLoopCount causing a
ReferenceError; fix it by declaring and initializing testLoopCount to a sensible
numeric iteration count (e.g., const testLoopCount = <N>) before the for (let i
= 0; i < testLoopCount; ++i) loop or by replacing testLoopCount with an existing
iteration variable (if one exists) so the loop uses a defined numeric value;
ensure the constant is >0 and placed in the same scope as the loop (near the top
of the test or just above the loop) so repeated allocation/JIT validation runs
as intended.
In `@JSTests/stress/temporal-boundary-values.js`:
- Around line 7-10: The helper shouldThrow has inverted logic: in the catch
block it currently re-throws when the caught error message matches the expected
prefix; change it so that shouldThrow re-throws the caught error only when
e.message does NOT start with the expected msg, and otherwise treat it as the
expected success (i.e., return/silently succeed). Update the catch in
shouldThrow to check e.message.startsWith(msg) and return on match, rethrow on
mismatch so wrong error messages no longer pass.
In `@JSTests/stress/temporal-int32-overflow-duration.js`:
- Line 46: Remove the duplicate assertion that repeats
Temporal.PlainYearMonth.from("-271821-04").toString() === "-271821-04" (the same
shouldBe call present twice); locate the repeated shouldBe invocation for
Temporal.PlainYearMonth.from("-271821-04").toString() and delete the second
occurrence (or replace it with a different, meaningful assertion if additional
coverage was intended).
In `@JSTests/stress/temporal-pre-lmt-timezone.js`:
- Around line 46-58: Rename the misleading variable isAmbiguous to reflect a DST
gap/nonexistent time (e.g., isNonexistent or isGap) in the test block that
constructs Temporal.ZonedDateTime earlier and later for 2025-03-09 02:30
America/Vancouver; update the assertion and any references to use the new name
so the test reads correctly (earlier and later produce different results because
the local time is non-existent).
In `@JSTests/stress/temporal-zoned-datetime-dst-gap.js`:
- Around line 9-12: The helper function shouldThrow has inverted logic: in the
catch it re-throws when the caught error's message does match the expected
prefix; change the condition so it re-throws only when the message does NOT
match the expected prefix (i.e. use a negated startsWith check) so correct
matches are swallowed and unexpected/errors from fn are propagated; update the
catch in shouldThrow accordingly.
In `@JSTests/wasm/stress/memory32-atomics-pointer-upper-bits.js`:
- Around line 45-47: The three assertions that check for an out-of-bounds trap
(the calls to exports.notify, exports.wait32, and exports.wait64) are too strict
by matching the entire RuntimeError message; change them to match only the
stable core text by using a regex or starts-with check for "Out of bounds memory
access" (e.g. assert.throws(..., WebAssembly.RuntimeError, /^Out of bounds
memory access/ or asserting error.message.startsWith("Out of bounds memory
access")) so variants with callsite context still pass.
In `@LayoutTests/fast/canvas/canvas-line-batch-flush.html`:
- Line 4: Replace the legacy test harness include in this test by swapping the
script src from "js-test-pre.js" to the newer "js-test.js" so the file uses the
same harness as the other updated tests (e.g., change-type.html); update the
<script> tag that currently references "js-test-pre.js" to reference
"js-test.js" and ensure any pre-test setup expectations in this file align with
the newer harness behavior.
In `@LayoutTests/fast/css-grid-layout/masonry-subgrid-excluded-legend-crash.html`:
- Line 4: The test uses "display: grid-lanes" which is a valid WebKit display
value when the gridLanes feature is enabled, so update the test to run with the
gridLanesEnabled feature flag on (instead of treating the value as invalid):
enable gridLanesEnabled in the test harness/setup (e.g. add the appropriate
layout-test meta or test-run option that turns on gridLanesEnabled) so the CSS
rule "display: grid-lanes" in this file is exercised under the feature gate and
the crash path is reached.
In `@LayoutTests/fast/dom/crash-with-bad-url.html`:
- Around line 4-7: The test currently relies on the subframe calling
testRunner.notifyDone() inside the injected image onerror handler, but that call
is guarded by if (window.testRunner) and may never run; update the test to
ensure async completion by either calling finishJSTest() from the top-level page
after creating the subframe or by making the subframe unconditionally call
testRunner.notifyDone() (or invoke a top-level finishJSTest() via postMessage
from the subframe) so the harness is always notified; locate the image onerror
handler in the subframe injection and the main test code around
finishJSTest()/testRunner.notifyDone() and implement one of these fixes.
In `@LayoutTests/fast/dom/pseudo-element-hidePopover-then-crash.html`:
- Line 31: Test mixes old synchronous harness with the new async harness: remove
the old synchronous harness includes and usage and keep the async pattern.
Specifically, delete the script tags referencing js-test-pre.js and
js-test-post.js (the lines that include those files) and any synchronous harness
assumptions, ensure window.jsTestIsAsync = true remains, and keep the
finishJSTest() call to complete the test; verify no other synchronous test
harness functions remain in the file (look for js-test-pre.js, js-test-post.js,
window.jsTestIsAsync, finishJSTest).
In `@LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html`:
- Around line 64-67: The test currently only calls nextTest() when
window.testRunner exists, leaving jsTestIsAsync = true and never calling
finishJSTest() when testRunner is absent; add an else branch that calls
finishJSTest() (or otherwise ensures finishJSTest() is invoked) when
window.testRunner is falsy so the async test completes. Locate the conditional
around window.testRunner and nextTest() and insert the finishJSTest() call in
the fallback path so the test always finishes.
In `@LayoutTests/TestExpectations`:
- Around line 7115-7116: The test
imported/w3c/web-platform-tests/navigation-api/navigate-event/dangling-navigate-event-after-bfcache-restore.html
is causing a crash during BFCache restore; update the TestExpectations entry to
mark it as a Crash (not just Skip) and annotate it with the rdar://179168475
tracking ID so it shows as a prioritized stability regression, and file/link a
bug/regression ticket referencing rdar://179168475 and the test path so the
crash can be triaged and fixed; ensure the TestExpectations line for that test
is changed from "[ Skip ]" to the appropriate crash expectation and add a
one-line TODO comment with the bug ID for follow-up.
---
Outside diff comments:
In `@LayoutTests/fast/events/popup-blocked-from-fake-user-gesture.html`:
- Around line 29-44: The test currently only calls finishJSTest() inside the if
(window.testRunner) block, so when window.testRunner is falsy the async test
never finishes; update the logic around the if (window.testRunner) guard (the
block that manipulates button, eventSender, clickHandler, window.event and calls
button.click()) to ensure finishJSTest() is always invoked regardless of
window.testRunner presence — either by adding an else that calls finishJSTest()
or by moving the finishJSTest() call out of the conditional so finishJSTest()
runs in all cases.
In `@LayoutTests/fast/events/popup-blocking-timers5.html`:
- Around line 22-31: The clickButton function currently only triggers a click
when window.eventSender exists, so when eventSender is missing clickHandler and
finishJSTest never run; update clickButton to handle the fallback case by either
invoking button.click() or calling clickHandler() directly and then calling
finishJSTest() when eventSender is unavailable (referencing clickButton,
clickHandler, finishJSTest, eventSender and the DOM element with id "test").
In `@LayoutTests/fast/events/touch/gesture/gesture-dblclick.html`:
- Around line 76-78: The else branch that runs when window.eventSender is
unavailable only calls debug("This test requires DumpRenderTree.") and never
completes the async test; update that branch to call finishJSTest() after
logging so the async test ends correctly (jsTestIsAsync is set to true); locate
the else block in gesture-dblclick.html that checks for window.eventSender and
add a finishJSTest() call there to avoid timeouts.
In
`@LayoutTests/fast/events/touch/gesture/touch-gesture-noscroll-body-yhidden.html`:
- Around line 142-143: The else branch that runs when window.eventSender is
missing only calls debug("This test requires DumpRenderTree. Touch scroll the
red rect to log."); but does not call finishJSTest(), causing an async test
timeout (jsTestIsAsync is true). Update that fallback else block to call
finishJSTest() after the debug() call so the test completes; locate the branch
that checks window.eventSender and add a finishJSTest() invocation to it.
---
Duplicate comments:
In `@JSTests/stress/async-generator-reentrant-resume-during-resolve.js`:
- Around line 24-32: The test adds a getter for Object.prototype.then and never
restores it; save the original descriptor for "then" before calling
Object.defineProperty (or detect if it was undefined), then ensure you restore
the original state in a finally/cleanup block after the test body (either by
re-defining the saved descriptor or deleting Object.prototype.then if it didn’t
exist). Locate the existing Object.defineProperty(Object.prototype, "then", {
get() { ... } }) usage (and related symbols thenGets, iter, log) and wrap the
test mutation so the original descriptor is restored regardless of test success
or failure to avoid leaking into other tests.
In `@JSTests/stress/temporal-plainyearmonth-constructor-exception-check.js`:
- Around line 5-7: The current initialization uses a self-referential let
binding: "let a4 = a4.withPlainTime(a3);" which triggers a ReferenceError from
the TDZ and prevents the intended Temporal.PlainYearMonth validation; change the
initialization to call withPlainTime on the already-constructed instance (use
"a2.withPlainTime(a3)" or otherwise use the constructed object instead of a4) so
the Temporal check occurs instead of the TDZ error, and ensure subsequent throws
still reference the correct variables (a1, a2, a3, a4).
In `@JSTests/stress/temporal-zdt-large-fold-disambiguation.js`:
- Around line 3-6: The shouldBe helper throws a TypeError when JSON.stringify
encounters BigInt (so assertion failures on epochNanoseconds hide the real
message); update the shouldBe function to safely serialize values (e.g., use a
replacer that converts BigInt to a string or detect typeof value === 'bigint'
and call toString()) before constructing the error message so the thrown Error
contains "expected … but got …" for BigInt comparisons; keep the function name
shouldBe and only change how actual and expected are stringified for the error
text.
In `@JSTests/stress/temporal-zoned-datetime-dst-fold.js`:
- Around line 7-10: The helper shouldThrow currently swallows unexpected
exceptions; update the function shouldThrow(fn, msg) so that it throws the
"should have thrown" error when fn() does not throw, and in the catch block
rethrow any exception that does not match the expected message (i.e., if
!e.message.startsWith(msg) then throw e), otherwise treat the expected exception
as success. Locate shouldThrow and adjust the try/catch accordingly to avoid
silently swallowing unrelated errors.
In `@JSTests/stress/wasm-table-constructor-wrapper-function-default.js`:
- Around line 87-95: The test comment and input disagree: the comment
"Non-function default values keep throwing" but the code passes a function in
the WebAssembly.Table constructor call new WebAssembly.Table({ element:
"funcref", initial: 1 }, () => 1); — either update the comment to state "Plain
JS functions are rejected as defaults" if you intend to assert rejection of
regular JS functions, or change the second argument to a non-function value
(e.g., replace () => 1 with 1 or "foo") to test actual non-function defaults;
modify the comment or the constructor call in the test so the intent and input
match (refer to the WebAssembly.Table constructor call in this file).
In
`@JSTests/test262/test/intl402/Temporal/PlainMonthDay/prototype/toLocaleString/basic.js`:
- Line 33: Remove the duplicated frontmatter entry by deleting the second
"features: [Temporal]" occurrence so only a single "features: [Temporal]"
remains in the file; search for the duplicate string "features: [Temporal]" and
remove the later instance to avoid metadata ambiguity.
In
`@LayoutTests/editing/selection/ios/tap-focused-input-clears-outside-selection.html`:
- Around line 25-53: The load handler assumes UIHelper exists and will throw
before finishJSTest(), causing the async test to hang; add an early guard at the
top of the addEventListener("load", ...) callback that checks for
window.UIHelper (or typeof UIHelper !== "undefined") and if missing logs a clear
failure and calls finishJSTest() (or otherwise terminates the test) so the test
never times out; reference the UIHelper usage (activateAndWaitForInputSessionAt,
longPressElement, activateElementAndWaitForInputSession) and ensure
finishJSTest() is invoked on the guard path.
In `@LayoutTests/fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html`:
- Around line 21-24: In the webgl branch where you check for
OES_element_index_uint (the if (contextType == "webgl") block that calls
context.getExtension("OES_element_index_uint") and testPassed("no extension")),
return immediately after logging the pass so execution does not fall through
into the UNSIGNED_INT drawElements path; specifically, add an early return right
after testPassed("no extension") to avoid calling drawElements with UNSIGNED_INT
when the extension is missing.
In `@LayoutTests/fast/canvas/webgl/gl-teximage-imagebitmap.html`:
- Around line 135-137: The rejection handler for the promise currently only
calls testFailed(String(e)) and must also call finishJSTest() so the async JS
test completes; update the arrow function (the rejection callback passed after
the success handler) to call testFailed(String(e)) followed by finishJSTest(),
ensuring window.jsTestIsAsync is respected.
In `@LayoutTests/fast/dom/HTMLLinkElement/subresource.html`:
- Line 6: The error handler for the image link does not call finishJSTest(), so
when window.jsTestIsAsync = true the test hangs on failure; update the onerror
path to ensure finishJSTest() is invoked (either by calling finishJSTest()
directly in the onerror="..." attribute alongside testFailed('onerror') or by
making testFailed(...) itself call finishJSTest()), referencing the existing
onerror handler and the finishJSTest function to ensure the test always
terminates on both success and failure.
In `@LayoutTests/fast/events/pointerleave-after-ancestor-removal.html`:
- Around line 30-43: The test can hang if window.eventSender is missing because
containerLeaveReceived never becomes true; update the logic around the
eventSender check (the block that calls eventSender.mouseMoveTo) to provide a
fallback when window.eventSender is falsy: either synthesize the pointer/leave
outcome by setting containerLeaveReceived = true (or dispatching the equivalent
DOM event) before calling UIHelper.waitForCondition(), so
UIHelper.waitForCondition(() => containerLeaveReceived) will not wait
indefinitely; adjust the code near eventSender, containerLeaveReceived, and
UIHelper.waitForCondition to implement this fallback.
In `@LayoutTests/fast/events/popup-blocking-timers2.html`:
- Line 5: Gate enabling async mode to the automated click path: only set
window.jsTestIsAsync = true when the automation eventSender is present (e.g.,
wrap the assignment in if (window.eventSender) { window.jsTestIsAsync = true;
}). Ensure the existing code paths that use window.eventSender (and the
automatic finishJSTest() trigger) remain unchanged so tests without eventSender
do not enter async mode and hang.
In `@LayoutTests/fast/events/touch/gesture/gesture-scroll.html`:
- Around line 137-138: The no-eventSender fallback branch only calls debug("This
test requires DumpRenderTree.") while jsTestIsAsync is set, causing a timeout;
update that branch so after the debug() call it also calls finishJSTest() to end
the async test. Locate the check for window.eventSender and add the
finishJSTest() invocation after the debug(...) call to ensure the test completes
when eventSender is unavailable.
In
`@LayoutTests/fast/events/touch/gesture/long-press-on-draggable-element-in-nested-iframes-triggers-drag.html`:
- Around line 15-17: The test currently calls finishJSTest() only when
window.testRunner exists, which can prevent completion in async mode
(window.jsTestIsAsync = true); remove the conditional guard around
finishJSTest() so that finishJSTest() is invoked unconditionally at the end of
the script (replace the if (window.testRunner) { finishJSTest(); } block with a
direct call to finishJSTest()), ensuring the async test harness always closes
the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| assert(accesses == "", "Should not be able to override lastIndex"); | ||
| let result = RegExp.prototype[Symbol.match].call(obj, "searchme"); | ||
| assert(accesses == "", "Should not be able to override lastIndex"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use explicit array checks instead of relying on coercion.
Lines 26, 28, 44, 66, 84, and 106 compare arrays to empty strings using loose equality (accesses == ""), which relies on JavaScript's coercion behavior. While this works (empty arrays coerce to ""), it's not idiomatic and reduces clarity.
♻️ Suggested refactor
- assert(accesses == "", "Should not be able to override lastIndex");
+ assert(accesses.length === 0, "Should not be able to override lastIndex");Apply similar changes to lines 28, 44, 66, 84, and 106.
Also applies to: 44-44, 66-66, 84-84, 106-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/microbenchmarks/regexp-prototype-match-observable-side-effects.js`
around lines 26 - 28, Change the loose equality checks that rely on coercion
(assert(accesses == "")) to explicit empty-array checks: use
assert(accesses.length === 0, "...") (or assert(Array.isArray(accesses) &&
accesses.length === 0, "...") if you want to be defensive) for each occurrence
where the test verifies no accesses (the assert calls around the
RegExp.prototype[Symbol.match].call(obj, ...) checks referencing the variable
accesses). Replace all instances of accesses == "" with the explicit length (or
Array.isArray + length) check so the intent is clear and not dependent on JS
coercion.
| assert(accesses == "", "unexpected call to overridden props"); | ||
| let result = RegExp.prototype[Symbol.match].call(proxy, "searchme"); | ||
| // Non-global path: just calls flags getter and exec, no lastIndex access. | ||
| assert(accesses.toString() == "flags,exec", "Proxy not able to observe some gets, got: " + accesses); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use strict equality for string comparison.
Line 109 uses loose equality (==) when comparing stringified array results. Prefer strict equality (===) for clarity and to avoid unexpected coercion.
♻️ Suggested fix
- assert(accesses.toString() == "flags,exec", "Proxy not able to observe some gets, got: " + accesses);
+ assert(accesses.toString() === "flags,exec", "Proxy not able to observe some gets, got: " + accesses);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/microbenchmarks/regexp-prototype-match-observable-side-effects.js` at
line 109, Replace the loose equality check in the assertion with a strict
equality check: in the assertion that compares accesses.toString() to
"flags,exec" (the assert call referencing the accesses variable), change the
operator from == to === so the test uses strict string comparison and avoids
coercion.
| Object.defineProperty(Object.prototype, "then", { | ||
| configurable: true, | ||
| get() { | ||
| if (phase === 1) { | ||
| phase = 2; | ||
| it.next().then(() => { log.push("h2"); }); | ||
| Promise.resolve().then(() => { log.push("m1"); }); | ||
| } | ||
| return undefined; | ||
| }, | ||
| }); | ||
|
|
||
| it.next(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider cleaning up Object.prototype modification.
The test defines a property on Object.prototype (line 17) but doesn't delete it after the test completes. While the getter only returns undefined and the test terminates after verification, best practice is to clean up global state modifications.
♻️ Recommended cleanup
drainMicrotasks();
shouldBe(log.join("|"), "h2|m1");
+delete Object.prototype.then;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/async-generator-next-on-completed-reentrant.js` around lines
17 - 30, The test modifies Object.prototype by defining a "then" accessor;
record the original descriptor (or presence) before defining it and restore it
after the test completes (after the it.next() invocation and any awaited
microtasks), or at minimum delete Object.prototype.then at the end of the test;
update the code around the getter definition for "then" and the final it.next()
call to ensure the original state is restored (use the saved originalDescriptor
or delete if none) so global prototype pollution is cleaned up.
| } | ||
| noInline(test); | ||
|
|
||
| for (let i = 0; i < testLoopCount; ++i) { |
There was a problem hiding this comment.
Undefined variable testLoopCount will crash the test.
The loop references testLoopCount, which is never defined in this file or provided by the JSC stress test harness. This will throw a ReferenceError before any test assertions run.
🔧 Proposed fix
-for (let i = 0; i < testLoopCount; ++i) {
+for (let i = 0; i < 1e4; ++i) {(Use an appropriate iteration count for DFG compilation; typical stress tests use 1e4 or higher.)
📝 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.
| for (let i = 0; i < testLoopCount; ++i) { | |
| for (let i = 0; i < 1e4; ++i) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/multi-put-by-val-mixed-dfg.js` at line 20, The for-loop uses
an undefined variable testLoopCount which causes a ReferenceError; define a
local iteration constant (e.g. const testLoopCount = 1e4 or another large number
appropriate for DFG stress) above the loop that contains for (let i = 0; i <
testLoopCount; ++i) so the loop can run—add the constant near the top of the
file or immediately before the loop to ensure the test executes and triggers DFG
compilation.
| } | ||
|
|
||
| // Run many times so the test covers all JIT tiers / repeated allocation paths. | ||
| for (let i = 0; i < testLoopCount; ++i) { |
There was a problem hiding this comment.
Undefined variable testLoopCount will cause ReferenceError.
Line 87 references testLoopCount but it's never defined in this file. The test will crash with ReferenceError: Can't find variable: testLoopCount before exercising the repeated allocation/JIT tier validation.
🐛 Proposed fix
+// Run many times so the test covers all JIT tiers / repeated allocation paths.
+const testLoopCount = 1e4;
for (let i = 0; i < testLoopCount; ++i) {📝 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.
| for (let i = 0; i < testLoopCount; ++i) { | |
| // Run many times so the test covers all JIT tiers / repeated allocation paths. | |
| const testLoopCount = 1e4; | |
| for (let i = 0; i < testLoopCount; ++i) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@JSTests/stress/promise-resolving-functions-name-and-length.js` at line 87,
The for-loop references an undefined variable testLoopCount causing a
ReferenceError; fix it by declaring and initializing testLoopCount to a sensible
numeric iteration count (e.g., const testLoopCount = <N>) before the for (let i
= 0; i < testLoopCount; ++i) loop or by replacing testLoopCount with an existing
iteration variable (if one exists) so the loop uses a defined numeric value;
ensure the constant is >0 and placed in the same scope as the loop (near the top
of the test or just above the loop) so repeated allocation/JIT validation runs
as intended.
| <!DOCTYPE html> | ||
| <style> | ||
| *:not(style):not(script) { | ||
| display: grid-lanes; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other uses of grid-lanes in the codebase and CSS specifications
echo "=== Searching for grid-lanes usage in test files ==="
rg -n "grid-lanes" --type html --type css -C 2
echo -e "\n=== Searching for grid-lanes in source code ==="
rg -n "grid-lanes" Source/ -C 2
echo -e "\n=== Checking CSS display property implementations ==="
rg -n "CSSPropertyDisplay|DisplayType" Source/WebCore/css/ -g "*.h" -g "*.cpp" | head -20Repository: oven-sh/WebKit
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== grid-lanes occurrences in Source/WebCore (limited) ==="
rg -n "grid-lanes" Source/WebCore -S --max-count 50
echo -e "\n=== grid-lanes occurrences in whole Source (limited to likely related identifiers) ==="
rg -n "GridLanes|gridLanes|grid lanes|Grid lanes" Source/WebCore -S --max-count 50
echo -e "\n=== CSS display parsing for grid-lanes (narrower search) ==="
rg -n "display\s*:\s*grid-lanes|Display.*grid-lanes|grid-lanes.*display" Source/WebCore -S --max-count 50Repository: oven-sh/WebKit
Length of output: 9607
Review comment
File: LayoutTests/fast/css-grid-layout/masonry-subgrid-excluded-legend-crash.html
Lines: 4-4
Snippet showing the final state of code at these lines
display: grid-lanes;
Comment
display: grid-lanes is a valid/implemented CSS display value in WebKit
WebKit’s CSS parser treats grid-lanes as a supported <display-inside> value (mapped to Style::DisplayType::BlockGridLanes / InlineGridLanes), so this isn’t an “invalid CSS” value. The main thing to check is that it’s feature-gated by the gridLanesEnabled setting—ensure the test runs with that flag enabled so the crash path is actually exercised.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/css-grid-layout/masonry-subgrid-excluded-legend-crash.html`
at line 4, The test uses "display: grid-lanes" which is a valid WebKit display
value when the gridLanes feature is enabled, so update the test to run with the
gridLanesEnabled feature flag on (instead of treating the value as invalid):
enable gridLanesEnabled in the test harness/setup (e.g. add the appropriate
layout-test meta or test-run option that turns on gridLanesEnabled) so the CSS
rule "display: grid-lanes" in this file is exercised under the feature gate and
the crash path is reached.
| <script src="../../resources/js-test.js"></script> | ||
| <script> | ||
| description('This tests that WebKit properly handles sub-frames with invalid URLs. It passes if it does not crash.'); | ||
| if (window.testRunner) | ||
| testRunner.waitUntilDone(); | ||
| window.jsTestIsAsync = true; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other tests using jsTestIsAsync without finishJSTest
rg -l 'jsTestIsAsync.*true' --type html LayoutTests/ | xargs rg -L 'finishJSTest' | head -20
# Check if this specific test passes
echo "---"
echo "Verify crash-with-bad-url.html test completes correctly in test runs"
rg -C3 'crash-with-bad-url.html' LayoutTests/TestExpectations || echo "Not in TestExpectations (likely passing)"Repository: oven-sh/WebKit
Length of output: 2062
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the specific test for jsTestIsAsync / finishJSTest / waitUntilDone / notifyDone
echo "=== crash-with-bad-url.html: relevant calls ==="
rg -n --hidden --no-ignore --context 2 'jsTestIsAsync|finishJSTest|waitUntilDone|notifyDone|testRunner' LayoutTests/fast/dom/crash-with-bad-url.html || true
# 2) Check whether finishJSTest exists anywhere in this test
echo "=== crash-with-bad-url.html: finishJSTest occurrences (raw) ==="
rg -n --hidden --no-ignore 'finishJSTest\(' LayoutTests/fast/dom/crash-with-bad-url.html || echo "no finishJSTest()"
# 3) Find other html tests that set jsTestIsAsync = true but do NOT contain finishJSTest()
echo "=== Other tests: jsTestIsAsync = true without finishJSTest() ==="
# Use rg -L correctly: -L prints filenames; we then verify by printing 2-3 lines around jsTestIsAsync in the candidates.
candidates=$(
rg -l 'jsTestIsAsync\s*=\s*true' --type html LayoutTests/ \
| xargs -r rg -L 'finishJSTest\(' \
)
echo "$candidates" | head -50
echo "--- count (capped to 200) ---"
echo "$candidates" | wc -l
# 4) For the first few candidates, show jsTestIsAsync and any testRunner.notifyDone usage
echo "=== First candidates: jsTestIsAsync + notifyDone context ==="
echo "$candidates" | head -10 | while read -r f; do
echo "--- $f ---"
rg -n --context 1 'jsTestIsAsync\s*=\s*true|notifyDone|finishJSTest\(' "$f" || true
doneRepository: oven-sh/WebKit
Length of output: 7022
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find and inspect the js-test.js implementation used by this test
echo "=== Locate js-test.js files ==="
fd -a "js-test.js" LayoutTests | head -20
js_test_path="$(fd -a "js-test.js" LayoutTests | head -1 || true)"
echo "=== Using js-test.js candidate: $js_test_path ==="
if [ -n "${js_test_path}" ]; then
echo "=== js-test.js: relevant parts (jsTestIsAsync/finishJSTest/notifyDone) ==="
rg -n "jsTestIsAsync|finishJSTest|notifyDone" "$js_test_path" || true
rg -n "function finishJSTest|finishJSTest\s*=" "$js_test_path" || true
rg -n "notifyDone" "$js_test_path" || true
fi
# 2) Check whether other tests use jsTestIsAsync=true + testRunner.notifyDone() without finishJSTest()
echo "=== Find tests: jsTestIsAsync=true and notifyDone but no finishJSTest() ==="
# Print filenames that contain both patterns
files_both=$(
rg -l 'jsTestIsAsync\s*=\s*true' --type html LayoutTests/ \
| xargs -r rg -l 'notifyDone' --type html \
)
# From those, exclude files containing finishJSTest(
candidates=$(
printf "%s\n" "$files_both" | xargs -r rg -L 'finishJSTest\(' --type html || true
)
echo "$candidates" | head -50
echo "--- count (capped) ---"
echo "$candidates" | wc -l
# 3) Directly inspect crash-with-bad-url.html for notifyDone vs finishJSTest
echo "=== crash-with-bad-url.html: notifyDone and finishJSTest context ==="
rg -n --context 2 'jsTestIsAsync\s*=\s*true|finishJSTest\(|notifyDone' LayoutTests/fast/dom/crash-with-bad-url.html || trueRepository: oven-sh/WebKit
Length of output: 2905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
js_test="LayoutTests/resources/js-test.js"
echo "=== js-test.js: lines ~100-180 (jsTestIsAsync gating) ==="
sed -n '100,190p' "$js_test" | nl -ba
echo "=== js-test.js: lines ~880-960 (finishJSTest + notifyDone) ==="
sed -n '880,960p' "$js_test" | nl -ba
echo "=== js-test.js: lines ~900-940 (finishJSTest function body) ==="
sed -n '900,940p' "$js_test" | nl -ba
echo "=== Find any other finish/async completion bookkeeping around jsTestIsAsync ==="
rg -n "jsTestIsAsync|finishJSTest|notifyDone|TestRunner" "$js_test" | head -80Repository: oven-sh/WebKit
Length of output: 177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
js_test="LayoutTests/resources/js-test.js"
echo "=== js-test.js: lines 100-190 ==="
sed -n '100,190p' "$js_test" | cat -n
echo "=== js-test.js: lines 880-960 ==="
sed -n '880,960p' "$js_test" | cat -n
echo "=== js-test.js: lines 900-940 (finishJSTest function body) ==="
sed -n '900,940p' "$js_test" | cat -n
echo "=== js-test.js: completion-related occurrences ==="
rg -n "jsTestIsAsync|finishJSTest|notifyDone|testRunner" "$js_test" | head -120Repository: oven-sh/WebKit
Length of output: 8537
Reassess async completion for crash-with-bad-url.html after js-test.js migration
js-test.jsdrives async completion viatestRunner.waitUntilDone()+testRunner.notifyDone()(normally fromfinishJSTest()), sofinishJSTest()is not strictly required.- This test omits
finishJSTest()but callstestRunner.notifyDone()directly from the subframe’s injectedimgonerrorhandler. - Remaining risk: that call is guarded by
if(window.testRunner)inside the subframe; iftestRunnerisn’t exposed there, the test may hang.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/dom/crash-with-bad-url.html` around lines 4 - 7, The test
currently relies on the subframe calling testRunner.notifyDone() inside the
injected image onerror handler, but that call is guarded by if
(window.testRunner) and may never run; update the test to ensure async
completion by either calling finishJSTest() from the top-level page after
creating the subframe or by making the subframe unconditionally call
testRunner.notifyDone() (or invoke a top-level finishJSTest() via postMessage
from the subframe) so the harness is always notified; locate the image onerror
handler in the subframe injection and the main test code around
finishJSTest()/testRunner.notifyDone() and implement one of these fixes.
| } | ||
| } | ||
| </style> | ||
| <script src="../../resources/js-test-pre.js"></script> |
There was a problem hiding this comment.
Incompatible test harness patterns mixed.
This test uses both the old synchronous harness (js-test-pre.js at line 31 and js-test-post.js at line 58) and the new async harness (window.jsTestIsAsync = true at line 34 and finishJSTest() at line 50). These patterns are incompatible and may cause the test to fail or produce incorrect results.
Based on the pattern used in the other migrated tests in this PR (files gesture-click.html, no-page-cache.html, autoscroll-in-iframe.html, and delete-text-in-composition.html), the correct approach is to use only the new async pattern.
🔧 Recommended fix
- <script src="../../resources/js-test-pre.js"></script>
+ <script src="../../resources/js-test.js"></script>
<script src="../../resources/ui-helper.js"></script>
<script>
window.jsTestIsAsync = true;
...
<div class="trigger"></div>
<div class="tooltip" popover="manual"></div>
<div id="console"></div>
- <script src="../../resources/js-test-post.js"></script>
</body>Also applies to: 34-34, 50-50, 58-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/dom/pseudo-element-hidePopover-then-crash.html` at line 31,
Test mixes old synchronous harness with the new async harness: remove the old
synchronous harness includes and usage and keep the async pattern. Specifically,
delete the script tags referencing js-test-pre.js and js-test-post.js (the lines
that include those files) and any synchronous harness assumptions, ensure
window.jsTestIsAsync = true remains, and keep the finishJSTest() call to
complete the test; verify no other synchronous test harness functions remain in
the file (look for js-test-pre.js, js-test-post.js, window.jsTestIsAsync,
finishJSTest).
| if (window.testRunner) { | ||
| testRunner.dumpAsText(); | ||
| testRunner.waitUntilDone(); | ||
|
|
||
| nextTest(); | ||
| } |
There was a problem hiding this comment.
Missing finishJSTest() when testRunner is unavailable.
The test conditionally starts only if window.testRunner exists (line 64). When testRunner is unavailable, no code runs and finishJSTest() is never called, causing a timeout in async mode (jsTestIsAsync = true at line 11).
🔧 Suggested fix
if (window.testRunner) {
nextTest();
+ } else {
+ debug("This test requires testRunner.");
+ finishJSTest();
}📝 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.
| if (window.testRunner) { | |
| testRunner.dumpAsText(); | |
| testRunner.waitUntilDone(); | |
| nextTest(); | |
| } | |
| if (window.testRunner) { | |
| nextTest(); | |
| } else { | |
| debug("This test requires testRunner."); | |
| finishJSTest(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html`
around lines 64 - 67, The test currently only calls nextTest() when
window.testRunner exists, leaving jsTestIsAsync = true and never calling
finishJSTest() when testRunner is absent; add an else branch that calls
finishJSTest() (or otherwise ensures finishJSTest() is invoked) when
window.testRunner is falsy so the async test completes. Locate the conditional
around window.testRunner and nextTest() and insert the finishJSTest() call in
the fallback path so the test always finishes.
| # Crashing. rdar://179168475 | ||
| imported/w3c/web-platform-tests/navigation-api/navigate-event/dangling-navigate-event-after-bfcache-restore.html [ Skip ] |
There was a problem hiding this comment.
Crash in BFCache navigation requires urgent attention.
This test is crashing during back-forward cache restoration with a dangling navigate event. BFCache-related crashes can indicate use-after-free or lifecycle issues that may affect real users during navigation. While properly tracked (rdar://179168475), ensure this is prioritized as a critical stability issue.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LayoutTests/TestExpectations` around lines 7115 - 7116, The test
imported/w3c/web-platform-tests/navigation-api/navigate-event/dangling-navigate-event-after-bfcache-restore.html
is causing a crash during BFCache restore; update the TestExpectations entry to
mark it as a Crash (not just Skip) and annotate it with the rdar://179168475
tracking ID so it shows as a prioritized stability regression, and file/link a
bug/regression ticket referencing rdar://179168475 and the test path so the
crash can be triaged and fixed; ensure the TestExpectations line for that test
is changed from "[ Skip ]" to the appropriate crash expectation and add a
one-line TODO comment with the bug ID for follow-up.
Preview Builds
|
WTF_DEPRECATED_MAKE_FAST_ALLOCATED_IMPL (and the compact variant) only defined operator new(size_t). For over-aligned classes like the alignas(64) Lexer, the class-scope operator new is selected without the align_val_t overload, so the alignment was silently dropped: makeUnique in the Parser constructor allocated the Lexer with fastMalloc (16-byte aligned), while the constructor's stores assume 64-byte alignment. On x86_64 clang emits aligned vector stores from that assumption, which fault (#GP, reported as SIGSEGV at address 0x0) when the allocation is not 64-byte aligned. Apple builds avoid this because TZONE malloc captures alignof; the fastMalloc fallback used by JSCOnly builds did not. Add operator new/new[](size_t, std::align_val_t) and the matching deletes routing through fastAlignedMalloc / fastCompactAlignedMalloc.
> [\!NOTE] > **Stacked on #31796** (base branch is `claude/webkit-upgrade-51cc3feb7298`). Retarget to `main` after #31796 merges. > > `WEBKIT_VERSION` currently points at the preview build `autobuild-preview-pr-251-10fc0cab` from [oven-sh/WebKit#251](oven-sh/WebKit#251). After that PR merges, bump it to the merge commit's `autobuild-<sha>` release before merging this PR. ## Bun-side changes - `VM::getHostFunction` gained an `unsigned length` parameter (name/length now live on the `NativeExecutable`), and `JSFunction::finishCreation(VM&, NativeExecutable*, unsigned, const String&)` was deleted upstream. Updated `JSWrappingFunction`, `JSFFIFunction`, `NapiClass`, and `JSSQLStatementConstructor` to pass length/name through `getHostFunction` and use the default `finishCreation(VM&)`. ## WebKit-fork-side changes (in the merge, oven-sh/WebKit#251) - Ported the `USE(BUN_JSC_ADDITIONS)` AsyncLocalStorage context wrapping to upstream's reworked internals: `InternalMicrotask::AsyncGeneratorResumeNext` → `AsyncGeneratorAwaitReturn`, and `Promise.prototype.finally` contexts now use `JSSlimPromiseReaction` instead of `JSPromiseCombinatorsGlobalContext`. - Cross-compile fixes: `mig` lookup falls back to `find_program` when `WebKitXcodeSDK.cmake` isn't included (Linux-hosted macOS build); the new `InlineCacheHandler::offsetOfUid() == 40` layout-drift `static_assert` is skipped on Windows, where the MSVC ABI ignores `[[no_unique_address]]` and the offset is 48. ## Verification - Full debug build against local WebKit; smoke-tested async generator `return`/`throw`, AsyncLocalStorage across `await`/`.finally()`/`for await`, `Promise.prototype.finally` semantics, sqlite, and `expect.extend`. - Test runs: `AsyncLocalStorage.test.ts`, `async-local-storage-thenable.test.ts`, `AsyncLocalStorage-tracking.test.ts` (74 pass), `ffi.test.js`, napi name tests — all green. # WebKit upgrade: `51cc3feb7298` → `24362e675175` 83 commits touching `Source/JavaScriptCore`, `Source/WTF`, and `Source/bmalloc`. ## Highlights (Bun-relevant) - **`JSType.h` is unchanged in this range** — no JSType additions or reordering, so Bun's JSType-based checks need no updates. - **NativeExecutable gains `name`/`length`, JSFunction `finishCreation` overloads deleted** (`a633a8abfee7`, [316443](https://bugs.webkit.org/show_bug.cgi?id=316443)). NativeExecutable now stores name and length the same way FunctionExecutable does, so `bind` on native functions stops hitting the slow path. Embedder-visible API changes: - `NativeExecutable::create(...)` and both `VM::getHostFunction(...)` overloads take a new `unsigned length` parameter before `name`. - `JSFunction::finishCreation(VM&, NativeExecutable*, unsigned length, const String& name)` (and the ASSERT-only `finishCreation(VM&)`) are deleted, replaced by `DECLARE_DEFAULT_FINISH_CREATION`. Subclasses that called the old overload must pass length/name through `NativeExecutable` instead. - `JSNativeStdFunction::create` / `JSFunctionWithFields::create` no longer take separate length/name arguments where the executable already carries them. New `NativeExecutable::length()` and `nameJSString(VM&)` accessors. - **Async generators rewritten to current spec** (`d096ff9cfae1`, [316447](https://bugs.webkit.org/show_bug.cgi?id=316447)). `InternalMicrotask::AsyncGeneratorResumeNext` is renamed to `AsyncGeneratorAwaitReturn` (enum in `Microtask.h`; corresponding link-time constant removed). The generator state machine replaces `AwaitingReturn` with a new `DrainingQueue` state and adds a `YieldNoAwait` suspend reason (reason bit-field widened from 1 to 2 bits). `%AsyncGeneratorPrototype%.return`/`.throw` move from JS builtins to C++ host functions. Fixes re-entrancy confusion when `Object.prototype.then` is patched. - **Promise internals cleanup** (`7b0aff184802`, [316553](https://bugs.webkit.org/show_bug.cgi?id=316553)). `JSPromiseCombinatorsGlobalContext` is no longer used as a generic cell holder: `Promise.prototype.finally` now stores its context in a `JSSlimPromiseReaction` instead. The combinator context itself now uses a `uint64_t` remaining-elements count. (Builds on `a633a8abfee7`, which already touched the same `finally` host functions.) - **GC / Heap changes**: - `e69c47917811` ([311420](https://bugs.webkit.org/show_bug.cgi?id=311420)): Heap now protects StringImpls swapped out by `JSString::swapToAtomString` while a `GCOwnedDataScope` is on the stack — `m_possiblyAccessedStringsFromConcurrentThreads` becomes a `(JSString*, String)` pair list pruned via conservative-root discovery instead of cleared wholesale. Fixes a dangling-buffer bug. - `c8e53c74403f` ([316635](https://bugs.webkit.org/show_bug.cgi?id=316635)): `Heap::clearConcurrentRetainedDataIfPossible()` no longer runs while concurrent marking is active — fixes a collector-thread use-after-free on racily-loaded StringImpls. - `441e3da20428` ([316713](https://bugs.webkit.org/show_bug.cgi?id=316713)): `deleteUnmarkedCompiledCode` now runs with an unset AtomStringTable in `Heap::runEndPhase`. - `4d73bc11dd6c` ([316385](https://bugs.webkit.org/show_bug.cgi?id=316385)): `FreeList::forEach` interval assert bounded by `MarkedBlock::blockSize`. - **Module loader fixes**: - `5c64352cd6cc` ([316615](https://bugs.webkit.org/show_bug.cgi?id=316615)): `GatherAvailableAncestors` / `AsyncModuleExecutionRejected` in `CyclicModuleRecord` made iterative — no more stack overflow on deep async module graphs (top-level-await chains). - `e46667fac721` ([316610](https://bugs.webkit.org/show_bug.cgi?id=316610)): deferred module namespace objects (`import defer`) no longer leak the synthetic `"then"` into `Object.keys`. - **WTF changes embedders may feel**: - `aae76637c06f` ([316554](https://bugs.webkit.org/show_bug.cgi?id=316554)): `URLParser`/IDNA — ASCII domains can no longer fail IDNA mapping, even when they start with `xn--`. Affects `WTF::URL` host parsing behavior. - `59604007e4c6` ([316511](https://bugs.webkit.org/show_bug.cgi?id=316511)): `clampToInteger<T>` in `MathExtras.h` now correctly clamps values below `INT_MIN`. - `3997b5c96e77` ([316692](https://bugs.webkit.org/show_bug.cgi?id=316692)): revert of an `AutomaticThread` change that introduced a race permanently inflating the active thread count (affected JIT/Wasm worklist threads). - `6667782c52fa` ([316510](https://bugs.webkit.org/show_bug.cgi?id=316510)): missing `return` statements fixed in `LazyRef.h`/`LazyUniqueRef.h`. - Removed files: `wtf/MainThreadData.h`, `wtf/StatisticsManager.{h,cpp}` (dead-code sweeps `012c64ce3ab1`, `5101cdc679ab`); JSC drops `dfg/DFGPropertyTypeKey.h` and the unused `TemporalTimeZone*` classes. - **Codebase-wide C++ modernization** that can affect Bun's C++ bindings compile: `ab23e0e34b7c` ([304023](https://bugs.webkit.org/show_bug.cgi?id=304023)) uses C++20 concepts across JSC (touches `WriteBarrier.h`, `CagedBarrierPtr.h`); `f582e488dbf4` ([316055](https://bugs.webkit.org/show_bug.cgi?id=316055)) replaces C-style arrays with `WTF::toArray()`; `66a98ce83600` ([316364](https://bugs.webkit.org/show_bug.cgi?id=316364)) guards `Platform.h` defines with `!defined()` checks. ## New language / runtime features - **`Temporal.ZonedDateTime` implemented** (`27ac373783f0`, [315939](https://bugs.webkit.org/show_bug.cgi?id=315939)) — ~7.5k lines; the largest change in the range. Follow-ups: carry non-primary time zones (`c39b3d4d67cb`, [316517](https://bugs.webkit.org/show_bug.cgi?id=316517)), spec-aligned option helpers / Duration internals and removal of the obsolete `TemporalTimeZone` classes (`063066dc87c7`, [316370](https://bugs.webkit.org/show_bug.cgi?id=316370)), `destroy` function for `TemporalZonedDateTime` (`1c8ae9884a85`, [316334](https://bugs.webkit.org/show_bug.cgi?id=316334)). - Class-field anonymous function names are now set at parse time instead of via the `SetFunctionName` bytecode (`b6a9b84dae1f`, [316646](https://bugs.webkit.org/show_bug.cgi?id=316646)). - Wasm: `Table` constructor fills funcref tables correctly when the default value is a wrapper function (`7a35a1699bc9`, [316280](https://bugs.webkit.org/show_bug.cgi?id=316280)). ## Performance - New DFG `MultiGetByVal` / `MultiPutByVal` nodes for polymorphic array access (`8f6bc9a16adf`, [315832](https://bugs.webkit.org/show_bug.cgi?id=315832)). - `RegExp.prototype[Symbol.match]` moved from JS builtin to C++ with DFG intrinsic support (`e922a2cecfac`, [316509](https://bugs.webkit.org/show_bug.cgi?id=316509)). - YARR regexp engine: auto-possession optimization (`2a8223d802c8`, [316491](https://bugs.webkit.org/show_bug.cgi?id=316491)), optimized ParenContext save/restore (`eef93d3c2048`, [316555](https://bugs.webkit.org/show_bug.cgi?id=316555)), FixedCount model changed from save-at-END to save-at-BEGIN (`a92d79b27748`, [316275](https://bugs.webkit.org/show_bug.cgi?id=316275)), `ParenthesesSubpatternFixedCount` now supports captures (`3f58e2018a6b`, [316599](https://bugs.webkit.org/show_bug.cgi?id=316599)). - Struct-layout optimizations: Parser and Lexer (`8243c6b69d66`, [316211](https://bugs.webkit.org/show_bug.cgi?id=316211)), InlineCacheHandler (`8cb7e38ecdc8`, [316163](https://bugs.webkit.org/show_bug.cgi?id=316163)); Wasm `FuncRefTable` entry size reduced (`8abf5256fdcb`, [316305](https://bugs.webkit.org/show_bug.cgi?id=316305)). - Promise combinators presize the result array from the iterable's size hint (`c6900eb69893`, [316548](https://bugs.webkit.org/show_bug.cgi?id=316548)); redundant eager `length` definition removed from `JSPromiseConstructor` (`deb8f86fbe49`, [316478](https://bugs.webkit.org/show_bug.cgi?id=316478)). - Temporal: ICU `UCalendar` cached per CalendarID (`7636f6149708`, [316569](https://bugs.webkit.org/show_bug.cgi?id=316569)). ## Fixes **Spec correctness / runtime:** - Map/Set iteration fast paths perform `IteratorClose` when the callback throws (`84a71a9868ed`, [316495](https://bugs.webkit.org/show_bug.cgi?id=316495)). - `String#split` RegExp fast path missed side effects of `ToString(this)` / `ToUint32(limit)` (`b4b15818d650`, [316508](https://bugs.webkit.org/show_bug.cgi?id=316508)). - `isDefinitelyNonThenable` Structure cache could go stale when the prototype belongs to another realm (`8d6b11214830`, [316506](https://bugs.webkit.org/show_bug.cgi?id=316506)) — affects promise resolution fast paths. - "Singleton" invalidation now propagates to the originating SymbolTable (`6da8ead481eb`, [316472](https://bugs.webkit.org/show_bug.cgi?id=316472)). - Fixed opcode assert on `Array.prototype.sort` OSR exit (`e7d51d19e065`, [316296](https://bugs.webkit.org/show_bug.cgi?id=316296)). - YARR: string-list fast path dropped a non-final empty alternative (`e6d0f57f8d04`, [316288](https://bugs.webkit.org/show_bug.cgi?id=316288)); interpreter greedy backtracking now tries up to max count (`5fe4838cb7d1`, [316378](https://bugs.webkit.org/show_bug.cgi?id=316378)). **Wasm:** - Name section parsing made thread-safe (`24362e675175`, [309538](https://bugs.webkit.org/show_bug.cgi?id=309538)). - IPInt `memory.atomic.notify`/`wait` and `memory.grow` mishandled dirty upper bits of i32 operands (`a0d2eebf9e13`, [316507](https://bugs.webkit.org/show_bug.cgi?id=316507)). - OMG tail-call patchpoint clobbers late pinned registers (`c18d1e3571f4`, [316227](https://bugs.webkit.org/show_bug.cgi?id=316227)). **Temporal / Intl hardening** (mostly crash and OOB fixes in the new Temporal code): - OOB read in `ISO8601::parseDate` on short invalid strings (`d58bad697e50`, [316366](https://bugs.webkit.org/show_bug.cgi?id=316366)); crash in `PlainMonthDay.from` with very large strings (`221dcc89aba8`, [316805](https://bugs.webkit.org/show_bug.cgi?id=316805)); double-throw crash in Temporal constructors (`19e18af9f088`, [316793](https://bugs.webkit.org/show_bug.cgi?id=316793)); `PlainDate` add/subtract day-range assertion (`2c290815d421`, [316368](https://bugs.webkit.org/show_bug.cgi?id=316368)). - Stricter ICU error handling (`178eab311235`, [316346](https://bugs.webkit.org/show_bug.cgi?id=316346)); `toIntegerWithTruncation` for Temporal conversions (`66267990831b`, [316369](https://bugs.webkit.org/show_bug.cgi?id=316369)); Japanese era fast-path validation (`31e50e893a11`, [316477](https://bugs.webkit.org/show_bug.cgi?id=316477)); date-spec invariants (`544a3bff9b31`, [316440](https://bugs.webkit.org/show_bug.cgi?id=316440)). - `Intl.DateTimeFormat` with Temporal types: era width preserved (`9cd3289437d5`, [316048](https://bugs.webkit.org/show_bug.cgi?id=316048)); calendar passed to ICU in BCP47 form (`45b638378595`, [315984](https://bugs.webkit.org/show_bug.cgi?id=315984)). **Build / misc:** - Non-unified and unified build fixes (`ee637a607df2`, [316381](https://bugs.webkit.org/show_bug.cgi?id=316381); `6c8b20e9f7b2`, [316374](https://bugs.webkit.org/show_bug.cgi?id=316374)); PlayStation SIMDUTF AVX-512 build fix (`08e30f68509b`, [316649](https://bugs.webkit.org/show_bug.cgi?id=316649)). - Build-parallelism work landed, was reverted, and re-landed (`c9e9995641cc` → `653a36adb39a` → `09f89f078e7d`, [316232](https://bugs.webkit.org/show_bug.cgi?id=316232)); assorted CMake configuration changes (Apple SDK/ICU handling, configure-time probe skipping). - Dead-code removal sweeps (`012c64ce3ab1`, [316520](https://bugs.webkit.org/show_bug.cgi?id=316520); `5101cdc679ab`, [316502](https://bugs.webkit.org/show_bug.cgi?id=316502)); libpas test harness changes (`e7665a906ab0`, [316595](https://bugs.webkit.org/show_bug.cgi?id=316595); `1a8a72a5bc1a`, [316457](https://bugs.webkit.org/show_bug.cgi?id=316457)). - Remaining commits in the range are Web Inspector / Site Isolation protocol work, CSS `calc-mix()`, MediaSession, and visionOS test changes that only incidentally touch generated inspector code under `Source/JavaScriptCore/inspector`.
Merge upstream WebKit
24362e675175into the Bun fork.Previous merge base:
51cc3feb7298(#248). 464 upstream commits, 83 touchingSource/JavaScriptCore/Source/WTF/Source/bmalloc.Conflict resolutions
runtime/JSMicrotask.cpp,runtime/JSPromisePrototype.cpp: ported theUSE(BUN_JSC_ADDITIONS)async-context (AsyncLocalStorage) wrapping to upstream's reworked promise/async-generator internals —InternalMicrotask::AsyncGeneratorResumeNext→AsyncGeneratorAwaitReturn(https://bugs.webkit.org/show_bug.cgi?id=316447) andPromise.prototype.finallycontexts now stored inJSSlimPromiseReactioninstead ofJSPromiseCombinatorsGlobalContext(https://bugs.webkit.org/show_bug.cgi?id=316553).runtime/JSFunction.cpp: took upstream's deletion of thefinishCreationoverloads (https://bugs.webkit.org/show_bug.cgi?id=316443); the fork's ASSERT reorder is obsolete.runtime/CodeCache.cpp: took upstream'srequires-clause modernization.heap/FreeListInlines.h: upstream adopted the sameMarkedBlock::blockSizeassert bound the fork already had; kept the fork's explanatory comment.heap/GCOwnedDataScope.h,runtime/GetPutInfo.h: kept the fork's quote-style includes, added upstream's new includes.Verified with a JSCOnly debug build plus a full Bun debug build and async-generator / AsyncLocalStorage /
Promise.prototype.finallytest runs.