Open
Conversation
pascoej
pushed a commit
that referenced
this pull request
Nov 30, 2022
…a rejected promise https://bugs.webkit.org/show_bug.cgi?id=247785 rdar://102325201 Reviewed by Yusuke Suzuki. Rest parameter should be caught in async function. So, running this JavaScript program should print "caught". ``` async function f(...[[]]) { } f().catch(e => print("caught")); ``` V8 (used console.log) ``` $ node input.js caught ``` GraalJS ``` $ js input.js caught ``` https://tc39.es/ecma262/#sec-async-function-definitions ... AsyncFunctionDeclaration[Yield, Await, Default] : async [no LineTerminator here] function BindingIdentifier[?Yield, ?Await] ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody } [+Default] async [no LineTerminator here] function ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody } AsyncFunctionExpression : async [no LineTerminator here] function BindingIdentifier[~Yield, +Await]opt ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody } ... According to the spec, it indicates `FormalParameters` is used for Async Function, where `FormalParameters` can be converted to `FunctionRestParameter`. https://tc39.es/ecma262/#sec-parameter-lists ... FormalParameters[Yield, Await] : [empty] FunctionRestParameter[?Yield, ?Await] FormalParameterList[?Yield, ?Await] FormalParameterList[?Yield, ?Await] , FormalParameterList[?Yield, ?Await] , FunctionRestParameter[?Yield, ?Await] ... And based on RS: EvaluateAsyncFunctionBody, it will invoke the promise.reject callback function with abrupt value ([[value]] of non-normal completion record). https://tc39.es/ecma262/#sec-runtime-semantics-evaluateasyncfunctionbody ... 2. Let declResult be Completion(FunctionDeclarationInstantiation(functionObject, argumentsList)). 3. If declResult is an abrupt completion, then a. Perform ! Call(promiseCapability.[[Reject]], undefined, « declResult.[[Value]] »). ... In that case, any non-normal results of evaluating rest parameters should be caught and passed to the reject callback function. To resolve this problem, we should allow the emitted RestParameterNode be wrapped by the catch handler for promise. However, we should remove `m_restParameter` and emit rest parameter byte code in `initializeDefaultParameterValuesAndSetupFunctionScopeStack` if we can prove that change has no side effect. In that case, we can only use one exception handler. Current fix is to add another exception handler. And move the handler byte codes to the bottom of code block in order to make other byte codes as much compact as possible. Input: ``` async function f(arg0, ...[[]]) { } f(); ``` Dumped Byte Codes: ``` ... bb#2 Predecessors: [ #1 ] [ 20] mov dst:loc9, src:<JSValue()>(const0) ... bb#3 Predecessors: [ WebKit#2 ] [ 29] get_rest_length dst:loc11, numParametersToSkip:1 ... bb#12 Predecessors: [ WebKit#8 WebKit#9 WebKit#10 ] [ 138] new_func_exp dst:loc10, scope:loc4, functionDecl:0 ... bb#13 Predecessors: [ ] [ 170] catch exception:loc10, thrownValue:loc8 [ 174] jmp targetLabel:8(->182) Successors: [ WebKit#15 ] bb#14 Predecessors: [ WebKit#7 WebKit#11 ] [ 176] catch exception:loc10, thrownValue:loc8 [ 180] jmp targetLabel:2(->182) Successors: [ WebKit#15 ] bb#15 Predecessors: [ WebKit#13 WebKit#14 ] [ 182] mov dst:loc12, src:Undefined(const1) ... Exception Handlers: 1: { start: [ 20] end: [ 29] target: [ 170] } synthesized catch 2: { start: [ 29] end: [ 138] target: [ 176] } synthesized catch ``` * JSTests/stress/catch-rest-parameter.js: Added. (throwError): (shouldThrow): (async f): (throwError.async f): (throwError.async let): (async let): (x.async f): (x): (async shouldThrow): * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::BytecodeGenerator): (JSC::BytecodeGenerator::initializeDefaultParameterValuesAndSetupFunctionScopeStack): * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h: Canonical link: https://commits.webkit.org/256864@main
pascoej
pushed a commit
that referenced
this pull request
Jan 4, 2023
https://bugs.webkit.org/show_bug.cgi?id=249765 rdar://103631099 Reviewed by Mark Lam. In ARM64, we are leveraging LDR style address, which can take 32bit index in addressing and zero-extend / sign-extend that in load/store. This is useful since WasmAddress' index is 32bit and we need to zero-extend it. However, we cannot use this addressing when there is an offset since this addressing cannot encode offset. As a result, we are emitting Move32 and Add64 when there is an offset. However, ARM64 can do even better for that case since ARM64 add / sub instructions also support LDR style extension. This patch adds AddZeroExtend64 and AddSignExtend64. They take 32bit second operand and extend it before adding. This is particularly useful when computing WasmAddress. We also leverage this in AirIRGenerator. In the added testb3, the generated code is changed as follows. Before: O2: testWasmAddressWithOffset()... Generated JIT code for Compilation: Code at [0x115f74980, 0x115f749a0): <0> 0x115f74980: pacibsp <4> 0x115f74984: stp fp, lr, [sp, #-16]! <8> 0x115f74988: mov fp, sp <12> 0x115f7498c: ubfx x0, x0, #0, WebKit#32; emitSave <16> 0x115f74990: add x0, x2, x0 <20> 0x115f74994: sturb w1, [x0, #1] <24> 0x115f74998: ldp fp, lr, [sp], WebKit#16 <28> 0x115f7499c: retab After: O2: testWasmAddressWithOffset()... Generated JIT code for Compilation: Code at [0x121108980, 0x1211089a0): <0> 0x121108980: pacibsp <4> 0x121108984: stp fp, lr, [sp, #-16]! <8> 0x121108988: mov fp, sp <12> 0x12110898c: add x0, x2, w0, uxtw; emitSave <16> 0x121108990: sturb w1, [x0, #1] <20> 0x121108994: ldp fp, lr, [sp], WebKit#16 <24> 0x121108998: retab * Source/JavaScriptCore/assembler/MacroAssemblerARM64.h: (JSC::MacroAssemblerARM64::addZeroExtend64): (JSC::MacroAssemblerARM64::addSignExtend64): * Source/JavaScriptCore/b3/B3LowerToAir.cpp: * Source/JavaScriptCore/b3/air/AirInstInlines.h: (JSC::B3::Air::isAddZeroExtend64Valid): (JSC::B3::Air::isAddSignExtend64Valid): * Source/JavaScriptCore/b3/air/AirOpcode.opcodes: Canonical link: https://commits.webkit.org/258259@main
pascoej
pushed a commit
that referenced
this pull request
Jan 7, 2023
https://bugs.webkit.org/show_bug.cgi?id=250196 rdar://98798050 Reviewed by Simon Fraser and Dean Jackson. WebKit has long accidentally depended on the combination of two somewhat unusual behavioral quirks in CGIOSurfaceContext: 1) (Source) If you make a CGImageRef from one CGIOSurfaceContext via CGIOSurfaceContextCreateImage, and mutate the original IOSurface under the hood (or in a different process) in a way that CGIOSurfaceContext does not know, CGIOSurfaceContextCreateImage will return the same CGImageRef when called later. 2) (Destination) If you make a CGImageRef from one CGIOSurfaceContext via CGIOSurfaceContextCreateImage, paint it into a different CGIOSurfaceContext, then mutate the original IOSurface, and paint the same CGImageRef again, the updated IOSurface contents will be used the second time. The second quirk has never worked with unaccelerated CoreGraphics bitmap context destinations. Instead, in the unaccelerated case, the CGImageRef acts as a snapshot of the surface at the time it was created. We've long had code to handle this, forcing CGIOSurfaceContextCreateImage to re-create the CGImageRef each time we paint it (by drawing an empty rect into the CGIOSurfaceContext), working around quirk #1 and thus bypassing quirk WebKit#2, if we're painting into an unaccelerated backing store. It turns out our CG display list backing store implementation behaves like a CG bitmap context (without quirk WebKit#2), and so currently any IOSurfaces painted into CG display list backing store from a CGImageRef created by CGIOSurfaceContextCreateImage (but not -CreateImageReference) become stale if painted multiple times. To avoid this, extend the workaround to apply to any destination context that claims that it needs the workaround, and use it whenever painting an IOSurface into anything other than a CGIOSurfaceContext. * Source/WebCore/platform/graphics/BifurcatedGraphicsContext.cpp: (WebCore::BifurcatedGraphicsContext::needsCachedNativeImageInvalidationWorkaround): * Source/WebCore/platform/graphics/BifurcatedGraphicsContext.h: Make BifurcatedGraphicsContext assume the more conservative mode of its two children. * Source/WebCore/platform/graphics/GraphicsContext.h: (WebCore::GraphicsContext::needsCachedNativeImageInvalidationWorkaround): Assume that by default, GraphicsContexts need the workaround. * Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp: (WebCore::GraphicsContextCG::needsCachedNativeImageInvalidationWorkaround): * Source/WebCore/platform/graphics/cg/GraphicsContextCG.h: GraphicsContextCG needs the workaround, except in the IOSurface->IOSurface case. * Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp: (WebCore::ImageBufferIOSurfaceBackend::finalizeDrawIntoContext): Confer with the GraphicsContext about its need for the workaround instead of hardcoding the behavior here. * Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm: CG display list graphics contexts need the workaround. Canonical link: https://commits.webkit.org/258586@main
pascoej
pushed a commit
that referenced
this pull request
Jan 25, 2023
https://bugs.webkit.org/show_bug.cgi?id=251063 rdar://104585575 Reviewed by Mark Lam and Justin Michaud. This patch enhances CallFrame::dump to support wasm frames in btjs stacktrace. The example is as follows. frame #0: 0x00000001035fca78 JavaScriptCore`JSC::functionBreakpoint(globalObject=0x000000012f410068, callFrame=0x000000016fdfa9d0) at JSDollarVM.cpp:2273:9 [opt] frame #1: 0x000000010ec44204 0x10eccc5dc frame WebKit#2: 0x000000010eccc5dc callback#Dwaxn6 [Baseline bc#50](Undefined) frame WebKit#3: 0x000000010ec4ca84 wasm-stub [WasmToJS](Wasm::Instance: 0x10d29da40) frame WebKit#4: 0x000000010ed0c060 <?>.wasm-function[1] [OMG](Wasm::Instance: 0x10d29da40) frame WebKit#5: 0x000000010ed100d0 jsToWasm#CWTx6k [FTL bc#22](Cell[JSModuleEnvironment]: 0x12f524540, Cell[WebAssemblyFunction]: 0x10d06a3a8, 1, 2, 3) frame WebKit#6: 0x000000010ec881b0 #D5ymZE [Baseline bc#733](Undefined, Cell[Generator]: 0x12f55c180, 1, Cell[Object]: 0x12f69dfc0, 0, Cell[JSLexicalEnvironment]: 0x12f52cee0) frame WebKit#7: 0x000000010ec3c008 asyncFunctionResume#A4ayYg [LLInt bc#49](Undefined, Cell[Generator]: 0x12f55c180, Cell[Object]: 0x12f69dfc0, 0) frame WebKit#8: 0x000000010ec3c008 promiseReactionJobWithoutPromise#D0yDF1 [LLInt bc#25](Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180) frame WebKit#9: 0x000000010ec80ec0 promiseReactionJob#EdShZz [Baseline bc#74](Undefined, Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180) frame WebKit#10: 0x000000010ec3c728 frame WebKit#11: 0x0000000103137560 JavaScriptCore`JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) [inlined] JSC::JITCode::execute(this=<unavailable>, vm=<unavailable>, protoCallFrame=<unavailable>) at JITCodeInlines.h:42:38 [opt] frame WebKit#12: 0x0000000103137524 JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, lexicalGlobalObject=<unavailable>, function=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) at Interpreter.cpp:1093:27 [opt] frame WebKit#13: 0x000000010349d6d0 JavaScriptCore`JSC::runJSMicrotask(globalObject=0x000000012f410068, identifier=(m_identifier = 81), job=JSValue @ x22, argument0=JSValue @ x26, argument1=JSValue @ x25, argument2=<unavailable>, argument3=<unavailable>) at JSMicrotask.cpp:98:9 [opt] frame WebKit#14: 0x00000001039dfc54 JavaScriptCore`JSC::VM::drainMicrotasks() (.cold.1) at VM.cpp:0:9 [opt] frame WebKit#15: 0x00000001035e58a4 JavaScriptCore`JSC::VM::drainMicrotasks() [inlined] JSC::MicrotaskQueue::dequeue(this=<unavailable>) at VM.cpp:0:9 [opt] frame WebKit#16: 0x00000001035e5894 JavaScriptCore`JSC::VM::drainMicrotasks(this=0x000000012f000000) at VM.cpp:1255:46 [opt] ... * Source/JavaScriptCore/interpreter/CallFrame.cpp: (JSC::CallFrame::dump const): Canonical link: https://commits.webkit.org/259262@main
pascoej
pushed a commit
that referenced
this pull request
Feb 20, 2023
https://bugs.webkit.org/show_bug.cgi?id=252379 <rdar://104303475> Reviewed by Antti Koivisto. While display boxes are positioned based on margin boxes, the left/right side of a display box do not include these margins. e.g. [display box #1]<- 100px margin ->[display box WebKit#2] width: 50px width: 50px margin-right: 100px; display box #1's right: 50px display box WebKit#2's left: 150px This patch makes sure when we place an out-of-flow box next to display box #1, we put it at 150px and not at 50px. * LayoutTests/fast/inline/out-of-flow-inline-with-previous-next-margin-expected.html: Added. * LayoutTests/fast/inline/out-of-flow-inline-with-previous-next-margin.html: Added. * Source/WebCore/layout/formattingContexts/inline/InlineFormattingGeometry.cpp: (WebCore::Layout::InlineFormattingGeometry::staticPositionForOutOfFlowInlineLevelBox const): Canonical link: https://commits.webkit.org/260380@main
pascoej
pushed a commit
that referenced
this pull request
Apr 14, 2023
https://bugs.webkit.org/show_bug.cgi?id=253907 rdar://102754257 Reviewed by Keith Miller and Justin Michaud. iterator_next and iterator_open can create a graph of DFG nodes. But this is problematic for OSR exit node liveness tracking. We have phantom insertion phase to keep uses of instructions alive properly. But that analysis has an assumption that one instruction cannot create a graph. As a result, the phase does block local analysis, and if the local is not used on that basic block, we do not insert phantoms. Let's see Block #0 @0 GetLocal(local0) @1 Use(@0) @2 Jump(#1) Block #1 @3 ForceOSRExit In #1, we do not know that local0 needs to be kept alive even if local0 is alive in bytecode. And phantom insertion phase cannot insert phantoms to keep them alive since local0 operand in #1 is not filled. This patch adds keepUsesOfCurrentInstructionAlive helper function and call it in the prologue of newly created basic block for one instruction. This inserts all the uses of the instruction explicitly via GetLocal. Block #0 @0 GetLocal(local0) @1 Use(@0) @2 Jump(#1) Block #1 @3 GetLocal(local0) @4 ForceOSRExit With this function, we insert @3 for local0, and now phatom insertion phase will see operand for local0 is filled in #1, and appropriately insert Phantom into #1 too. * JSTests/stress/iterator-next-osr-exit-dead-1.js: Added. * JSTests/stress/iterator-next-osr-exit-dead-2.js: Added. (i.i.switch): * Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp: (JSC::computeUsesForBytecodeIndexImpl): * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::keepUsesOfCurrentInstructionAlive): (JSC::DFG::ByteCodeParser::parseBlock): Canonical link: https://commits.webkit.org/261668@main
pascoej
pushed a commit
that referenced
this pull request
Apr 14, 2023
https://bugs.webkit.org/show_bug.cgi?id=254666 Reviewed by Antti Koivisto. Consider the following case: <containing-block> <layout-boundary> Inline-content <out-of-flow-box> </layout-boundary> </containing-block> e.g. <div style="overflow: hidden"> some text <div style="position: absolute"></div> </div> 1. "inline content" gets mutated and the associated renderers are marked dirty. During #1 we climb the ancestor chain and mark containing blocks dirty to ensure the subsequent layout has a correct entry point. We either (most of the time) stop at the ICB (RenderView) during this walk or at a layout-boundary. 2. Subsequent layout is initiated starting at layout-boundary. The geometry of the freshly laid out inline-content may affect the out-of-flow-box's static position. In inline layout code (both legacy and IFC) at this point we only set the "static" position assuming that layout eventually reaches the out-of-flow-box's containing block which would set the final top/left coords. However since this layout is bound to layout-boundary's subtree, we never get to the containing-block. While this is an invalidation bug where we fail to mark the containing-block dirty (by not stopping at layout-boundary), it's expensive to figure out if there's a descendent of the layout-boundary with an ancestor containing block (outside of layout-boundary's subtree). Instead set the out-of-flow-box's coordinates here at inline layout and let the containing block update it as part of the normal layout flow (when we actually get to the containing block). This is technically correct since this renderer's position is its static position until the containing block updates it as applicable (and the special "static position" handling could be considered as a render tree artifact). * LayoutTests/fast/block/positioning/static_out_of_flow_inside_layout_boundary-expected.html: Added. * LayoutTests/fast/block/positioning/static_out_of_flow_inside_layout_boundary.html: Added. * Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp: (WebCore::LayoutIntegration::LineLayout::updateRenderTreePositions): Canonical link: https://commits.webkit.org/262470@main
pascoej
pushed a commit
that referenced
this pull request
Apr 20, 2023
…r/backspace/enter combo in long text https://bugs.webkit.org/show_bug.cgi?id=254989 <rdar://problem/108215532> Reviewed by Antti Koivisto. This bugs caused by an incorrectly computed (damaged) line index which caused us exiting inline layout too early (and not producing content for the new newline). With partial layout we 1. first compute the damaged line index which is the entry point for the subsequent inline layout. 2. run inline layout until we see no change in generated display boxes anymore (in many cases the damage only affects a range of lines and not the full set) With the following content: First line 1\n Second line 2\n Third line 3\n When a new \n is inserted between the last \n and [3], we compute the damage position by looking at the _start_ of the previous sibling. In this case the previous sibling is "First line 1\nSecond line 2\nThird line 3\n" which means we damage the content starting from line #0. The subsequent inline layout starts generating display boxes starting from line #0 and bails out at the end of line #1 since the set of display boxes are getting generated match what we had before. This fix ensures that when we insert some content, the damaged line index is computed based on the insertion point (end of the previous sibling). * Source/WebCore/layout/formattingContexts/inline/invalidation/InlineInvalidation.cpp: (WebCore::Layout::damagedLineIndex): (WebCore::Layout::inlineItemPositionForDamagedContentPosition): (WebCore::Layout::InlineInvalidation::textInserted): (WebCore::Layout::InlineInvalidation::inlineLevelBoxInserted): Canonical link: https://commits.webkit.org/263113@main
pascoej
pushed a commit
that referenced
this pull request
May 31, 2023
https://bugs.webkit.org/show_bug.cgi?id=245225 <rdar://problem/100278323> Reviewed by Simon Fraser. Non-initial line-height value gets leaked into the inner text control making the associated inline-block inline level box too tall. It causes 2 highly visible bugs with single-line type of text controls. 1, it results in tall line box pushing the rest of the baseline align inline content downward (test case #1) 2, text content inside the input is not visible at all unless the input is active and user starts typing (test case WebKit#2) (It's quite bad as focusing the input still produces blank content and the user has to start typing to see existing text in the input). <div>some text<input style="height: 50px; line-height: 1000" placeholder="and more"></div> _______________ | _____________ | <- input || and more || <- "forced positioned" placeholder control ||_____________|| | | <- inner text control | | | | some text | | <- computed baseline position (this is where the input box value ("text content") would end up) | | | | |_____________| Note that this is normal inline-block behavior where the baseline alignment is based off of the inline-block's last line even when this last line overflows the border box (and produces layout overflow). However not only does it look unacceptable for single-line input boxes but also our custom layout and painting logic inside RenderTextControlSingleLine slightly disagrees with this constrained set and produces unexpected content placement. (Current behavior is closer to what happens if the inline-block had "overflow: hidden", -which puts the baseline position at the bottom of the margin box) In this patch we override the inherited line-height value to initial unless the input box's height is auto -in which case it is actually ok to be driven by the content height (i.e. line-height based inflate is ok). This change improves interoperability and makes WebKit match other rendering engines' behavior. * Source/WebCore/html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::createInnerTextStyle): Canonical link: https://commits.webkit.org/264613@main
pascoej
pushed a commit
that referenced
this pull request
Jul 31, 2023
https://bugs.webkit.org/show_bug.cgi?id=255872 rdar://108738795 Reviewed by Darin Adler. It turns out that JSON, HTTP, and XML all use the same whitespace definition, so let's make them share it. Also correct an existing comment for that function as \v is not part of isASCIIWhitespace(), but \f is. Furthermore, remove the "optimization" from these whitespace functions per a comment from Chris Dumez at WebKit#13080 (comment): > Just verified out of curiosity and llvm does generate the same code > with -O2 (tried both arm64 and x86_64): > > isXMLSpace1(char): // @isXMLSpace1(char) > mov x8, WebKit#9728 // =0x2600 > and w9, w0, #0xff > movk x8, #1, lsl WebKit#32 > cmp w9, WebKit#33 > cset w9, lo > lsr x8, x8, x0 > and w0, w9, w8 > ret > isXMLSpace2(char): // @isXMLSpace2(char) > mov x8, WebKit#9728 // =0x2600 > and w9, w0, #0xff > movk x8, #1, lsl WebKit#32 > cmp w9, WebKit#33 > cset w9, lo > lsr x8, x8, x0 > and w0, w9, w8 > ret > > Ahmad-S792 Let's simplify the code then. * Source/WTF/wtf/ASCIICType.h: (WTF::isASCIIWhitespace): (WTF::isJSONOrHTTPOrXMLWhitespace): (WTF::isJSONOrHTTPWhitespace): Deleted. * Source/WTF/wtf/JSONValues.cpp: (WTF::JSONImpl::Value::parseJSON): * Source/WTF/wtf/text/StringToIntegerConversion.h: * Source/WebCore/Modules/cache/DOMCache.cpp: (WebCore::hasResponseVaryStarHeaderValue): * Source/WebCore/Modules/cache/DOMCacheEngine.cpp: (WebCore::DOMCacheEngine::queryCacheMatch): * Source/WebCore/Modules/fetch/FetchBodyConsumer.cpp: (WebCore::parseParameters): (WebCore::parseMIMEType): (WebCore::FetchBodyConsumer::packageFormData): * Source/WebCore/Modules/fetch/FetchHeaders.cpp: (WebCore::canWriteHeader): (WebCore::appendToHeaderMap): (WebCore::FetchHeaders::set): (WebCore::FetchHeaders::filterAndFill): * Source/WebCore/mathml/MathMLPresentationElement.cpp: (WebCore::MathMLPresentationElement::parseMathMLLength): * Source/WebCore/mathml/MathMLTokenElement.cpp: (WebCore::MathMLTokenElement::convertToSingleCodePoint): * Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp: (WebCore::ContentSecurityPolicyDirectiveList::parse): * Source/WebCore/platform/ReferrerPolicy.cpp: (WebCore::parseReferrerPolicy): * Source/WebCore/platform/network/DataURLDecoder.cpp: (WebCore::DataURLDecoder::DecodeTask::process): * Source/WebCore/platform/network/HTTPParsers.cpp: (WebCore::parseContentTypeOptionsHeader): (WebCore::parseClearSiteDataHeader): (WebCore::parseRange): (WebCore::parseCrossOriginResourcePolicyHeader): * Source/WebCore/platform/network/HTTPParsers.h: (WebCore::addToAccessControlAllowList): * Source/WebCore/platform/network/ParsedContentType.cpp: (WebCore::skipSpaces): (WebCore::parseToken): (WebCore::ParsedContentType::create): (WebCore::ParsedContentType::setContentType): * Source/WebCore/platform/network/ResourceResponseBase.cpp: (WebCore::ResourceResponseBase::containsInvalidHTTPHeaders const): * Source/WebCore/platform/network/TimingAllowOrigin.cpp: (WebCore::passesTimingAllowOriginCheck): * Source/WebCore/xml/XMLHttpRequest.cpp: (WebCore::XMLHttpRequest::setRequestHeader): * Source/WebCore/xml/XPathFunctions.cpp: (WebCore::XPath::FunNormalizeSpace::evaluate const): * Source/WebCore/xml/XPathParser.cpp: (WebCore::XPath::Parser::skipWS): * Source/WebCore/xml/XPathUtil.cpp: (WebCore::XPath::isXMLSpace): Deleted. * Source/WebCore/xml/XPathUtil.h: * Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp: (WebKit::CacheStorage::updateVaryInformation): * Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp: (WebKit::WebSocketTask::WebSocketTask): * Source/WebKit/NetworkProcess/storage/CacheStorageRecord.h: (WebKit::CacheStorageRecordInformation::updateVaryHeaders): * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::shouldSkipDecidePolicyForResponse const): Canonical link: https://commits.webkit.org/266253@main
webkit-commit-queue
pushed a commit
that referenced
this pull request
Oct 19, 2023
https://bugs.webkit.org/show_bug.cgi?id=263269 <rdar://problem/117086061> Reviewed by Ross Kirsling. The spec has at least 8 occurrences of > It is a Syntax Error if the LexicallyDeclaredNames of X contains any duplicate entries. early error rules that preclude duplicate lexical declarations. For backwards-compatibility, LexicallyDeclaredNames [1] skips top-level function declarations inside `ScriptBody : StatementList` by invoking TopLevelLexicallyDeclaredNames [2], which returns an empty list for them [3]. However, the semantics of LexicallyDeclaredNames is entirely different for `ModuleItem` (also please see note #1). The fact that top-level function declarations are lexical in module code is also evident during binding initialization [4]. This change makes top-level function declarations in module code behave like `let` rather than `var`, introducing early errors that come with it, like disallowing duplicates with `var` and `function`. Since inside declareFunction() we can't rely neither on `m_scriptMode` (which is always `Module`, even for nested functions that absolutely should not throw SyntaxError for duplicate declarations), nor on `m_parseMode` (it's already the parse mode of the declared function itself), this change introduces isModuleCode() [5], refactoring parse mode handling in Scope. Also, this patch aligns declareFunctionAsLet() with declareVariable(), called for `let` declarations, by adding `m_declaredVariables` check, which may fail only in module code. Removes now incorrect (for module code only) ASSERT that isn't particularly useful given how simple declareFunction() now is. Aligns JSC with V8 and SpiderMonkey. [1]: https://tc39.es/ecma262/#sec-static-semantics-lexicallydeclarednames [2]: https://tc39.es/ecma262/#sec-static-semantics-toplevellexicallydeclarednames [3]: https://tc39.es/ecma262/#prod-HoistableDeclaration [4]: https://tc39.es/ecma262/#sec-source-text-module-record-initialize-environment (step 24.iii) [5]: https://tc39.es/ecma262/#sec-types-of-source-code * JSTests/modules/async-function-export.js: Moved to JSTests/stress/modules-syntax-error.js. * JSTests/stress/modules-syntax-error.js: * JSTests/test262/expectations.yaml: Mark 8 tests as passing. * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::BytecodeGenerator): * Source/JavaScriptCore/parser/Parser.cpp: (JSC::Parser<LexerType>::Parser): (JSC::Parser<LexerType>::parseFunctionInfo): (JSC::Parser<LexerType>::parseMemberExpression): * Source/JavaScriptCore/parser/Parser.h: (JSC::Scope::setSourceParseMode): (JSC::Scope::isGlobalCode const): (JSC::Scope::isModuleCode const): (JSC::Scope::declareFunctionAsLet): (JSC::Scope::setIsGlobalCode): (JSC::Scope::setIsModuleCode): (JSC::Parser::declareFunction): (JSC::Scope::setIsGlobalCodeScope): Deleted. (JSC::Scope::isGlobalCodeScope const): Deleted. Canonical link: https://commits.webkit.org/269485@main
pascoej
pushed a commit
that referenced
this pull request
Dec 19, 2023
https://bugs.webkit.org/show_bug.cgi?id=266153 Reviewed by Antti Koivisto. This patch adds support for "ruby-align: space-around" on both bidi and non-bidi content. "ruby-align: space-around" consists of 2 adjustments at run level. - run spacing at justification opportunities (similar to text-align: justify) - content offsetting to distribute one extra justification opportunity space before/after base content In this patch we start computing the offset values at step #1 and pass them over to InlineContentBuilder where the final ruby processing happens (using visual order). InlineContentAligner takes these offset values and adjusts (aligns) the display boxes accordingly. What makes it a bit more verbose is the fact that while non-bidi base runs have the correct spacing (e.g. when annotation is wider than the base), we lose that information at visual reordering (<- we essentially have a fast path for non-bidi content, where as we build the Line for line breaking we also compute horizontal content positions) It simply means that while non-bidi content, offsetting means just moving content inside the base without pushing adjacent content/expanding enclosing inline boxes, bidi content needs both (see AdjustContentOnlyInsideRubyBase). * Source/WebCore/layout/formattingContexts/inline/InlineContentAligner.cpp: (WebCore::Layout::shiftDisplayBox): (WebCore::Layout::expandInlineBox): (WebCore::Layout::alignmentOffset): (WebCore::Layout::expandInlineBoxWithDescendants): (WebCore::Layout::shiftRubyBaseContentByAlignmentOffset): (WebCore::Layout::InlineContentAligner::applyRubyAlignSpaceAround): (WebCore::Layout::InlineContentAligner::applyRubyBaseAlignmentOffset): (WebCore::Layout::InlineContentAligner::applyRubyAlign): Deleted. * Source/WebCore/layout/formattingContexts/inline/InlineContentAligner.h: * Source/WebCore/layout/formattingContexts/inline/InlineLine.h: * Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp: (WebCore::Layout::LineBuilder::layoutInlineContent): * Source/WebCore/layout/formattingContexts/inline/LineLayoutResult.h: * Source/WebCore/layout/formattingContexts/inline/TextOnlySimpleLineBuilder.cpp: (WebCore::Layout::TextOnlySimpleLineBuilder::layoutInlineContent): * Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.cpp: (WebCore::Layout::InlineDisplayContentBuilder::build): (WebCore::Layout::InlineDisplayContentBuilder::processRubyContent): * Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayContentBuilder.h: * Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.cpp: (WebCore::Layout::RubyFormattingContext::applyRubyAlignOnBaseContent): (WebCore::Layout::RubyFormattingContext::applyRubyAlign): (WebCore::Layout::RubyFormattingContext::applyAlignmentOffsetList): (WebCore::Layout::applyRubyAlignOnBaseContent): Deleted. * Source/WebCore/layout/formattingContexts/inline/ruby/RubyFormattingContext.h: Canonical link: https://commits.webkit.org/271836@main
pascoej
pushed a commit
that referenced
this pull request
Jan 29, 2024
https://bugs.webkit.org/show_bug.cgi?id=268227 Reviewed by Antti Koivisto. This is in preparation for being able to run a range of content through the simple line builder (where the range has text only content e.g. <div><span>this is text content</span></div> 1. Keep track of the number of inline boxes (m_inlineBoxCount) 2. m_isTextAndForcedLineBreakOnlyContent only tracks _content_ type of inline level boxes now (excluding inline boxes, see #1) 3. and it does not track if content needs bidi reordering anymore (we already track that information in m_contentRequiresVisualReordering) 4. Add InlineContentCache::InlineItems::ContentAttributes to hold all these bits. 5. Remove dedicated set functions and pass these bit through InlineContentCache::InlineItems::set/replace. * Source/WebCore/layout/formattingContexts/inline/InlineContentCache.h: (WebCore::Layout::InlineContentCache::InlineItems::requiresVisualReordering const): (WebCore::Layout::InlineContentCache::InlineItems::hasTextAndLineBreakOnlyContent const): (WebCore::Layout::InlineContentCache::InlineItems::hasInlineBoxes const): (WebCore::Layout::InlineContentCache::InlineItems::inlineBoxCount const): (WebCore::Layout::InlineContentCache::InlineItems::set): (WebCore::Layout::InlineContentCache::InlineItems::replace): (WebCore::Layout::InlineContentCache::InlineItems::append): Deleted. (WebCore::Layout::InlineContentCache::InlineItems::clear): Deleted. (WebCore::Layout::InlineContentCache::InlineItems::setRequiresVisualReordering): Deleted. (WebCore::Layout::InlineContentCache::InlineItems::setIsNonBidiTextAndForcedLineBreakOnlyContent): Deleted. (WebCore::Layout::InlineContentCache::InlineItems::isNonBidiTextAndForcedLineBreakOnlyContent const): Deleted. * Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp: (WebCore::Layout::InlineFormattingContext::layout): (WebCore::Layout::InlineFormattingContext::minimumMaximumContentSize): (WebCore::Layout::InlineFormattingContext::minimumContentSize): (WebCore::Layout::InlineFormattingContext::maximumContentSize): * Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp: (WebCore::Layout::InlineItemsBuilder::build): (WebCore::Layout::InlineItemsBuilder::traverseUntilDamaged): (WebCore::Layout::InlineItemsBuilder::collectInlineItems): (WebCore::Layout::InlineItemsBuilder::handleTextContent): (WebCore::Layout::InlineItemsBuilder::handleInlineBoxStart): (WebCore::Layout::InlineItemsBuilder::handleInlineBoxEnd): (WebCore::Layout::InlineItemsBuilder::handleInlineLevelBox): * Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.h: * Source/WebCore/layout/formattingContexts/inline/IntrinsicWidthHandler.cpp: (WebCore::Layout::IntrinsicWidthHandler::IntrinsicWidthHandler): (WebCore::Layout::IntrinsicWidthHandler::minimumContentSize): (WebCore::Layout::IntrinsicWidthHandler::maximumContentSize): (WebCore::Layout::IntrinsicWidthHandler::computedIntrinsicWidthForConstraint): (WebCore::Layout::IntrinsicWidthHandler::simplifiedMaximumWidth): * Source/WebCore/layout/formattingContexts/inline/IntrinsicWidthHandler.h: (WebCore::Layout::IntrinsicWidthHandler::inlineItemList const): * Source/WebCore/layout/formattingContexts/inline/TextOnlySimpleLineBuilder.cpp: (WebCore::Layout::TextOnlySimpleLineBuilder::isEligibleForSimplifiedTextOnlyInlineLayout): * Source/WebCore/layout/formattingContexts/inline/TextOnlySimpleLineBuilder.h: Canonical link: https://commits.webkit.org/273632@main
pascoej
pushed a commit
that referenced
this pull request
Apr 15, 2024
https://bugs.webkit.org/show_bug.cgi?id=272292 Reviewed by Antti Koivisto. 1. In some cases (e.g. pseudo content) a subtree root gets detached first followed by the descendant renderers 2. Repaint functions start with traversing all the way to the RenderView just to check if dispatching repaint is safe -which in most cases comes back true (normal at-layout repaint, invalidation, standard tree teardown) This patch fixes a correctness and a performance issue: correctness: In case of #1, when calling willBeRemovedFromTree() (right before deleting the descendant renderer), the renderer is already detached from the render tree. It is still attached to the root of the subtree (which is already detached), but has no access to the rest of the render tree (i.e. willBeRemovedFromTree is technically just willBeRemovedFromParent) performance: ensuring correctness eliminates the need for checking if the renderer is still attached. (Sadly ::imageChanged gets called indirectly through initializeStyle before the initial attach) * Source/WebCore/rendering/RenderObject.cpp: (WebCore::RenderObject::repaint const): Use isDescendantOf instead of isRooted() as I am planning on completely remove isRooted(). (WebCore::RenderObject::repaintRectangle const): (WebCore::RenderObject::repaintSlowRepaintObject const): * Source/WebCore/rendering/updating/RenderTreeBuilder.cpp: (WebCore::RenderTreeBuilder::destroy): (WebCore::RenderTreeBuilder::detachFromRenderElement): * Source/WebCore/rendering/updating/RenderTreeBuilder.h: Canonical link: https://commits.webkit.org/277186@main
pascoej
pushed a commit
that referenced
this pull request
Jun 4, 2024
https://bugs.webkit.org/show_bug.cgi?id=275019 <rdar://128067952> Reviewed by Antti Koivisto. 1. Backdrop render is always anchored to the viewport regardless of where the associated content renderer is 2. While destroying the content renderer we also remove the backdrop renderer In order to let invalidateLineLayout kick in (see RenderTreeBuilder::detachFromRenderElement) we have to make sure the content renderer is not considered as the "destroy root" of the backdrop (see #1). * LayoutTests/fast/dynamic/backdrop-remove-crash-expected.txt: Added. * LayoutTests/fast/dynamic/backdrop-remove-crash.html: Added. * Source/WebCore/rendering/updating/RenderTreeUpdater.cpp: (WebCore::RenderTreeUpdater::tearDownRenderers): Canonical link: https://commits.webkit.org/279651@main
pascoej
pushed a commit
that referenced
this pull request
Jul 1, 2024
…terpolate https://bugs.webkit.org/show_bug.cgi?id=275993 rdar://130704075 Reviewed by Matt Woodrow. We had three separate issues that would lead us to visually animate when one of the values in a given interval is a non-invertible matrix: 1. The method that determines whether it's possible to interpolate between two `transform` values would only account for `matrix()` values and not `matrix3d()`. 2. The `transform` property animation wrapper would not implement the `canInterpolate()` method and would thus always indicate that two `transform` values could be interpolated. This caused CSS Transitions to run even when the values would not a discrete interpolation. 3. Even if we correctly determined that two `transform` values should yield discrete interpolation, we would delegate an accelerated animation to Core Animation and that animation's behavior would differ an visibly interpolate. In this patch, we fill all three issues. First, we introduce a new `TransformOperations::containsNonInvertibleMatrix()` method which will check whether a `matrix()` or `matrix3d()` value that is not invertible is contained in the list of transform operations. We now use this function in `TransformOperations::shouldFallBackToDiscreteAnimation()` to address issue #1. Then, we add a `canInterpolate()` implementation to `AcceleratedTransformOperationsPropertyWrapper` which calls in the now-correct `TransformOperations::shouldFallBackToDiscreteAnimation()` to address issue WebKit#2. Finally, we add a new flag on `BlendingKeyframes` to determine whether a keyframe contains a `transform` value with a non-invertible matrix and we consult that flag in `KeyframeEffect::canBeAccelerated()` to determine whether an animation should be delegated to Core Animation, addressing issue WebKit#3. We add new WPT tests to check the correct interpolation behavior of `transform` when a non-invertible `matrix3d()` value is used, that no CSS Transition can be started with such a value, and finally that no animation is visibly run to catch the Core Animation case. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-007-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-007.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-discrete-interpolation-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-discrete-interpolation-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-discrete-interpolation.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-no-transition-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-non-invertible-no-transition.html: Added. * Source/WebCore/animation/BlendingKeyframes.cpp: (WebCore::BlendingKeyframes::analyzeKeyframe): * Source/WebCore/animation/BlendingKeyframes.h: (WebCore::BlendingKeyframes::hasDiscreteTransformInterval const): * Source/WebCore/animation/CSSPropertyAnimation.cpp: * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::canBeAccelerated const): * Source/WebCore/platform/graphics/transforms/TransformOperations.cpp: (WebCore::TransformOperations::containsNonInvertibleMatrix const): (WebCore::TransformOperations::shouldFallBackToDiscreteAnimation const): * Source/WebCore/platform/graphics/transforms/TransformOperations.h: Canonical link: https://commits.webkit.org/280466@main
pascoej
pushed a commit
that referenced
this pull request
Aug 13, 2024
…text run https://bugs.webkit.org/show_bug.cgi?id=277716 rdar://133309470 Reviewed by Matthieu Dubet. This patch implements the processing of text-autospace: ideogram-alpha only within an element. We don't yet handle element boundaries here. Although we pass SpacingState context from one ComplexTextController to another, we do that here in a limited way, only for measuring text for layout and for painting. There are other places in code which this will be necessary, for example, for handling element boundaries. 1. During the construction of ComplexTextController, we call ::adjustGlyphsAndAdvances which already iterates through glyphs and adjust spacing for other reasons. Now we process each pair of characters related to these glyphs here, adding the spacing necessary before the "current" character. For that reason, the SpacingState stores information about the previous character of a run. We also save the measured spacing in a new parallel vector m_textAutoSpaceSpacings. At this phase we can only manipulate a glyph advance, however, for adding space "before" a glyph, we need to move the glyph to the logical right, which is done later on ::advance. 2. ComplexTextController::advance is called for both layout and painting, but during painting it has access to a GlyphBuffer and it add glyphs into it. We are introducing a new GlyphBuffer::add function that also takes the glyph's origin, so we can manipulate the origin as necessary by adding the previous calculated spacing. 3. Doing #1 and WebKit#2 is already enough for painting the extra spacing between relevant characters according to their classes. Howeverm the width measured during layout would be broken because IFC splits text content into inlineTextItem(s) and measure the width of each item independently. This means that we already have to handle SpacingState passing here, otherwise we are not able to handle spacing between characters on the boundary of different InlineTextItem. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-autospace/text-autospace-ideogram-alpha-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-autospace/text-autospace-ideogram-alpha-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/text-autospace/text-autospace-ideogram-alpha-001.html: Added. * Source/WTF/wtf/text/CharacterProperties.h: (WTF::isPunctuation): (WTF::isOpeningPunctuation): (WTF::isClosingPunctuation): (WTF::isOfScriptType): (WTF::eastAsianWidth): (WTF::isEastAsianFullWidth): (WTF::isCJKSymbolOrPunctuation): * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp: (WebCore::Layout::InlineItemsBuilder::computeInlineTextItemWidths): * Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp: (WebCore::Layout::TextUtil::width): * Source/WebCore/layout/formattingContexts/inline/text/TextUtil.h: (WebCore::Layout::TextUtil::width): * Source/WebCore/platform/graphics/ComplexTextController.cpp: (WebCore::ComplexTextController::ComplexTextController): (WebCore::ComplexTextController::advance): (WebCore::ComplexTextController::adjustGlyphsAndAdvances): (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/graphics/ComplexTextController.h: (WebCore::ComplexTextController::ComplexTextRun::textAutospaceSize const): * Source/WebCore/platform/graphics/FontCascade.cpp: (WebCore::FontCascade::width const): (WebCore::FontCascade::codePath const): * Source/WebCore/platform/graphics/GlyphBuffer.h: (WebCore::GlyphBuffer::add): * Source/WebCore/platform/graphics/TextRun.cpp: * Source/WebCore/platform/graphics/TextRun.h: * Source/WebCore/platform/graphics/WidthCache.h: (WebCore::WidthCache::add): (WebCore::WidthCache::invalidateCacheForTextSpacing): * Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp: (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm: (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/graphics/skia/ComplexTextControllerSkia.cpp: (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/text/TextSpacing.cpp: Added. (WebCore::TextAutospace::shouldApplySpacing const): (WebCore::TextAutospace::textAutospaceSize): (WebCore::TextSpacing::isIdeograph): (WebCore::TextSpacing::isNonIdeographicNumeral): (WebCore::TextSpacing::characterClass): * Source/WebCore/platform/text/TextSpacing.h: (WebCore::TextAutospace::hasIdeographAlpha const): (WebCore::TextAutospace::hasIdeographNumeric const): Canonical link: https://commits.webkit.org/282192@main
webkit-commit-queue
pushed a commit
that referenced
this pull request
Aug 20, 2024
…text run https://bugs.webkit.org/show_bug.cgi?id=277716 rdar://133309470 Reviewed by Matthieu Dubet. We are relanding this patch as its first version was reverted due to performance reasons. On the current iteration we are avoiding classifying characters when not needed (text-autospace: no-autospace). We also won't keep the parralel vector for the added spacing in such a case. Original patch description: This patch implements the processing of text-autospace: ideogram-alpha only within an element. We don't yet handle element boundaries here. Although we pass SpacingState context from one ComplexTextController to another, we do that here in a limited way, only for measuring text for layout and for painting. There are other places in code which this will be necessary, for example, for handling element boundaries. 1. During the construction of ComplexTextController, we call ::adjustGlyphsAndAdvances which already iterates through glyphs and adjust spacing for other reasons. Now we process each pair of characters related to these glyphs here, adding the spacing necessary before the "current" character. For that reason, the SpacingState stores information about the previous character of a run. We also save the measured spacing in a new parallel vector m_textAutoSpaceSpacings. At this phase we can only manipulate a glyph advance, however, for adding space "before" a glyph, we need to move the glyph to the logical right, which is done later on ::advance. 2. ComplexTextController::advance is called for both layout and painting, but during painting it has access to a GlyphBuffer and it add glyphs into it. We are introducing a new GlyphBuffer::add function that also takes the glyph's origin, so we can manipulate the origin as necessary by adding the previous calculated spacing. 3. Doing #1 and WebKit#2 is already enough for painting the extra spacing between relevant characters according to their classes. Howeverm the width measured during layout would be broken because IFC splits text content into inlineTextItem(s) and measure the width of each item independently. This means that we already have to handle SpacingState passing here, otherwise we are not able to handle spacing between characters on the boundary of different InlineTextItem. * Source/WTF/wtf/text/CharacterProperties.h: (WTF::isPunctuation): (WTF::isOpeningPunctuation): (WTF::isClosingPunctuation): (WTF::isOfScriptType): (WTF::eastAsianWidth): (WTF::isEastAsianFullWidth): (WTF::isCJKSymbolOrPunctuation): * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp: (WebCore::Layout::InlineItemsBuilder::computeInlineTextItemWidths): * Source/WebCore/layout/formattingContexts/inline/text/TextUtil.cpp: (WebCore::Layout::TextUtil::width): * Source/WebCore/layout/formattingContexts/inline/text/TextUtil.h: (WebCore::Layout::TextUtil::width): * Source/WebCore/platform/graphics/ComplexTextController.cpp: (WebCore::ComplexTextController::ComplexTextController): (WebCore::ComplexTextController::advance): (WebCore::ComplexTextController::adjustGlyphsAndAdvances): (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/graphics/ComplexTextController.h: (WebCore::ComplexTextController::ComplexTextRun::textAutospaceSize const): * Source/WebCore/platform/graphics/FontCascade.cpp: (WebCore::FontCascade::width const): (WebCore::FontCascade::codePath const): * Source/WebCore/platform/graphics/GlyphBuffer.h: (WebCore::GlyphBuffer::add): * Source/WebCore/platform/graphics/TextRun.cpp: * Source/WebCore/platform/graphics/TextRun.h: * Source/WebCore/platform/graphics/WidthCache.h: (WebCore::WidthCache::add): (WebCore::WidthCache::invalidateCacheForTextSpacing): * Source/WebCore/platform/graphics/coretext/ComplexTextControllerCoreText.mm: (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp: (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/graphics/skia/ComplexTextControllerSkia.cpp: (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): * Source/WebCore/platform/text/TextSpacing.cpp: Added. (WebCore::TextAutospace::shouldApplySpacing const): (WebCore::TextAutospace::textAutospaceSize): (WebCore::TextSpacing::isIdeograph): (WebCore::TextSpacing::isNonIdeographicNumeral): (WebCore::TextSpacing::characterClass): * Source/WebCore/platform/text/TextSpacing.h: (WebCore::TextAutospace::hasIdeographAlpha const): (WebCore::TextAutospace::hasIdeographNumeric const): Canonical link: https://commits.webkit.org/282511@main
pascoej
pushed a commit
that referenced
this pull request
Sep 28, 2024
…r_ overflow https://bugs.webkit.org/show_bug.cgi?id=279486 Reviewed by Antti Koivisto. Let's call 1. lastHyphenPosition when we are dealing with the non-overflowing runs (this is when we can't break the overflowing part of the content and try to break runs _before_ the overflowing point). Since these runs are not overflowing, we should simply pick the last hyphenation position. 2. firstHyphenPosition when even the first hyphenation would produce overflowing content (e.g. minimum-content with computation) 3. hyphenPositionBefore when dealing with normal overflowing breaking (neither #1 nor WebKit#2) * LayoutTests/fast/inline/overflowing-content-with-hypens-expected.html: Added. * LayoutTests/fast/inline/overflowing-content-with-hypens.html: Added. * Source/WebCore/layout/formattingContexts/inline/InlineContentBreaker.cpp: (WebCore::Layout::firstTextRunIndex): (WebCore::Layout::InlineContentBreaker::processOverflowingContent const): (WebCore::Layout::limitBeforeValue): (WebCore::Layout::limitAfterValue): (WebCore::Layout::hasEnoughContentForHyphenation): (WebCore::Layout::firstHyphenPosition): (WebCore::Layout::lastHyphenPosition): (WebCore::Layout::hyphenPositionBefore): (WebCore::Layout::InlineContentBreaker::tryBreakingTextRun const): (WebCore::Layout::InlineContentBreaker::tryHyphenationAcrossOverflowingInlineTextItems const): (WebCore::Layout::hyphenPosition): Deleted. Canonical link: https://commits.webkit.org/283528@main
pascoej
pushed a commit
that referenced
this pull request
Sep 28, 2024
…ter follows to the same value https://bugs.webkit.org/show_bug.cgi?id=279570 rdar://135851156 Reviewed by Keith Miller. Let's consider the following FTL graph. BB#0 @0 = NewObject() Jump #1 BB#1 PutByOffset(@0, 0, @x) Jump WebKit#2 BB#2 ... @z = ... @1 = GetByOffset(@x, 0) Branch(@1, WebKit#3, WebKit#4) BB#3 PutByOffset(@0, 0, @z) Jump WebKit#5 BB#4 PutByOffset(@0, 0, @z) Jump WebKit#5 BB#5 Jump WebKit#2 Now, we would like to eliminate @0 object allocation. And we are computing SSA for pointers of fields of the that object which gets eliminated. Consider about @x's fields' SSA. PutByOffset becomes Def and GetByOffset becomes Use. And the same field will get the same SSA variable. So we first puts Defs and compute Phis based on that. In ObjectAllocationSinking phase, we had a fast path when the both SSA variable is following to the same value. Let's see BB#5. Because BB#3 and BB#4 defines Defs, dominance frontier BB#5 will need to introduce Phi. But interestingly, both SSA variable is following to the same @z. As a result, we were not inserting Phi for this case. But this is wrong. Inserted Phi is a Def, and based on that, we will further introduce Phis with that. If we omit inserting Phi in BB#5, we will not insert Phi into BB#2 while BB#2 will merge BB#1's Def And BB#5's Phi's Def. As a result, in BB#2, we think this variable is following to BB#1's Def. But that's wrong and BB#5's Phi exists. This patch removes this fast path to fix the issue. * JSTests/stress/object-allocation-sinking-phi-insertion-for-pointers.js: Added. (Queue): (Queue.prototype.enqueue): (Queue.prototype.dequeue): (i.queue.dequeue): * Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp: Canonical link: https://commits.webkit.org/283558@main
pascoej
pushed a commit
that referenced
this pull request
Nov 13, 2024
https://bugs.webkit.org/show_bug.cgi?id=282741 rdar://139412312 Reviewed by Youenn Fablet. Pass colorspace information to the created CVPixelBuffer. We add utility methods to construct the colorspace data from the vpcC box and VPx bytestream should the information not be provided on construction. We prefer colorspace information from this source order given colorspace > description data (vpcC) > inband bytestream. Added tests verifying that black are pure black, and yellow are almost pure yellow with 601 videos and video range. Technically they should be exactly pure (255, 255, 0), however, compression artifacts with the source makes it not so. Fly-by #1: m_isClosed can be accessed concurrently on the decoder's or caller's workqueue. Make it atomic. Fly-by WebKit#2: Make relevant members const and add annotation about where some members can be accessed from. * LayoutTests/media/content/test-h264-601-videorange.mp4: Added. * LayoutTests/media/content/test-vp8-601-videorange.webm: Added. * LayoutTests/media/content/test-vp9-601-videorange.webm: Added. * LayoutTests/media/media-source/media-source-vp8-hiddenframes.html: We can reduce the fuzz range now that both the VT decoder (mac) will return the same colours as VideoDecoder (ios family) * LayoutTests/media/media-video-fullrange.html: Wait a maximum of 500ms for the promise to be resolved as the rVFC callback may not always be called. * LayoutTests/media/media-video-videorange-expected.txt: Added. * LayoutTests/media/media-video-videorange.html: Added. * LayoutTests/media/media-vp8-hiddenframes.html: We can reduce the fuzz range now that both the VT decoder (mac) will return the same colours as VideoDecoder (ios family) * LayoutTests/platform/mac-wk1/TestExpectations: * LayoutTests/platform/wpe/TestExpectations: * Source/WebCore/Modules/webcodecs/WebCodecsVideoDecoder.cpp: (WebCore::createVideoDecoderConfig): * Source/WebCore/platform/VideoDecoder.h: * Source/WebCore/platform/graphics/VP9Utilities.cpp: (WebCore::vPCodecConfigurationRecordFromVPXByteStream): (WebCore::convertToPlatformVideoColorPrimaries): (WebCore::convertToPlatformVideoTransferCharacteristics): (WebCore::convertToPlatformVideoMatrixCoefficients): (WebCore::colorSpaceFromVPCodecConfigurationRecord): (WebCore::vpcCFromVPXByteStream): Deleted. * Source/WebCore/platform/graphics/VP9Utilities.h: * Source/WebCore/platform/graphics/cocoa/CMUtilities.h: * Source/WebCore/platform/graphics/cocoa/CMUtilities.mm: (WebCore::convertToCMTransferFunction): Add transfer value for BT601 (smpte170m) which is the same as 709.2 transfer. (WebCore::attachColorSpaceToPixelBuffer): * Source/WebCore/platform/graphics/cocoa/VP9UtilitiesCocoa.mm: Move methods to VP9Utilities.cpp (WebCore::convertToMatrixCoefficients): (WebCore::createVideoInfoFromVPCodecConfigurationRecord): (WebCore::convertToPlatformVideoColorPrimaries): Deleted. (WebCore::convertToPlatformVideoTransferCharacteristics): Deleted. (WebCore::convertToPlatformVideoMatrixCoefficients): Deleted. * Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.h: * Source/WebCore/platform/graphics/cocoa/WebCoreDecompressionSession.mm: (WebCore::WebCoreDecompressionSession::decodeSampleInternal): Retrieve colorspace from CMSampleBuffer and give it to the decoder initialization. (WebCore::WebCoreDecompressionSession::enqueueDecodedSample): Fly-by: the last video frame in a webm doesn't have a duration. A logic error would have caused to never notify the listener that the frame at currentTime had been decoded, leading to the play() promise to never be resolved (nor rVFC callback to be called) (WebCore::WebCoreDecompressionSession::initializeVideoDecoder): * Source/WebCore/platform/libwebrtc/LibWebRTCVPXVideoDecoder.cpp: (WebCore::LibWebRTCVPXInternalVideoDecoder::decode): (WebCore::LibWebRTCVPXInternalVideoDecoder::LibWebRTCVPXInternalVideoDecoder): (WebCore::LibWebRTCVPXInternalVideoDecoder::createPixelBuffer): (WebCore::LibWebRTCVPXInternalVideoDecoder::Decoded): Canonical link: https://commits.webkit.org/286474@main
pascoej
pushed a commit
that referenced
this pull request
Nov 22, 2024
…layout dependent state https://bugs.webkit.org/show_bug.cgi?id=283395 Reviewed by Antti Koivisto. There are isSkippedContentRoot functions atm. 1. WebCore::isSkippedContentRoot(style, element) 2. and RenderObject::isSkippedContentRoot (see ContentVisibilityForceLayoutScope, for cases when we need to look inside c-v subtrees for geometry) and returns false when we are supposed to ignore content-visibility. This is always scoped to a layout frame (as opposed to painting, hittesting etc) The codebase is sprinkled with isSkippedContentRoot() calls, some of which exercise #1 while others call into WebKit#2 in a seemingly random fashion (e.g. even painting calls the "let's consult the ignore bit" variant). This patch replaces these 2 functions with 1. LocalFrameViewLayoutContext::isSkippedContentRootForLayout() 2. WebCore::isSkippedContentRoot(renderer) Where during layout we call layoutContext().isSkippedContentRootForLayout() (surprisingly small number) and the rest simply calls WebKit#2. (Note, there's a highly specific, 3rd use case in StyleAdjuster, which should be moved out to a place where we could use the WebCore::isSkippedContentRoot(renderer) variant). * Source/WebCore/dom/Document.cpp: (WebCore::CallbackForContainIntrinsicSize): (WebCore::Document::caretPositionFromPoint): * Source/WebCore/editing/TextIterator.cpp: (WebCore::TextIterator::advance): * Source/WebCore/page/LocalFrameViewLayoutContext.cpp: (WebCore::LocalFrameViewLayoutContext::isSkippedContentForLayout const): (WebCore::LocalFrameViewLayoutContext::isSkippedContentRootForLayout const): * Source/WebCore/page/LocalFrameViewLayoutContext.h: * Source/WebCore/rendering/RenderBlock.cpp: (WebCore::RenderBlock::simplifiedLayout): (WebCore::RenderBlock::layoutPositionedObject): (WebCore::RenderBlock::paintContents): (WebCore::RenderBlock::adjustBorderBoxRectForPainting): (WebCore::RenderBlock::paintRectToClipOutFromBorder): (WebCore::RenderBlock::paintExcludedChildrenInBorder): * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::layoutBlockChildren): * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::foregroundIsKnownToBeOpaqueInRect const): (WebCore::RenderBox::explicitIntrinsicInnerWidth const): (WebCore::RenderBox::explicitIntrinsicInnerHeight const): * Source/WebCore/rendering/RenderElement.cpp: (WebCore::RenderElement::styleWillChange): (WebCore::RenderElement::layoutIfNeeded): (WebCore::RenderElement::isSkippedContentRoot const): Deleted. * Source/WebCore/rendering/RenderElement.h: (WebCore::RenderObject::isSkippedContentRoot const): Deleted. * Source/WebCore/rendering/RenderElementInlines.h: (WebCore::RenderElement::shouldApplyInlineSizeContainment const): (WebCore::RenderElement::shouldApplySizeContainment const): (WebCore::RenderElement::shouldApplySizeOrInlineSizeContainment const): (WebCore::isSkippedContentRoot): * Source/WebCore/rendering/RenderGrid.cpp: (WebCore::RenderGrid::layoutPositionedObject): * Source/WebCore/rendering/RenderObject.cpp: (WebCore::RenderObject::isSkippedContentForLayout const): Deleted. * Source/WebCore/rendering/RenderObject.h: * Source/WebCore/rendering/RenderObjectInlines.h: (WebCore::RenderObject::layoutContext const): * Source/WebCore/rendering/RenderReplaced.cpp: (WebCore::RenderReplaced::paint): * Source/WebCore/rendering/RenderWidget.cpp: (WebCore::RenderWidget::paint): * Source/WebCore/rendering/style/RenderStyle.h: * Source/WebCore/rendering/style/RenderStyleInlines.h: (WebCore::doesSizeContainmentApplyByStyle): (WebCore::isSkippedContentRoot): Deleted. * Source/WebCore/rendering/updating/RenderTreeUpdater.cpp: (WebCore::RenderTreeUpdater::updateElementRenderer): * Source/WebCore/style/StyleAdjuster.cpp: (WebCore::Style::Adjuster::adjust const): Canonical link: https://commits.webkit.org/286858@main
pascoej
pushed a commit
that referenced
this pull request
Feb 18, 2025
…pector rdar://98891055 https://bugs.webkit.org/show_bug.cgi?id=283092 Reviewed by Ryosuke Niwa and BJ Burg. There currently exists a message WebInspectorUIProxy::OpenLocalInspectorFrontend, which the web process sends to the UI process to show Web Inspector for the current web page. This introduces security risks as a compromised website may find its way to send arbitrary messages to the UI process, opening Web Inspector and weakening the web content sandbox. The reason this message exists is because there are useful ways the web process needs to open Web Inspector with initiative. Normally, Web Inspector is opened via one of the Develop menu's items, which is controlled by the UI process. However, Web Inspector can also be opened without being prompted by the UI process first, in these places: 1. In a web page's context menu, the "Inspect Element" option 2. Inside Web Inspector, if the Debug UI is enabled, on the top right corner, a button to open inspector^2 3. In WebKitTestRunner, via the TestRunner::showWebInspector function This patch makes it so that web process can no longer send a message to a UI process to open Web Inspector. This means web process cannot open Web Inspector at will -- it must be either due to the UI process's demand, or it's in one of the above three cases. More details below. I have tested that this change preserves the above three special cases and does prevent the web page from opening Web Inspector at will. - Cases #1 and WebKit#2 can be tested from the UI. - Case WebKit#3 can be tested with a WebKit test involving Web Inspector. I ran the test LayoutTests/inspector/console/js-completions.html, where I saw the test crashing without special treatment for this case. - To verify that the web page can't open Web Inspector, I followed the reproduction steps from the Radar and saw Web Inspector no longer opens, and opening the external URL also failed as expected. * Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.messages.in: * Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.h: * Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp: (WebKit::WebInspectorUIProxy::connect): - If the UI process wants to open Web Inspector, it sends a WebInspector::Show command to the web process. This patch makes that command take an async reply, so that the anticipated WebInspectorUIProxy::OpenLocalInspectorFrontend message from the web process can now be delivered through that async reply instead. This ensures that OpenLocalInspectorFrontend can only be done when initiated from the UI process (due to user interaction). (WebKit::WebInspectorUIProxy::markAsUnderTest): (WebKit::WebInspectorUIProxy::openLocalInspectorFrontend): (WebKit::WebInspectorUIProxy::closeFrontendPageAndWindow): - To avoid relying on the web process for potentially sensitive parameters, I reworked and removed the canAttach and underTest arguments from openLocalInspectorFrontend. These two values are now stored and managed in the UI process instead, instead of being passed from the web process all the time. - For canAttach, I noticed that the WebInspectorUIProxyMac::platformCanAttach method already implements the same logic as the web process's WebInspector::canAttachWindow. I filed https://webkit.org/b/283435 as a follow-up to clean up the webProcessCanAttach parameter, the canAttachWindow function in the web process, and potentially the m_attached field too, which all become obsolete due to this change. - I couldn't figure out what the `if (m_attached)` in canAttachWindow check does, and to me it had no effect, as this function is not called while inspector is open. - For underTest, I'm now letting the test runner directly set the flag on the WebInspectorUIProxy, as part of my fix to address case WebKit#3 from above. (WebKit::WebInspectorUIProxy::showConsole): (WebKit::WebInspectorUIProxy::showResources): (WebKit::WebInspectorUIProxy::showMainResourceForFrame): (WebKit::WebInspectorUIProxy::togglePageProfiling): - As the web process can longer call OpenLocalInspectorFrontend, call show/connect/openLocalInspectorFrontend here in the UI process instead. (WebKit::WebInspectorUIProxy::requestOpenLocalInspectorFrontend): - To preserve the open inspector^2 button (case WebKit#2 from above), we still maintain this message, but we ignore it unless it's for opening inspector^2, thus renaming the message as a request. This is all assuming that the Web Inspector is not a compromised web process, so we allow that message from it to come through. * Source/WebKit/WebProcess/Inspector/WebInspector.messages.in: * Source/WebKit/WebProcess/Inspector/WebInspector.h: * Source/WebKit/WebProcess/Inspector/WebInspector.cpp: (WebKit::WebInspector::show): - The Show message now takes an async reply, which is used to replace sending WebInspectorUIProxy::OpenLocalInspectorFrontend later. (WebKit::WebInspector::showConsole): (WebKit::WebInspector::showResources): (WebKit::WebInspector::showMainResourceForFrame): (WebKit::WebInspector::startPageProfiling): (WebKit::WebInspector::stopPageProfiling): - Calling inspectorController()->show() no longer does anything, since it's now the UI process's job to show Web Inspector first, for these functions to merely switch to the appropriate tabs. * Source/WebKit/WebProcess/Inspector/WebInspector.cpp: (WebKit::WebInspector::openLocalInspectorFrontend): * Source/WebKit/WebProcess/Inspector/WebInspectorClient.cpp: (WebKit::WebInspectorClient::openLocalFrontend): - Adapt to the command's reworked version. - This is maintained to allow the opening of inspector^2 from the web process (case WebKit#2 from above). For opening inspector^1, this message will be ignored by the UI process. * Source/WebKit/UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::contextMenuItemSelected): - When the "Inspect Element" context menu item is selected (case #1 from above), since the web process may not be privileged to open Web Inspector, handle the showing of inspector here in UI process. * Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp: (WTR::TestRunner::showWebInspector): * Tools/WebKitTestRunner/TestInvocation.cpp: (WTR::TestInvocation::didReceiveMessageFromInjectedBundle): * Source/WebKit/UIProcess/API/C/WKPagePrivate.h: * Source/WebKit/UIProcess/API/C/WKPage.cpp: (WKPageShowWebInspectorForTesting): - Preserve letting the WebKitTestRunner open Web Inspector (case WebKit#3 from above). - Adapt to the change that we now also let the UI process know about the underTest flag for case WebKit#3, rather than letting UI process rely on the value reported by the web process. * Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.h: * Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp: (WKBundlePageShowInspectorForTest): Deleted. - No longer used due to my special fix for case WebKit#3. Originally-landed-as: 283286.537@safari-7620-branch (694a9b5). rdar://144667626 Canonical link: https://commits.webkit.org/290260@main
pascoej
pushed a commit
that referenced
this pull request
Feb 25, 2025
https://bugs.webkit.org/show_bug.cgi?id=287440 rdar://145011222 Reviewed by Ryosuke Niwa. Addressing some more smart pointer warnings in WebProcess part #1. * Source/WebKit/WebProcess/WebProcess.cpp: (WebKit::WebProcess::initializeWebProcess): (WebKit::WebProcess::ensureNetworkProcessConnection): (WebKit::WebProcess::prepareToSuspend): (WebKit::WebProcess::markAllLayersVolatile): (WebKit::WebProcess::libWebRTCNetwork): (WebKit::WebProcess::setUseGPUProcessForMedia): * Source/WebKit/WebProcess/WebProcess.h: Canonical link: https://commits.webkit.org/290637@main
pascoej
pushed a commit
that referenced
this pull request
Feb 25, 2025
https://bugs.webkit.org/show_bug.cgi?id=288102 rdar://145222010 Reviewed by Yusuke Suzuki. Added the notion of a string list to a parsed RegExp that is in the form of /^(?:break|case|which|do|for)/ with an optional trailing $. Such a RegExp will not backtrack and therefore we can streamline the code we emit for such a pattern. This change involves recognizing beginning of string anchored alternations of strings while parsing and then treating the generation of JIT code differently for these patterns. This includes changing how conditional branching works, specifically that instead of the "fall through on match" for each term, to a "jump on match" for the whole alternation. The current code generated for the "case" elternative is: 8:Term PatternCharacter checked-offset:(3) 'c' <156> 0x11381430c: add w1, w1, WebKit#2 <160> 0x113814310: cmp w1, w2 <164> 0x113814314: b.hi 0x113814444 -> <468> 10:Term PatternCharacter checked-offset:(4) 'c' <168> 0x113814318: sub x17, x0, WebKit#4 <172> 0x11381431c: ldr w17, [x17, x1] <176> 0x113814320: movz w16, #0x6163 <180> 0x113814324: movk w16, #0x6573, lsl WebKit#16 -> 0x65736163 <184> 0x113814328: cmp w17, w16 <188> 0x11381432c: b.ne 0x113814444 -> <468> 11:Term PatternCharacter checked-offset:(4) 'a' already handled 12:Term PatternCharacter checked-offset:(4) 's' already handled 13:Term PatternCharacter checked-offset:(4) 'e' already handled 14:NestedAlternativeNext minimum-size:(5),checked-offset:(5) <192> 0x113814330: movz x16, #0x4444 <196> 0x113814334: movk x16, #0x1381, lsl WebKit#16 <200> 0x113814338: movk x16, #0x8001, lsl WebKit#32 <204> 0x11381433c: movk x16, #0xc973, lsl WebKit#48 -> 0x113814444 JIT PC <208> 0x113814340: stur x16, [sp, WebKit#8] <212> 0x113814344: b 0x113814404 -> <404> With some additional backtracking code: 9:NestedAlternativeNext minimum-size:(4),checked-offset:(4) <468> 0x113814444: sub w1, w1, WebKit#2 <472> 0x113814448: b 0x113814348 -> <216> With this change, the processing of "case" becomes: 9:StringListAlternativeNext minimum-size:(4),checked-offset:(4) <132> 0x12a8285c4: sub w1, w1, #1 <136> 0x12a8285c8: cmp w1, w2 <140> 0x12a8285cc: b.hi 0x12a8285e8 -> <168> 10:Term PatternCharacter checked-offset:(4) 'c' <144> 0x12a8285d0: sub x17, x0, WebKit#4 <148> 0x12a8285d4: ldr w17, [x17, x1] <152> 0x12a8285d8: movz w16, #0x6163 <156> 0x12a8285dc: movk w16, #0x6573, lsl WebKit#16 -> 0x65736163 <160> 0x12a8285e0: cmp w17, w16 <164> 0x12a8285e4: b.eq 0x12a82866c -> <300> 11:Term PatternCharacter checked-offset:(4) 'a' already handled 12:Term PatternCharacter checked-offset:(4) 's' already handled 13:Term PatternCharacter checked-offset:(4) 'e' already handled 14:StringListAlternativeNext minimum-size:(5),checked-offset:(5) With no backtracking code. We are able to eliminate one branch and the saving of the continuation PC for backtracking. The code size to process these string list RegExp is reduces. For the example RegExp above, the prior version created 1940 bytes (485 instructions) of code while the code created with this 1392 bytes (345 instructions) of code, a nearly 30% reduction in code. This change is a ~18% progression on the new regexp-keyword-parsing microbenchmark: Baseline YarrStringList regexp-keyword-parsing 136.7065+-0.9807 ^ 116.0161+-1.1791 ^ definitely 1.1783x faster <geometric> 136.7065+-0.9807 ^ 116.0161+-1.1791 ^ definitely 1.1783x faster * JSTests/microbenchmarks/regexp-keyword-parsing.js: Added. (arrayToString): (objectToString): (dumpValue): (compareArray): (compareGroups): (testRegExp): (testRegExpSyntaxError): (let.re.break.case.catch.continue.debugger.default.else.finally.if): (let.re1.break.case.catch.continue.debugger.default.else.finally.if): * JSTests/stress/regexp-parsing-tokens.js: Added. (arrayToString): (objectToString): (dumpValue): (compareArray): (compareGroups): (testRegExp): (testRegExpSyntaxError): * Source/JavaScriptCore/yarr/YarrJIT.cpp: * Source/JavaScriptCore/yarr/YarrPattern.cpp: (JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd): (JSC::Yarr::YarrPatternConstructor::checkForTerminalParentheses): (JSC::Yarr::PatternAlternative::dump): (JSC::Yarr::PatternTerm::dump): * Source/JavaScriptCore/yarr/YarrPattern.h: (JSC::Yarr::PatternTerm::PatternTerm): (JSC::Yarr::PatternAlternative::PatternAlternative): Canonical link: https://commits.webkit.org/290791@main
pascoej
pushed a commit
that referenced
this pull request
Feb 26, 2025
https://bugs.webkit.org/show_bug.cgi?id=288102 rdar://145222010 Reviewed by Yusuke Suzuki. Added the notion of a string list to a parsed RegExp that is in the form of /^(?:break|case|which|do|for)/ with an optional trailing $. Such a RegExp will not backtrack and therefore we can streamline the code we emit for such a pattern. This change involves recognizing beginning of string anchored alternations of strings while parsing and then treating the generation of JIT code differently for these patterns. This includes changing how conditional branching works, specifically that instead of the "fall through on match" for each term, to a "jump on match" for the whole alternation. Fixed a bug in the original version where we weren't properly checking the nested alternatives to see if they only contain fixed single count PatternCharacter terms. The current code generated for the "case" elternative is: 8:Term PatternCharacter checked-offset:(3) 'c' <156> 0x11381430c: add w1, w1, WebKit#2 <160> 0x113814310: cmp w1, w2 <164> 0x113814314: b.hi 0x113814444 -> <468> 10:Term PatternCharacter checked-offset:(4) 'c' <168> 0x113814318: sub x17, x0, WebKit#4 <172> 0x11381431c: ldr w17, [x17, x1] <176> 0x113814320: movz w16, #0x6163 <180> 0x113814324: movk w16, #0x6573, lsl WebKit#16 -> 0x65736163 <184> 0x113814328: cmp w17, w16 <188> 0x11381432c: b.ne 0x113814444 -> <468> 11:Term PatternCharacter checked-offset:(4) 'a' already handled 12:Term PatternCharacter checked-offset:(4) 's' already handled 13:Term PatternCharacter checked-offset:(4) 'e' already handled 14:NestedAlternativeNext minimum-size:(5),checked-offset:(5) <192> 0x113814330: movz x16, #0x4444 <196> 0x113814334: movk x16, #0x1381, lsl WebKit#16 <200> 0x113814338: movk x16, #0x8001, lsl WebKit#32 <204> 0x11381433c: movk x16, #0xc973, lsl WebKit#48 -> 0x113814444 JIT PC <208> 0x113814340: stur x16, [sp, WebKit#8] <212> 0x113814344: b 0x113814404 -> <404> With some additional backtracking code: 9:NestedAlternativeNext minimum-size:(4),checked-offset:(4) <468> 0x113814444: sub w1, w1, WebKit#2 <472> 0x113814448: b 0x113814348 -> <216> With this change, the processing of "case" becomes: 9:StringListAlternativeNext minimum-size:(4),checked-offset:(4) <132> 0x12a8285c4: sub w1, w1, #1 <136> 0x12a8285c8: cmp w1, w2 <140> 0x12a8285cc: b.hi 0x12a8285e8 -> <168> 10:Term PatternCharacter checked-offset:(4) 'c' <144> 0x12a8285d0: sub x17, x0, WebKit#4 <148> 0x12a8285d4: ldr w17, [x17, x1] <152> 0x12a8285d8: movz w16, #0x6163 <156> 0x12a8285dc: movk w16, #0x6573, lsl WebKit#16 -> 0x65736163 <160> 0x12a8285e0: cmp w17, w16 <164> 0x12a8285e4: b.eq 0x12a82866c -> <300> 11:Term PatternCharacter checked-offset:(4) 'a' already handled 12:Term PatternCharacter checked-offset:(4) 's' already handled 13:Term PatternCharacter checked-offset:(4) 'e' already handled 14:StringListAlternativeNext minimum-size:(5),checked-offset:(5) With no backtracking code. We are able to eliminate one branch and the saving of the continuation PC for backtracking. The code size to process these string list RegExp is reduces. For the example RegExp above, the prior version created 1940 bytes (485 instructions) of code while the code created with this 1392 bytes (345 instructions) of code, a nearly 30% reduction in code. This change is a ~18% progression on the new regexp-keyword-parsing microbenchmark: Baseline YarrStringList regexp-keyword-parsing 136.7065+-0.9807 ^ 116.0161+-1.1791 ^ definitely 1.1783x faster <geometric> 136.7065+-0.9807 ^ 116.0161+-1.1791 ^ definitely 1.1783x faster * JSTests/microbenchmarks/regexp-keyword-parsing.js: Added. (arrayToString): (objectToString): (dumpValue): (compareArray): (compareGroups): (testRegExp): (testRegExpSyntaxError): (let.re.break.case.catch.continue.debugger.default.else.finally.if): (let.re1.break.case.catch.continue.debugger.default.else.finally.if): * JSTests/stress/regexp-parsing-tokens.js: Added. (arrayToString): (objectToString): (dumpValue): (compareArray): (compareGroups): (testRegExp): (testRegExpSyntaxError): * Source/JavaScriptCore/yarr/YarrJIT.cpp: * Source/JavaScriptCore/yarr/YarrPattern.cpp: (JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd): (JSC::Yarr::YarrPatternConstructor::checkForTerminalParentheses): (JSC::Yarr::PatternAlternative::dump): (JSC::Yarr::PatternTerm::dump): * Source/JavaScriptCore/yarr/YarrPattern.h: (JSC::Yarr::PatternTerm::PatternTerm): (JSC::Yarr::PatternAlternative::PatternAlternative): Canonical link: https://commits.webkit.org/290982@main
pascoej
pushed a commit
that referenced
this pull request
Feb 27, 2025
https://bugs.webkit.org/show_bug.cgi?id=264576 rdar://114997939 Reviewed by BJ Burg. (This work was done in collaboration with Razvan and was based on his draft at WebKit@377f3e1.) This commit enables automatically inspecting and pausing the ServiceWorkerDebuggable. The idea is similar to the same functionalities with the JSContext/JSGlobalObjectDebuggable. The general flow is: 1. When the debuggable is first created, we optionally mark it as inspectable. 2. As soon as the debuggable is marked inspectable, its main thread (the thread that it was created on) gets blocked. 3. When the auto-launched Web Inspector frontend finishes initializing, it notifies the backend. - It's important for the debuggable to wait for this signal because a genuine auto-inspection must appear attached to the debuggable before it begins execution, respecting any breakpoints set early on in its script (where auto-pausing is basically a breakpoint before line 1). 4. The backend unpauses the blocked debuggable. If auto-pausing was requested, tell the debugger agent to pause. The service worker begins executing script unless its worker thread was specified to start in the WorkerThreadStartMode::WaitForInspector. During that waiting period, the worker thread can perform tasks sent into its debugging run loop, until it's signaled to stop waiting and continue to execute the script like normal. This commit makes use of that interface to make the service worker pause (when justified, i.e. developerExtrasEnabled) before running the above flow resembling auto-inspecting a JSContext. * Source/WebCore/workers/service/context/ServiceWorkerThread.cpp: (WebCore::threadStartModeFromSettings): (WebCore::ServiceWorkerThread::ServiceWorkerThread): - When there is potentially a remote inspector that would like to auto-inspect, make it so that the thread waits on start before executing its script. * Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h: * Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp: (WebCore::ServiceWorkerThreadProxy::ServiceWorkerThreadProxy): (WebCore::ServiceWorkerThreadProxy::threadStartedRunningDebuggerTasks): - Setting inspectability is step #1 in the above flow. - In step WebKit#2, calling `debuggable->setInspectable(true)` might block already, but we don't want that until the worker thread is setup and have the run loop be in debug mode, so we do that in a callback instead. - In step WebKit#4, when connection to the inspector completes or fails, the setInspectable call only returns then, so we unblock the worker thread to resume code execution. * Source/WebCore/inspector/agents/worker/WorkerDebuggerAgent.h: * Source/WebCore/inspector/WorkerInspectorController.h: * Source/WebCore/inspector/WorkerInspectorController.cpp: (WebCore::WorkerInspectorController::frontendInitialized): (WebCore::WorkerInspectorController::connectFrontend): (WebCore::WorkerInspectorController::disconnectFrontend): (WebCore::WorkerInspectorController::createLazyAgents): (WebCore::WorkerInspectorController::ensureDebuggerAgent): * Source/WebCore/workers/service/context/ServiceWorkerDebuggable.cpp: (WebCore::ServiceWorkerDebuggable::connect): * Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.h: * Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp: (WebCore::ServiceWorkerInspectorProxy::connectToWorker): - Mimic the logic for auto-inspecting a JSContext/JSGlobalObjectDebuggable. * Source/JavaScriptCore/inspector/protocol/Inspector.json: - Step WebKit#3 in the above flow, notify the backend when frontend completes setting up. * Source/WebCore/workers/service/context/ServiceWorkerDebuggable.h: - Allow service workers to be auto-inspected. (This is checked at https://github.com/rcaliman-apple/WebKit/blob/eng/Web-Inspector-Automatically-connect-Web-Inspector-to-ServiceWorker/Source/JavaScriptCore/inspector/remote/RemoteInspectionTarget.cpp#L95) * Source/WTF/wtf/PlatformEnableCocoa.h: - Add feature flag just in case. Canonical link: https://commits.webkit.org/291167@main
pascoej
pushed a commit
that referenced
this pull request
Apr 2, 2025
https://bugs.webkit.org/show_bug.cgi?id=290862 <rdar://147215658> Reviewed by Antti Koivisto. "Reusing block" type mutations (see RenderTreeBuilder::Inline::splitFlow) followed by float removal may lead to an unexpected state where we have a float to remove, but we have already destroyed m_floatingObjects, causing us to incorrectly assume that the float no longer belongs here (markSiblingsWithFloatsForLayout) and, therefore, does not need to be removed from sibling blocks (in case it is intrusive). What happens here is: 1. tree mutation makes an anon block reused (pre block) 2. a float is removed from said anon block's subtree At #1 we call removeFloatingObjects() which simply clears and destroys m_floatingObjects on the anon block. Now at WebKit#2, when we try to remove this float from sibling block containers by calling RenderBlockFlow::markSiblingsWithFloatsForLayout, and we consult m_floatingObjects to see if there's any float associated with the block and we early return as we had already cleared this set at #1. This patch ensures that when markSiblingsWithFloatsForLayout is called with a valid float, we always try to clean up sibling content. * LayoutTests/fast/block/float-remove-after-block-collapse-crash-expected.txt: Added. * LayoutTests/fast/block/float-remove-after-block-collapse-crash.html: Added. * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout): Change for (siblings) for (set items) to for (set items) for (siblings) so that the 'for (siblings)' logic can be moved to a lambda and used when there's a valid incoming float. Canonical link: https://commits.webkit.org/293094@main
webkit-commit-queue
pushed a commit
that referenced
this pull request
Apr 2, 2025
…n addFloatsToNewParent https://bugs.webkit.org/show_bug.cgi?id=290898 <rdar://143296265> Reviewed by Antti Koivisto. In this patch 1. we let m_floatingObjects go stale on the skipped root (we already do that for the skipped subtree by not running layout) 2. we descend into skipped subtrees while cleaning up floats even when m_floatingObjects is stale/empty Having up-to-date m_floatingObjects on the skipped root, while stale m_floatingObjects on the skipped subtree can lead to issues when (#1) a previously intrusive float (WebKit#2) becomes non-intrusive and (WebKit#3) eventually gets deleted prevents us from being able to cleanup m_floatingObjects in skipped subtree(s). at #1 m_floatingObjects is populated with the intrusive float (both skipped root and renderers in skipped subtree) and at WebKit#2 since we only run layout on the skipped root, m_floatingObjects gets updated by removing this previously intrusive float (skipped subtree becomes stale) and at WebKit#3 we don't descend into the skipped subtree to cleanup m_floatingObjects since the skipped root does not have this float anymore (removed at WebKit#2). * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout): Canonical link: https://commits.webkit.org/293119@main
pascoej
pushed a commit
that referenced
this pull request
Apr 25, 2025
…n addFloatsToNewParent https://bugs.webkit.org/show_bug.cgi?id=290898 <rdar://149579273> Reviewed by Antti Koivisto. In this patch 1. we let m_floatingObjects go stale on the skipped root (we already do that for the skipped subtree by not running layout) 2. we descend into skipped subtrees while cleaning up floats even when m_floatingObjects is stale/empty Having up-to-date m_floatingObjects on the skipped root, while stale m_floatingObjects on the skipped subtree can lead to issues when (#1) a previously intrusive float (WebKit#2) becomes non-intrusive and (WebKit#3) eventually gets deleted prevents us from being able to cleanup m_floatingObjects in skipped subtree(s). at #1 m_floatingObjects is populated with the intrusive float (both skipped root and renderers in skipped subtree) and at WebKit#2 since we only run layout on the skipped root, m_floatingObjects gets updated by removing this previously intrusive float (skipped subtree becomes stale) and at WebKit#3 we don't descend into the skipped subtree to cleanup m_floatingObjects since the skipped root does not have this float anymore (removed at WebKit#2). * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout): Canonical link: https://commits.webkit.org/293889@main
webkit-commit-queue
pushed a commit
that referenced
this pull request
Apr 28, 2025
…rsor smoothly rdar://148762897 https://bugs.webkit.org/show_bug.cgi?id=292042 Reviewed by Devin Rousso. This is a combination of two issues: 1. If the WKInspectorWKWebView has safeAreaInsets set and therefore _obscuredContentInsets configured, adjusting its dimensions does not take that into account. Assuming there is a left inset configured to be X and the new docked inspector width requested by the frontend is W, the frontend is not aware of the extra X pixels needed when rendering the WKInspectorWKWebView (because the frontend lives inside that web view), which then needs to be (W + X) pixels wide instead. 2. The setAttachedWindowHeight/Width commands in the backend take an `unsigned int` as the new dimension's type, but the frontend computes in floating point (JavaScript's Number), which always gets rounded down when the new dimension gets passed in and coerced to an int. This makes the new view just slightly smaller than required over the course of dragging. * Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm: (WebKit::WebInspectorUIProxy::platformSetAttachedWindowHeight): (WebKit::WebInspectorUIProxy::platformSetAttachedWindowWidth): - Fix issue #1. * Source/WebInspectorUI/UserInterface/Base/Main.js: - Fix issue WebKit#2. - Adding the rounding on resulting dimension alone was sufficient to fix the "always-shrinking" bug. However, resizing still didn't feel completely smooth; the inspector wouldn't shrink by itself, but it still felt like having some lag following the moving cursor. I took a simpler approach to compute the delta, where we use the original dimension as the standard instead of the last frame, which turns out to be extremely smooth and exactly what we wanted. Tested the fix manually on various inspector zoom factors from 60% to 240%, combined with or without an _obscuredContentInsets applied on the WKInspectorWKWebView. Canonical link: https://commits.webkit.org/294123@main
pascoej
pushed a commit
that referenced
this pull request
May 28, 2025
https://bugs.webkit.org/show_bug.cgi?id=293470 Reviewed by Antti Koivisto. 1. In WebKit, column spanners are moved out of their original insertion position and get reparented under the column container. 2. "is this renderer inside a skipped subtree" logic consults parent state. Now the parent of a column spanner (#1) may be completely outside of 'c-v: h' tricking us into believing the spanner is not hidden. * LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner.html: Added. * Source/WebCore/rendering/RenderBlock.cpp: (WebCore::RenderBlock::paintChild): * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::layoutBlockChildren): * Source/WebCore/rendering/RenderBox.h: * Source/WebCore/rendering/RenderBoxInlines.h: (WebCore::RenderBox::isColumnSpanner const): * Source/WebCore/rendering/RenderObject.cpp: (WebCore::RenderObject::isSkippedContent const): Canonical link: https://commits.webkit.org/295335@main
pascoej
pushed a commit
that referenced
this pull request
May 28, 2025
https://bugs.webkit.org/show_bug.cgi?id=293337 rdar://151740794 Reviewed by Yijia Huang and Justin Michaud. The current MovHintRemoval's analysis looks weird. We should just do liveness analysis globally and use this information for MovHint removal. 1. "Use" is a node which may exit. When exit happens, we should keep all use of live locals at this bytecode exit location alive. 2. "Def" is MovHint. We kill the locals here. And doing fixpoint analysis and using this information to remove MovHint. Also, pruning Availability in OSRAvailabilityAnalysisPhase via bytecode liveness is wrong: they need to keep live nodes from DFG for example. 0: PutHint @x, PROP(@y) 1: OSR exit point #1 (here, loc0 is not alive) 2: -- Pruning happens -- 3: MovHint @x, loc0 4: OSR exit point WebKit#2 (here, loc0 is alive) In this case pruning point will remove (0)'s heap availability since @x is not alive from bytecode at (1), but this is wrong as we need this in (4). In this patch, we remove pruneByLiveness in DFGOSRAvailabilityAnalysisPhase. This pruning should happen by the user of DFGOSRAvailabilityAnalysisPhase instead, and it is already happening (see FTLLowerToB3's pruneByLiveness in exit site, which is right. And ObjectAllocationSinking is pruning with CombinedLiveness, this is right since it also accounts Node's liveness in addition to bytecode's liveness.). Let's just make availability just compute the availability for all things, and then we prune some of unnecessary ones at each use of this information. * Source/JavaScriptCore/dfg/DFGMovHintRemovalPhase.cpp: * Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp: (JSC::DFG::OSRAvailabilityAnalysisPhase::run): Canonical link: https://commits.webkit.org/295369@main
webkit-commit-queue
pushed a commit
that referenced
this pull request
May 28, 2025
https://bugs.webkit.org/show_bug.cgi?id=293456 Reviewed by Antti Koivisto. 1. setPreferredLogicalWidthsDirty was introduced at 17615@main modeled after setNeedsLayout 2. As with setNeedsLayout, setPreferredLogicalWidthsDirty has the ability to only mark the current renderer dirty (as opposed to the default behavior of marking ancestors as well) 3. Initially (17615@main) MarkOnlyThis was passed in only on the RenderView (as it has no parent) 4. Later at 17621@main, MarkOnlyThis was used to fix a specific bug where an absolute positioned box with percent padding had incorrect size after viewport resize. Here is what happened there: RenderView RenderBlockFlow (html) RenderBlockFlow (body) RenderBlockFlow (absolute positioned box with % padding) - absolute positioned box uses shrink-to-fit sizing when width is auto. The final width is the combination of its min/max widths and the available space. - % padding is resolved against the containing block's width. Now with viewport resize, where the absolute positioned box's containing block is the RenderView - min/max _content_ values stay the same but - the viewport's new size affects the padding value so the box's final min/max values do change. Min/max values (aka preferred width) are cached on the renderers and we don't recompute them unless PreferredLogicalWidthsDirty bit is set to true (similar to needsLayout bit). We mark renderers dirty in two distinct places: #1 when content/style changes before layout or WebKit#2 during layout as we figure we need to invalidate more content In many cases (e.g. resize) in order to evaluate the extent of the damage up front and mark all affected renderers dirty would require a full tree walk, so instead we rely on layout to mark additional renderers dirty as needed. ...which is how we fixed the viewport resize bug in 17621@main. if (RelayoutChildren::Yes && renderer.preferredWidthDependsOnAvailableSpace()) renderer.setPreferredLogicalWidthsDirty(true, MarkOnlyThis) - check during layout if the current renderer's final width depends on the available space - if it does, mark the preferred widths dirty This ensures that by the time we get to computeLogicalWidth() -where we compute the final size of the renderer- preferredLogicalWidths bit is already dirty. It guarantees that we re-compute our min/max values, allowing the new padding value to be incorporated. Now consider a scenario where this positioned box has fixed width (e.g. width: 100px) - we still mark preferred widths dirty (we have to) but - in computeLogicalWidth(), we will not re-compute them as we simply take the fixed width (instead of running the shrink-to-fit logic) - and preferredWidths stays dirty So just because we have a box with preferredWidthDependsOnAvailableSpace(), it does not necessarily mean we run shrink-to-fit sizing and as a result we may not clear the dirty bit. ...and this is where setPreferredLogicalWidthsDirty differs from setNeedsLayout. Whenever we call setNeedsLayout(MarkOnlyThis), it is always followed by a layoutIfNeeded() call, clearing the needsLayout bit, while preferredWidths may remain dirty. While it is crucial that no needsLayout bit is set as returning from layout, preferredWidths bits can stay dirty throughout the entire lifetime of a renderer if they are never used. The reason why having a stuck dirty bit is problematic though, is because we rely on them when marking ancestor chain dirty. The default behavior of both needsLayout and preferredLogicalWidthsDirty is MarkContainingBlockChain (when a renderer changes it's likely that parent changes too). With MarkContainingBlockChain, we climb the ancestor chain and mark containers dirty, unless we find an already dirty container. This performance optimization helps to avoid to walk the ancestor chain every time a renderer is marked dirty, but it also assumes that if a renderer is dirty all its ancestors are dirty as well. So now JS mutates some style and we try to mark our ancestor chain dirty to let our containers know that at the subsequent layout they need to re-compute their min/max values if their sizing relies on them. ...but in setPreferredLogicalWidthsDirty, we bail out too early when we come across this stuck renderer and never mark the parents dirty. So maybe 17621@main should have picked MarkContainingBlockChain and not MarkOnlyThis to fully invalidate the ancestor chain. Before considering that let's take a look at how min/max values are used. In block layout we first visit the parent, compute its width and descend into the children and pass in the parent width as available space. If the parent's width depends on the size of the children (e.g. width: min-content), we simply ask the children for their min/max widths. There's a special "preferred width" codepath in our block layout implementation. This codepath computes min/max widths and caches them on the renderers. Important to note that this happens _before_ running layout on the child renderers. (this may sound like some form of circular dependency, but CSS spec is clear about how to resolve cases where child min/max widths depend on the final size of the parent width) What it means is by the time we run layout on a renderer, the parent may have already "forced" the renderer to re-compute the stale min/max widths. So now imagine we are in the renderer's layout code now and come across this line if (RelayoutChildren::Yes && renderer.preferredWidthDependsOnAvailableSpace()) renderer.setPreferredLogicalWidthsDirty(true, MarkOnlyThis) This makes us to re-compute the min/max values even if they are clean (and this is the result of not being able to effectively run invalidation up front, before layout) With MarkOnlyThis, the worst case scenario (beside the sticky bit bug) is that we may end up running min/max computation twice; first triggered by our parent followed by this line above. However, with MarkContainingBlockChain, we would keep re-computing valid and clean min/max values at every layout on the ancestors as well. (as ancestors would see their dirty min/max values at the next layout the first time and then they would re-compute them, followed by us marking them dirty again and so on) While MarkContainingBlockChain is normally what we do as changing min/max values on the child may affect the ancestors too, it is too late to use it at layout due to block layout's "preferred width first -> layout second" order. The fundamental issue here is that we can't tell if the renderer's min/max values got cleared in the current layout frame by ancestors running preferred with computation on their subtree. If we could, we would either 1, not call renderer.setPreferredLogicalWidthsDirty(true, MarkOnlyThis) at all i.e. min/max values are really really clear, so let's just reuse them 2, or call it by passing in MarkContainingBlockChain (and go with #1 at subsequent layout if applicable) TLDR; While it's okay for preferredWidths to stay dirty across layouts, when a renderer is dirty all its ancestors have to be dirty. Calling setPreferredLogicalWidthsDirty() with MarkOnlyThis during layout is also fine as long as we managed to clear it before finishing layout. Here let's just fix the sticky bit by making sure ancestor chain is always fully marked. - add a rare bit to indicate if we used MarkOnlyThis on this renderer - adjust the "if this renderer dirty its parent must be dirty" logic by consulting the MarkOnlyThis bit. * LayoutTests/fast/dynamic/percent-padding-with-shrink-to-fit-parent-expected.html: Added. * LayoutTests/fast/dynamic/percent-padding-with-shrink-to-fit-parent.html: Added. * Source/WebCore/rendering/RenderObject.cpp: (WebCore::RenderObject::setPreferredLogicalWidthsDirty): (WebCore::RenderObject::invalidateContainerPreferredLogicalWidths): * Source/WebCore/rendering/RenderObject.h: Canonical link: https://commits.webkit.org/295501@main
pascoej
pushed a commit
that referenced
this pull request
Sep 2, 2025
https://bugs.webkit.org/show_bug.cgi?id=297724 rdar://157024791 Reviewed by Antti Koivisto. 1. out-of-flow boxes participate first in in-flow layout as if they were in-flow boxes where we compute their static position. This static position becomes their final position when inset (left, right, top, bottom) is auto. 2. as a second step, as we reach the out-of-flow box's containing block we run layout again and compute the final position (this might just be what we computed at #1 in case of auto inset) Now we mark the out-of-flow box dirty at #1 and expect WebKit#2 to clear the box by moving it to its final position. However in case of subtree layout where the layout root has an out-of-flow descendant while the containing block is an ancestor of the layout root, WebKit#2 will never happen (we bail out of layout before reaching the containing block). "setNeedsLayout" was added at 254969@main to mimic what legacy line layout did. However starting from 262470@main, we already move the box at #1 meaning that WebKit#2 does not need to happen if the box is statically positioned only. (If the out-of-flow box was not-statically positioned, subtree layout would not start "below" its containing block) * LayoutTests/TestExpectations: * Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp: (WebCore::LayoutIntegration::LineLayout::updateRenderTreePositions): Canonical link: https://commits.webkit.org/299021@main
pascoej
pushed a commit
that referenced
this pull request
Sep 30, 2025
https://bugs.webkit.org/show_bug.cgi?id=299504 rdar://161294228 Reviewed by Yijia Huang. Previous one 300327@main worked for the same basic block's load, but it didn't work for the load in dominators. This patch updates CSE rules to make immutable load elimination work with dominators' load. Like, BB#0 @0: Load(@x, immutable) @1: CCall(...) # potentially clobber everything Branch ... #1, WebKit#2 BB#1 @2: CCall(...) # potentially clobber everything Jump WebKit#3 BB#2 @3: CCall(...) # potentially clobber everything Jump WebKit#3 BB#3 @4: Load(@x, immutable) ... Then @4 should be replaced with Identity(@0) as dominator BB#0 is having immutable load @0 matching to @4. Tests: Source/JavaScriptCore/b3/testb3_1.cpp Source/JavaScriptCore/b3/testb3_8.cpp * Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp: * Source/JavaScriptCore/b3/testb3.h: * Source/JavaScriptCore/b3/testb3_1.cpp: (run): * Source/JavaScriptCore/b3/testb3_8.cpp: (testLoadImmutableDominated): (testLoadImmutableNonDominated): Canonical link: https://commits.webkit.org/300562@main
pascoej
pushed a commit
that referenced
this pull request
Sep 30, 2025
…on-in-child.html is failing https://bugs.webkit.org/show_bug.cgi?id=299628 rdar://161203486 Reviewed by Basuke Suzuki. Suppose we have a mainframe and an iframe and this series of navigations happens: 1. iframe fragment navigates to "/#a" 2. main frame fragment navigates to "/#1" 3. main frame fragment navigates to "/WebKit#2" 4. main frame fragment navigates to "/WebKit#3" 5. iframe goes back 6. iframe fragment navigates to "/#b" After Step 5, the UI Process b/f list should be: A) mainframe - URL - ItemID A ** iframe - URL - ItemID A B) mainframe - URL - ItemID B ** iframe - URL/#a - ItemID B C) mainframe - URL/#1 - ItemID C ** iframe - URL/#a - ItemID C D) mainframe - URL/WebKit#2 - ItemID D ** iframe - URL/#a - ItemID D E) mainframe - URL/WebKit#3 - ItemID E ** iframe - URL/#a - ItemID E The mainframe's Navigation object's m_entries should be: A) mainframe - URL - ItemID A C) mainframe - URL/#1 - ItemID C D) mainframe - URL/WebKit#2 - ItemID D E) mainframe - URL/WebKit#3 - ItemID E <--- current index The iframe's Navigation object's m_entries should be: A) ** iframe - URL - ItemID A <--- current index E) ** iframe - URL/#a - ItemID E According to this layout test, after Step 6: The mainframe's Navigation object's m_entries should be: A) mainframe - URL - ItemID A <--- current index The iframe's Navigation object's m_entries should be: A) ** iframe - URL - ItemID A F) ** iframe - URL/#b - ItemID F <--- current index So when a subframe has a PUSH same-document navigation and disposes of any forward entries, any parent frame must do the same. This test was failing because the parent frame was not disposing of its forward entries. To fix this, we add a new function to recusively dispose of all forward entries in any parent frames when a subframe has a PUSH same-document navigation. We use the ItemID to determine what entry must stay and then dispose of any entries that come after that one. * LayoutTests/imported/w3c/web-platform-tests/navigation-api/per-entry-events/dispose-for-navigation-in-child-expected.txt: * Source/WebCore/page/LocalDOMWindowProperty.cpp: (WebCore::LocalDOMWindowProperty::protectedFrame const): * Source/WebCore/page/LocalDOMWindowProperty.h: * Source/WebCore/page/Navigation.cpp: (WebCore::Navigation::updateNavigationEntry): (WebCore::Navigation::disposeOfForwardEntriesInParents): Call recursivelyDisposeOfForwardEntriesInParents on the main frame, which will traverse down the frame tree, and for each frame until we reach the subframe that actually navigated, dispose of any forward entries. (WebCore::Navigation::recursivelyDisposeOfForwardEntriesInParents): (WebCore::Navigation::updateForNavigation): This is called for same-document navigations. If it's a PUSH navigation, call disposeOfForwardEntriesInParents. The ItemID that we keep in these parent frames is the current ItemID right before this PUSH operation happens. * Source/WebCore/page/Navigation.h: Canonical link: https://commits.webkit.org/300721@main
pascoej
pushed a commit
that referenced
this pull request
Oct 11, 2025
rdar://161694748 https://bugs.webkit.org/show_bug.cgi?id=299920 Reviewed by Ryosuke Niwa. Prior to this patch, DocumentInlines.h was the most expensive header used in WebCore; it is included 303 times in a WebCore Unified build, and on this machine, each include of that file too the compile on average 1.5s to parse, for a total of 8m CPU minutes of time spent parsing this header. To diagnose why this header is so expensive, I commented out all the implementations from the body of this header and ran a compile, collecting a list of all the errors encountered. I then collected the counts of each error message, and grouped liked errors together. It became clear that most uses of DocumentInlines.h were to pull in a very narrow set of inlined methods, but to get those few methods had to include a very large and very heavy header. To address this, DocumentInlines.h was broken up into separate headers, each which includes only Document.h and one or two other headers. Usually the second header was required to define the return type of the method, and the type definition is required because the return type is a CheckedPtr or RefPtr. However because the return value is typically used at the call site, that return value header would have been included already, meaning this style of header has zero effective additional cost. The files were created in descending order of the number of uses: - DocumentView.h - 187 - DocumentPage.h - 175 - DocumentQuirks.h - 84 - DocumentSecurityOrigin.h - 54 - DocumentResourceLoader.h - 49 - DocumentMarkers.h - 27 - DocumentEventLoop.h - 26 - DocumentSettingsValues.h - 23 - DocumentWindow.h - 19 The remaining grab-bag of inlined methods had less than 10 uses each and were left in DocumentInlines.h. After this patch, DocumentInlines.h is used in just over 100 implementation files and no headers. None of the new Document*.h headers or DocumentInlines.h appear in the top 100 most expensive header files. DocumentInlines.h dropped from the #1 most expensive header taking 8m to parse to WebKit#389 and 5.7s of parsing. The most expensive new header is DocumentPage.h at WebKit#124 and 25s of parsing. Canonical link: https://commits.webkit.org/300989@main
pascoej
pushed a commit
that referenced
this pull request
Nov 4, 2025
…are trimmed https://bugs.webkit.org/show_bug.cgi?id=301794 Reviewed by Antti Koivisto. 1. In 'TextBoxPainter::collectDecoratingBoxesForBackgroundPainting', make 'decoratingBoxLocation' represent the actual position of the decorating box itself, not the position of the text content inside it (the inline box). (While these typically align, in cases with trimmed sides, the inline box can be offset while the text content remains in its original position.) 2. Make sure that 'underlineOffsetForTextBoxPainting' consistently returns the underline position relative to the decorating inline box - not the text _inside_ the decorating inline box (see #1). Test: fast/inline/text-box-trim-with-underline2.html * LayoutTests/fast/inline/text-box-trim-with-underline2-expected.html: Added. * LayoutTests/fast/inline/text-box-trim-with-underline2.html: Added. * Source/WebCore/rendering/TextBoxPainter.cpp: (WebCore::TextBoxPainter::collectDecoratingBoxesForBackgroundPainting): * Source/WebCore/style/InlineTextBoxStyle.cpp: (WebCore::textBoxEdgeAdjustmentForUnderline): (WebCore::underlineOffsetForTextBoxPainting): * Source/WebCore/style/InlineTextBoxStyle.h: Canonical link: https://commits.webkit.org/302435@main
webkit-commit-queue
pushed a commit
that referenced
this pull request
Jan 14, 2026
https://bugs.webkit.org/show_bug.cgi?id=304917 rdar://167529315 Reviewed by Marcus Plutowski. This patch implements chain of compare with ARM64 ccmp / ccmn. Let's say, if (x0 == 0 && x1 == 1) { // target-block } Then this will be compiled via LLVM into wasm. And this will create BitAnd(Equal(a, 0), Equal(b, 1)). Then ARM64 ccmp can handle it as follows. cmp x0, #0 ccmp x1, #1, #0, eq // cmp x1, #1 when flag is eq and override, otherwise set #0 to flag b.eq target-block This reduces small weird basic blocks and reduces prediction miss, and reduces code size. We introduce CompareOnFlags, CompareConditionallyOnFlags, and BranchOnFlags Air opcodes. All of them are annotated as /effects since this relies on the current flag, and they are not producing register output while it has side effects. This ensures that we are not removing these instructions, and we are not hoisting etc. randomly. We introduced V8's compare chain detection mechanism and using it for chained cmp code generation. Tests: Source/JavaScriptCore/b3/testb3_1.cpp Source/JavaScriptCore/b3/testb3_8.cpp * Source/JavaScriptCore/assembler/MacroAssemblerARM64.h: (JSC::MacroAssemblerARM64::compareOnFlags32): (JSC::MacroAssemblerARM64::compareOnFlags64): (JSC::MacroAssemblerARM64::compareOnFlagsFloat): (JSC::MacroAssemblerARM64::compareOnFlagsDouble): (JSC::MacroAssemblerARM64::compareConditionallyOnFlags32): (JSC::MacroAssemblerARM64::compareConditionallyOnFlags64): (JSC::MacroAssemblerARM64::compareConditionallyOnFlagsFloat): (JSC::MacroAssemblerARM64::compareConditionallyOnFlagsDouble): (JSC::MacroAssemblerARM64::branchOnFlags): * Source/JavaScriptCore/b3/B3LowerToAir.cpp: * Source/JavaScriptCore/b3/air/AirOpcode.opcodes: * Source/JavaScriptCore/b3/air/AirOptimizeBlockOrder.cpp: (JSC::B3::Air::optimizeBlockOrder): * Source/JavaScriptCore/b3/testb3.h: * Source/JavaScriptCore/b3/testb3_1.cpp: (run): * Source/JavaScriptCore/b3/testb3_8.cpp: (testCCmpAnd32): (testCCmpAnd64): (testCCmpOr32): (testCCmpOr64): (testCCmpAndAnd32): (testCCmpOrOr32): (testCCmpAndOr32): (testCCmnAnd32WithNegativeImm): (testCCmnAnd64WithNegativeImm): (testCCmpWithLargePositiveImm): (testCCmpWithLargeNegativeImm): (testCCmpSmartOperandOrdering32): (testCCmpSmartOperandOrdering64): (testCCmpOperandCommutation32): (testCCmpOperandCommutation64): (testCCmpCombinedOptimizations): (testCCmpZeroRegisterOptimization32): (testCCmpZeroRegisterOptimization64): (testCCmpMixedAndOr32): (testCCmpMixedOrAnd32): (testCCmpNegatedAnd32): (testCCmpNegatedOr32): (testCCmpMixedWidth32And64): (testCCmpMixedWidth64And32): Canonical link: https://commits.webkit.org/305493@main
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
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.
Pull Request Template
File a Bug
All changes should be associated with a bug. The WebKit project is currently using Bugzilla as our bug tracker. Note that multiple changes may be associated with a single bug.
Provided Tooling
The WebKit Project strongly recommends contributors use
Tools/Scripts/git-webkitto generate pull requests. See Setup and Contributing Code for how to do this.Template
If a contributor wishes to file a pull request manually, the template is below. Manually-filed pull requests should contain their commit message as the pull request description, and their commit message should be formatted like the template below.
Additionally, the pull request should be mentioned on Bugzilla, labels applied to the pull request matching the component and version of the Bugzilla associated with the pull request and the pull request assigned to its author.