Skip to content

Comments

Create blank.yml#1

Open
pascoej wants to merge 1 commit intomainfrom
actioneeeee
Open

Create blank.yml#1
pascoej wants to merge 1 commit intomainfrom
actioneeeee

Conversation

@pascoej
Copy link
Owner

@pascoej pascoej commented Nov 11, 2022

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-webkit to 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.

< bug title >
https://bugs.webkit.org/show_bug.cgi?id=#####

Reviewed by NOBODY (OOPS!).

* path/changed.ext:
(function):
(class.function):

@pascoej pascoej marked this pull request as ready for review November 11, 2022 10:33
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant