Upgrade to upstream WebKit d81bcc3d833c - #263
Merged
Merged
Conversation
…::highlightStateForRenderer https://bugs.webkit.org/show_bug.cgi?id=318175 rdar://180977788 Reviewed by Alan Baradlay. The branch is entered only when `&renderer == m_renderRange.start()`. Since renderer is a reference, &renderer is never null, so m_renderRange.start() is guaranteed non-null inside the block. The two further m_renderRange.start() checks were therefore always true: the leading term of the Both test and the guard on the Start return. Drop both dead checks. The Start return becomes the unconditional fall-through after the Both case, and the end() null check is retained since end() can legitimately be null. No change in behavior. * Source/WebCore/rendering/RenderHighlight.cpp: (WebCore::RenderHighlight::highlightStateForRenderer): Canonical link: https://commits.webkit.org/316115@main
…OB Write https://bugs.webkit.org/show_bug.cgi?id=313360 rdar://174662781 Reviewed by Dan Glastonbury. Share identifiers for render and compute pipelines to avoid validation succeeding for a given bind group + render pipeline which was validated for the hash key identical to the same bind group but with a compute pipeline which has the same key. * Source/WebGPU/WebGPU/ComputePipeline.mm: * Source/WebGPU/WebGPU/Device.h: * Source/WebGPU/WebGPU/RenderPipeline.mm: Originally-landed-as: 305413.739@safari-7624-branch (427fc7a6307d). rdar://180429272 Canonical link: https://commits.webkit.org/316116@main
…16 via color-space early-return <https://bugs.webkit.org/show_bug.cgi?id=314590> <rdar://176483321> Reviewed by Gerald Squelart. convertImagePixelsFromFloat16ToFloat16() early-returned on a color-space mismatch without writing the destination. The destination buffer is allocated via JSC::Float16Array::tryCreateUninitialized() and ImageBufferBackend:: getPixelBuffer() only zero-fills on a size mismatch, so an RGBA16F getPixelBuffer() readback whose output color space differed from the backing IOSurface's left the destination populated with uninitialized Gigacage::Primitive heap. RemoteImageBuffer::getPixelBufferWithNewMemory() then memcpy'd those bytes into WebContent-mapped shared memory causing a WebContent<->GPUProcess heap disclosure. Float16<->Float16 color-space conversion is unimplemented (the 8-bit sibling path also doesn't perform it). Remove the early return so the destination is always populated with the source pixels (alpha-format conversion applied), matching the sibling's behavior and closing the disclosure. Test: ipc/rgba16f-getpixelbuffer-colorspace-mismatch-heap-disclosure.html * LayoutTests/ipc/rgba16f-getpixelbuffer-colorspace-mismatch-heap-disclosure-expected.txt: Added. * LayoutTests/ipc/rgba16f-getpixelbuffer-colorspace-mismatch-heap-disclosure.html: Added. * Source/WebCore/platform/graphics/PixelBufferConversion.cpp: (WebCore::convertImagePixelsFromFloat16ToFloat16): Originally-landed-as: 305413.933@safari-7624-branch (e7a0eda7b7ff). rdar://180428968 Canonical link: https://commits.webkit.org/316117@main
…an index between InlineCallFrames https://bugs.webkit.org/show_bug.cgi?id=314850 rdar://176966728 Reviewed by Yusuke Suzuki. In DFGArgumentsEliminationPhase::eliminateCandidatesThatInterfere(), the removeViaKill lambda contains two nested loops. The outer loop iterates over inline call frames. For each frame, the inner loop is expected to iterate the nodes of the given basic block in reverse order in the range [0, nodeIndex). However, because the inner loop mutates the lambda parameter nodeIndex directly, this only works as intended for the first inline call frame. Each subsequent call frame begins where the previous one left off. The change introduces a separate iteration variable for the inner loop, which starts off at nodeIndex for each inline call frame. nodeIndex parameter is marked as 'const' to make explicit the expectation that it should not change. Test: JSTests/stress/arguments-elimination-multiple-inline-call-frames.js Originally-landed-as: 305413.912@safari-7624-branch (3270ebdb7366). rdar://180435783 Canonical link: https://commits.webkit.org/316118@main
…in should report the sizes of the stripped binaries and libs. https://bugs.webkit.org/show_bug.cgi?id=318205 Reviewed by Nikolas Zimmermann. Ensure to strip the binary and libraries before reporting the size. That is done in a temporal file to not affect the original ones because the user may want to keep those with debug symbols to use gdb or similar. * Tools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.py: (get_stripped_object_size): (get_basenames_and_sizes): (get_browser_relevant_objects_glib): Canonical link: https://commits.webkit.org/316119@main
…ser activation https://bugs.webkit.org/show_bug.cgi?id=313478 rdar://174964081 Reviewed by Matthew Finkel. The completion handler for requestStorageAccess() preserved the user gesture whenever the prompt was not shown, including on the no-gesture fast-reject path. This synthesized a UserGestureIndicator from nothing, allowing the .catch handler to call gesture-gated APIs from a cross-site iframe with zero user interaction. Gate gesture preservation on a real gesture having existed at call time. Test: http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate.html * LayoutTests/http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate-expected.txt: Added. * LayoutTests/http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate.html: Added. * LayoutTests/http/tests/storageAccess/resources/request-storage-access-without-gesture-check-activation-iframe.html: Added. * Source/WebCore/dom/DocumentStorageAccess.cpp: (WebCore::DocumentStorageAccess::requestStorageAccess): (WebCore::DocumentStorageAccess::requestStorageAccessQuirk): * Source/WebCore/dom/DocumentStorageAccess.h: * Source/WebCore/page/ChromeClient.h: (WebCore::ChromeClient::requestStorageAccess): * Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp: (WebKit::WebResourceLoadStatisticsStore::requestStorageAccess): * Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h: * Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp: (WebKit::NetworkConnectionToWebProcess::requestStorageAccess): * Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h: * Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in: * Source/WebKit/Scripts/webkit/messages.py: (headers_for_type): * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in: * Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp: (WebKit::WebChromeClient::requestStorageAccess): * Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h: * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::requestStorageAccess): * Source/WebKit/WebProcess/WebPage/WebPage.h: Originally-landed-as: 305413.854@safari-7624-branch (6fae18f756d2). rdar://180436613 Canonical link: https://commits.webkit.org/316120@main
rdar://174740124 https://bugs.webkit.org/show_bug.cgi?id=313917 Reviewed by Simon Fraser. * Source/WebCore/platform/mock/mediasource/MockBox.cpp: (WebCore::MockInitializationBox::MockInitializationBox): (WebCore::MockSampleBox::MockSampleBox): Originally-landed-as: 305413.813@safari-7624-branch (4f7a3cfefe44). rdar://180437504 Canonical link: https://commits.webkit.org/316121@main
…th()/setBorderBoxHeight() https://bugs.webkit.org/show_bug.cgi?id=318039 <rdar://problem/180975454> Reviewed by Antti Koivisto. Follow-up to the width()/height() -> borderBoxWidth()/borderBoxHeight() rename. setWidth()/setHeight() set the border box width/height, but the bare names didn't say so -- the unqualified odd ones out next to setBorderBoxSize() and the borderBox* getters. Name them for what they are. Mechanical rename across WebCore; no behavior change. * Source/WebCore/layout/integration/flex/LayoutIntegrationFlexLayout.cpp: (FlexLayout::layout): (FlexLayout::updateRenderers): * Source/WebCore/layout/integration/grid/LayoutIntegrationGridLayout.cpp: (GridLayout::updateGridItemRenderers): (GridLayout::updateFormattingContextRootRenderer): * Source/WebCore/rendering/RenderBox.h: * Source/WebCore/rendering/RenderBoxInlines.h: (RenderBox::setLogicalHeight): (RenderBox::setLogicalWidth): * Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp: (RenderDeprecatedFlexibleBox::layoutBlock): (RenderDeprecatedFlexibleBox::layoutHorizontalBox): (RenderDeprecatedFlexibleBox::layoutSingleClampedFlexItem): (RenderDeprecatedFlexibleBox::layoutVerticalBox): * Source/WebCore/rendering/RenderFrameSet.cpp: (RenderFrameSet::layout): (resetFrameRendererAndDescendants): (RenderFrameSet::positionFrames): * Source/WebCore/rendering/RenderListMarker.cpp: (RenderListMarker::layout): * Source/WebCore/rendering/RenderReplaced.cpp: (RenderReplaced::layout): * Source/WebCore/rendering/RenderScrollbarPart.cpp: (RenderScrollbarPart::layoutHorizontalPart): (RenderScrollbarPart::layoutVerticalPart): (RenderScrollbarPart::computeScrollbarWidth): (RenderScrollbarPart::computeScrollbarHeight): (RenderScrollbarPart::paintIntoRect): * Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp: (RenderSVGForeignObject::updateLogicalWidth): Canonical link: https://commits.webkit.org/316122@main
…erBoxRect() https://bugs.webkit.org/show_bug.cgi?id=318020 Reviewed by Antti Koivisto. These call sites only read the size of borderBoxRect(); none of them use its location. borderBoxRect() is { 0, 0, borderBoxSize() }, so constructing the positioned rect just to call .size()/.width()/.height()/.isEmpty() is wasteful and hides the intent. Use the direct accessor (borderBoxSize(), borderBoxWidth(), borderBoxHeight()) instead. No behavior change. * Source/WebCore/rendering/RenderBox.cpp: (RenderBox::reflectionOffset): * Source/WebCore/rendering/svg/RenderSVGRoot.cpp: (RenderSVGRoot::paint): (RenderSVGRoot::boundingRects): * Source/WebCore/style/values/transforms/StyleTransformList.cpp: (Blending<TransformList>::blend): Canonical link: https://commits.webkit.org/316123@main
https://bugs.webkit.org/show_bug.cgi?id=314295 rdar://176438133 Reviewed by Ryosuke Niwa. Stop capturing `host` and `protocol` by reference in the lambda since the lambda can get called asynchronously. * Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm: (WebKit::WebsiteDataStore::beginAppBoundDomainCheck): Originally-landed-as: 305413.851@safari-7624-branch (3602ee8745a7). rdar://180438476 Canonical link: https://commits.webkit.org/316124@main
…extGL::readPixelsInline https://bugs.webkit.org/show_bug.cgi?id=312564 rdar://174640403 Reviewed by Dan Glastonbury. Allocate the read pixels area with zero init. * Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp: (WebKit::RemoteGraphicsContextGL::readPixelsInline): Originally-landed-as: 305413.708@safari-7624-branch (0c204da15932). rdar://180438472 Canonical link: https://commits.webkit.org/316125@main
https://bugs.webkit.org/show_bug.cgi?id=314906 rdar://176813834 Reviewed by Darin Adler. FilterImage provides three different representations of the image: ImageBuffer, Unpremultiplied PixelBuffer and Premultiplied PixelBuffer. These three buffers are lazily created. Each of them is created only when it is needed. But keep in mind they are copies of each other. In other words, when one is created it has to copy the pixels from the existing buffers. Otherwise it has to be zero-filled. A problem may happen when copying the pixels from one buffer to a newly created buffer fails. In this case we send uninitialized PixelBuffer which may expose user private data. Not able to copy existing pixels to the PixelBuffer should be treated as an error. So a null PixelBuffer should be returned in this case. * Source/WebCore/platform/graphics/cpu/arm/filters/FEBlendNeonApplier.cpp: (WebCore::FEBlendNeonApplier::apply const): * Source/WebCore/platform/graphics/filters/FilterImage.cpp: (WebCore::copyImageBytes): (WebCore::FilterImage::pixelBuffer): (WebCore::FilterImage::getPixelBuffer): (WebCore::FilterImage::copyPixelBuffer): * Source/WebCore/platform/graphics/filters/FilterImage.h: Originally-landed-as: 305413.919@safari-7624-branch (dbf10417aa9e). rdar://180436188 Canonical link: https://commits.webkit.org/316126@main
https://bugs.webkit.org/show_bug.cgi?id=312415 rdar://174714198 Reviewed by Yijia Huang. In YarrJIT negativeOffsetIndexedAddress computes a negative offset from a base address but the function mistakenly uses the original value instead of the adjusted value. This patch fixes it to use the adjusted value. Test: JSTests/stress/yarr-negative-offset.js * JSTests/stress/yarr-negative-offset.js: Added. (catch): * Source/JavaScriptCore/yarr/YarrJIT.cpp: Originally-landed-as: 305413.685@safari-7624-branch (30c8460241f3). rdar://180436598 Canonical link: https://commits.webkit.org/316127@main
…sStructure in DFG https://bugs.webkit.org/show_bug.cgi?id=313252 rdar://175498631 Reviewed by Yijia Huang. This patch fixes the abstract interpreter's structure prediction for the spread operator. Instead of only cellButterflyStructure(CopyOnWriteArrayWithContiguous), it now also takes cellButterflyOnlyAtomStringsStructure into account. Test: JSTests/stress/spread-with-OnlyAtomStringsStructure.js * JSTests/stress/spread-with-OnlyAtomStringsStructure.js: Added. (index): * Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): Originally-landed-as: 305413.768@safari-7624-branch (62a1052dd1fe). rdar://180435430 Canonical link: https://commits.webkit.org/316128@main
…rse iterator when a Prev/Prevunique cursor's next-higher index entry is deleted <https://bugs.webkit.org/show_bug.cgi?id=313818> <rdar://174678764> Reviewed by Sihui Liu. MemoryIndexCursor caches an IndexValueStore::Iterator across IPC messages. For Prev/Prevunique cursors this wraps std::set<IDBKeyData>::reverse_iterator objects (one for the outer IndexValueStore::m_orderedKeys set, one nested in IndexValueEntry::Iterator for the inner per-index-key set). A std::reverse_iterator stores a forward base() iterator pointing one element past the logical position; for libc++ std::set that is a raw __tree_node*. MemoryIndexCursor::indexValueChanged() only invalidates m_currentIterator when the changed (key, primaryKey) equals the cursor's current logical position. Deleting the next-higher index key (Variant A, outer set) or the next-higher primary key under a shared index key (Variant B, inner set) therefore frees exactly the __tree_node base() references while the guard early-returns. The next IterateCursor IPC executes ++m_reverseIterator => --base() => __tree_prev_iter(freed_node), a heap-use-after-free in com.apple.WebKit.Networking reachable from a compromised WebContent process via NetworkStorageManager IPC with WCP-controlled IDBDatabaseIdentifier.m_isTransient = true forcing MemoryIDBBackingStore. Fix by gating the equality early-return on info().isDirectionForward(). For reverse cursors we now invalidate m_currentIterator on any index mutation; iterate() already re-seeks via reverseFind(m_currentKey, m_currentPrimaryKey) when the iterator is invalid. Both removeEntriesWithValueKey() erase paths (outer m_orderedKeys.erase() and inner IndexValueEntry::removeKey()) reach indexValueChanged() through MemoryIndex::notifyCursorsOfValueChange(), so both variants are closed. * LayoutTests/storage/indexeddb/index-cursor-reverse-delete-next-higher-key-private-expected.txt: Added. * LayoutTests/storage/indexeddb/index-cursor-reverse-delete-next-higher-key-private.html: Added. * Source/WebCore/Modules/indexeddb/server/MemoryIndexCursor.cpp: (WebCore::IDBServer::MemoryIndexCursor::indexValueChanged): Originally-landed-as: 305413.842@safari-7624-branch (d14d12e915f3). rdar://180436983 Canonical link: https://commits.webkit.org/316129@main
…iations https://bugs.webkit.org/show_bug.cgi?id=312470 rdar://174919762 Reviewed by Dan Glastonbury. The implementation of DrawElements would be inconsistent: The implementation has multiple stages of possible intermediate buffers. At various stages, offsets and counts were mixed between incorrect stages. Historically the confusions were partly due to the logic loading the primitive ranges sometimes from generate intermediary buffers, sometimes from the original client-provided buffer. This was partly fixed to load only from client-provided buffers and client arrays in "WebGL: consecutive UNSIGNED_BYTE drawElements() draw in wrong location", but this caused a regression by other part of the code using counts from incorrect stages. The implementation strategy would not working for non-strip topologies, such as GL_TRIANGLES, in primitive restart cases when provoking vertex rewrite was needed. The provoking vertex rewrite operates per primitive. For strip topologies, this works as each vertex index also defines a primitive. For non-strip topologies, this is not possible as the provoking vertex generation shader cannot know where the primitive starts. Example: GL_TRIANGLES of {0, ff, 1, 2, 3, 4} would be dispatched as primitives starting at 0 and 2, but the proper dispatch list would contain just primitive at 1. Furthermore, the logic to determine drawn index ranges was based on scanning for primitive restart ranges, and then converting those to draw index ranges. This algorithm was brittle and contained quite many confusions. Fix by: - Resolve the draw index ranges by resolving the draw index ranges instead of restart ranges. These index ranges are then intersected with the client provided offset and count, which define the first index, last index range pair. - Consistently resolve the draw index ranges ƒrom the client buffers. - Keep all intermediate buffers consistent with the first index, count: the intermediate buffers always have the similar unused prefix than the original client-provided index buffer. - Keep all the intermediate buffers offsets separated from the client-provided offset (first index). The intermediate buffers are allocated from the buffer pool, so they're not separate buffers but buffer, offset pairs. Keep these buffer, offset pairs separate from the client first index - For non-strip topology draws, provide the draw ranges already to the provoking vertex shader helper. The provoking vertex shader is only run for the real draw indices instead of the full draw. This allows the shader to function correctly. * Source/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxproj: * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.h: (rx::DrawIndexRange::DrawIndexRange): (rx::BufferMtl::DrawIndexRangeCache::DrawIndexRangeCache): (rx::IndexRange::IndexRange): Deleted. (rx::BufferMtl::RestartRangeCache::RestartRangeCache): Deleted. * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm: (rx::BufferMtl::markConversionBuffersDirty): (rx::BufferMtl::clearConversionBuffers): (rx::CalculateDrawIndexRanges): (rx::BufferMtl::getDrawIndexRanges): (rx::BufferMtl::GetDrawIndexRangesFromClientData): (rx::IndexConversionBufferMtl::getRangeForConvertedBuffer): Deleted. (rx::CalculateRestartRanges): Deleted. (rx::BufferMtl::getRestartIndices): Deleted. (rx::BufferMtl::GetRestartIndicesFromClientData): Deleted. * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ContextMtl.h: (rx::ContextMtl::getProvokingVertexHelper): * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ContextMtl.mm: (rx::ContextMtl::drawElementsImpl): * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProvokingVertexHelper.h: * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProvokingVertexHelper.mm: (rx::ProvokingVertexHelper::preconditionIndexBuffer): (rx::ProvokingVertexHelper::generateIndexBuffer): * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/VertexArrayMtl.h: * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/VertexArrayMtl.mm: (rx::AppendDrawCommands): (rx::AppendDrawCommandRanges): (rx::VertexArrayMtl::resolveDrawElementsDraw): (rx::VertexArrayMtl::getIndexBuffer): Deleted. (rx::VertexArrayMtl::getDrawIndices): Deleted. * Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/VertexArrayMtl_unittest.mm: Added. * Source/ThirdParty/ANGLE/src/tests/gl_tests/DrawElementsTest.cpp: (angle::convertIndexBufferContents): Originally-landed-as: 305413.713@safari-7624-branch (23eba5e02916). rdar://180437073 Canonical link: https://commits.webkit.org/316130@main
…assEncoder::clampIndexBufferToValidValues https://bugs.webkit.org/show_bug.cgi?id=315196 rdar://176882934 Reviewed by Mike Wyrzykowski. RenderPassEncoder::clampIndexBufferToValidValues guards baseVertex magnitude with -baseVertex > static_cast<int64_t>(minVertexCount), but the unary negation runs in 32-bit signed arithmetic before the comparison widens, so baseVertex == INT32_MIN wraps to itself and passes the guard. The fix widens baseVertex to int64_t before negation and sign extension preserves all valid negative baseVertex values, so the guard's behaviour is unchanged for every input except INT32_MIN, whose magnitude (2^31) is now representable and correctly rejected. Test: fast/webgpu/nocrash/fuzz-176882934.html * LayoutTests/fast/webgpu/nocrash/fuzz-176882934-expected.txt: Added. * LayoutTests/fast/webgpu/nocrash/fuzz-176882934.html: Added. * Source/WebGPU/WebGPU/RenderPassEncoder.mm: (WebGPU::RenderPassEncoder::clampIndexBufferToValidValues): Originally-landed-as: 305413.952@safari-7624-branch (04553772a362). rdar://180436823 Canonical link: https://commits.webkit.org/316131@main
… callsites https://bugs.webkit.org/show_bug.cgi?id=313063 rdar://174492346 Reviewed by Keith Miller. After a re-tier, when a fresh BBQCallee replaces a retired one, m_osrEntryCallees may hold a stale weak ref to an OMGOSREntryCallee not owned by the current BBQCallee. Currently, updateCallsitesToCallUs will not track this since it assumes that this OMGOSREntryCallee will be owned by the BBQCallee This patch fixes it by unconditionally keeping the OMGOSREntryCallee alive while we update all the callsites within it. The assert in ~BBQCallee is void now since an OMGOSREntryCallee can now have multiple owners. * Source/JavaScriptCore/wasm/WasmCallee.cpp: (JSC::Wasm::BBQCallee::~BBQCallee): * Source/JavaScriptCore/wasm/WasmCalleeGroup.cpp: (JSC::Wasm::CalleeGroup::updateCallsitesToCallUs): Originally-landed-as: 305413.784@safari-7624-branch (84a5f91f00a5). rdar://180437899 Canonical link: https://commits.webkit.org/316132@main
https://bugs.webkit.org/show_bug.cgi?id=312391 rdar://173799670 Reviewed by Sihui Liu. When an IndexedDB connection is closed while a version change transaction is active, UniqueIDBDatabase::connectionClosedFromClient successfully closes the version change transaction however it does not clear any other pending transactions due to an early return when clearing version change transactions. Instead of being defensive and clearing the pending transaciton in the case of the early return, we should reject any incoming transactions which come on a version change connection. Only whenever a version change operation completes, can a new transaction be established. So, this patch rejects any incoming transactions which come while a version change operation is active. The IndexedDB spec describes the steps for starting a new transaction: The transaction(storeNames, mode, options) method steps are: 1. If a live upgrade transaction is associated with the connection, throw an "InvalidStateError" DOMException. https://w3c.github.io/IndexedDB/#dom-idbdatabase-transaction This check already happens in the web process during IDBDatabase::transaction where it returns an "InvalidStateError" in this state. However, this doesn't account for a compromised web process who avoids this client side check and makes it across IPC to invoke transaction creation in the NetworkProcess. This patch adds a check for this scenario of a comprimised web process who manages to start a new transaction during a version change, if this is detected, we return early and don't move forward with creating the transaction This patch checks that the current connection isn't the stored UniqueIDBDatabase::m_versionChangeDatabaseConnection. That member variable will point to the active version change connection, if any, and it is cleared when the version change completes. It also checks if m_versionChangeTransaction is non-null since it will only be null once the version change transaction is completed. * LayoutTests/ipc/networksindexeddb-close-connection-during-version-change-expected.txt: Added. * LayoutTests/ipc/networksindexeddb-close-connection-during-version-change.html: Added. * Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp: (WebCore::IDBServer::UniqueIDBDatabase::isVersionChangeTransactionActive const): * Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h: * Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp: (WebKit::NetworkStorageManager::establishTransaction): (WebKit::NetworkStorageManager::databaseConnectionPendingClose): Originally-landed-as: 305413.734@safari-7624-branch (6b2393e40648). rdar://180437837 Canonical link: https://commits.webkit.org/316133@main
…BindGroupCompatibility allows incompatible auto/explicit layout pairing https://bugs.webkit.org/show_bug.cgi?id=315496 rdar://176812014 Reviewed by Mike Wyrzykowski. We were using weak pointers to compare the autogenerated pipeline layouts when checking for compatibility of two bind groups, however the weak pointer can be null in two scenarios: when the layout is explicit or when the pipeline layout has been destroyed. That could result in incorrectly considering two bind groups compatible when one is pipeline layout is explicit and the other has been destroyed. In order to avoid that we explicitly check whether both bind groups use auto generated layouts. Test: fast/webgpu/regression/repro_176812014.html * LayoutTests/fast/webgpu/regression/repro_176812014-expected.txt: Added. * LayoutTests/fast/webgpu/regression/repro_176812014.html: Added. * Source/WebGPU/WebGPU/BindGroupLayout.mm: (WebGPU::BindGroupLayout::errorValidatingBindGroupCompatibility const): Originally-landed-as: 305413.964@safari-7624-branch (48e38ee5acbc). rdar://180428905 Canonical link: https://commits.webkit.org/316134@main
…marker / close() rdar://175520011 Reviewed by Jean-Yves Avenard WebCodecsAudioData is annotated with ReportExtraMemoryCost, so the generated JS wrapper's visitChildren calls WebCodecsAudioData::memoryCost() from a concurrent GC marker thread. memoryCost() dereferenced m_data.audioData (a RefPtr<PlatformRawAudioData>) without synchronization while close() on the main thread assigns m_data.audioData = nullptr and frees the PlatformRawAudioData, leading to a heap-use-after-free in PlatformRawAudioDataCocoa::memoryCost(). Cache the memory cost in a std::atomic<size_t> on WebCodecsAudioData, computed once at construction time on the main thread and zeroed in close(), so the GC-thread memoryCost() never touches the RefPtr. This matches the existing pattern in ImageBitmap. * LayoutTests/fast/webcodecs/audio-data-close-during-gc-crash-expected.txt: Added. * LayoutTests/fast/webcodecs/audio-data-close-during-gc-crash.html: Added. * Source/WebCore/Modules/webcodecs/WebCodecsAudioData.cpp: (WebCore::WebCodecsAudioData::WebCodecsAudioData): (WebCore::WebCodecsAudioData::close): * Source/WebCore/Modules/webcodecs/WebCodecsAudioData.h: (WebCore::WebCodecsAudioData::memoryCost const): Originally-landed-as: 305413.845@safari-7624-branch (2102c1dad1d3). rdar://180438342 Canonical link: https://commits.webkit.org/316135@main
…Set from HashMap mutation during keys() iteration https://bugs.webkit.org/show_bug.cgi?id=315538 rdar://177375129 Reviewed by Timothy Hatcher. When WebExtensionStorageSQLiteStore::setKeyedData hits an INSERT error mid-batch (e.g. SQLITE_FULL), it returns a keysSuccessfullySet vector that is non-empty but smaller than the input map. The completion lambda in storageSet then iterated `data.keys()` while calling `data.remove()` inside the loop. `keys()` returns a live iterator range backed by raw pointers into the HashTable buffer, and remove() can shrink/rehash and free that buffer, leaving the iterator dangling — a heap UAF in the UI Process driven by IPC from the WebContent process. Replace the iterate-and-mutate loop with HashMap::removeIf, which walks the table safely. * Source/WebKit/UIProcess/Extensions/API/WebExtensionContextAPIStorage.cpp: (WebKit::WebExtensionContext::storageSet): Originally-landed-as: 305413.961@safari-7624-branch (de9fb008197a). rdar://180427449 Canonical link: https://commits.webkit.org/316136@main
…resize/transfer in mapFn callback. https://bugs.webkit.org/show_bug.cgi?id=312513 rdar://174428778 Reviewed by Yusuke Suzuki. Adjusts TypedArray.from to properly handle cases where the map function changes the bounds of the arraylike input. Test: JSTests/stress/typedarray-from-oob.js * JSTests/stress/typedarray-from-oob.js: Added. (testResize): (testDetach): * Source/JavaScriptCore/builtins/TypedArrayConstructor.js: (from): Originally-landed-as: 305413.702@safari-7624-branch (e99a325bb9b8). rdar://180429231 Canonical link: https://commits.webkit.org/316137@main
https://bugs.webkit.org/show_bug.cgi?id=318132 Reviewed by Nikolas Zimmermann. Fix animateMotion-spline-invalid-keyTimes.html by keeping the animation invalid when keyTimes list does not end with 1 for the spline calcMode [1]. [1] https://svgwg.org/specs/animations/#KeyTimesAttribute * LayoutTests/imported/w3c/web-platform-tests/svg/animations/animateMotion-spline-invalid-keyTimes-expected.txt: * Source/WebCore/svg/SVGAnimationElement.cpp: (WebCore::SVGAnimationElement::startedActiveInterval): Canonical link: https://commits.webkit.org/316138@main
https://bugs.webkit.org/show_bug.cgi?id=314747 rdar://176792407 Reviewed by Keith Miller. The DFG barrier insertion phase works by tracking an "epoch" serial number, which is bumped every time it encounters a node that can GC. The assumption is that each DFG node either does a store, which would need to be considered for barrier insertion, or performs a GC. MultiPutByOffset is a "fat" node that can GC and perform a store after that GC, in sequence. The current analysis therefore incorrectly elides the barrier for it. This PR fixes by special casing. Test: JSTests/stress/multi-put-by-offset-reallocating-storage-needs-write-barrier.js * JSTests/stress/multi-put-by-offset-reallocating-storage-needs-write-barrier.js: Added. (foo): * Source/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cpp: Originally-landed-as: 305413.909@safari-7624-branch (2b4933eb1658). rdar://180427769 Canonical link: https://commits.webkit.org/316139@main
…le due to missing import of defining module 'WebCore_Private' [#MemberImportVisibility] https://bugs.webkit.org/show_bug.cgi?id=318219 rdar://181020438 Unreviewed build fix. * Source/WebKit/UIProcess/mac/WKAppKitGestureController.swift: Canonical link: https://commits.webkit.org/316140@main
https://bugs.webkit.org/show_bug.cgi?id=318220 Unreviewed gardening. Forgot to reset LBSE specific results after 315674@main, fix that. * LayoutTests/platform/ios/svg/compositing/anonymous-RenderSVGViewportContainer-no-repaints-expected.txt: * LayoutTests/platform/ios/svg/compositing/segment-removed-after-anchor-decomposited-layer-tree-expected.txt: Added. * LayoutTests/platform/ios/svg/compositing/transform-change-repainting-viewBox-repaintRects-expected.txt: Added. * LayoutTests/platform/ios/svg/transforms/nested-svg-transform-attribute-creates-layer-expected.txt: Added. Canonical link: https://commits.webkit.org/316141@main
https://bugs.webkit.org/show_bug.cgi?id=318180 rdar://180990441 Reviewed by Tadeu Zagallo. This patch adds operationGetByValArrayStorageInt, which is tailored for ArrayStorage slow path operation. DFG / FTL can use this when we already speculate ArrayStorage, but it is a slow path. ToT Patched get-by-val-array-storage-sparse-hole 122.9244+-1.1533 ^ 62.5947+-9.6089 ^ definitely 1.9638x faster get-by-val-array-storage-sparse 117.8623+-0.4612 ^ 84.7042+-0.5769 ^ definitely 1.3915x faster Tests: JSTests/microbenchmarks/get-by-val-array-storage-sparse-hole.js JSTests/microbenchmarks/get-by-val-array-storage-sparse.js * JSTests/microbenchmarks/get-by-val-array-storage-sparse-hole.js: Added. (get array): * JSTests/microbenchmarks/get-by-val-array-storage-sparse.js: Added. (get array): * Source/JavaScriptCore/dfg/DFGOperations.cpp: (JSC::DFG::getByValArrayStorageInt): (JSC::DFG::JSC_DEFINE_JIT_OPERATION): * Source/JavaScriptCore/dfg/DFGOperations.h: * Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: (JSC::DFG::SpeculativeJIT::compileGetByVal): * Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp: (JSC::DFG::SpeculativeJIT::compileGetByVal): * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::compileGetByValImpl): Canonical link: https://commits.webkit.org/316142@main
…of the process-global map for better message checking rdar://174702519 Reviewed by Ben Nham. When considering whether a given web process should have access to a given back/forward entry, the global map is the wrong tool. Check on a per-process basis instead. Test: Tools/TestWebKitAPI/Tests/WebKit/WKBackForwardListTests.mm * Source/WebKit/UIProcess/WebBackForwardList.cpp: (WebKit::messageCheckItemURLs): (WebKit::WebBackForwardList::backForwardAddItemShared): (WebKit::WebBackForwardList::backForwardSetChildItem): (WebKit::WebBackForwardList::backForwardUpdateItem): * Source/WebKit/UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::checkURLReceivedFromWebProcess): * Tools/TestWebKitAPI/Tests/WebKit/WKBackForwardListTests.mm: (<script>): (ForgedFileURLItemIsRejected)): Originally-landed-as: 305413.831@safari-7624-branch (249edf86a9f5). rdar://180428225 Canonical link: https://commits.webkit.org/316143@main
https://bugs.webkit.org/show_bug.cgi?id=266832 rdar://120284006 Reviewed by Elika Etemad. SVG text on a path (and any per-character-positioned text) was laid out one Unicode code point at a time: SVGTextLayoutEngine opened a fresh single-code-point fragment for every code point whenever a per-character condition fired (text on a path, an explicit rotate, vertical text, …), and SVGTextBoxPainter then shaped each fragment's substring in isolation. A lone combining mark shaped on its own (e.g. U+0301, or the Devanagari virama U+094D) makes the font emit a dotted circle. SVG 2 section 11 addresses the text-positioning attributes (x/y/dx/dy/rotate) per Unicode code point, but it also defines a "typographic character" a UAX #29 grapheme cluster such as a base letter plus its combining marks, or a Devanagari syllable as an indivisible unit whose internal glyph arrangement is "not user controllable". So code points are addressed, but typographic characters are positioned and shaped. WebKit placed code points. Compute grapheme-cluster boundaries over the renderer text with an ICU character-break iterator, keyed by the same code-unit offset the layout loop walks, and only let a cluster-start code point open a new fragment. A continuation code point (a combining mark, a Devanagari continuation, the second regional indicator of a flag) now joins the current fragment, so the painter hands base + marks to the shaper together. This is font-independent, so it groups base + mark even when the font has no composed glyph -- precisely the dotted-circle case. Per-code-point metrics, character-data lookups, and the getNumberOfChars / query contract are unchanged; the fragment simply spans multiple metrics entries, which recordTextFragment already sums. Matches Gecko and Blink, which both treat the typographic character as the positioning unit. For pure ASCII / precomposed Latin-1 text every code point is a grapheme boundary, so behavior is unchanged there. Tests: imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks-ref.html imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks.html * LayoutTests/imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks.html: Added. * Source/WebCore/rendering/svg/SVGTextLayoutEngine.cpp: (WebCore::SVGTextLayoutEngine::layoutTextOnLineOrPath): Canonical link: https://commits.webkit.org/316144@main
…ction rdar://174708287 Reviewed by Per Arne Vollan (OOPS\!). establishSharedWorkerContextConnection accepts a WebContent-supplied Site with no MESSAGE_CHECK, allowing a compromised web process to hijack SharedWorker context connections for arbitrary domains. This mirrors the fix applied to the ServiceWorker equivalent (establishSWContextConnection) in rdar://107063897. Two changes: 1. Add allowsFirstPartyForCookies validation with MESSAGE_CHECK_COMPLETION before creating the context connection, matching the ServiceWorker pattern. 2. In WebSharedWorkerServer::addContextConnection, skip contextConnectionCreated when the domain already has a registered connection (replace debug-only ASSERT with runtime guard). No new tests. Covered by existing SharedWorker tests. * Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp: (WebKit::NetworkConnectionToWebProcess::establishSharedWorkerContextConnection): * Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorkerServer.cpp: (WebKit::WebSharedWorkerServer::addContextConnection): Originally-landed-as: 305413.712@safari-7624-branch (701d9d99f21d). rdar://180428538 Canonical link: https://commits.webkit.org/316191@main
https://bugs.webkit.org/show_bug.cgi?id=318260 rdar://181055085 Reviewed by Tim Nguyen. * LayoutTests/imported/w3c/web-platform-tests/css/css-mixins/at-function-cssom-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/cssom/variable-names-expected.txt: * Source/WebCore/css/StyleProperties.cpp: (WebCore::StyleProperties::asTextInternal const): Use serializeIdentifier. Canonical link: https://commits.webkit.org/316192@main
…vigation early-return reaches Cache::storeRedirect with unrestored cachePartition https://bugs.webkit.org/show_bug.cgi?id=314862 rdar://176914483 Reviewed by Alex Christensen. A compromised WebContent process can poison the Network Process persistent disk cache for an arbitrary cross-origin partition. NetworkConnectionToWebProcess::useRedirectionForCurrentNavigation accepts a WebContent-supplied ResourceResponse with no validation and stashes it on the loader as m_redirectionForCurrentNavigation. When the loader's WillSendRequest async reply later delivers a WebContent-supplied ResourceRequest, NetworkResourceLoader::continueWillSendRequest takes the m_redirectionForCurrentNavigation early-return into willSendRedirectedRequest *before* the setCachePartition(originalRequest().cachePartition()) restore, so NetworkCache::Cache::storeRedirect keys the on-disk record by the unvalidated WebContent-chosen {cachePartition, url} and persists the WebContent-supplied 301 with WebContent-chosen Location:. Fix this by: 1. Hoisting the setCachePolicy/setCachePartition restore to the top of continueWillSendRequest so every path through the function — including the m_redirectionForCurrentNavigation, service-worker and m_shouldRestartLoad early returns — uses the original (Network-Process-side) cache partition, never the value round-tripped through the WebContent process. 2. Promoting the debug ASSERT(isMainFrameLoad()) and ASSERT(response.isRedirection()) at the NetworkConnectionToWebProcess::useRedirectionForCurrentNavigation IPC entry point to MESSAGE_CHECKs. The legitimate sender (WebPage::useRedirectionForCurrentNavigation, driven by RedirectSOAuthorizationSession in the UI process) only ever targets the main-frame main-resource loader with a redirection response; anything else is a compromised WebContent process. Test: ipc/use-redirection-for-current-navigation-message-check.html * LayoutTests/ipc/use-redirection-for-current-navigation-message-check-expected.txt: Added. * LayoutTests/ipc/use-redirection-for-current-navigation-message-check.html: Added. * Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp: (WebKit::NetworkConnectionToWebProcess::useRedirectionForCurrentNavigation): * Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp: (WebKit::NetworkResourceLoader::continueWillSendRequest): Originally-landed-as: 305413.915@safari-7624-branch (a547e707161f). rdar://180438355 Canonical link: https://commits.webkit.org/316193@main
https://bugs.webkit.org/show_bug.cgi?id=318239 rdar://181044227 Unreviewed CI fix. The webkit-swift-interface patch no longer applies cleanly, blocking build perf testing. * Tools/Scripts/measure-build-time: Canonical link: https://commits.webkit.org/316194@main
…ockManager::query() https://bugs.webkit.org/show_bug.cgi?id=312456 rdar://174652399 Reviewed by Ryosuke Niwa. WebLockManager::MainThreadBridge::query() was taking in a CompletionHandler but it may sometimes fail to call its completion handler. This happened when the worker thread is exiting, causing `ScriptExecutionContext::ensureOnContextThread()` to fail. Not calling the completion handler is bad but what's worse is that the completion handler would end up getting destroyed on the main thread. The completion handler was capturing a promise from the worker thread, which led to security bugs. To address the issue: 1. Have WebLockManager::MainThreadBridge::query() take in a Function instead of a CompletionHandler given that it cannot always call its callback. 2. Have WebLockManager store the promise in a HashMap and only capture a promise identifier in the MainThreadBridge::query() lambda instead of the promise itself. This pattern was already used for other promises in this class. Test: workers/weblock-manager-query-crash.html * LayoutTests/workers/weblock-manager-query-crash-expected.txt: Added. * LayoutTests/workers/weblock-manager-query-crash.html: Added. * Source/WebCore/Modules/web-locks/WebLockManager.cpp: (WebCore::WebLockManager::MainThreadBridge::abortLockRequest): (WebCore::WebLockManager::MainThreadBridge::query): (WebCore::WebLockManager::query): (WebCore::WebLockManager::clientIsGoingAway): * Source/WebCore/Modules/web-locks/WebLockManager.h: Originally-landed-as: 305413.688@safari-7624-branch (0b964ced2532). rdar://180436136 Canonical link: https://commits.webkit.org/316195@main
…ad of moving it https://bugs.webkit.org/show_bug.cgi?id=318189 rdar://180998972 Reviewed by Alan Baradlay. LineState's constructor takes `FlexLayoutItems&& flexLayoutItems` (an rvalue reference), signaling intent to move. However, inside the member initializer list a named rvalue-reference parameter is an lvalue, so `flexLayoutItems(flexLayoutItems)` selected the Vector copy constructor and deep-copied the whole Vector<FlexLayoutItem, s_flexLayoutItemsInitialCapacity> on every flex line. The caller already passes WTF::move(lineItems), so the intended behavior was always a move. Wrap the parameter in WTF::move() to elide the per-line copy. * Source/WebCore/rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::LineState::LineState): Canonical link: https://commits.webkit.org/316196@main
…n-hover.html after 315468@main https://bugs.webkit.org/show_bug.cgi?id=318268 Unreviewed. 315468@main added a new test, which needs a LBSE specific baseline (different repaint rect order / batching). * LayoutTests/platform/mac-tahoe-wk2-lbse-text/svg/repaint/svg-outline-repaint-on-hover-expected.txt: Added. Canonical link: https://commits.webkit.org/316197@main
https://bugs.webkit.org/show_bug.cgi?id=318264 rdar://181058988 Reviewed by Wenson Hsieh. In order to help some clients move off _WKJSHandle on to WKJSHandle, we need to provide equivalent and similarly named interfaces. * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _getSelectorPathData:completionHandler:]): * Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h: * Source/WebKit/UIProcess/API/Cocoa/_WKTextExtraction.h: * Source/WebKit/UIProcess/API/Cocoa/_WKTextExtraction.mm: (-[_WKTextExtractionConfiguration targetNodeHandle]): (-[_WKTextExtractionConfiguration setTargetNodeHandle:]): (-[_WKTextExtractionResult requestHandleForNodeIdentifier:searchText:completionHandler:]): (-[_WKTextExtractionResult requestContainerHandleForNodeIdentifier:searchText:completionHandler:]): (-[_WKTextExtractionResult requestContainerHandleForSearchTexts:nodeIdentifier:completionHandler:]): Canonical link: https://commits.webkit.org/316198@main
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+55
to
+72
| TinyLRUCache(const TinyLRUCache& other) | ||
| : m_cacheBuffer(other.m_cacheBuffer) | ||
| , m_size(other.m_size) | ||
| { | ||
| invalidateIterators(); | ||
| other.invalidateIterators(); | ||
| } | ||
|
|
||
| TinyLRUCache& operator=(const TinyLRUCache& other) | ||
| { | ||
| if (this == &other) | ||
| return *this; | ||
| invalidateIterators(); | ||
| other.invalidateIterators(); | ||
| m_cacheBuffer = other.m_cacheBuffer; | ||
| m_size = other.m_size; | ||
| return *this; | ||
| } |
There was a problem hiding this comment.
🟡 The new copy constructor and copy-assignment operator call other.invalidateIterators() on a const TinyLRUCache&, but invalidateIterators() (line 192) is non-const and writes m_findResult (line 236, not mutable). Because the class is a template these members are only instantiated on use, so this is a latent compile error: the first caller to copy a TinyLRUCache will fail to build with "discards qualifiers". This is upstream code so it should not block this merge, but worth reporting upstream — the simplest fix is to drop the other.invalidateIterators() calls (a const source can never have an outstanding FindResult since findIfCached() is non-const), or alternatively make m_findResult mutable and invalidateIterators() const.
Extended reasoning...
What the bug is. Upstream's rework of TinyLRUCache adds explicit copy operations:
TinyLRUCache(const TinyLRUCache& other)
: m_cacheBuffer(other.m_cacheBuffer)
, m_size(other.m_size)
{
invalidateIterators();
other.invalidateIterators(); // <-- line 60
}
TinyLRUCache& operator=(const TinyLRUCache& other)
{
if (this == &other)
return *this;
invalidateIterators();
other.invalidateIterators(); // <-- line 68
...
}Both take other by const TinyLRUCache&. invalidateIterators() is declared as a non-const member at line 192:
void invalidateIterators()
{
if (m_findResult) {
...
m_findResult = nullptr;
}
}and m_findResult at line 236 is FindResult* m_findResult { nullptr }; — not mutable. There is no const overload. Calling a non-const member on a const reference is ill-formed.
Why it builds today. TinyLRUCache is a class template, so member function bodies are only instantiated when they are used ([temp.inst]/4). Nothing in the tree copies a TinyLRUCache: the new Tools/TestWebKitAPI/Tests/WTF/TinyLRUCache.cpp exercises get/findIfCached/insert/clear but never copy-constructs or copy-assigns, and a grep over Source/ finds no copies either. So neither the 43-platform CI nor the JSCOnly artifact ever instantiates lines 60/68, and the latent error is hidden.
Step-by-step proof. Take any concrete instantiation:
TinyLRUCache<int, int> a;TinyLRUCache<int, int> b(a);— instantiates the copy constructor.- In the body,
otherhas typeconst TinyLRUCache<int, int>&. other.invalidateIterators()selectsvoid TinyLRUCache::invalidateIterators()(no const overload). The implicit object parameter is non-constTinyLRUCache&.- Binding a non-const reference to a const object is ill-formed → "passing 'const TinyLRUCache<int, int>' as 'this' argument discards qualifiers".
I confirmed this by extracting the class skeleton into a standalone TU and compiling with g++ -std=c++20; both the copy ctor and copy assignment fail with exactly that diagnostic. The same applies under clang.
Why existing safeguards don't catch it. The author explicitly defined the copy operations (and = deleted the move operations), so copyability is clearly intended — this is not the rule-of-five fallout the compiler would warn about. Template lazy instantiation hides the error from the build, and the new unit test never exercises copy. CI is green precisely because the broken members are dead code today.
Impact. Low / latent. No current build breaks (Bun's JSCOnly artifacts included). But the public API now advertises copyability that does not actually compile: the first WebKit/Bun change that copies a TinyLRUCache (e.g. storing one by value in a copyable struct) will fail at the point of instantiation, far from this header, with a confusing diagnostic.
How to fix. Two equivalent options:
- Drop the
other.invalidateIterators()calls. Aconstsource can never have a liveFindResult: the only producer isfindIfCached(), which is non-const, and every mutating member (get,insert,clear,findIfCacheditself) callsinvalidateIterators()first. So ifotheris reachable only through aconstreference,other.m_findResultis already null and the call is a no-op anyway. - Or declare
mutable FindResult* m_findResult { nullptr };and markinvalidateIterators()const, if upstream wants to keep the defensive call.
Since this header arrives verbatim from upstream WebKit and is not in Bun's patch set, the right action here is to land the merge as-is and file the fix upstream; flagging so it is on record.
|
Caution Review failedFailed to post review comments. We encountered an issue with GitHub. Use ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (265)
💤 Files with no reviewable changes (10)
⏰ Context from checks skipped due to timeout. (45)
🧰 Additional context used📓 Path-based instructions (6)**/Source/JavaScriptCore/runtime/**/*.{h,cpp,mm}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
Source/JavaScriptCore/**/*.{cpp,h}📄 CodeRabbit inference engine (Source/JavaScriptCore/CLAUDE.md)
Files:
**/Source/JavaScriptCore/{jit,dfg,ftl}/**/*.{h,cpp,mm}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/Source/JavaScriptCore/**/*.{cpp,mm}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
Source/JavaScriptCore/{llint/LowLevelInterpreter.asm,jit/JIT.cpp,dfg/DFGSpeculativeJIT.cpp,ftl/FTLLowerDFGToB3.cpp}📄 CodeRabbit inference engine (Source/JavaScriptCore/CLAUDE.md)
Files:
Source/JavaScriptCore/builtins/**/*.js📄 CodeRabbit inference engine (Source/JavaScriptCore/CLAUDE.md)
Files:
🪛 ast-grep (0.44.0)LayoutTests/fast/webcodecs/audio-data-close-during-gc-crash.html[error] 57-57: React's useState should not be directly called (usestate-direct-usage) [warning] 57-57: Avoid using the initial state variable in setState (setstate-same-var) LayoutTests/webaudio/biquadfilternode-set-type-channel-count-race.html[warning] 41-41: Avoid using the initial state variable in setState (setstate-same-var) [error] 41-41: React's useState should not be directly called (usestate-direct-usage) LayoutTests/workers/weblock-manager-query-crash.html[warning] 31-31: Avoid using the initial state variable in setState (setstate-same-var) LayoutTests/fast/mediasession/metadata/artwork-image-loader-callback-crash.html[error] 16-19: React's useState should not be directly called (usestate-direct-usage) [error] 31-38: React's useState should not be directly called Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice. (usestate-direct-usage) [warning] 37-37: Avoid using the initial state variable in setState (setstate-same-var) JSTests/stress/regexp-many-non-greedy-paren-groups.js[warning] 6-6: Do not use variable for regular expressions (regexp-non-literal) [warning] 6-6: Detects non-literal values in regular expressions (detect-non-literal-regexp) JSTests/stress/yarr-negative-offset.js[warning] 29-29: Detects non-literal values in regular expressions (detect-non-literal-regexp) LayoutTests/ipc/networksindexeddb-close-connection-during-version-change.html[warning] 14-14: Avoid using the initial state variable in setState (setstate-same-var) LayoutTests/fast/webgpu/regression/repro_176812014.html[warning] 39-39: Avoid using the initial state variable in setState (setstate-same-var) JSTests/stress/arguments-elimination-multiple-inline-call-frames.js[warning] 37-37: Avoid (detect-eval-with-expression) LayoutTests/http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate.html[warning] 23-23: Avoid using the initial state variable in setState (setstate-same-var) [error] 23-23: React's useState should not be directly called (usestate-direct-usage) [error] 27-42: React's useState should not be directly called (usestate-direct-usage) LayoutTests/ipc/forged-resource-load-statistics-storage-access.html[warning] 10-10: Avoid using the initial state variable in setState (setstate-same-var) Tools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.py[error] 46-46: Command coming from incoming request (subprocess-from-request) 🪛 Cppcheck (2.21.0)Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp[error] 329-329: failed to evaluate (syntaxError) Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp[style] 1382-1382: The function 'isVersionChangeTransactionActive' is never used. (unusedFunction) Source/WebCore/css/parser/CSSCustomPropertySyntax.cpp[style] 267-267: The function 'serializeCustomPropertySyntaxAsCSSType' is never used. (unusedFunction) Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp[style] 342-342: The function 'getOwnPrivatePropertyMethods' is never used. (unusedFunction) Tools/TestWebKitAPI/Tests/WTF/TinyLRUCache.cpp[error] 45-45: syntax error (syntaxError) 🪛 HTMLHint (1.9.2)LayoutTests/fast/css/highlight-pseudo-fill-applies-color-filter-expected.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 8-8: <title> must be present in tag. (title-require) LayoutTests/fast/css/highlight-pseudo-fill-applies-color-filter.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 8-8: <title> must be present in tag. (title-require) LayoutTests/http/tests/storageAccess/resources/request-storage-access-without-gesture-check-activation-iframe.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 21-21: <title> must be present in tag. (title-require) LayoutTests/webaudio/biquadfilternode-set-type-channel-count-race.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 52-52: <title> must be present in tag. (title-require) LayoutTests/http/tests/security/contentSecurityPolicy/directive-name-case-insensitive.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 9-9: <title> must be present in tag. (title-require) LayoutTests/workers/weblock-manager-query-crash.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 38-38: <title> must be present in tag. (title-require) LayoutTests/fast/mediasession/metadata/artwork-image-loader-callback-crash.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 44-44: <title> must be present in tag. (title-require) LayoutTests/http/wpt/webcodecs/audio-encoder-pcm-format-change.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 6-6: <title> must be present in tag. (title-require) LayoutTests/fast/mediastream/applyConstraints-with-takePhoto.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 7-7: <title> must be present in tag. (title-require) LayoutTests/storage/indexeddb/index-cursor-reverse-delete-next-higher-key-private.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 2-2: Doctype must be declared before any non-comment content. (doctype-first) [error] 6-6: <title> must be present in tag. (title-require) LayoutTests/fast/webgpu/nocrash/fuzz-176882934.html[error] 2-2: Doctype must be declared before any non-comment content. (doctype-first) LayoutTests/ipc/networksindexeddb-close-connection-during-version-change.html[error] 2-2: Doctype must be declared before any non-comment content. (doctype-first) LayoutTests/http/wpt/site-isolation/history-traversal/history-traversal-navigate-parent-while-child-loading.html[warning] 5-5: A <iframe> element must have an accessible name. (frame-title-require) LayoutTests/media/media-source/media-source-gap-policy.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) LayoutTests/http/tests/security/contentSecurityPolicy/directive-name-case-insensitive-strict-dynamic.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 7-7: <title> must be present in tag. (title-require) LayoutTests/fast/webgpu/regression/repro_176812014.html[error] 2-2: Doctype must be declared before any non-comment content. (doctype-first) LayoutTests/http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) [error] 46-46: <title> must be present in tag. (title-require) LayoutTests/ipc/forged-resource-load-statistics-storage-access.html[warning] 2-2: An lang attribute must be present on elements. (html-lang-require) LayoutTests/ipc/rgba16f-getpixelbuffer-colorspace-mismatch-heap-disclosure.html[error] 2-2: Doctype must be declared before any non-comment content. (doctype-first) 🪛 OpenGrep (1.23.0)JSTests/stress/regress-174630697.js[ERROR] 17-17: new Function() with dynamic input can execute arbitrary code. Avoid dynamic code evaluation entirely, or use a safe alternative. (coderabbit.code-injection.new-function-js) JSTests/stress/arguments-elimination-multiple-inline-call-frames.js[ERROR] 38-38: eval() with dynamic input can execute arbitrary code. Avoid dynamic code evaluation entirely, or use a safe alternative. (coderabbit.code-injection.eval-js) 🪛 Pylint (4.0.6)Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py[warning] 149-149: Access to a protected member _options of a client class (W0212) [warning] 150-150: Access to a protected member _port of a client class (W0212) [warning] 151-151: Access to a protected member _create_port_for_driver of a client class (W0212) [warning] 151-151: Access to a protected member _port of a client class (W0212) [warning] 152-152: Access to a protected member _collect_tests of a client class (W0212) Tools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.py[warning] 47-47: 'subprocess.run' used without explicitly defining the value for 'check'. (W1510) 🪛 Ruff (0.15.20)Tools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.py[error] 47-47: (S603) [warning] 47-47: Add explicit (PLW1510) [error] 47-47: Starting a process with a partial executable path (S607) [warning] 49-49: Avoid specifying long messages outside the exception class (TRY003) [warning] 52-55: Use Replace (SIM105) [warning] 63-63: Avoid specifying long messages outside the exception class (TRY003) Hidden Review Stack ArtifactWalkthroughThis PR bundles numerous unrelated changes: JavaScriptCore JIT/runtime work (ArrayStorage GetByVal fast path, Repatch locking, YarrJIT code-size limiting), WebCore render border-box setter refactor, MediaSource gap-tolerance policy rework, storage-access user-gesture enum rename, CSS custom-property/dashed-function and CSP case-sensitivity fixes, WebKit IPC message-check hardening, history about:blank back-forward fix, fullscreen titlebar overlay feature, WebGPU pipeline-ID unification, pixel-buffer color-space heap-disclosure fix, inspector private-member support, and a large volume of new/updated layout tests, build configuration, and tooling. ChangesJavaScriptCore Engine
WebCore Rendering & Media
WebKit IPC & Storage Access
WebGPU & CSS
Miscellaneous & Tests
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 37
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp (1)
330-353: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
OutOfMemoryerror reported even when the failure is "size exceeds inline limit", not an actual allocation failure.When
replyImageBytes > readPixelsInlineSizeLimit,pixelsis never allocated (stays default/empty), sopixels.sizeInBytes() == replyImageBytesis false and the code falls intocontext->addError(GCGLErrorCode::OutOfMemory)— even though no allocation was attempted or failed. This conflates two distinct conditions: "value too large for the inline transfer path" (a policy/size-limit case, analogous togetBufferSubDataInline'sInvalidOperationfor its size check) versus a genuinetryZeroedMallocfailure. Consider distinguishing the two cases so the correct GL error is surfaced to the WebGL client.♻️ Possible fix sketch
- MallocSpan<uint8_t> pixels; - if (replyImageBytes && replyImageBytes <= readPixelsInlineSizeLimit) - pixels = MallocSpan<uint8_t>::tryZeroedMalloc(replyImageBytes); - - RefPtr context = m_context; - std::optional<WebCore::IntSize> readArea; - if (pixels.sizeInBytes() == replyImageBytes) - readArea = context->readPixelsWithStatus(rect, format, type, packReverseRowOrder, pixels.mutableSpan()); - else - context->addError(GCGLErrorCode::OutOfMemory); + RefPtr context = m_context; + std::optional<WebCore::IntSize> readArea; + MallocSpan<uint8_t> pixels; + if (replyImageBytes > readPixelsInlineSizeLimit) { + context->addError(GCGLErrorCode::InvalidOperation); + } else if (replyImageBytes) { + pixels = MallocSpan<uint8_t>::tryZeroedMalloc(replyImageBytes); + if (pixels.sizeInBytes() == replyImageBytes) + readArea = context->readPixelsWithStatus(rect, format, type, packReverseRowOrder, pixels.mutableSpan()); + else + context->addError(GCGLErrorCode::OutOfMemory); + } else + readArea = context->readPixelsWithStatus(rect, format, type, packReverseRowOrder, pixels.mutableSpan());🤖 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 `@Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp` around lines 330 - 353, `RemoteGraphicsContextGL::readPixelsInline` is reporting `GCGLErrorCode::OutOfMemory` whenever the request exceeds the inline transfer limit, even though no allocation was attempted. Update the `readPixelsInline` flow to distinguish the inline-size policy check from a real `tryZeroedMalloc` failure: if `replyImageBytes` is too large for the inline path, surface the appropriate non-OOM error instead of `OutOfMemory`, and only call `context->addError(GCGLErrorCode::OutOfMemory)` when allocation was actually attempted and failed. Use the existing `readPixelsInlineSizeLimit`, `replyImageBytes`, and `pixels` handling to keep the decision localized in `RemoteGraphicsContextGL::readPixelsInline`.Tools/TestWebKitAPI/Tests/WebKit/WKWebView/ObscuredContentInsets.mm (1)
964-988: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider refactoring the original test to reuse the new
createFullScreenCapableWindow()helper.
ScrollPocketCoversFullScreenTitlebar(lines 976-988) still inlines the exact same window/toolbar/titlebar-accessory setup that was just extracted intocreateFullScreenCapableWindow(). Applying the helper there too would remove the now-triplicated duplication across this file.♻️ Proposed refactor to reuse the helper in the original test
- auto styleMask = NSWindowStyleMaskTitled | NSWindowStyleMaskClosable | NSWindowStyleMaskResizable | NSWindowStyleMaskFullSizeContentView; - RetainPtr toolbar = adoptNS([[NSToolbar alloc] initWithIdentifier:@"ScrollPocketTestToolbar"]); - RetainPtr window = adoptNS([[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) styleMask:styleMask backing:NSBackingStoreBuffered defer:NO]); - - [window setCollectionBehavior:[window collectionBehavior] | NSWindowCollectionBehaviorFullScreenPrimary]; - [window setToolbar:toolbar.get()]; - - // Attach a titlebar accessory so AppKit does not autohide the fullscreen toolbar mid-test. - RetainPtr accessoryViewController = adoptNS([[NSTitlebarAccessoryViewController alloc] init]); - [accessoryViewController setView:adoptNS([[NSView alloc] initWithFrame:NSMakeRect(0, 0, 1, 1)]).get()]; - [accessoryViewController setLayoutAttribute:NSLayoutAttributeRight]; - [window addTitlebarAccessoryViewController:accessoryViewController.get()]; + RetainPtr window = createFullScreenCapableWindow();Note: this requires moving the
createFullScreenCapableWindow()definition aboveScrollPocketCoversFullScreenTitlebar, or forward-declaring it.Also applies to: 1035-1051
🤖 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 `@Tools/TestWebKitAPI/Tests/WebKit/WKWebView/ObscuredContentInsets.mm` around lines 964 - 988, `ScrollPocketCoversFullScreenTitlebar` still duplicates the full-screen window setup that was extracted into `createFullScreenCapableWindow()`. Update this test to call the helper instead of inlining the `NSWindow`, `NSToolbar`, and `NSTitlebarAccessoryViewController` setup, and ensure the helper is visible before this test either by moving its definition earlier or adding a forward declaration. Also apply the same refactor to the other duplicated test mentioned in the diff.Source/WebCore/platform/graphics/filters/FilterImage.cpp (1)
281-294: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDon’t cache a pixel buffer when the fallback copy fails.
pixelBufferis assigned beforecopyImageBytes; if the copy fails, the slot remains non-null, so a later call returns the partially initialized buffer from Line 266. Publish the buffer only after the copy succeeds.Proposed fix
- pixelBuffer = m_allocator.createPixelBuffer(format, logicalSize); - if (!pixelBuffer) + RefPtr newPixelBuffer = m_allocator.createPixelBuffer(format, logicalSize); + if (!newPixelBuffer) return nullptr; if (alphaFormat == AlphaPremultiplication::Unpremultiplied) { if (auto& sourcePixelBuffer = pixelBufferSlot(AlphaPremultiplication::Premultiplied)) { - if (!copyImageBytes(*sourcePixelBuffer, *pixelBuffer)) + if (!copyImageBytes(*sourcePixelBuffer, *newPixelBuffer)) return nullptr; } } else { if (auto& sourcePixelBuffer = pixelBufferSlot(AlphaPremultiplication::Unpremultiplied)) { - if (!copyImageBytes(*sourcePixelBuffer, *pixelBuffer)) + if (!copyImageBytes(*sourcePixelBuffer, *newPixelBuffer)) return nullptr; } } + pixelBuffer = WTF::move(newPixelBuffer); return pixelBuffer.get();🤖 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 `@Source/WebCore/platform/graphics/filters/FilterImage.cpp` around lines 281 - 294, The pixel buffer is being stored in the cache before the fallback copy succeeds, so a failed copy leaves a partially initialized buffer visible to later calls. In FilterImage::pixelBufferForFormat, keep the newly created buffer local until copyImageBytes completes successfully, then publish it to the slot; if the copy fails, return nullptr without updating the cached pixelBufferSlot state.
🤖 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/stress/slowputarraystorage-stale-structure-bit.js`:
- Around line 10-18: The proxy invariant test around proxyGet.length is too weak
because it still passes when no TypeError is thrown and the ordinary target
value is returned. Update the try/catch blocks to explicitly fail when the
invariant violation does not throw, using the existing proxyGet.length check and
the TypeError validation so the stale-structure regression only passes when the
engine actually raises the expected error.
In `@JSTests/stress/typedarray-from-oob.js`:
- Around line 26-35: The try/catch around Int32Array.from in
typedarray-from-oob.js is swallowing all failures, which can hide unrelated
regressions before the buffer is actually detached. Narrow the handling so only
the expected detach-related exception path is accepted, and let any other
exception escape or fail the test explicitly; keep the test logic around the
Int32Array.from callback, arrayBuffer.transfer, and callbacks count intact while
making unexpected errors visible.
In `@LayoutTests/fast/mediastream/applyConstraints-with-takePhoto.html`:
- Around line 11-29: The media cleanup is only happening at the end of the test
body, so failures before the final stop calls can leave capture active and
affect later tests. Update the test around the existing
getUserMedia/ImageCapture flow to register cleanup with the test harness (using
the appropriate media cleanup helper) so both originalTrack and clonedTrack are
always stopped even if takePhoto or applyConstraints fails.
In `@LayoutTests/fast/webcodecs/audio-data-close-during-gc-crash.html`:
- Around line 48-58: The GC-pressure test is using a microtask reschedule in
closeOne via Promise.resolve().then(closeOne), which can starve the repeating
pressureGC task loop. Change closeOne to yield back to the task queue with a
task-based reschedule such as setTimeout(closeOne, 0), keeping the existing
pressureGC and closeOne flow intact so the GC-pressure loop continues to run.
In `@LayoutTests/fast/webgpu/nocrash/fuzz-176882934.html`:
- Around line 4-59: The run() test flow can reject before reaching
testRunner.notifyDone(), leaving waitUntilDone() hanging indefinitely. Update
run() to use try/finally around the main WebGPU setup and draw calls, and make
sure testRunner.notifyDone() is always called in the finally path. Also handle a
missing adapter/device in run() explicitly so this nocrash test reports a clear
failure instead of timing out.
In
`@LayoutTests/http/wpt/site-isolation/history-traversal/history-traversal-navigate-parent-while-child-loading.html`:
- Line 5: The iframe in the test markup is missing an accessible name, which
triggers HTMLHint. Update the iframe element used in this test to include a
stable title attribute so it has a name without changing test behavior, keeping
the existing iframe id and overall setup intact.
In
`@LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/highlight-fill-color-ignores-stroke-color-ref.html`:
- Around line 10-18: The reference still depends on the highlight API via
main::highlight(...) and CSS.highlights.set(), so both the test and reference
can share the same bug. Update the reference in the highlighted reftest to
render the green PASS with ordinary styling only, removing the ::highlight-based
rule and the CSS.highlights.set call while keeping the reference element
(main#target / class highlight_reftest) visually equivalent.
In `@LayoutTests/ipc/forged-resource-load-statistics-storage-access.html`:
- Around line 87-100: The current `HasStorageAccess` test path treats a missing
reply the same as an explicit “no access” response, so a timeout can incorrectly
pass; update the logic around `hsaReply` and the
`CoreIPC.Networking.NetworkConnectionToWebProcess.HasStorageAccess` callback to
fail explicitly when no reply arrives after the wait loop, and only report PASS
when a real reply is received and `hasStorageAccess` is false.
In `@LayoutTests/ipc/loadping-firstpartyforcookies-message-check.html`:
- Around line 138-143: The assertion in the invalid-message test is racing the
reply from TakeInvalidMessageStringForTesting because messageCheck is read
immediately after registering the callback. Update the test to await the
callback result before asserting, using the existing reply handler around
CoreIPC.Networking.NetworkConnectionToWebProcess.TakeInvalidMessageStringForTesting
so the check runs only after messageCheck has been populated.
- Around line 8-10: The `if (!window.IPC)` guard in the test should not silently
pass when the IPC test hook is unavailable; instead, make the
`loadping-firstpartyforcookies-message-check` test fail so missing
`IPCTestingAPIEnabled` is caught. Update the early-exit logic around
`window.IPC` to report a failure through the test harness rather than calling
`done()` and returning, keeping the check in the same test flow so configuration
regressions surface immediately.
In `@LayoutTests/ipc/use-redirection-for-current-navigation-message-check.html`:
- Around line 54-66: The current test only exercises an invalid
resourceLoadIdentifier path, so it never reaches the new
loader->isMainFrameLoad() guard in UseRedirectionForCurrentNavigation. Add an
additional case in use-redirection-for-current-navigation-message-check.html
that targets a live loader path with a non-main-frame navigation, so the check
is actually executed and the authorization regression is covered. Keep the
existing non-redirect rejection assertion, but ensure the new case uses the same
CoreIPC.Networking.NetworkConnectionToWebProcess.UseRedirectionForCurrentNavigation
entry point and validates the main-frame-only behavior.
In
`@LayoutTests/storage/indexeddb/index-cursor-reverse-delete-next-higher-key-private.html`:
- Around line 8-19: Mark this IndexedDB regression test as asynchronous so the
harness stays open until the cursor callbacks complete. Set jsTestIsAsync before
the call to finishJSTest in the test logic, and keep the existing IndexedDB flow
in the openonsuccess/onupgradeneeded handlers unchanged so the reverse
cursor/UAF check runs deterministically.
In `@LayoutTests/webaudio/biquadfilternode-set-type-channel-count-race.html`:
- Around line 2-4: Add the missing document metadata to this test fixture by
updating the top-level HTML structure in the test document: set a lang attribute
on the root html element and add a title element inside head. Use the existing
document skeleton near the html/head/script tags to locate the change and ensure
the new test page satisfies HTMLHint requirements.
In `@Source/JavaScriptCore/builtins/TypedArrayConstructor.js`:
- Around line 119-120: The typed array guard in TypedArrayConstructor.js is
checking the result length before detachment, which can read length from a
detached view after mapFn changes it. Update the short-circuit condition in the
result-handling path so `@isDetached`(result) is evaluated before
`@typedArrayLength`(result), matching the source guard’s order in the same
TypedArrayConstructor logic.
In `@Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp`:
- Around line 458-500: The instance private-member collection in
JSInjectedScriptHost::getPrivateProperties skips the inspected class’s own scope
when the target is a direct Class.prototype object. After recovering the
constructor from the object’s direct constructor property, seed the current
class scope into the instance-scopes walk before iterating via
getPrototypeDirect so the class’s own private methods/accessors are included,
while keeping the existing brand checks and duplicate SymbolTable filtering.
In `@Source/JavaScriptCore/yarr/YarrJIT.cpp`:
- Line 4528: The new code-size cutoff should not be enforced during inline
RegExp emission, because compileInline() uses a caller-owned assembler and does
not recover from mid-stream aborts like the main compile path. Update the
loop/guard around the emission in YarrJIT::compileInline() so it does not
consult hasExceededCodeSizeLimit() or otherwise stop generation partway through.
Keep the cutoff only on the non-inline compile path, and preserve the current
generate()/backtrack() flow without introducing a failure state in
compileInline().
In `@Source/WebCore/bindings/js/JSDOMPromiseDeferred.h`:
- Around line 427-456: The promise rejection path in callPromisePairFunction is
unconditionally rejecting both JSPromise instances even if one was already
settled by the functor, which can violate the pending-only requirement. Update
rejectPromisesWithExceptionIfAny usage so it only rejects promises that are
still pending, or otherwise guard each promise’s state before calling reject,
and keep the fallback rebuild via convertDictionaryToJS in sync with the
settled/rejected outcomes.
In `@Source/WebCore/bindings/scripts/CodeGeneratorJS.pm`:
- Around line 6372-6376: Generate a concise fix in
GenerateImplementationCustomFunctionCall() so custom operations marked with
ReturnsPromisePair forward both promise outputs, not just the first one. The
current trampoline/template argument update in CodeGeneratorJS.pm is
insufficient because the call path for [Custom, ReturnsPromisePair] still omits
promise2; update the call signature construction in the custom-function branch
to pass the second promise argument whenever the extended attribute is present,
using the existing ReturnsPromisePair handling as the entry point.
In `@Source/WebCore/css/StyleProperties.cpp`:
- Around line 308-309: Mirror the custom-property escaping used in
StyleProperties::asTextInternal inside PropertyReference::cssText(). The issue
is that cssText() still appends raw cssName() for CSSPropertyCustom, so update
that branch to serialize the identifier from the CSSCustomPropertyValue name via
serializeIdentifier(), matching the existing handling in the other text path and
keeping the rest of cssText() unchanged.
In `@Source/WebCore/loader/HistoryController.cpp`:
- Around line 889-890: The HistoryController::updateCurrentItem same-URL path is
not refreshing the item’s isInitialAboutBlank state, so an initial empty
document that later becomes explicit about:blank stays misclassified. Update the
same-URL branch in updateCurrentItem() to re-read
documentLoader->isInitialAboutBlank() and apply it to the current item, matching
the initialization done in initializeItem() so the flag stays accurate after
navigation updates.
In `@Source/WebCore/platform/audio/ios/MediaDeviceRoute.mm`:
- Line 245: The seek-tolerance contract in MediaDeviceRoute is still unresolved,
so add a tracked follow-up for the zero-tolerance assumption in the seek path.
Update the relevant MediaDeviceRoute seek logic to explicitly call out the
missing tolerance support, and either implement the proper seek-with-tolerance
behavior or create a clearly linked follow-up task from the same
MediaDeviceRoute code path so it won’t be lost.
In `@Source/WebCore/platform/graphics/MediaSourcePrivate.cpp`:
- Around line 388-400: Refresh m_audioBuffered whenever the active-source set
changes, since it is derived from m_activeSourceBuffers and must stay in sync.
Update the add/remove active-source paths in MediaSourcePrivate, especially
removeSourceBuffer(), so the recomputation happens after the buffer is removed
from m_activeSourceBuffers and before gap-tolerance decisions use it. Use the
existing audio-range aggregation logic around m_audioBuffered,
audioBufferedRanges(), and gapToleranceAtTime to keep the update in one place
and avoid stale ranges from removed SourceBuffer entries.
- Around line 631-633: Update nextStallTime() in MediaSourcePrivate.cpp so
tolerated gaps are resolved before falling back to duration(). The current early
return on ranges.find(currentTime) == notFound bypasses cases that
hasBufferedTime() treats as buffered, such as start gaps and audio-covered gaps.
Adjust the logic around currentTime, buffered(), and hasBufferedTime() so these
tolerated gaps map to the correct stall time instead of returning stallAtTime
based on duration().
In `@Source/WebCore/platform/graphics/PixelBufferConversion.cpp`:
- Around line 361-363: The RGBA16F path is still performing a straight channel
copy even after removing the color-space mismatch guard, so mismatched spaces
can be reported as successfully converted. Update the relevant
PixelBufferConversion routine to route RGBA16F color-space mismatches through a
real conversion path, such as the existing RGBA8 intermediary, or add an
explicit unsupported-path fallback that fails instead of returning success. Keep
the copy-only behavior only for same-color-space RGBA16F cases, and preserve the
current safety around uninitialized destination buffers.
In `@Source/WebCore/platform/graphics/SourceBufferPrivate.cpp`:
- Around line 824-829: The gap-tolerance lambda in
SourceBufferPrivate::createTrackBuffer is capturing the original trackId, but
updateTrackIds() later remaps TrackBuffer keys so the callback can use a stale
ID. Update the callback to resolve the current track ID at call time, or rebind
the callback when updateTrackIds() changes the mapping, and make sure the
gapToleranceAtTime lookup uses the remapped track identity rather than the
pre-remap one.
In `@Source/WebCore/style/StyleSubstitutionResolver.cpp`:
- Around line 246-281: The substituted argument handling in
StyleSubstitutionResolver is dropping attr() taint, so caller-supplied
dashed-function parameters can lose their tainted state before body evaluation.
Update the parameter flow around createFirstValidVariableData,
CSSCustomPropertyValue::createSyntaxAll, and the substitutedArguments storage so
IsAttrTainted is preserved alongside each argument (or defer substitution until
parameter resolution), and ensure any CSSVariableData built from argument tokens
carries that taint through to the final resolved value.
- Around line 79-95: The internal first-valid path is currently keyed off the
author-reachable function name "-internal-first-valid", which lets authored
custom-property values trigger the special handling, including the
m_registration == nullptr bypass. Update createFirstValidVariableData and the
function-token dispatch in StyleSubstitutionResolver so this uses an
internal-only marker/representation that cannot be synthesized from CSS text,
and ensure the execution path only recognizes that internal form rather than a
raw function name.
In `@Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp`:
- Around line 913-929: The current guard in
XMLDocumentParserLibxml2::appendCurrentNode only checks parent/document changes,
but constructor-side child or attribute mutations can still slip through. Update
this path to use the same fallback-style post-validation used by
Document::createElement/constructElementWithFallback, so the element is
validated after custom-element construction and rejected consistently if its
tree state was mutated. Locate the logic around parserAppendChild and
stopParsing in XMLDocumentParserLibxml2::appendCurrentNode and replace the
partial invariant check with the fuller validation behavior.
In `@Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorkerServer.cpp`:
- Around line 185-190: The addContextConnection path in WebSharedWorkerServer
silently returns when m_contextConnections.add finds an existing
ContextConnectionKey, leaving a new contextConnection untracked and causing
removeContextConnection to mismatch later. Update addContextConnection to either
verify the duplicate is the same ContextConnection object or explicitly
replace/tear down the new connection before returning, and keep the
ContextConnectionKey handling consistent with
contextConnectionCreated/removeContextConnection.
In `@Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp`:
- Around line 2052-2055: The early return in establishTransaction is silently
dropping a normal transaction start, so replace it with a failure path instead
of a no-op. In NetworkStorageManager::establishTransaction, where the CheckedPtr
database and isVersionChangeTransactionActive() guard are used, change the
conditional handling to MESSAGE_CHECK or another explicit error response so the
client transaction is not stranded without a server-side counterpart.
In `@Source/WebKit/UIProcess/API/Cocoa/WKUserContentController.mm`:
- Around line 234-242: The addBuffer:name:contentWorld: path in
WKUserContentController currently allows unsupported id values to leave
bufferToAdd nil, and then dereferences _buffer unconditionally. Add an explicit
validation step after the NSData/WKJSScriptingBuffer casting so unsupported
inputs are rejected before calling
protect(*_userContentControllerProxy)->addJSBuffer, and make sure the internal
_addBuffer: path uses the same guard so both entry points fail safely instead of
crashing.
In `@Source/WebKit/UIProcess/mac/WebPageProxyMac.mm`:
- Around line 637-640: The temporary PDF directory guard in WebPageProxyMac.mm
is using a raw string prefix check, which can accept sibling paths that only
share the same leading characters. Keep the existing single-component
validation, and update the containment check around the path comparison to
require a directory-bound prefix (for example by comparing against the
standardized pdfDirectoryPath with a trailing path separator) so only files
truly inside the temporary PDF directory are accepted.
In `@Source/WTF/wtf/PlatformHave.h`:
- Around line 1262-1268: The HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT guard is
duplicated with overlapping platform/version conditions, so consolidate the two
checks into a single definition or make the later block conditional on the macro
not already being defined. Update the related preprocessor logic in
PlatformHave.h around HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT so the Mac
160000–259999 and the existing Mac 260000+/non-Mac coverage are handled without
redefining the macro.
In `@Source/WTF/wtf/TinyLRUCache.h`:
- Around line 55-72: Remove the unnecessary other.invalidateIterators() calls
from TinyLRUCache’s copy constructor and operator=; both TinyLRUCache(const
TinyLRUCache&) and TinyLRUCache::operator= should only invalidate the
destination object’s iterators before copying m_cacheBuffer and m_size, since
calling a non-const method on the const source breaks compilation.
In `@Tools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.py`:
- Line 47: The subprocess call in browser_binary_size.py is not lint-clean
because it relies on a PATH lookup for strip and omits an explicit check
argument. Update the logic around the subprocess.run call in the
browser_binary_size flow to resolve the strip binary once before use, then
invoke subprocess.run with that resolved executable and check=False explicitly,
keeping the existing capture_output and text behavior unchanged.
In `@Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp`:
- Around line 339-353: The new ImageBuffer test can still dereference null after
non-fatal EXPECT_NE checks, so move the setup assertions in
ImageBufferTests::... to fatal checks. Use ASSERT_NE for the ImageBuffer
returned by ImageBuffer::create and for the pixelBuffer returned by
imageBuffer->getPixelBuffer before calling imageBuffer->context() or
pixelBuffer->bytes(), so the test stops immediately on setup failure.
In `@Tools/TestWebKitAPI/Tests/WebKit/WKWebView/JSBuffer.mm`:
- Line 41: The NSData ownership in the JSBuffer test is incorrect:
`dataWithBytes:length:` returns an autoreleased +0 object, so `adoptNS` in
`JSBuffer.mm` will over-release it. Update the `evenLength` initialization to
use `retainPtr(...)` instead, or construct the NSData with an `alloc/init` path,
keeping the fix localized to the `evenLength` setup in this test.
---
Outside diff comments:
In `@Source/WebCore/platform/graphics/filters/FilterImage.cpp`:
- Around line 281-294: The pixel buffer is being stored in the cache before the
fallback copy succeeds, so a failed copy leaves a partially initialized buffer
visible to later calls. In FilterImage::pixelBufferForFormat, keep the newly
created buffer local until copyImageBytes completes successfully, then publish
it to the slot; if the copy fails, return nullptr without updating the cached
pixelBufferSlot state.
In `@Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp`:
- Around line 330-353: `RemoteGraphicsContextGL::readPixelsInline` is reporting
`GCGLErrorCode::OutOfMemory` whenever the request exceeds the inline transfer
limit, even though no allocation was attempted. Update the `readPixelsInline`
flow to distinguish the inline-size policy check from a real `tryZeroedMalloc`
failure: if `replyImageBytes` is too large for the inline path, surface the
appropriate non-OOM error instead of `OutOfMemory`, and only call
`context->addError(GCGLErrorCode::OutOfMemory)` when allocation was actually
attempted and failed. Use the existing `readPixelsInlineSizeLimit`,
`replyImageBytes`, and `pixels` handling to keep the decision localized in
`RemoteGraphicsContextGL::readPixelsInline`.
In `@Tools/TestWebKitAPI/Tests/WebKit/WKWebView/ObscuredContentInsets.mm`:
- Around line 964-988: `ScrollPocketCoversFullScreenTitlebar` still duplicates
the full-screen window setup that was extracted into
`createFullScreenCapableWindow()`. Update this test to call the helper instead
of inlining the `NSWindow`, `NSToolbar`, and `NSTitlebarAccessoryViewController`
setup, and ensure the helper is visible before this test either by moving its
definition earlier or adding a forward declaration. Also apply the same refactor
to the other duplicated test mentioned in the diff.
🪄 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: bcbd3924-937a-40ef-ad50-61e4787a8a1d
📒 Files selected for processing (265)
Configurations/Version.xcconfigJSTests/microbenchmarks/get-by-val-array-storage-sparse-hole.jsJSTests/microbenchmarks/get-by-val-array-storage-sparse.jsJSTests/stress/arguments-elimination-multiple-inline-call-frames.jsJSTests/stress/array-indexof-ensure-still-alive.jsJSTests/stress/dfg-ensure-absence-cached-dictionary-then.jsJSTests/stress/multi-put-by-offset-reallocating-storage-needs-write-barrier.jsJSTests/stress/regexp-many-non-greedy-paren-groups.jsJSTests/stress/regress-174630697.jsJSTests/stress/slowputarraystorage-stale-structure-bit.jsJSTests/stress/spread-with-OnlyAtomStringsStructure.jsJSTests/stress/typedarray-from-oob.jsJSTests/stress/yarr-negative-offset.jsJSTests/wasm/gc/private-fields-and-methods.jsLayoutTests/fast/css/highlight-pseudo-fill-applies-color-filter-expected.htmlLayoutTests/fast/css/highlight-pseudo-fill-applies-color-filter.htmlLayoutTests/fast/custom-elements/xml-parser-reparent-during-construction-crash-expected.txtLayoutTests/fast/custom-elements/xml-parser-reparent-during-construction-crash.xhtmlLayoutTests/fast/mediasession/metadata/artwork-image-loader-callback-crash-expected.txtLayoutTests/fast/mediasession/metadata/artwork-image-loader-callback-crash.htmlLayoutTests/fast/mediastream/applyConstraints-with-takePhoto-expected.txtLayoutTests/fast/mediastream/applyConstraints-with-takePhoto.htmlLayoutTests/fast/webcodecs/audio-data-close-during-gc-crash-expected.txtLayoutTests/fast/webcodecs/audio-data-close-during-gc-crash.htmlLayoutTests/fast/webgpu/nocrash/fuzz-176882934-expected.txtLayoutTests/fast/webgpu/nocrash/fuzz-176882934.htmlLayoutTests/fast/webgpu/pipeline-id-collision-bindgroup-validation-bypass-expected.txtLayoutTests/fast/webgpu/pipeline-id-collision-bindgroup-validation-bypass.htmlLayoutTests/fast/webgpu/regression/repro_176812014-expected.txtLayoutTests/fast/webgpu/regression/repro_176812014.htmlLayoutTests/http/tests/security/contentSecurityPolicy/directive-name-case-insensitive-expected.txtLayoutTests/http/tests/security/contentSecurityPolicy/directive-name-case-insensitive-strict-dynamic-expected.txtLayoutTests/http/tests/security/contentSecurityPolicy/directive-name-case-insensitive-strict-dynamic.htmlLayoutTests/http/tests/security/contentSecurityPolicy/directive-name-case-insensitive.htmlLayoutTests/http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate-expected.txtLayoutTests/http/tests/storageAccess/request-storage-access-rejected-without-gesture-should-not-activate.htmlLayoutTests/http/tests/storageAccess/resources/request-storage-access-without-gesture-check-activation-iframe.htmlLayoutTests/http/wpt/site-isolation/history-traversal/history-traversal-navigate-parent-while-child-loading-expected.txtLayoutTests/http/wpt/site-isolation/history-traversal/history-traversal-navigate-parent-while-child-loading.htmlLayoutTests/http/wpt/webcodecs/audio-encoder-pcm-format-change-expected.txtLayoutTests/http/wpt/webcodecs/audio-encoder-pcm-format-change.htmlLayoutTests/imported/w3c/web-platform-tests/css/css-mixins/at-function-cssom-expected.txtLayoutTests/imported/w3c/web-platform-tests/css/css-mixins/dashed-function-cycles-expected.txtLayoutTests/imported/w3c/web-platform-tests/css/css-mixins/dashed-function-eval-expected.txtLayoutTests/imported/w3c/web-platform-tests/css/css-mixins/function-attr-expected.txtLayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/highlight-fill-color-ignores-stroke-color-expected.htmlLayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/highlight-fill-color-ignores-stroke-color-ref.htmlLayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/highlight-fill-color-ignores-stroke-color.htmlLayoutTests/imported/w3c/web-platform-tests/css/cssom/variable-names-expected.txtLayoutTests/imported/w3c/web-platform-tests/svg/animations/animateMotion-spline-invalid-keyTimes-expected.txtLayoutTests/imported/w3c/web-platform-tests/svg/styling/attr-style-media-dynamic-expected.txtLayoutTests/imported/w3c/web-platform-tests/svg/styling/attr-style-type-dynamic-expected.txtLayoutTests/imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks-expected.htmlLayoutTests/imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks-ref.htmlLayoutTests/imported/w3c/web-platform-tests/svg/text/reftests/textpath-combining-marks.htmlLayoutTests/inspector/runtime/getDisplayableProperties-expected.txtLayoutTests/inspector/runtime/getProperties-expected.txtLayoutTests/ipc/coreipc.jsLayoutTests/ipc/forged-resource-load-statistics-storage-access-expected.txtLayoutTests/ipc/forged-resource-load-statistics-storage-access.htmlLayoutTests/ipc/loadping-firstpartyforcookies-message-check-expected.txtLayoutTests/ipc/loadping-firstpartyforcookies-message-check.htmlLayoutTests/ipc/networksindexeddb-close-connection-during-version-change-expected.txtLayoutTests/ipc/networksindexeddb-close-connection-during-version-change.htmlLayoutTests/ipc/rgba16f-getpixelbuffer-colorspace-mismatch-heap-disclosure-expected.txtLayoutTests/ipc/rgba16f-getpixelbuffer-colorspace-mismatch-heap-disclosure.htmlLayoutTests/ipc/use-redirection-for-current-navigation-message-check-expected.txtLayoutTests/ipc/use-redirection-for-current-navigation-message-check.htmlLayoutTests/media/media-source/media-source-fudge-factor-expected.txtLayoutTests/media/media-source/media-source-fudge-factor.htmlLayoutTests/media/media-source/media-source-gap-policy-expected.txtLayoutTests/media/media-source/media-source-gap-policy.htmlLayoutTests/navigation-api/navigation-navigate-throwing-argument-conversion-expected.txtLayoutTests/navigation-api/navigation-navigate-throwing-argument-conversion.htmlLayoutTests/platform/ios/TestExpectationsLayoutTests/platform/ios/svg/compositing/anonymous-RenderSVGViewportContainer-no-repaints-expected.txtLayoutTests/platform/ios/svg/compositing/segment-removed-after-anchor-decomposited-layer-tree-expected.txtLayoutTests/platform/ios/svg/compositing/transform-change-repainting-viewBox-repaintRects-expected.txtLayoutTests/platform/ios/svg/transforms/nested-svg-transform-attribute-creates-layer-expected.txtLayoutTests/platform/mac-tahoe-wk2-lbse-text/svg/repaint/svg-outline-repaint-on-hover-expected.txtLayoutTests/storage/indexeddb/index-cursor-reverse-delete-next-higher-key-private-expected.txtLayoutTests/storage/indexeddb/index-cursor-reverse-delete-next-higher-key-private.htmlLayoutTests/webaudio/biquadfilternode-set-type-channel-count-race-expected.txtLayoutTests/webaudio/biquadfilternode-set-type-channel-count-race.htmlLayoutTests/workers/weblock-manager-query-crash-expected.txtLayoutTests/workers/weblock-manager-query-crash.htmlSource/JavaScriptCore/builtins/TypedArrayConstructor.jsSource/JavaScriptCore/bytecode/Repatch.cppSource/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.hSource/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cppSource/JavaScriptCore/dfg/DFGGraph.cppSource/JavaScriptCore/dfg/DFGOperations.cppSource/JavaScriptCore/dfg/DFGOperations.hSource/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cppSource/JavaScriptCore/dfg/DFGSpeculativeJIT64.cppSource/JavaScriptCore/dfg/DFGStoreBarrierInsertionPhase.cppSource/JavaScriptCore/ftl/FTLLowerDFGToB3.cppSource/JavaScriptCore/inspector/InjectedScriptSource.jsSource/JavaScriptCore/inspector/JSInjectedScriptHost.cppSource/JavaScriptCore/inspector/JSInjectedScriptHost.hSource/JavaScriptCore/inspector/JSInjectedScriptHostPrototype.cppSource/JavaScriptCore/runtime/JSObjectInlines.hSource/JavaScriptCore/runtime/OptionsList.hSource/JavaScriptCore/runtime/Structure.cppSource/JavaScriptCore/runtime/Structure.hSource/JavaScriptCore/wasm/WasmCallee.cppSource/JavaScriptCore/wasm/WasmCalleeGroup.cppSource/JavaScriptCore/yarr/YarrJIT.cppSource/JavaScriptCore/yarr/YarrJIT.hSource/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxprojSource/WTF/wtf/PlatformHave.hSource/WTF/wtf/TinyLRUCache.hSource/WebCore/Modules/indexeddb/server/MemoryIndexCursor.cppSource/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cppSource/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.hSource/WebCore/Modules/mediasession/MediaMetadata.cppSource/WebCore/Modules/mediasession/MediaMetadata.hSource/WebCore/Modules/mediasource/MediaSource.cppSource/WebCore/Modules/mediasource/MediaSource.hSource/WebCore/Modules/web-locks/WebLockManager.cppSource/WebCore/Modules/web-locks/WebLockManager.hSource/WebCore/Modules/webaudio/BiquadFilterNode.cppSource/WebCore/Modules/webcodecs/WebCodecsAudioData.cppSource/WebCore/Modules/webcodecs/WebCodecsAudioData.hSource/WebCore/bindings/js/JSDOMOperationReturningPromise.hSource/WebCore/bindings/js/JSDOMPromiseDeferred.cppSource/WebCore/bindings/js/JSDOMPromiseDeferred.hSource/WebCore/bindings/scripts/CodeGeneratorJS.pmSource/WebCore/bindings/scripts/test/JS/JSTestObj.cppSource/WebCore/css/CSSFunctionRule.cppSource/WebCore/css/StyleProperties.cppSource/WebCore/css/parser/CSSCustomPropertySyntax.cppSource/WebCore/css/parser/CSSCustomPropertySyntax.hSource/WebCore/dom/DocumentStorageAccess.cppSource/WebCore/dom/DocumentStorageAccess.hSource/WebCore/history/HistoryItem.hSource/WebCore/layout/integration/flex/LayoutIntegrationFlexLayout.cppSource/WebCore/layout/integration/grid/LayoutIntegrationGridLayout.cppSource/WebCore/loader/HistoryController.cppSource/WebCore/page/ChromeClient.hSource/WebCore/page/csp/ContentSecurityPolicy.cppSource/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cppSource/WebCore/page/csp/ContentSecurityPolicySourceList.cppSource/WebCore/platform/audio/cocoa/AudioEncoderCocoa.cppSource/WebCore/platform/audio/ios/MediaDeviceRoute.hSource/WebCore/platform/audio/ios/MediaDeviceRoute.mmSource/WebCore/platform/graphics/BitmapImageSource.cppSource/WebCore/platform/graphics/DestinationColorSpace.cppSource/WebCore/platform/graphics/DestinationColorSpace.hSource/WebCore/platform/graphics/MediaPlayerPrivateWirelessPlayback.cppSource/WebCore/platform/graphics/MediaPlayerPrivateWirelessPlayback.hSource/WebCore/platform/graphics/MediaSourcePrivate.cppSource/WebCore/platform/graphics/MediaSourcePrivate.hSource/WebCore/platform/graphics/PixelBufferConversion.cppSource/WebCore/platform/graphics/PlatformTimeRanges.cppSource/WebCore/platform/graphics/PlatformTimeRanges.hSource/WebCore/platform/graphics/SourceBufferPrivate.cppSource/WebCore/platform/graphics/SourceBufferPrivate.hSource/WebCore/platform/graphics/TrackBuffer.cppSource/WebCore/platform/graphics/TrackBuffer.hSource/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cppSource/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mmSource/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.hSource/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mmSource/WebCore/platform/graphics/cocoa/MediaPlayerPrivateWebM.mmSource/WebCore/platform/graphics/cpu/arm/filters/FEBlendNeonApplier.cppSource/WebCore/platform/graphics/filters/FilterImage.cppSource/WebCore/platform/graphics/filters/FilterImage.hSource/WebCore/platform/graphics/filters/software/FEGaussianBlurSoftwareApplier.cppSource/WebCore/platform/mock/mediasource/MockBox.cppSource/WebCore/rendering/RenderBox.cppSource/WebCore/rendering/RenderBox.hSource/WebCore/rendering/RenderBoxInlines.hSource/WebCore/rendering/RenderDeprecatedFlexibleBox.cppSource/WebCore/rendering/RenderFlexibleBox.cppSource/WebCore/rendering/RenderFrameSet.cppSource/WebCore/rendering/RenderHighlight.cppSource/WebCore/rendering/RenderListMarker.cppSource/WebCore/rendering/RenderReplaced.cppSource/WebCore/rendering/RenderScrollbarPart.cppSource/WebCore/rendering/RenderTableCell.cppSource/WebCore/rendering/StyledMarkedText.cppSource/WebCore/rendering/svg/RenderSVGForeignObject.cppSource/WebCore/rendering/svg/RenderSVGRoot.cppSource/WebCore/rendering/svg/SVGTextLayoutEngine.cppSource/WebCore/style/StyleBuilder.cppSource/WebCore/style/StyleBuilder.hSource/WebCore/style/StyleSubstitutionResolver.cppSource/WebCore/style/StyleSubstitutionResolver.hSource/WebCore/style/values/transforms/StyleTransformList.cppSource/WebCore/svg/SVGAnimationElement.cppSource/WebCore/svg/SVGGeometryElement.cppSource/WebCore/svg/SVGGeometryElement.hSource/WebCore/svg/SVGStyleElement.cppSource/WebCore/xml/parser/XMLDocumentParserLibxml2.cppSource/WebGPU/WebGPU/BindGroupLayout.mmSource/WebGPU/WebGPU/ComputePipeline.mmSource/WebGPU/WebGPU/Device.hSource/WebGPU/WebGPU/RenderPassEncoder.mmSource/WebGPU/WebGPU/RenderPipeline.mmSource/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cppSource/WebKit/GPUProcess/webrtc/UserMediaCaptureManagerProxy.cppSource/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cppSource/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.hSource/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cppSource/WebKit/NetworkProcess/NetworkConnectionToWebProcess.hSource/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.inSource/WebKit/NetworkProcess/NetworkResourceLoadParameters.hSource/WebKit/NetworkProcess/NetworkResourceLoadParameters.serialization.inSource/WebKit/NetworkProcess/NetworkResourceLoader.cppSource/WebKit/NetworkProcess/SharedWorker/WebSharedWorkerServer.cppSource/WebKit/NetworkProcess/storage/NetworkStorageManager.cppSource/WebKit/Scripts/process-entitlements.shSource/WebKit/Scripts/webkit/messages.pySource/WebKit/Shared/SessionState.cppSource/WebKit/Shared/SessionState.hSource/WebKit/Shared/SessionState.serialization.inSource/WebKit/Shared/WebCoreArgumentCoders.serialization.inSource/WebKit/Shared/WebCoreArgumentCodersPlatform.serialization.inSource/WebKit/Shared/WebPageCreationParameters.hSource/WebKit/Shared/WebPageCreationParameters.serialization.inSource/WebKit/Shared/cf/CoreIPCSecTrust.mmSource/WebKit/UIProcess/API/Cocoa/WKUserContentController.hSource/WebKit/UIProcess/API/Cocoa/WKUserContentController.mmSource/WebKit/UIProcess/API/Cocoa/WKWebView.mmSource/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.hSource/WebKit/UIProcess/API/Cocoa/_WKTextExtraction.hSource/WebKit/UIProcess/API/Cocoa/_WKTextExtraction.mmSource/WebKit/UIProcess/Extensions/API/WebExtensionContextAPIStorage.cppSource/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPIRuntimeCocoa.mmSource/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPITabsCocoa.mmSource/WebKit/UIProcess/WebBackForwardList.cppSource/WebKit/UIProcess/WebPageProxy.cppSource/WebKit/UIProcess/WebPageProxy.hSource/WebKit/UIProcess/WebProcessProxy.cppSource/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mmSource/WebKit/UIProcess/mac/WKAppKitGestureController.swiftSource/WebKit/UIProcess/mac/WebPageProxyMac.mmSource/WebKit/UIProcess/mac/WebViewImpl.hSource/WebKit/UIProcess/mac/WebViewImpl.mmSource/WebKit/WebProcess/Model/WebModelPlayer.mmSource/WebKit/WebProcess/Network/WebLoaderStrategy.cppSource/WebKit/WebProcess/WebCoreSupport/SessionStateConversion.cppSource/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cppSource/WebKit/WebProcess/WebCoreSupport/WebChromeClient.hSource/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mmSource/WebKit/WebProcess/WebPage/WebPage.cppSource/WebKit/WebProcess/WebPage/WebPage.hSource/WebKit/WebProcess/WebPage/WebPage.messages.inTools/Scripts/measure-build-timeTools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.pyTools/Scripts/webkitpy/layout_tests/controllers/manager.pyTools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.pyTools/TestWebKitAPI/CMakeLists.txtTools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxprojTools/TestWebKitAPI/Tests/WTF/TinyLRUCache.cppTools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cppTools/TestWebKitAPI/Tests/WebKit/WKBackForwardListTests.mmTools/TestWebKitAPI/Tests/WebKit/WKWebView/JSBuffer.mmTools/TestWebKitAPI/Tests/WebKit/WKWebView/ObscuredContentInsets.mmTools/TestWebKitAPI/Tests/WebKit/WKWebView/SiteIsolation.mmTools/TestWebKitAPI/Tests/WebKit/WKWebView/UIDelegate.mmWebsites/bugs.webkit.org/PrettyPatch/PrettyPatch.rbWebsites/bugs.webkit.org/committers-autocomplete.jsWebsites/bugs.webkit.org/extensions/Commits/Extension.pm
💤 Files with no reviewable changes (10)
- LayoutTests/media/media-source/media-source-fudge-factor.html
- Source/WebCore/Modules/mediasource/MediaSource.h
- LayoutTests/media/media-source/media-source-fudge-factor-expected.txt
- Source/JavaScriptCore/wasm/WasmCallee.cpp
- Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.h
- Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.serialization.in
- Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm
- Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h
- Source/WebCore/Modules/mediasource/MediaSource.cpp
- Source/WebKit/Scripts/process-entitlements.sh
Comment on lines
+10
to
+18
| try { | ||
| let lengthLie = proxyGet.length; | ||
| if (lengthLie === 999) { | ||
| throw "\"get\" trap successfully returned a lying value (999) for a non-configurable, non-writable property!"; | ||
| } | ||
| } catch (e) { | ||
| if (!(e instanceof TypeError)) { | ||
| throw "Expected TypeError for \"get\" trap invariant violation, got: " + e; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail when the invariant check does not throw.
Both blocks currently pass if the engine skips the proxy trap path and returns the ordinary target result instead of throwing. That makes this regression test too weak for the stale-structure bug it is trying to pin down.
Suggested fix
try {
- let lengthLie = proxyGet.length;
- if (lengthLie === 999) {
- throw "\"get\" trap successfully returned a lying value (999) for a non-configurable, non-writable property!";
- }
+ proxyGet.length;
+ throw "Expected TypeError for \"get\" trap invariant violation, but no exception was thrown";
} catch (e) {
if (!(e instanceof TypeError)) {
throw "Expected TypeError for \"get\" trap invariant violation, got: " + e;
}
}
@@
try {
- let setSuccess = Reflect.set(proxySet, "length", 999);
- if (setSuccess === true && target.length !== 999) {
- throw "Reflect.set returned true claiming success on a non-configurable, non-writable property!";
- }
+ Reflect.set(proxySet, "length", 999);
+ throw "Expected TypeError for \"set\" trap invariant violation, but no exception was thrown";
} catch (e) {
if (!(e instanceof TypeError)) {
throw "Expected TypeError for \"set\" trap invariant violation, got: " + e;
}
}Also applies to: 25-33
🤖 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/slowputarraystorage-stale-structure-bit.js` around lines 10 -
18, The proxy invariant test around proxyGet.length is too weak because it still
passes when no TypeError is thrown and the ordinary target value is returned.
Update the try/catch blocks to explicitly fail when the invariant violation does
not throw, using the existing proxyGet.length check and the TypeError validation
so the stale-structure regression only passes when the engine actually raises
the expected error.
Comment on lines
+26
to
+35
| try { | ||
| Int32Array.from(source, (val, index) => { | ||
| if (index === detachAt) | ||
| arrayBuffer.transfer(); | ||
| callbacks++; | ||
| return val; | ||
| }); | ||
| } catch (e) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't swallow unexpected exceptions in the detach regression.
This catch makes the test pass even when Int32Array.from() fails before the buffer is actually detached, so unrelated breakage can silently slip through.
Suggested fix
let callbacks = 0;
+ let didDetach = false;
try {
Int32Array.from(source, (val, index) => {
- if (index === detachAt)
+ if (index === detachAt) {
arrayBuffer.transfer();
+ didDetach = true;
+ }
callbacks++;
return val;
});
} catch (e) {
- return;
+ if (didDetach)
+ return;
+ throw e;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| Int32Array.from(source, (val, index) => { | |
| if (index === detachAt) | |
| arrayBuffer.transfer(); | |
| callbacks++; | |
| return val; | |
| }); | |
| } catch (e) { | |
| return; | |
| } | |
| let callbacks = 0; | |
| let didDetach = false; | |
| try { | |
| Int32Array.from(source, (val, index) => { | |
| if (index === detachAt) { | |
| arrayBuffer.transfer(); | |
| didDetach = true; | |
| } | |
| callbacks++; | |
| return val; | |
| }); | |
| } catch (e) { | |
| if (didDetach) | |
| return; | |
| throw e; | |
| } |
🤖 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/typedarray-from-oob.js` around lines 26 - 35, The try/catch
around Int32Array.from in typedarray-from-oob.js is swallowing all failures,
which can hide unrelated regressions before the buffer is actually detached.
Narrow the handling so only the expected detach-related exception path is
accepted, and let any other exception escape or fail the test explicitly; keep
the test logic around the Int32Array.from callback, arrayBuffer.transfer, and
callbacks count intact while making unexpected errors visible.
Comment on lines
+11
to
+29
| const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 320 } }); | ||
| const originalTrack = stream.getVideoTracks()[0]; | ||
| const clonedTrack = originalTrack.clone(); | ||
| const imageCapture = new ImageCapture(originalTrack); | ||
|
|
||
| for (let round = 0; round < 5; round++) { | ||
| const promises = []; | ||
| for (let i = 0; i < 10; i++) { | ||
| promises.push(imageCapture.takePhoto({imageWidth:1280, imageHeight:720}).catch(()=>{})); | ||
| promises.push(clonedTrack.applyConstraints({ | ||
| width:{ideal:320+(i*100)}, height:{ideal:240+(i*75)}, frameRate:{ideal:15+i} | ||
| }).catch(()=>{})); | ||
| } | ||
| await Promise.allSettled(promises); | ||
| } | ||
|
|
||
| originalTrack.stop(); | ||
| clonedTrack.stop(); | ||
| }, "applyConstraints while takePhoto is happening on a cloned track"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Register media cleanup with the harness.
The tracks are only stopped on the happy path. Any failure before Lines 27-28 leaves capture running and can contaminate later media tests.
Suggested fix
const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 320 } });
const originalTrack = stream.getVideoTracks()[0];
const clonedTrack = originalTrack.clone();
+ t.add_cleanup(() => {
+ originalTrack.stop();
+ clonedTrack.stop();
+ });
const imageCapture = new ImageCapture(originalTrack);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 320 } }); | |
| const originalTrack = stream.getVideoTracks()[0]; | |
| const clonedTrack = originalTrack.clone(); | |
| const imageCapture = new ImageCapture(originalTrack); | |
| for (let round = 0; round < 5; round++) { | |
| const promises = []; | |
| for (let i = 0; i < 10; i++) { | |
| promises.push(imageCapture.takePhoto({imageWidth:1280, imageHeight:720}).catch(()=>{})); | |
| promises.push(clonedTrack.applyConstraints({ | |
| width:{ideal:320+(i*100)}, height:{ideal:240+(i*75)}, frameRate:{ideal:15+i} | |
| }).catch(()=>{})); | |
| } | |
| await Promise.allSettled(promises); | |
| } | |
| originalTrack.stop(); | |
| clonedTrack.stop(); | |
| }, "applyConstraints while takePhoto is happening on a cloned track"); | |
| const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 320 } }); | |
| const originalTrack = stream.getVideoTracks()[0]; | |
| const clonedTrack = originalTrack.clone(); | |
| t.add_cleanup(() => { | |
| originalTrack.stop(); | |
| clonedTrack.stop(); | |
| }); | |
| const imageCapture = new ImageCapture(originalTrack); | |
| for (let round = 0; round < 5; round++) { | |
| const promises = []; | |
| for (let i = 0; i < 10; i++) { | |
| promises.push(imageCapture.takePhoto({imageWidth:1280, imageHeight:720}).catch(()=>{})); | |
| promises.push(clonedTrack.applyConstraints({ | |
| width:{ideal:320+(i*100)}, height:{ideal:240+(i*75)}, frameRate:{ideal:15+i} | |
| }).catch(()=>{})); | |
| } | |
| await Promise.allSettled(promises); | |
| } | |
| originalTrack.stop(); | |
| clonedTrack.stop(); | |
| }, "applyConstraints while takePhoto is happening on a cloned track"); |
🤖 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/mediastream/applyConstraints-with-takePhoto.html` around
lines 11 - 29, The media cleanup is only happening at the end of the test body,
so failures before the final stop calls can leave capture active and affect
later tests. Update the test around the existing getUserMedia/ImageCapture flow
to register cleanup with the test harness (using the appropriate media cleanup
helper) so both originalTrack and clonedTrack are always stopped even if
takePhoto or applyConstraints fails.
Comment on lines
+48
to
+58
| Promise.resolve().then(closeOne); | ||
| } | ||
|
|
||
| function pressureGC() { | ||
| if (Date.now() - startTime > DURATION_MS) | ||
| return; | ||
| for (let i = 0; i < 20; i++) { | ||
| let tmp = new AudioData(init); | ||
| tmp.close(); | ||
| } | ||
| setTimeout(pressureGC, 10); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='LayoutTests/fast/webcodecs/audio-data-close-during-gc-crash.html'
wc -l "$file"
sed -n '1,140p' "$file"Repository: oven-sh/WebKit
Length of output: 1650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Model a task queue with microtasks and timers to show whether an infinite
# microtask chain can prevent a repeating timer from re-entering.
microtasks = 0
timer_runs = 0
# Initial synchronous task schedules one microtask and one timer.
queue_microtask = True
queue_timer = True
# Simulate the event loop for a few turns.
for turn in range(3):
# Drain microtasks before any timer/task.
while queue_microtask:
queue_microtask = False
microtasks += 1
# closeOne() re-enqueues itself via Promise.resolve().then(closeOne)
queue_microtask = True
# In real JS, this would continue indefinitely and prevent timers.
if microtasks > 10:
break
if microtasks > 10:
break
if queue_timer:
timer_runs += 1
# pressureGC() re-schedules itself with setTimeout
queue_timer = True
print(f"microtasks_drained={microtasks}, timer_runs={timer_runs}")
PYRepository: oven-sh/WebKit
Length of output: 188
Yield back to the task queue here. Promise.resolve().then(closeOne) can keep the microtask queue non-empty, which starves the repeating setTimeout(pressureGC, 10) callback after the first pass. Use a task-based reschedule like setTimeout(closeOne, 0) so the GC-pressure loop actually runs.
Suggested fix
- Promise.resolve().then(closeOne);
+ setTimeout(closeOne, 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Promise.resolve().then(closeOne); | |
| } | |
| function pressureGC() { | |
| if (Date.now() - startTime > DURATION_MS) | |
| return; | |
| for (let i = 0; i < 20; i++) { | |
| let tmp = new AudioData(init); | |
| tmp.close(); | |
| } | |
| setTimeout(pressureGC, 10); | |
| setTimeout(closeOne, 0); | |
| } | |
| function pressureGC() { | |
| if (Date.now() - startTime > DURATION_MS) | |
| return; | |
| for (let i = 0; i < 20; i++) { | |
| let tmp = new AudioData(init); | |
| tmp.close(); | |
| } | |
| setTimeout(pressureGC, 10); |
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 57-57: React's useState should not be directly called
Context: setTimeout(pressureGC, 10)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 57-57: Avoid using the initial state variable in setState
Context: setTimeout(pressureGC, 10)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 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/webcodecs/audio-data-close-during-gc-crash.html` around
lines 48 - 58, The GC-pressure test is using a microtask reschedule in closeOne
via Promise.resolve().then(closeOne), which can starve the repeating pressureGC
task loop. Change closeOne to yield back to the task queue with a task-based
reschedule such as setTimeout(closeOne, 0), keeping the existing pressureGC and
closeOne flow intact so the GC-pressure loop continues to run.
Comment on lines
+4
to
+59
| async function run() { | ||
| const adapter = await navigator.gpu.requestAdapter(); | ||
| const device = await adapter.requestDevice(); | ||
|
|
||
| const indexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.INDEX, mappedAtCreation: true }); | ||
| new Uint32Array(indexBuffer.getMappedRange()).set([0, 1, 2, 0]); | ||
| indexBuffer.unmap(); | ||
|
|
||
| const vertexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.VERTEX, mappedAtCreation: true }); | ||
| new Float32Array(vertexBuffer.getMappedRange()).set([1.0, 2.0, 3.0, 4.0]); | ||
| vertexBuffer.unmap(); | ||
|
|
||
| const shaderModule = device.createShaderModule({ code: ` | ||
| @vertex fn vs(@location(0) pos: f32) -> @builtin(position) vec4<f32> { | ||
| return vec4<f32>(pos, 0.0, 0.0, 1.0); | ||
| } | ||
| @fragment fn fs() -> @location(0) vec4<f32> { | ||
| return vec4<f32>(1.0, 0.0, 0.0, 1.0); | ||
| } | ||
| `}); | ||
| const pipeline = device.createRenderPipeline({ | ||
| layout: 'auto', | ||
| vertex: { | ||
| module: shaderModule, entryPoint: 'vs', | ||
| buffers: [{ arrayStride: 4, stepMode: 'vertex', attributes: [{ shaderLocation: 0, offset: 0, format: 'float32' }] }] | ||
| }, | ||
| fragment: { module: shaderModule, entryPoint: 'fs', targets: [{ format: 'bgra8unorm' }] } | ||
| }); | ||
|
|
||
| const colorTexture = device.createTexture({ size: [1, 1], format: 'bgra8unorm', usage: GPUTextureUsage.RENDER_ATTACHMENT }); | ||
| const renderPassDescriptor = { | ||
| colorAttachments: [{ view: colorTexture.createView(), clearValue: [0, 0, 0, 1], loadOp: 'clear', storeOp: 'store' }] | ||
| }; | ||
|
|
||
| function draw(baseVertex) { | ||
| const commandEncoder = device.createCommandEncoder(); | ||
| const pass = commandEncoder.beginRenderPass(renderPassDescriptor); | ||
| pass.setPipeline(pipeline); | ||
| pass.setVertexBuffer(0, vertexBuffer); | ||
| pass.setIndexBuffer(indexBuffer, 'uint32'); | ||
| pass.drawIndexed(3, 1, 0, baseVertex, 0); | ||
| pass.end(); | ||
| device.queue.submit([commandEncoder.finish()]); | ||
| return device.queue.onSubmittedWorkDone(); | ||
| } | ||
|
|
||
| await draw(0); | ||
| await draw(-2147483648); | ||
|
|
||
| debug('Pass'); | ||
| globalThis.testRunner?.notifyDone(); | ||
| } | ||
|
|
||
| globalThis.testRunner?.dumpAsText(); | ||
| globalThis.testRunner?.waitUntilDone(); | ||
| run(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Always release waitUntilDone() on failure.
Any rejection before Line 54 leaves the test runner waiting forever. Wrap the body in try/finally and handle a missing adapter so this nocrash test fails diagnostically instead of timing out.
Suggested fix
async function run() {
- const adapter = await navigator.gpu.requestAdapter();
- const device = await adapter.requestDevice();
+ try {
+ const adapter = await navigator.gpu.requestAdapter();
+ if (!adapter)
+ throw new Error("WebGPU adapter unavailable");
+
+ const device = await adapter.requestDevice();
- const indexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.INDEX, mappedAtCreation: true });
- new Uint32Array(indexBuffer.getMappedRange()).set([0, 1, 2, 0]);
- indexBuffer.unmap();
+ const indexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.INDEX, mappedAtCreation: true });
+ new Uint32Array(indexBuffer.getMappedRange()).set([0, 1, 2, 0]);
+ indexBuffer.unmap();
- const vertexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.VERTEX, mappedAtCreation: true });
- new Float32Array(vertexBuffer.getMappedRange()).set([1.0, 2.0, 3.0, 4.0]);
- vertexBuffer.unmap();
+ const vertexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.VERTEX, mappedAtCreation: true });
+ new Float32Array(vertexBuffer.getMappedRange()).set([1.0, 2.0, 3.0, 4.0]);
+ vertexBuffer.unmap();
- const shaderModule = device.createShaderModule({ code: `
+ const shaderModule = device.createShaderModule({ code: `
`@vertex` fn vs(`@location`(0) pos: f32) -> `@builtin`(position) vec4<f32> {
return vec4<f32>(pos, 0.0, 0.0, 1.0);
}
@@
- const pipeline = device.createRenderPipeline({
+ const pipeline = device.createRenderPipeline({
layout: 'auto',
vertex: {
module: shaderModule, entryPoint: 'vs',
@@
- const colorTexture = device.createTexture({ size: [1, 1], format: 'bgra8unorm', usage: GPUTextureUsage.RENDER_ATTACHMENT });
- const renderPassDescriptor = {
- colorAttachments: [{ view: colorTexture.createView(), clearValue: [0, 0, 0, 1], loadOp: 'clear', storeOp: 'store' }]
- };
+ const colorTexture = device.createTexture({ size: [1, 1], format: 'bgra8unorm', usage: GPUTextureUsage.RENDER_ATTACHMENT });
+ const renderPassDescriptor = {
+ colorAttachments: [{ view: colorTexture.createView(), clearValue: [0, 0, 0, 1], loadOp: 'clear', storeOp: 'store' }]
+ };
- function draw(baseVertex) {
- const commandEncoder = device.createCommandEncoder();
- const pass = commandEncoder.beginRenderPass(renderPassDescriptor);
- pass.setPipeline(pipeline);
- pass.setVertexBuffer(0, vertexBuffer);
- pass.setIndexBuffer(indexBuffer, 'uint32');
- pass.drawIndexed(3, 1, 0, baseVertex, 0);
- pass.end();
- device.queue.submit([commandEncoder.finish()]);
- return device.queue.onSubmittedWorkDone();
- }
+ function draw(baseVertex) {
+ const commandEncoder = device.createCommandEncoder();
+ const pass = commandEncoder.beginRenderPass(renderPassDescriptor);
+ pass.setPipeline(pipeline);
+ pass.setVertexBuffer(0, vertexBuffer);
+ pass.setIndexBuffer(indexBuffer, 'uint32');
+ pass.drawIndexed(3, 1, 0, baseVertex, 0);
+ pass.end();
+ device.queue.submit([commandEncoder.finish()]);
+ return device.queue.onSubmittedWorkDone();
+ }
- await draw(0);
- await draw(-2147483648);
+ await draw(0);
+ await draw(-2147483648);
- debug('Pass');
- globalThis.testRunner?.notifyDone();
+ debug('Pass');
+ } catch (error) {
+ testFailed(String(error));
+ } finally {
+ globalThis.testRunner?.notifyDone();
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function run() { | |
| const adapter = await navigator.gpu.requestAdapter(); | |
| const device = await adapter.requestDevice(); | |
| const indexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.INDEX, mappedAtCreation: true }); | |
| new Uint32Array(indexBuffer.getMappedRange()).set([0, 1, 2, 0]); | |
| indexBuffer.unmap(); | |
| const vertexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.VERTEX, mappedAtCreation: true }); | |
| new Float32Array(vertexBuffer.getMappedRange()).set([1.0, 2.0, 3.0, 4.0]); | |
| vertexBuffer.unmap(); | |
| const shaderModule = device.createShaderModule({ code: ` | |
| @vertex fn vs(@location(0) pos: f32) -> @builtin(position) vec4<f32> { | |
| return vec4<f32>(pos, 0.0, 0.0, 1.0); | |
| } | |
| @fragment fn fs() -> @location(0) vec4<f32> { | |
| return vec4<f32>(1.0, 0.0, 0.0, 1.0); | |
| } | |
| `}); | |
| const pipeline = device.createRenderPipeline({ | |
| layout: 'auto', | |
| vertex: { | |
| module: shaderModule, entryPoint: 'vs', | |
| buffers: [{ arrayStride: 4, stepMode: 'vertex', attributes: [{ shaderLocation: 0, offset: 0, format: 'float32' }] }] | |
| }, | |
| fragment: { module: shaderModule, entryPoint: 'fs', targets: [{ format: 'bgra8unorm' }] } | |
| }); | |
| const colorTexture = device.createTexture({ size: [1, 1], format: 'bgra8unorm', usage: GPUTextureUsage.RENDER_ATTACHMENT }); | |
| const renderPassDescriptor = { | |
| colorAttachments: [{ view: colorTexture.createView(), clearValue: [0, 0, 0, 1], loadOp: 'clear', storeOp: 'store' }] | |
| }; | |
| function draw(baseVertex) { | |
| const commandEncoder = device.createCommandEncoder(); | |
| const pass = commandEncoder.beginRenderPass(renderPassDescriptor); | |
| pass.setPipeline(pipeline); | |
| pass.setVertexBuffer(0, vertexBuffer); | |
| pass.setIndexBuffer(indexBuffer, 'uint32'); | |
| pass.drawIndexed(3, 1, 0, baseVertex, 0); | |
| pass.end(); | |
| device.queue.submit([commandEncoder.finish()]); | |
| return device.queue.onSubmittedWorkDone(); | |
| } | |
| await draw(0); | |
| await draw(-2147483648); | |
| debug('Pass'); | |
| globalThis.testRunner?.notifyDone(); | |
| } | |
| globalThis.testRunner?.dumpAsText(); | |
| globalThis.testRunner?.waitUntilDone(); | |
| run(); | |
| async function run() { | |
| try { | |
| const adapter = await navigator.gpu.requestAdapter(); | |
| if (!adapter) | |
| throw new Error("WebGPU adapter unavailable"); | |
| const device = await adapter.requestDevice(); | |
| const indexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.INDEX, mappedAtCreation: true }); | |
| new Uint32Array(indexBuffer.getMappedRange()).set([0, 1, 2, 0]); | |
| indexBuffer.unmap(); | |
| const vertexBuffer = device.createBuffer({ size: 16, usage: GPUBufferUsage.VERTEX, mappedAtCreation: true }); | |
| new Float32Array(vertexBuffer.getMappedRange()).set([1.0, 2.0, 3.0, 4.0]); | |
| vertexBuffer.unmap(); | |
| const shaderModule = device.createShaderModule({ code: ` | |
| `@vertex` fn vs(`@location`(0) pos: f32) -> `@builtin`(position) vec4<f32> { | |
| return vec4<f32>(pos, 0.0, 0.0, 1.0); | |
| } | |
| `@fragment` fn fs() -> `@location`(0) vec4<f32> { | |
| return vec4<f32>(1.0, 0.0, 0.0, 1.0); | |
| } | |
| `}); | |
| const pipeline = device.createRenderPipeline({ | |
| layout: 'auto', | |
| vertex: { | |
| module: shaderModule, entryPoint: 'vs', | |
| buffers: [{ arrayStride: 4, stepMode: 'vertex', attributes: [{ shaderLocation: 0, offset: 0, format: 'float32' }] }] | |
| }, | |
| fragment: { module: shaderModule, entryPoint: 'fs', targets: [{ format: 'bgra8unorm' }] } | |
| }); | |
| const colorTexture = device.createTexture({ size: [1, 1], format: 'bgra8unorm', usage: GPUTextureUsage.RENDER_ATTACHMENT }); | |
| const renderPassDescriptor = { | |
| colorAttachments: [{ view: colorTexture.createView(), clearValue: [0, 0, 0, 1], loadOp: 'clear', storeOp: 'store' }] | |
| }; | |
| function draw(baseVertex) { | |
| const commandEncoder = device.createCommandEncoder(); | |
| const pass = commandEncoder.beginRenderPass(renderPassDescriptor); | |
| pass.setPipeline(pipeline); | |
| pass.setVertexBuffer(0, vertexBuffer); | |
| pass.setIndexBuffer(indexBuffer, 'uint32'); | |
| pass.drawIndexed(3, 1, 0, baseVertex, 0); | |
| pass.end(); | |
| device.queue.submit([commandEncoder.finish()]); | |
| return device.queue.onSubmittedWorkDone(); | |
| } | |
| await draw(0); | |
| await draw(-2147483648); | |
| debug('Pass'); | |
| } catch (error) { | |
| testFailed(String(error)); | |
| } finally { | |
| globalThis.testRunner?.notifyDone(); | |
| } | |
| } | |
| globalThis.testRunner?.dumpAsText(); | |
| globalThis.testRunner?.waitUntilDone(); | |
| run(); |
🤖 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/webgpu/nocrash/fuzz-176882934.html` around lines 4 - 59, The
run() test flow can reject before reaching testRunner.notifyDone(), leaving
waitUntilDone() hanging indefinitely. Update run() to use try/finally around the
main WebGPU setup and draw calls, and make sure testRunner.notifyDone() is
always called in the finally path. Also handle a missing adapter/device in run()
explicitly so this nocrash test reports a clear failure instead of timing out.
Comment on lines
+1262
to
+1268
| #if ((PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 160000) \ | ||
| || ((PLATFORM(IOS) || PLATFORM(MACCATALYST)) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 190000) \ | ||
| || (PLATFORM(VISION) && __VISION_OS_VERSION_MIN_REQUIRED >= 30000)) \ | ||
| || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 120000) \ | ||
| || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 190000) | ||
| #define HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT 1 | ||
| #endif |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm there are exactly two unconditional definitions of this macro in the file.
rg -n 'define HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT' Source/WTF/wtf/PlatformHave.hRepository: oven-sh/WebKit
Length of output: 258
🏁 Script executed:
#!/bin/bash
sed -n '1208,1272p' Source/WTF/wtf/PlatformHave.hRepository: oven-sh/WebKit
Length of output: 3014
Consolidate the overlapping HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT guards
PlatformHave.h defines this macro twice with overlapping conditions. The later block adds Mac 160000–259999 coverage, while the earlier block already enables it for Mac 260000+ and all non-Mac platforms. Merge these into one condition (or make the later one !defined(...)) to avoid the redundant duplicate.
🤖 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 `@Source/WTF/wtf/PlatformHave.h` around lines 1262 - 1268, The
HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT guard is duplicated with overlapping
platform/version conditions, so consolidate the two checks into a single
definition or make the later block conditional on the macro not already being
defined. Update the related preprocessor logic in PlatformHave.h around
HAVE_WK_SECURE_CODING_AVOUTPUTCONTEXT so the Mac 160000–259999 and the existing
Mac 260000+/non-Mac coverage are handled without redefining the macro.
Comment on lines
+55
to
+72
| TinyLRUCache(const TinyLRUCache& other) | ||
| : m_cacheBuffer(other.m_cacheBuffer) | ||
| , m_size(other.m_size) | ||
| { | ||
| invalidateIterators(); | ||
| other.invalidateIterators(); | ||
| } | ||
|
|
||
| TinyLRUCache& operator=(const TinyLRUCache& other) | ||
| { | ||
| if (this == &other) | ||
| return *this; | ||
| invalidateIterators(); | ||
| other.invalidateIterators(); | ||
| m_cacheBuffer = other.m_cacheBuffer; | ||
| m_size = other.m_size; | ||
| return *this; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for const-qualified invalidateIterators overloads and any copy/assignment usage of TinyLRUCache.
ast-grep run --pattern 'void invalidateIterators() const' --lang cpp Source/WTF/wtf/TinyLRUCache.h
rg -nP --type=cpp -B2 -A2 'TinyLRUCache<[^>]*>\s+\w+\s*=\s*\w+\s*;|TinyLRUCache<[^>]*>\s*\(\s*\w+\s*\)' Source/Repository: oven-sh/WebKit
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -e
printf '== TinyLRUCache.h size ==\n'
wc -l Source/WTF/wtf/TinyLRUCache.h
printf '\n== TinyLRUCache.h outline ==\n'
ast-grep outline Source/WTF/wtf/TinyLRUCache.h --match TinyLRUCache --view expanded || true
printf '\n== Relevant lines around invalidateIterators ==\n'
sed -n '1,260p' Source/WTF/wtf/TinyLRUCache.h | nl -ba | sed -n '1,260p' | rg -n 'invalidateIterators|TinyLRUCache\(|operator=' -n -C 3
printf '\n== Search for TinyLRUCache usages ==\n'
rg -n --type=cpp 'TinyLRUCache<' Source/ | head -200
printf '\n== Search for copy/assignment of TinyLRUCache variable names ==\n'
rg -n --type=cpp 'TinyLRUCache<[^>]+>\s+\w+\s*=\s*\w+\s*;' Source/ || true
rg -n --type=cpp 'TinyLRUCache<[^>]+>\s+\w+\s*\(\s*\w+\s*\)' Source/ || trueRepository: oven-sh/WebKit
Length of output: 372
🏁 Script executed:
#!/bin/bash
set -e
printf '== TinyLRUCache.h (relevant sections) ==\n'
sed -n '1,240p' Source/WTF/wtf/TinyLRUCache.h | cat -n
printf '\n== TinyLRUCache usages in Source/ ==\n'
rg -n --type=cpp 'TinyLRUCache<' Source/ || true
printf '\n== Any copy/assignment sites for TinyLRUCache ==\n'
rg -n --type=cpp 'TinyLRUCache<[^>]+>\s+\w+\s*=\s*\w+\s*;' Source/ || true
rg -n --type=cpp 'TinyLRUCache<[^>]+>\s+\w+\s*\(\s*\w+\s*\)' Source/ || trueRepository: oven-sh/WebKit
Length of output: 12511
Remove the other.invalidateIterators() calls
Copying from a const TinyLRUCache& invokes a non-const member on other, so any copy/assignment use of this template will fail to compile. The extra invalidation is unnecessary anyway; only the destination object's existing iterator state needs clearing.
🤖 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 `@Source/WTF/wtf/TinyLRUCache.h` around lines 55 - 72, Remove the unnecessary
other.invalidateIterators() calls from TinyLRUCache’s copy constructor and
operator=; both TinyLRUCache(const TinyLRUCache&) and TinyLRUCache::operator=
should only invalidate the destination object’s iterators before copying
m_cacheBuffer and m_size, since calling a non-const method on the const source
breaks compilation.
| fd, tmp_path = tempfile.mkstemp(prefix=f'{PLUGIN_NAME}_', suffix='.stripped') | ||
| os.close(fd) | ||
| try: | ||
| result = subprocess.run(['strip', '--strip-all', '-o', tmp_path, object_path], capture_output=True, text=True) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the strip subprocess call lint-clean.
Line 47 uses a PATH-resolved executable and omits check=..., matching the provided Ruff/Pylint findings. Resolve the binary once and pass check=False explicitly.
Proposed fix
+import shutil
import subprocess
import tempfile
@@
def get_stripped_object_size(object_path):
fd, tmp_path = tempfile.mkstemp(prefix=f'{PLUGIN_NAME}_', suffix='.stripped')
os.close(fd)
try:
- result = subprocess.run(['strip', '--strip-all', '-o', tmp_path, object_path], capture_output=True, text=True)
+ strip_path = shutil.which('strip')
+ if not strip_path:
+ raise RuntimeError("'strip' executable not found")
+ result = subprocess.run([strip_path, '--strip-all', '-o', tmp_path, object_path], capture_output=True, text=True, check=False)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| result = subprocess.run(['strip', '--strip-all', '-o', tmp_path, object_path], capture_output=True, text=True) | |
| import shutil | |
| import subprocess | |
| import tempfile | |
| strip_path = shutil.which('strip') | |
| if not strip_path: | |
| raise RuntimeError("'strip' executable not found") | |
| result = subprocess.run([strip_path, '--strip-all', '-o', tmp_path, object_path], capture_output=True, text=True, check=False) |
🧰 Tools
🪛 Pylint (4.0.6)
[warning] 47-47: 'subprocess.run' used without explicitly defining the value for 'check'.
(W1510)
🪛 Ruff (0.15.20)
[error] 47-47: subprocess call: check for execution of untrusted input
(S603)
[warning] 47-47: subprocess.run without explicit check argument
Add explicit check=False
(PLW1510)
[error] 47-47: Starting a process with a partial executable path
(S607)
🤖 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 `@Tools/Scripts/webkitpy/browserperfdash/plans/browser_binary_size.py` at line
47, The subprocess call in browser_binary_size.py is not lint-clean because it
relies on a PATH lookup for strip and omits an explicit check argument. Update
the logic around the subprocess.run call in the browser_binary_size flow to
resolve the strip binary once before use, then invoke subprocess.run with that
resolved executable and check=False explicitly, keeping the existing
capture_output and text behavior unchanged.
Source: Linters/SAST tools
Comment on lines
+339
to
+353
| RefPtr<ImageBuffer> imageBuffer = ImageBuffer::create(size, RenderingMode::Unaccelerated, RenderingPurpose::Unspecified, scale, sourceColorSpace, sourcePixelFormat); | ||
| EXPECT_NE(nullptr, imageBuffer); | ||
|
|
||
| auto& context = imageBuffer->context(); | ||
| context.fillRect(fillRect, Color::green); | ||
|
|
||
| auto getPixelBufferAllZeros = [&](const FloatRect& rect) { | ||
| RetainPtr platformColorSpace = adoptCF(CGColorSpaceCreateWithName(kCGColorSpaceGenericCMYK)); | ||
| auto destinationColorSpace = DestinationColorSpace(WTF::move(platformColorSpace)); | ||
| PixelBufferFormat destinationPixelFormat { AlphaPremultiplication::Unpremultiplied, PixelFormat::RGBA8, destinationColorSpace }; | ||
|
|
||
| RefPtr pixelBuffer = imageBuffer->getPixelBuffer(destinationPixelFormat, enclosingIntRect(rect)); | ||
| EXPECT_NE(nullptr, pixelBuffer); | ||
|
|
||
| auto bytes = pixelBuffer->bytes(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Avoid null dereferences in the new test.
EXPECT_NE is non-fatal, so setup failures can still crash at imageBuffer->context() or pixelBuffer->bytes().
Proposed fix
RefPtr<ImageBuffer> imageBuffer = ImageBuffer::create(size, RenderingMode::Unaccelerated, RenderingPurpose::Unspecified, scale, sourceColorSpace, sourcePixelFormat);
- EXPECT_NE(nullptr, imageBuffer);
+ ASSERT_NE(nullptr, imageBuffer);
@@
RefPtr pixelBuffer = imageBuffer->getPixelBuffer(destinationPixelFormat, enclosingIntRect(rect));
EXPECT_NE(nullptr, pixelBuffer);
+ if (!pixelBuffer)
+ return false;
auto bytes = pixelBuffer->bytes();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| RefPtr<ImageBuffer> imageBuffer = ImageBuffer::create(size, RenderingMode::Unaccelerated, RenderingPurpose::Unspecified, scale, sourceColorSpace, sourcePixelFormat); | |
| EXPECT_NE(nullptr, imageBuffer); | |
| auto& context = imageBuffer->context(); | |
| context.fillRect(fillRect, Color::green); | |
| auto getPixelBufferAllZeros = [&](const FloatRect& rect) { | |
| RetainPtr platformColorSpace = adoptCF(CGColorSpaceCreateWithName(kCGColorSpaceGenericCMYK)); | |
| auto destinationColorSpace = DestinationColorSpace(WTF::move(platformColorSpace)); | |
| PixelBufferFormat destinationPixelFormat { AlphaPremultiplication::Unpremultiplied, PixelFormat::RGBA8, destinationColorSpace }; | |
| RefPtr pixelBuffer = imageBuffer->getPixelBuffer(destinationPixelFormat, enclosingIntRect(rect)); | |
| EXPECT_NE(nullptr, pixelBuffer); | |
| auto bytes = pixelBuffer->bytes(); | |
| RefPtr<ImageBuffer> imageBuffer = ImageBuffer::create(size, RenderingMode::Unaccelerated, RenderingPurpose::Unspecified, scale, sourceColorSpace, sourcePixelFormat); | |
| ASSERT_NE(nullptr, imageBuffer); | |
| auto& context = imageBuffer->context(); | |
| context.fillRect(fillRect, Color::green); | |
| auto getPixelBufferAllZeros = [&](const FloatRect& rect) { | |
| RetainPtr platformColorSpace = adoptCF(CGColorSpaceCreateWithName(kCGColorSpaceGenericCMYK)); | |
| auto destinationColorSpace = DestinationColorSpace(WTF::move(platformColorSpace)); | |
| PixelBufferFormat destinationPixelFormat { AlphaPremultiplication::Unpremultiplied, PixelFormat::RGBA8, destinationColorSpace }; | |
| RefPtr pixelBuffer = imageBuffer->getPixelBuffer(destinationPixelFormat, enclosingIntRect(rect)); | |
| EXPECT_NE(nullptr, pixelBuffer); | |
| if (!pixelBuffer) | |
| return false; | |
| auto bytes = pixelBuffer->bytes(); |
🤖 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 `@Tools/TestWebKitAPI/Tests/WebCore/ImageBufferTests.cpp` around lines 339 -
353, The new ImageBuffer test can still dereference null after non-fatal
EXPECT_NE checks, so move the setup assertions in ImageBufferTests::... to fatal
checks. Use ASSERT_NE for the ImageBuffer returned by ImageBuffer::create and
for the pixelBuffer returned by imageBuffer->getPixelBuffer before calling
imageBuffer->context() or pixelBuffer->bytes(), so the test stops immediately on
setup failure.
|
|
||
| RetainPtr oddLength = adoptNS([[WKJSScriptingBuffer alloc] initWithData:[NSData dataWithBytes:"abc" length:3]]); | ||
| RetainPtr evenLength = adoptNS([[_WKJSBuffer alloc] initWithData:[NSData dataWithBytes:"abcd" length:4]]); | ||
| RetainPtr evenLength = adoptNS([NSData dataWithBytes:"abcd" length:4]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'RetainPtr.h' Source/WTF --exec sed -n '1,220p' {}
rg -n 'adoptNS\(\[NSData dataWithBytes' Tools/TestWebKitAPI SourceRepository: oven-sh/WebKit
Length of output: 9429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file context
sed -n '1,120p' Tools/TestWebKitAPI/Tests/WebKit/WKWebView/JSBuffer.mm
# Find the adoptNS implementation and surrounding ownership helpers
rg -n "adoptNS|adoptNSArc|retainPtr\(" Source/WTF -g 'RetainPtr.h' -A 40 -B 20
# Check whether this test file is compiled under ARC or MRR-related conditions
rg -n "JSBuffer.mm|ARC|OBJC_ARC|CLANG_ENABLE_OBJC_ARC|fobjc-arc" Tools TestWebKitAPI Source -g '!**/build/**'Repository: oven-sh/WebKit
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- JSBuffer.mm ---\n'
sed -n '1,90p' Tools/TestWebKitAPI/Tests/WebKit/WKWebView/JSBuffer.mm
printf '\n--- RetainPtr ownership docs ---\n'
sed -n '1,120p' Source/WTF/wtf/RetainPtr.h
printf '\n--- adoptNS implementation ---\n'
rg -n "template<typename T>.*adoptNS|adoptNSArc|adoptNS\\(" Source/WTF/wtf/RetainPtr.h -A 20 -B 10
printf '\n--- ARC flags near this test target ---\n'
rg -n "JSBuffer\\.mm|OBJC_ARC|CLANG_ENABLE_OBJC_ARC|fobjc-arc" Tools/TestWebKitAPI Source --max-count 40Repository: oven-sh/WebKit
Length of output: 44418
Don't adoptNS an autoreleased NSData. dataWithBytes:length: returns a +0 object, so this RetainPtr will over-release it when it goes out of scope. Use retainPtr(...) here or switch to an alloc/init construction.
🤖 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 `@Tools/TestWebKitAPI/Tests/WebKit/WKWebView/JSBuffer.mm` at line 41, The
NSData ownership in the JSBuffer test is incorrect: `dataWithBytes:length:`
returns an autoreleased +0 object, so `adoptNS` in `JSBuffer.mm` will
over-release it. Update the `evenLength` initialization to use `retainPtr(...)`
instead, or construct the NSData with an `alloc/init` path, keeping the fix
localized to the `evenLength` setup in this test.
Preview Builds
|
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Jun 30, 2026
…cc3d833c) Add a regression test for #32793: a Date.now() call site that tiers into the DFG/FTL used to stop honoring the setSystemTime() override, because operationDateNow returned the wall clock instead of going through JSGlobalObject::jsDateNow(). Fixed upstream of Bun in oven-sh/WebKit#257, which this bump delivers.
Collaborator
Author
|
Verification against this PR's preview artifact is done. CI here produced all 43 platform artifacts with no job failures. I built a full Bun debug binary against It did turn up one thing, now reflected in the PR description. #257 changed This is ready to land. Please use a merge commit, not a squash, so the upstream parent link survives for the next upgrade. |
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Jun 30, 2026
oven-sh/WebKit#263 has landed on main as c9ad5813fd23, so the preview tag (autobuild-preview-pr-263-f652829d) is replaced by the real one, which is the hand-off the file's own comment called for. It supersedes the #261 tag: #263 is a further upstream upgrade (to d81bcc3d833c) stacked on top of #261's merge, so one version now carries both. Also surface the child's stderr in the three spawned fixture tests when the child exits nonzero, so an abort shows its stack trace in the assertion diff instead of only a wrong exit code.
robobun
added a commit
to oven-sh/bun
that referenced
this pull request
Jun 30, 2026
…x makes pass The NaN sentinel for JSGlobalObject::overridenDateNow (578331b, paired with oven-sh/WebKit#263) makes negative and pre-epoch clock values work, so the three fake-timers tests that assert negative `now` flooring and ticks on the negative side of the epoch now pass. A `.failing` test that passes is reported as a failure, so the stale markers have to come off. The remaining `.failing` markers in the Sinon suite cover unrelated features (custom install targets, huge-timeout overflow) and are unaffected.
Jarred-Sumner
pushed a commit
to oven-sh/bun
that referenced
this pull request
Jul 1, 2026
Bumps `vendor/WebKit` to upstream `WebKit/WebKit@d81bcc3d833c` (2026-06-30). 887 upstream commits since the last Bun sync point (`b5ba38a21e17`, 2026-06-16); 117 touch `Source/JavaScriptCore`, 54 touch `Source/WTF`, 10 touch `Source/bmalloc`. The fork-side work is split across two merge PRs: - oven-sh/WebKit#261: `b5ba38a21e17` to `ce481c4cfedb`, merged as `0d9ee56ba4f6`. - oven-sh/WebKit#263: `ce481c4cfedb` to `d81bcc3d833c`, based on the fork's current `main`, so it also carries oven-sh/WebKit#257 and oven-sh/WebKit#262, which landed on the fork between the two merges and which fix real Bun bugs (see below). > [!NOTE] > oven-sh/WebKit#263 has landed on `main` as `c9ad5813fd23`, and `WEBKIT_VERSION` now points at that commit's release, `autobuild-c9ad5813fd23bd8b98b0738abc3d037ec716aa92`. Nothing else is pending on the WebKit side. It was landed as a squash rather than a merge commit, but `c9ad5813fd23`'s git tree is byte-identical to the preview head `f652829d78`'s tree (verified: both are `470eee60f5`), so the final artifacts are built from exactly the source all of the verification below ran against. The squash does drop the upstream parent link, so whoever does the next fork upgrade will need a merge-base fixup like the one that followed oven-sh/WebKit#251. Fixes #32793 ## Bun changes required by this upgrade ### From `b5ba38a2 -> ce481c4c`: three builtin rewrites Upstream removed three bytecode intrinsics that Bun's built-in JS modules use (https://bugs.webkit.org/show_bug.cgi?id=318076 and https://bugs.webkit.org/show_bug.cgi?id=318028). Bumping the version without these rewrites aborts on every `Buffer.from()` call: ``` ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); ``` and on `new console.Console(stream).table(...)` (the global `console.table` is native and was unaffected). - `src/js/builtins/JSBufferConstructor.ts`: `$tryGetByIdWithWellKnownSymbol(value, "toPrimitive")` becomes an ordinary `value[Symbol.toPrimitive]` read, which is exactly what Node's `lib/buffer.js` does. The removed intrinsic never invoked accessors, so this is also a Node-compat improvement: a getter-defined `Symbol.toPrimitive` is now honored. `test/js/node/buffer-from-symbol-to-primitive.test.ts` covers it and fails on the released Bun. - `src/js/builtins/ConsoleObject.ts`: `$isMapIterator` / `$isSetIterator` become `require("node:util/types").isMapIterator / isSetIterator`, which is exactly what Node's `internal/console/constructor.js` does. `test/js/node/console/console-table-iterators.test.ts` is a snapshot test that passes on both the released Bun and this branch, proving the rewrite is behavior-preserving (there was no prior coverage for `Console#table` on a Map/Set iterator). - `src/js/builtins.d.ts`: the declarations for all ten removed intrinsics are deleted so nothing reaches for them again. ### From oven-sh/WebKit#257: `jest` fake-time code must use the new `NaN` sentinel oven-sh/WebKit#257 changed the "no override is active" sentinel on `JSGlobalObject::overridenDateNow` from `-1` to `NaN` (see `JSGlobalObject::jsDateNow()`). Bun's reset paths still wrote `-1`, which the new semantics read as a *live* override frozen at `-1` ms, so `jest.useRealTimers()` and `setSystemTime()` with no argument left `Date.now()` stuck at `1969-12-31T23:59:59.999Z`. `test/js/bun/test/test-timers.test.ts` ("we can go back in time") caught this against a Bun built from the #263 artifact. - `src/runtime/test_runner/timers/FakeTimers.rs`: `CurrentTime::clear` (the `jest.useRealTimers()` path) writes `f64::NAN` instead of `-1.0`. - `src/jsc/bindings/JSMockFunction.cpp`, `JSMock__jsSetSystemTime`: the reset value becomes `PNaN`. The old `>= 0` / `std::isnormal()` gating existed only to serve the `-1` sentinel and silently dropped every pre-epoch and zero override (a bug the #257 review called out); with the `NaN` sentinel every real timestamp is a valid override, so `setSystemTime(-1)` and `setSystemTime(new Date("1960-01-01"))` now work. A new test in `test/js/bun/test/test-timers.test.ts` covers the pre-epoch, zero, `-1`, and no-argument-reset cases and fails on Bun 1.4.0. - Deleted `JSMock__jsUseRealTimers` from `JSMockFunction.cpp`: nothing registers it (`jest.useRealTimers` is the Rust `use_real_timers` in `FakeTimers.rs`), and it was the third place that would have needed the sentinel change. `ce481c4c -> d81bcc3d` itself (the 84 upstream commits) requires nothing: no bytecode intrinsics changed, `JSType.h` did not change, and the only WebCore code-generator change is to a helper Bun does not use. ## Bun bugs fixed by the fork-side commits this carries - oven-sh/WebKit#257: `Date.now()` is tagged `DateNowIntrinsic`, so a hot call site is inlined by the DFG/FTL as a `DateNow` node, and `operationDateNow` returned the wall clock instead of reading `JSGlobalObject::overridenDateNow`. A `setSystemTime()` override therefore silently wore off as soon as a `Date.now()` call site tiered up (#32793). `test/regression/issue/32793.test.ts` spawns the loop from that issue and asserts the override never diverges; on the released Bun it diverges at iteration 99, and it also fails against a build of this branch's previous pin (`0d9ee56ba4f6`, which predates #257), so the test isolates exactly this commit. - oven-sh/WebKit#262: a top-level `import()` whose embedder fetch rejects with a value that is not an `ErrorInstance` (a single-message transpile failure rejects with a `BuildMessage`) poisoned its module registry entry, and every later importer of the same file hung forever. The Bun-side regression test lives in #33149, which also pins a WebKit preview; once this lands, #33149 reduces to its test plus its `scripts/sync-webkit-source.ts` improvement. ## Why the regression tests are spawned fixtures Without the three builtin rewrites, a Bun built against this WebKit aborts on the first `Buffer.from()` call: ``` ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); ``` `Buffer.from` is fundamental enough that this takes down the `bun test` runner itself, before any test file finishes loading and before the JUnit reporter can write anything. So `buffer-from-symbol-to-primitive.test.ts` and `console-table-iterators.test.ts` live in two small files of their own and spawn a child Bun instead of calling the affected APIs in process: a child abort becomes an ordinary `exitCode`/`stdout` assertion failure, whereas an in-process call takes the parent test runner down with it and reports nothing. Their fail-before is "this branch minus the `src/js/` rewrites", which is a deterministic SIGABRT: ```sh git checkout $(git merge-base HEAD origin/main) -- src/js/ # keep scripts/ (the WEBKIT_VERSION bump) bun bd test test/js/node/buffer-from-symbol-to-primitive.test.ts # => ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(...) (SIGABRT) git checkout HEAD -- src/js/ ``` `test/js/bun/test/test-timers.test.ts` is the equivalent fail-before for the sentinel change: with the `src/` diff reverted but the new WebKit kept, both "we can go back in time" and the new `setSystemTime` test fail. `test/regression/issue/32793.test.ts` is different from all of the above: its fix lives entirely in the vendored engine, selected by `scripts/build/deps/webkit.ts` rather than by anything under `src/`. So its before/after is a function of which WebKit artifact is linked, not of the `src/` diff: it fails on the released Bun (1.4.0), fails against this branch built with the previous pin `0d9ee56ba4f6`, and passes against this branch built with the #263 artifact. ## Verification All of the following were run against a debug Bun built from this branch and the `autobuild-preview-pr-263-f652829d` artifact. - The four test files this PR adds or modifies (`test/js/bun/test/test-timers.test.ts`, `test/regression/issue/32793.test.ts`, `test/js/node/buffer-from-symbol-to-primitive.test.ts`, `test/js/node/console/console-table-iterators.test.ts`): 5 tests, all pass. Every one fails in the right "before" state as described above. - Every test file in the repo that exercises `setSystemTime` / `useFakeTimers` / `useRealTimers` / `advanceTimers*` (the blast radius of the sentinel change): `test/js/bun/test/fake-timers/fake-timers.test.ts`, `test/js/bun/test/fake-timers/sinonjs/fake-timers.test.ts`, `test/js/bun/cron/in-process-cron.test.ts`, `test/regression/issue/25869.test.ts`, `test/regression/issue/26284.test.ts`, and the five `test/js/third_party/jsonwebtoken/*.test.js` suites. 249 pass, 1 todo, 0 fail. - One more file turned out to be in that blast radius: `test/js/bun/test/fake-timers/sinonjs/issue-207.test.ts`. The sentinel change makes pre-epoch and negative clock values work, so its three long-`.failing` tests ("should floor negative now values", "should floor negative start times", "should handle ticks on the negative side of the Epoch") started passing, which Bun's runner reports as a failure until the stale marker is removed. `d2309c92c2` removes them; all ten tests in the file pass against this branch. - `test/js/node/buffer.test.js`: 539 pass, 1 pre-existing skip, 0 fail. `test/js/node/console/console.test.ts`: 7 pass. Neither file is modified by this PR. - oven-sh/WebKit#263's preview build produced all 43 platform artifacts with no job failures. ## Upstream changelog <details> <summary><code>ce481c4c</code> -> <code>d81bcc3d</code> (the new hop in this revision)</summary> Memory safety and security: - `TypedArray.from()` out-of-bounds read via resizable `ArrayBuffer` resize/transfer inside the `mapFn` callback - Stale structure bit in `SlowPutArrayStorage` - Do not cache property absence on dictionary structures - Missing `codeBlock->m_lock` in `repatchGetBySlowPathCall` - Insert a write barrier for `MultiPutByOffset` when it can reallocate storage - Unconditionally keep `OMGOSREntryCallee` alive while updating its callsites - Disallow defining private names on WasmGC objects - Size limit on Yarr-generated code Correctness: - DFG spread did not account for `cellButterflyOnlyAtomStringsStructure` - YarrJIT `negativeOffsetIndexedAddress` discarded the adjusted base register - `DFGArgumentsEliminationPhase::removeViaKill` did not reset the node scan index between `InlineCallFrame`s Other: - An `ArrayStorage`-specific `GetByVal` DFG operation - ES2022 private method support in the Web Inspector - WTF: `TinyLRUCache::findIfCached` / `insert` </details> <details> <summary>Security / correctness fixes (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - Use-after-free after Wasm memory grow via a stale pointer folded by DFGConstantFoldingPhase - Keep `JSWebAssemblyMemory` alive from wasm-originated `JSArrayBuffer`s (UAF) - Use-after-free of `StreamingCompiler::m_ticket` - `BBQCallee` kept alive between callsite collection and repatch (upstreams and extends oven-sh/WebKit#210) - `Array.from(arguments)` fast path bails out past `MAX_STORAGE_VECTOR_LENGTH` - `JSArray::fastFlat` bounds-checked against the maximum legal length - `Array#flat` fast path was dropping prototype-backed holes in nested arrays - `Array.from(map.keys())` fast path was ignoring `Symbol.iterator` overrides - `RegExp#@@split` crash on a huge flags string - `LiteralParser` takes the slow path if the original structure changes - Missing exit-profile bails on some DFG nodes - `IteratorClose` in `forEachInIterable` uses the Map/Set's own realm's iterator structure - YARR: JIT backreference with a non-BMP character was skipping the next match start position - Wasm Memory64: fix parsing of data segment init expressions - Track `customSlotBase` for `CustomAccessorGetter` / `CustomAccessorSetter` - Set `MayStoreHole` appropriately; `"entries"` ArrayIterator emits `ExitOK` before `NewArray` - `poisonedDeadOSRExitValue` made crashier and on by default in debug builds </details> <details> <summary>Language features and spec alignment (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - BigInt Math proposal implemented - Temporal: spec-alignment passes across `PlainDate`, `PlainTime`, `Duration`, `Instant`, plus a unified `ParseISODateTime` - `Intl.PluralRules` reflects `compactDisplay` in its skeleton and `resolvedOptions()` </details> <details> <summary>Performance (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> JavaScript-visible fast paths: - `JSON.stringify` space/indentation argument handled by `FastStringifier` - Fast path for the `RegExp#flags` getter; cached named-capture `groups` Structure on `RegExp` - Map/Set iterator `next` and `%RegExpStringIteratorPrototype%.next` moved to C++ - int32 fast path in `parseInt` for short decimal strings - `NumericStrings` int cache lookup inlined into DFG/FTL `ToString(Int32)` - `Object#isPrototypeOf` uses the `InstanceOf` DFG node - `Object.freeze` / `seal` / `preventExtensions` no longer allocate `ArrayStorage` for `NonArray` objects - Map/Set `set` + `delete` churn no longer expands then immediately shrinks - Avoided an intermediate `StringImpl` allocation feeding `StringBuilder` into `makeString` - Wasm `intoCharCodeArray` bulk-copies 8-bit strings; unnecessary watchpoints avoided - `DFG::GetByVal` widens to `SpecBytecodeTop` instead of forcing an OSR exit on an empty value profile YARR (the RegExp engine): - `latin1Table` for efficient Char8 filtering, reused for Char16 and the interpreter - `StringList` optimization when captures are unused; atom fast path enabled for unicode patterns JIT compile time (a large batch aimed at DFG/FTL/B3/Air throughput): - Semi-NCA algorithm for `WTF::Dominators`; reverse post-order in `WTF::Liveness` and B3 `ReduceStrength`; `SparseBitVector` in `DFGLivenessAnalysis` - Unified `SSACalculator`; dominance frontiers computed once per graph - `AirFixObviousSpills`, `AirEliminateDeadCode`, `Air::RegLiveness`, and the greedy register allocator all got cheaper - B3 CSE reimplemented without `B3::Variable`; `B3::Value::effects` uses a constant table - `ReduceStrength` avoids fixed-point iteration; `InferSwitch` disabled in B3 for Wasm - Fixup-inserted `RegExp` primordial `TryGetById` chains replaced with a single `CheckStructure` - Parser: create save points only when actually needed - Baseline/LLInt profile `op_unsigned` overflow and report int32 overflow to the DFG </details> <details> <summary>WTF, bmalloc, and the build system (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - `SaturatedArithmetic.h` and its APIs renamed to `Saturating` (Bun does not use these directly) - Checked arithmetic helpers now require two or more arguments - `OrderedHashTable` load-factor policy aligned with `HashTable` - `JSON::Value::dump` no longer corrupts large string values via `PrintStream` truncation - Structured clone: the remaining terminal JS value tags moved from WebCore into JSC (`CloneSerializerBase.h` / `StructuredCloneTags.h`). Bun keeps its own fork of `SerializedScriptValue.cpp`, so this is additive. - Workaround for a GCC 14+ false `-Wuninitialized` in `Variant`; `[[msvc::no_unique_address]]` used on MSVC - WTF / libpas exception codes changed to `0xbb08` - libpas: per-heap tagging policy rework; `pas_deallocate` split into inline-only and casual variants; PGO applied to WTF/bmalloc - CMake: prefix headers expanded, headermap generation reworked, stale staged headers pruned, and the Darwin platform block in `BPlatform.h` / `pas_platform.h` no longer excludes CMake builds (restored in the fork, see oven-sh/WebKit#261; without it the Linux-to-macOS cross lane fails to link `gigacageEnabledForProcess`) </details>
This was referenced Jul 2, 2026
liooil
pushed a commit
to liooil/poly
that referenced
this pull request
Aug 7, 2026
Bumps `vendor/WebKit` to upstream `WebKit/WebKit@d81bcc3d833c` (2026-06-30). 887 upstream commits since the last Bun sync point (`b5ba38a21e17`, 2026-06-16); 117 touch `Source/JavaScriptCore`, 54 touch `Source/WTF`, 10 touch `Source/bmalloc`. The fork-side work is split across two merge PRs: - oven-sh/WebKit#261: `b5ba38a21e17` to `ce481c4cfedb`, merged as `0d9ee56ba4f6`. - oven-sh/WebKit#263: `ce481c4cfedb` to `d81bcc3d833c`, based on the fork's current `main`, so it also carries oven-sh/WebKit#257 and oven-sh/WebKit#262, which landed on the fork between the two merges and which fix real Bun bugs (see below). > [!NOTE] > oven-sh/WebKit#263 has landed on `main` as `c9ad5813fd23`, and `WEBKIT_VERSION` now points at that commit's release, `autobuild-c9ad5813fd23bd8b98b0738abc3d037ec716aa92`. Nothing else is pending on the WebKit side. It was landed as a squash rather than a merge commit, but `c9ad5813fd23`'s git tree is byte-identical to the preview head `f652829d78`'s tree (verified: both are `470eee60f5`), so the final artifacts are built from exactly the source all of the verification below ran against. The squash does drop the upstream parent link, so whoever does the next fork upgrade will need a merge-base fixup like the one that followed oven-sh/WebKit#251. Fixes #32793 ## Bun changes required by this upgrade ### From `b5ba38a2 -> ce481c4c`: three builtin rewrites Upstream removed three bytecode intrinsics that Bun's built-in JS modules use (https://bugs.webkit.org/show_bug.cgi?id=318076 and https://bugs.webkit.org/show_bug.cgi?id=318028). Bumping the version without these rewrites aborts on every `Buffer.from()` call: ``` ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); ``` and on `new console.Console(stream).table(...)` (the global `console.table` is native and was unaffected). - `src/js/builtins/JSBufferConstructor.ts`: `$tryGetByIdWithWellKnownSymbol(value, "toPrimitive")` becomes an ordinary `value[Symbol.toPrimitive]` read, which is exactly what Node's `lib/buffer.js` does. The removed intrinsic never invoked accessors, so this is also a Node-compat improvement: a getter-defined `Symbol.toPrimitive` is now honored. `test/js/node/buffer-from-symbol-to-primitive.test.ts` covers it and fails on the released Bun. - `src/js/builtins/ConsoleObject.ts`: `$isMapIterator` / `$isSetIterator` become `require("node:util/types").isMapIterator / isSetIterator`, which is exactly what Node's `internal/console/constructor.js` does. `test/js/node/console/console-table-iterators.test.ts` is a snapshot test that passes on both the released Bun and this branch, proving the rewrite is behavior-preserving (there was no prior coverage for `Console#table` on a Map/Set iterator). - `src/js/builtins.d.ts`: the declarations for all ten removed intrinsics are deleted so nothing reaches for them again. ### From oven-sh/WebKit#257: `jest` fake-time code must use the new `NaN` sentinel oven-sh/WebKit#257 changed the "no override is active" sentinel on `JSGlobalObject::overridenDateNow` from `-1` to `NaN` (see `JSGlobalObject::jsDateNow()`). Bun's reset paths still wrote `-1`, which the new semantics read as a *live* override frozen at `-1` ms, so `jest.useRealTimers()` and `setSystemTime()` with no argument left `Date.now()` stuck at `1969-12-31T23:59:59.999Z`. `test/js/bun/test/test-timers.test.ts` ("we can go back in time") caught this against a Bun built from the #263 artifact. - `src/runtime/test_runner/timers/FakeTimers.rs`: `CurrentTime::clear` (the `jest.useRealTimers()` path) writes `f64::NAN` instead of `-1.0`. - `src/jsc/bindings/JSMockFunction.cpp`, `JSMock__jsSetSystemTime`: the reset value becomes `PNaN`. The old `>= 0` / `std::isnormal()` gating existed only to serve the `-1` sentinel and silently dropped every pre-epoch and zero override (a bug the #257 review called out); with the `NaN` sentinel every real timestamp is a valid override, so `setSystemTime(-1)` and `setSystemTime(new Date("1960-01-01"))` now work. A new test in `test/js/bun/test/test-timers.test.ts` covers the pre-epoch, zero, `-1`, and no-argument-reset cases and fails on Bun 1.4.0. - Deleted `JSMock__jsUseRealTimers` from `JSMockFunction.cpp`: nothing registers it (`jest.useRealTimers` is the Rust `use_real_timers` in `FakeTimers.rs`), and it was the third place that would have needed the sentinel change. `ce481c4c -> d81bcc3d` itself (the 84 upstream commits) requires nothing: no bytecode intrinsics changed, `JSType.h` did not change, and the only WebCore code-generator change is to a helper Bun does not use. ## Bun bugs fixed by the fork-side commits this carries - oven-sh/WebKit#257: `Date.now()` is tagged `DateNowIntrinsic`, so a hot call site is inlined by the DFG/FTL as a `DateNow` node, and `operationDateNow` returned the wall clock instead of reading `JSGlobalObject::overridenDateNow`. A `setSystemTime()` override therefore silently wore off as soon as a `Date.now()` call site tiered up (#32793). `test/regression/issue/32793.test.ts` spawns the loop from that issue and asserts the override never diverges; on the released Bun it diverges at iteration 99, and it also fails against a build of this branch's previous pin (`0d9ee56ba4f6`, which predates #257), so the test isolates exactly this commit. - oven-sh/WebKit#262: a top-level `import()` whose embedder fetch rejects with a value that is not an `ErrorInstance` (a single-message transpile failure rejects with a `BuildMessage`) poisoned its module registry entry, and every later importer of the same file hung forever. The Bun-side regression test lives in #33149, which also pins a WebKit preview; once this lands, #33149 reduces to its test plus its `scripts/sync-webkit-source.ts` improvement. ## Why the regression tests are spawned fixtures Without the three builtin rewrites, a Bun built against this WebKit aborts on the first `Buffer.from()` call: ``` ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); ``` `Buffer.from` is fundamental enough that this takes down the `bun test` runner itself, before any test file finishes loading and before the JUnit reporter can write anything. So `buffer-from-symbol-to-primitive.test.ts` and `console-table-iterators.test.ts` live in two small files of their own and spawn a child Bun instead of calling the affected APIs in process: a child abort becomes an ordinary `exitCode`/`stdout` assertion failure, whereas an in-process call takes the parent test runner down with it and reports nothing. Their fail-before is "this branch minus the `src/js/` rewrites", which is a deterministic SIGABRT: ```sh git checkout $(git merge-base HEAD origin/main) -- src/js/ # keep scripts/ (the WEBKIT_VERSION bump) bun bd test test/js/node/buffer-from-symbol-to-primitive.test.ts # => ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(...) (SIGABRT) git checkout HEAD -- src/js/ ``` `test/js/bun/test/test-timers.test.ts` is the equivalent fail-before for the sentinel change: with the `src/` diff reverted but the new WebKit kept, both "we can go back in time" and the new `setSystemTime` test fail. `test/regression/issue/32793.test.ts` is different from all of the above: its fix lives entirely in the vendored engine, selected by `scripts/build/deps/webkit.ts` rather than by anything under `src/`. So its before/after is a function of which WebKit artifact is linked, not of the `src/` diff: it fails on the released Bun (1.4.0), fails against this branch built with the previous pin `0d9ee56ba4f6`, and passes against this branch built with the #263 artifact. ## Verification All of the following were run against a debug Bun built from this branch and the `autobuild-preview-pr-263-f652829d` artifact. - The four test files this PR adds or modifies (`test/js/bun/test/test-timers.test.ts`, `test/regression/issue/32793.test.ts`, `test/js/node/buffer-from-symbol-to-primitive.test.ts`, `test/js/node/console/console-table-iterators.test.ts`): 5 tests, all pass. Every one fails in the right "before" state as described above. - Every test file in the repo that exercises `setSystemTime` / `useFakeTimers` / `useRealTimers` / `advanceTimers*` (the blast radius of the sentinel change): `test/js/bun/test/fake-timers/fake-timers.test.ts`, `test/js/bun/test/fake-timers/sinonjs/fake-timers.test.ts`, `test/js/bun/cron/in-process-cron.test.ts`, `test/regression/issue/25869.test.ts`, `test/regression/issue/26284.test.ts`, and the five `test/js/third_party/jsonwebtoken/*.test.js` suites. 249 pass, 1 todo, 0 fail. - One more file turned out to be in that blast radius: `test/js/bun/test/fake-timers/sinonjs/issue-207.test.ts`. The sentinel change makes pre-epoch and negative clock values work, so its three long-`.failing` tests ("should floor negative now values", "should floor negative start times", "should handle ticks on the negative side of the Epoch") started passing, which Bun's runner reports as a failure until the stale marker is removed. `d2309c92c2` removes them; all ten tests in the file pass against this branch. - `test/js/node/buffer.test.js`: 539 pass, 1 pre-existing skip, 0 fail. `test/js/node/console/console.test.ts`: 7 pass. Neither file is modified by this PR. - oven-sh/WebKit#263's preview build produced all 43 platform artifacts with no job failures. ## Upstream changelog <details> <summary><code>ce481c4c</code> -> <code>d81bcc3d</code> (the new hop in this revision)</summary> Memory safety and security: - `TypedArray.from()` out-of-bounds read via resizable `ArrayBuffer` resize/transfer inside the `mapFn` callback - Stale structure bit in `SlowPutArrayStorage` - Do not cache property absence on dictionary structures - Missing `codeBlock->m_lock` in `repatchGetBySlowPathCall` - Insert a write barrier for `MultiPutByOffset` when it can reallocate storage - Unconditionally keep `OMGOSREntryCallee` alive while updating its callsites - Disallow defining private names on WasmGC objects - Size limit on Yarr-generated code Correctness: - DFG spread did not account for `cellButterflyOnlyAtomStringsStructure` - YarrJIT `negativeOffsetIndexedAddress` discarded the adjusted base register - `DFGArgumentsEliminationPhase::removeViaKill` did not reset the node scan index between `InlineCallFrame`s Other: - An `ArrayStorage`-specific `GetByVal` DFG operation - ES2022 private method support in the Web Inspector - WTF: `TinyLRUCache::findIfCached` / `insert` </details> <details> <summary>Security / correctness fixes (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - Use-after-free after Wasm memory grow via a stale pointer folded by DFGConstantFoldingPhase - Keep `JSWebAssemblyMemory` alive from wasm-originated `JSArrayBuffer`s (UAF) - Use-after-free of `StreamingCompiler::m_ticket` - `BBQCallee` kept alive between callsite collection and repatch (upstreams and extends oven-sh/WebKit#210) - `Array.from(arguments)` fast path bails out past `MAX_STORAGE_VECTOR_LENGTH` - `JSArray::fastFlat` bounds-checked against the maximum legal length - `Array#flat` fast path was dropping prototype-backed holes in nested arrays - `Array.from(map.keys())` fast path was ignoring `Symbol.iterator` overrides - `RegExp#@@split` crash on a huge flags string - `LiteralParser` takes the slow path if the original structure changes - Missing exit-profile bails on some DFG nodes - `IteratorClose` in `forEachInIterable` uses the Map/Set's own realm's iterator structure - YARR: JIT backreference with a non-BMP character was skipping the next match start position - Wasm Memory64: fix parsing of data segment init expressions - Track `customSlotBase` for `CustomAccessorGetter` / `CustomAccessorSetter` - Set `MayStoreHole` appropriately; `"entries"` ArrayIterator emits `ExitOK` before `NewArray` - `poisonedDeadOSRExitValue` made crashier and on by default in debug builds </details> <details> <summary>Language features and spec alignment (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - BigInt Math proposal implemented - Temporal: spec-alignment passes across `PlainDate`, `PlainTime`, `Duration`, `Instant`, plus a unified `ParseISODateTime` - `Intl.PluralRules` reflects `compactDisplay` in its skeleton and `resolvedOptions()` </details> <details> <summary>Performance (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> JavaScript-visible fast paths: - `JSON.stringify` space/indentation argument handled by `FastStringifier` - Fast path for the `RegExp#flags` getter; cached named-capture `groups` Structure on `RegExp` - Map/Set iterator `next` and `%RegExpStringIteratorPrototype%.next` moved to C++ - int32 fast path in `parseInt` for short decimal strings - `NumericStrings` int cache lookup inlined into DFG/FTL `ToString(Int32)` - `Object#isPrototypeOf` uses the `InstanceOf` DFG node - `Object.freeze` / `seal` / `preventExtensions` no longer allocate `ArrayStorage` for `NonArray` objects - Map/Set `set` + `delete` churn no longer expands then immediately shrinks - Avoided an intermediate `StringImpl` allocation feeding `StringBuilder` into `makeString` - Wasm `intoCharCodeArray` bulk-copies 8-bit strings; unnecessary watchpoints avoided - `DFG::GetByVal` widens to `SpecBytecodeTop` instead of forcing an OSR exit on an empty value profile YARR (the RegExp engine): - `latin1Table` for efficient Char8 filtering, reused for Char16 and the interpreter - `StringList` optimization when captures are unused; atom fast path enabled for unicode patterns JIT compile time (a large batch aimed at DFG/FTL/B3/Air throughput): - Semi-NCA algorithm for `WTF::Dominators`; reverse post-order in `WTF::Liveness` and B3 `ReduceStrength`; `SparseBitVector` in `DFGLivenessAnalysis` - Unified `SSACalculator`; dominance frontiers computed once per graph - `AirFixObviousSpills`, `AirEliminateDeadCode`, `Air::RegLiveness`, and the greedy register allocator all got cheaper - B3 CSE reimplemented without `B3::Variable`; `B3::Value::effects` uses a constant table - `ReduceStrength` avoids fixed-point iteration; `InferSwitch` disabled in B3 for Wasm - Fixup-inserted `RegExp` primordial `TryGetById` chains replaced with a single `CheckStructure` - Parser: create save points only when actually needed - Baseline/LLInt profile `op_unsigned` overflow and report int32 overflow to the DFG </details> <details> <summary>WTF, bmalloc, and the build system (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - `SaturatedArithmetic.h` and its APIs renamed to `Saturating` (Bun does not use these directly) - Checked arithmetic helpers now require two or more arguments - `OrderedHashTable` load-factor policy aligned with `HashTable` - `JSON::Value::dump` no longer corrupts large string values via `PrintStream` truncation - Structured clone: the remaining terminal JS value tags moved from WebCore into JSC (`CloneSerializerBase.h` / `StructuredCloneTags.h`). Bun keeps its own fork of `SerializedScriptValue.cpp`, so this is additive. - Workaround for a GCC 14+ false `-Wuninitialized` in `Variant`; `[[msvc::no_unique_address]]` used on MSVC - WTF / libpas exception codes changed to `0xbb08` - libpas: per-heap tagging policy rework; `pas_deallocate` split into inline-only and casual variants; PGO applied to WTF/bmalloc - CMake: prefix headers expanded, headermap generation reworked, stale staged headers pruned, and the Darwin platform block in `BPlatform.h` / `pas_platform.h` no longer excludes CMake builds (restored in the fork, see oven-sh/WebKit#261; without it the Linux-to-macOS cross lane fails to link `gigacageEnabledForProcess`) </details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge upstream WebKit
d81bcc3d833cinto the Bun fork.Previous merge base:
ce481c4cfedb(#261, merged earlier today as0d9ee56ba4f6). 84 upstream commits since then; 13 touchSource/JavaScriptCore(23 files), 2 touchSource/WTF(2 files), none touchSource/bmalloc. The branch is based on the currentmain(8ea8da715eca), so it also carries #257 and #262, which landed after #261.The merge commit's second parent is
d81bcc3d833cfce4e4b892e4c3758454c01498b6, upstreammainas of the time this PR was opened.What this brings in
Memory safety and security fixes in JSC:
TypedArray.from()out-of-bounds read via resizableArrayBufferresize/transfer inside themapFncallbackSlowPutArrayStoragecodeBlock->m_lockinrepatchGetBySlowPathCallMultiPutByOffsetwhen it can reallocate storageOMGOSREntryCalleealive while updating its callsitesCorrectness fixes:
cellButterflyOnlyAtomStringsStructurenegativeOffsetIndexedAddressdiscarded the adjusted base registerDFGArgumentsEliminationPhase::removeViaKilldid not reset the node scan index betweenInlineCallFramesPlus an
ArrayStorage-specificGetByValDFG operation, ES2022 private method support in the Web Inspector, andTinyLRUCache::findIfCached/insertin WTF.Conflict resolutions
None. The merge was conflict free.
Nine files are modified by both the fork's patch set and the upstream range (
DFGAbstractInterpreterInlines.h,DFGOperations.cpp/.h,FTLLowerDFGToB3.cpp,JSObjectInlines.h,OptionsList.h,Structure.h,WasmCalleeGroup.cpp,YarrJIT.h); all nine auto-merged. The one with a real semantic interaction isWasmCalleeGroup.cpp: upstream (https://bugs.webkit.org/show_bug.cgi?id=313063) deleted thebbqCalleeKeptAlivebookkeeping and made theOMGOSREntryCalleekeepalive unconditional, while the fork's only delta there is the#if ENABLE(WEBASSEMBLY_BBQJIT)guard around theVector<Ref<BBQCallee>>declaration (carried forward from #261). The merged function keeps the guard on the declaration, and every use ofkeepAliveBBQCalleesis inside the same guard, so the configurations that build withWEBASSEMBLY_BBQJIToff still compile. The fork's post-#261 changes (operationDateNow(JSGlobalObject*)from #257, thesetFetchErrorbranch inJSMicrotask.cppfrom #262, and theBUILDING_WITH_CMAKEguards inBPlatform.h/pas_platform.h) are all present in the merged tree.Heads up for the matching oven-sh/bun bump
From the 84 upstream commits in this merge: nothing.
src/js/).Source/JavaScriptCore/runtime/JSType.hdid not change, so Bun'sJSType.rsmirror stays in sync.ReturnsPromisePairtrampoline fix (https://bugs.webkit.org/show_bug.cgi?id=313618); Bun does not usecallPromisePairFunctionor theReturnsPromisePairattribute and builds against the JSCOnly artifacts, so this does not reach it..github/workflowsdid not change.From #257, which this branch carries because it is based on the current
main: one required Bun-side change. #257 movedJSGlobalObject::overridenDateNow's "no override is active" sentinel from-1toNaN. Bun'sjest.useRealTimers()/setSystemTime()reset paths still wrote-1, which the new semantics read as a live override frozen at-1ms, so after a resetDate.now()stayed stuck at1969-12-31T23:59:59.999Z. I found this by building Bun against this PR's preview artifact and running Bun's fake-timer tests; the fix (writeNaN) ships in oven-sh/bun#33133 together with theWEBKIT_VERSIONbump.oven-sh/bun#33133 currently pins this PR's preview release (
autobuild-preview-pr-263-f652829d) and will be repointed at the merge commit once this lands.Verification
autobuild-preview-pr-263-f652829d).bun-webkit-linux-amd64-debug-asanartifact from this PR. Against it I ran every Bun test file that exercises the fake-timer API (the blast radius of DFG/FTL: have the Date.now() intrinsic honor overridenDateNow #257) plus theBuffer,console, and JSC-builtin suites affected by Upgrade to upstream WebKit ce481c4cfedb #261's rewrites: 800 tests, 0 failures. Details in Upgrade WebKit to d81bcc3d833c bun#33133.Please land this with a merge commit rather than a squash, for the same reason as #261: a squash drops the upstream parent link and breaks the merge base for the next upgrade.