Skip to content

Conversation

berelig
Copy link

@berelig berelig commented Feb 17, 2021

Missing period after the subdomain.

Missing period before the subdomain.
@JonWBedard
Copy link
Member

@berelig we aren't ready to start using GitHub pull requests yet (although it's something we're working towards), for now, please file a bug via bugs.webkit.org and upload a patch with Tools/Scripts/webkit-patch upload. You can do all of this from your GitHub checkout.

@JonWBedard JonWBedard closed this Feb 17, 2021
webkit-commit-queue pushed a commit that referenced this pull request Mar 17, 2021
https://bugs.webkit.org/show_bug.cgi?id=223082

Patch by Kimmo Kinnunen <kkinnunen@apple.com> on 2021-03-17
Reviewed by Kenneth Russell.

Source/WebCore:

* CMakeLists.txt:
* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
* platform/graphics/cocoa/GraphicsContextGLOpenGLCocoa.mm:
(WebCore::InitializeEGLDisplay):
(WebCore::GraphicsContextGLOpenGL::GraphicsContextGLOpenGL):
* platform/graphics/opengl/GraphicsContextGLOpenGL.h:
Hold the default EGLDisplay with a scoped holder that counts
the references to the default display.
(WebCore::GraphicsContextGLOpenGL::releaseAllResourcesIfUnused):
Add a call that uninitializes the ANGLE default display if there are no uses of
the default display.

Source/WebKit:

Schedule a check for releasing the ANGLE default display when global
count of remote graphics contexts reach zero. This should decrease the
memory use of sessions where WebGL is not always on.

Dispatch the check 0.2s after hitting zero, so that the optimization still
affects page navigations but maybe does not redundantly deinitialize /
reinitialize ANGLE in the cases where the context #1 is created and destroyed
frequently.

* GPUProcess/graphics/RemoteGraphicsContextGL.cpp:
(WebKit::dispatchReleaseAllResourcesIfUnused):
(WebKit::RemoteGraphicsContextGL::initialize):
(WebKit::RemoteGraphicsContextGL::stopListeningForIPC):

Canonical link: https://commits.webkit.org/235402@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274557 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit that referenced this pull request Apr 7, 2021
    REGRESSION(r268615): images flicker on apple.com/ios/ios-14
    https://bugs.webkit.org/show_bug.cgi?id=221054
    <rdar://problem/72880447>

    Reviewed by Dean Jackson.

    Source/WebCore:

    When we added support for accelerated animations of individual transform properties in r268615 (bug 217842),
    we made it so that base values of each transform-related property had a non-interpolating animation in the
    Core Animation animations list that would combine with interpolating animations for that property as additive
    animations. Prior to any of those animations, we'd reset the combined transform with an identity transform
    as another non-interpolating animation.

    However, we neglected to consider the case where one of the interpolating animations would not start right
    away if a positive delay was set. In the case of this apple.com page, the target element would be composited
    due to a "will-change: transform" style, and a non-animated "transform" was set as well as an animation for
    the "transform" property with a delay.

    Since we had a "transform" animation, we'd create a Core Animation animations lists as follows:

        1. non-interpolating, non-additive animation set to the identity matrix
        2. interpolating, additive animation with the keyframes set in the CSS animation, with a begin time
           set to the current time plus the specified delay

    The result of this was that during the animation delay, the static "transform" property was overridden
    by animation #1 until animation #2 would kick in.

    We now make it so that for each transform-related property, we create a non-interpoloating, additive animation
    to represent the static value for that property for the duration of any potential delay until the first
    interpolating animation for this property starts.

    In this example, the Core Animation animations list is now as follows:

        1. non-interpolating, non-additive animation set to the identity matrix
        2. non-interpolating, additive animation set to the static transform value
        3. interpolating, additive animation with the keyframes set in the CSS animation, with a begin time
           set to the current time plus the specified delay

    We implement this with a new lambda function within GraphicsLayerCA::updateAnimations() called
    addAnimationsForProperty() which adds a non-interpolating animation in two cases:

        1. if there is no animation for this property at all, making it last forever
        2. if all animations have a delay, making it last until the first animation starts

    Tests: webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer.html
           webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer.html
           webanimations/scale-property-and-scale-animation-with-delay-on-forced-layer.html
           webanimations/transform-property-and-transform-animation-with-delay-on-forced-layer.html
           webanimations/translate-property-and-translate-animation-with-delay-on-forced-layer.html

    * platform/graphics/ca/GraphicsLayerCA.cpp:
    (WebCore::GraphicsLayerCA::updateAnimations):

    LayoutTests:

    Add a series of tests ensuring that starting an animation for transform-related properties does not clobber the static
    value for this property. We only run those tests on WK2 because running those in WK1 is flaky as there doesn't seem
    to be a solid test utility to determine that Core Animation animations have been committed, even with long delays
    that would make tests run slow.

    * TestExpectations:
    * platform/ios-wk2/TestExpectations:
    * platform/mac-wk2/TestExpectations:
    * webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/resources/wait-until-animations-are-committed.js: Added.
    * webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/scale-property-and-scale-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/scale-property-and-scale-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/transform-property-and-transform-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/transform-property-and-transform-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/translate-property-and-translate-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/translate-property-and-translate-animation-with-delay-on-forced-layer.html: Added.

    Canonical link: https://commits.webkit.org/233429@trunk
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272004 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/232923.106@safari-611-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-611-branch@272264 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit that referenced this pull request Apr 7, 2021
    [JSC] Insert PhantomLocal just before SetLocal for |this| to ensure liveness
    https://bugs.webkit.org/show_bug.cgi?id=221353
    <rdar://problem/70373862>

    Reviewed by Saam Barati.

    Let's consider the following case before SSA conversion.

        BB#0:
            SetArgumentDefinitely(this)
            ...
        @A: SomethingFun()
            MoveHint(@A, this)
            SetLocal(@A, this)
            Jump #1

        BB#1:
            ...
            ExitOK (this point)
            ...
        @b: SomethingFun()
            MoveHint(@b, this)
            SetLocal(@b, this)
            ...

        BB#2: (Catch entry point)
            ...
        @c: SetArgumentDefinitely(this)
            ...
            Jump #1

    We have two entry points. And BB#0 sets @A to |this| while BB#2 does not update |this|, so it is using @c.
    We have several patterns we can store |this|: arrow functions' |this| loading, derived constructors' |this| update. So we can see
    SetLocal(@x, this) at arbitrary code points in CodeBlocks having them.

    The problem is that DFG strongly assumed that |this| is initialized in the root basic block only once. So usually, we do not insert Flush/PhantomLocal for |this|.
    But this is problematic when we can store |this| at arbitrary basic blocks since we do not properly insert Flush/PhantomLocal(this) in BB#1's just before Store.

    Not inserting that in the above case makes |this| dead in BB#1's head liveness. Then we do not properly insert Phi(BB#0, BB#2) for |this|.
    This is OK for non |this| locals since literally that local is not used at all in BB#1. But |this| is special since it is always live in bytecode.
    So, OSR availability will be broken in the above graph: at ExitOK place, |this| must be live in bytecode. But |this| is pointing ConflictingFlush since
    BB#0 says @A and BB#2 says @c while we do not have Phi.

    The problem is that we do not keep liveness of |this| properly in BB#1. When setting a new |this|, we insert PhantomLocal to keep liveness so that appropriate Phi
    will be inserted when two predecessors have different DFG nodes for |this|, and this graph can appear in arrow functions, derived constructors, and code with catch.

    * dfg/DFGByteCodeParser.cpp:
    (JSC::DFG::ByteCodeParser::setArgument):
    * dfg/DFGVariableAccessDataDump.cpp:
    (JSC::DFG::VariableAccessDataDump::dump const):

    Canonical link: https://commits.webkit.org/233677@trunk
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272349 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/232923.164@safari-611-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-611-branch@272533 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit that referenced this pull request Apr 12, 2021
    REGRESSION(r268615): images flicker on apple.com/ios/ios-14
    https://bugs.webkit.org/show_bug.cgi?id=221054
    <rdar://problem/72880447>

    Reviewed by Dean Jackson.

    Source/WebCore:

    When we added support for accelerated animations of individual transform properties in r268615 (bug 217842),
    we made it so that base values of each transform-related property had a non-interpolating animation in the
    Core Animation animations list that would combine with interpolating animations for that property as additive
    animations. Prior to any of those animations, we'd reset the combined transform with an identity transform
    as another non-interpolating animation.

    However, we neglected to consider the case where one of the interpolating animations would not start right
    away if a positive delay was set. In the case of this apple.com page, the target element would be composited
    due to a "will-change: transform" style, and a non-animated "transform" was set as well as an animation for
    the "transform" property with a delay.

    Since we had a "transform" animation, we'd create a Core Animation animations lists as follows:

        1. non-interpolating, non-additive animation set to the identity matrix
        2. interpolating, additive animation with the keyframes set in the CSS animation, with a begin time
           set to the current time plus the specified delay

    The result of this was that during the animation delay, the static "transform" property was overridden
    by animation #1 until animation #2 would kick in.

    We now make it so that for each transform-related property, we create a non-interpoloating, additive animation
    to represent the static value for that property for the duration of any potential delay until the first
    interpolating animation for this property starts.

    In this example, the Core Animation animations list is now as follows:

        1. non-interpolating, non-additive animation set to the identity matrix
        2. non-interpolating, additive animation set to the static transform value
        3. interpolating, additive animation with the keyframes set in the CSS animation, with a begin time
           set to the current time plus the specified delay

    We implement this with a new lambda function within GraphicsLayerCA::updateAnimations() called
    addAnimationsForProperty() which adds a non-interpolating animation in two cases:

        1. if there is no animation for this property at all, making it last forever
        2. if all animations have a delay, making it last until the first animation starts

    Tests: webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer.html
           webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer.html
           webanimations/scale-property-and-scale-animation-with-delay-on-forced-layer.html
           webanimations/transform-property-and-transform-animation-with-delay-on-forced-layer.html
           webanimations/translate-property-and-translate-animation-with-delay-on-forced-layer.html

    * platform/graphics/ca/GraphicsLayerCA.cpp:
    (WebCore::GraphicsLayerCA::updateAnimations):

    LayoutTests:

    Add a series of tests ensuring that starting an animation for transform-related properties does not clobber the static
    value for this property. We only run those tests on WK2 because running those in WK1 is flaky as there doesn't seem
    to be a solid test utility to determine that Core Animation animations have been committed, even with long delays
    that would make tests run slow.

    * TestExpectations:
    * platform/ios-wk2/TestExpectations:
    * platform/mac-wk2/TestExpectations:
    * webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/resources/wait-until-animations-are-committed.js: Added.
    * webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/scale-property-and-scale-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/scale-property-and-scale-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/transform-property-and-transform-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/transform-property-and-transform-animation-with-delay-on-forced-layer.html: Added.
    * webanimations/translate-property-and-translate-animation-with-delay-on-forced-layer-expected.html: Added.
    * webanimations/translate-property-and-translate-animation-with-delay-on-forced-layer.html: Added.

    Canonical link: https://commits.webkit.org/233429@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272004 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/232923.106@safari-611-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-611-branch@272264 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit that referenced this pull request Apr 12, 2021
    [JSC] Insert PhantomLocal just before SetLocal for |this| to ensure liveness
    https://bugs.webkit.org/show_bug.cgi?id=221353
    <rdar://problem/70373862>

    Reviewed by Saam Barati.

    Let's consider the following case before SSA conversion.

        BB#0:
            SetArgumentDefinitely(this)
            ...
        @A: SomethingFun()
            MoveHint(@A, this)
            SetLocal(@A, this)
            Jump #1

        BB#1:
            ...
            ExitOK (this point)
            ...
        @b: SomethingFun()
            MoveHint(@b, this)
            SetLocal(@b, this)
            ...

        BB#2: (Catch entry point)
            ...
        @c: SetArgumentDefinitely(this)
            ...
            Jump #1

    We have two entry points. And BB#0 sets @A to |this| while BB#2 does not update |this|, so it is using @c.
    We have several patterns we can store |this|: arrow functions' |this| loading, derived constructors' |this| update. So we can see
    SetLocal(@x, this) at arbitrary code points in CodeBlocks having them.

    The problem is that DFG strongly assumed that |this| is initialized in the root basic block only once. So usually, we do not insert Flush/PhantomLocal for |this|.
    But this is problematic when we can store |this| at arbitrary basic blocks since we do not properly insert Flush/PhantomLocal(this) in BB#1's just before Store.

    Not inserting that in the above case makes |this| dead in BB#1's head liveness. Then we do not properly insert Phi(BB#0, BB#2) for |this|.
    This is OK for non |this| locals since literally that local is not used at all in BB#1. But |this| is special since it is always live in bytecode.
    So, OSR availability will be broken in the above graph: at ExitOK place, |this| must be live in bytecode. But |this| is pointing ConflictingFlush since
    BB#0 says @A and BB#2 says @c while we do not have Phi.

    The problem is that we do not keep liveness of |this| properly in BB#1. When setting a new |this|, we insert PhantomLocal to keep liveness so that appropriate Phi
    will be inserted when two predecessors have different DFG nodes for |this|, and this graph can appear in arrow functions, derived constructors, and code with catch.

    * dfg/DFGByteCodeParser.cpp:
    (JSC::DFG::ByteCodeParser::setArgument):
    * dfg/DFGVariableAccessDataDump.cpp:
    (JSC::DFG::VariableAccessDataDump::dump const):

    Canonical link: https://commits.webkit.org/233677@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272349 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/232923.164@safari-611-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-611-branch@272533 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Jun 8, 2021
https://bugs.webkit.org/show_bug.cgi?id=226751

Reviewed by Simon Fraser.

Source/WebCore:

This patch adds the initial support for content like this:
<table>
  <tr>
    <td style="width: 10%"></td><td style="width: 90%"></td>
  </tr>
</table>

Percent values work in mysterious ways in cases when the table has no fixed width.

1. The smaller the percent value is, the wider the table may become.

 Percent values are resolved against the cell's border box (so essentially they are resolved
 against their own content as opposed to the table/containing block) and the formula is slightly different.

    <td style="padding: 5px; width: 20%;"></td> : produces a 10px wide border box (horizontal border: 0px, padding: 10px, content: 0px).
      The maximum constraint is resolved to 50px (width / percent * 100)
    <td style="padding: 5px; width: 100%;"></td> : produces a 10px wide border box and the maximum constraint is resolved to 10px.

  This maximum constraint value turns into the available width for the table content and becomes the final table width.

2. With multiple rows, we pick the highest _percent_ value for each column (as opposed to the resolved values).

      <tr><td style="width: 20%"></td></tr> (assum same 5px padding on both sides)
      <tr><td style="width: 80%"></td></tr>

  While the second row's cell has a higher maximum constraint value (50px see #1) since we only look at the raw percent values,
  this content only produces a 12.5px wide table.

3. The percent values do not accumulate across columns but instead we pick the largest one to represent the entire table's max constraint width.

    <tr><td style="width: 60%"></td><td style="width: 40%"></td></tr>

  60% resolves to 16.6px
  40% resolves to 25px and we use the 25px value as the width for the entire table (and not 16.6px + 25px).

4. Since we pick the highest percent values across rows for each columns, we may end up with > 100%.
  In such cases we start dropping percent values for subsequent columns:

    <tr><td style="width: 20%;"></td><td style="width: 80%;"></td></tr>
    <tr><td style="width: 60%;"></td><td style="width: 10%;"></td></tr>

  First column width is max(20%, 60%) -> 60%
  Second column width is max(80%, 10%) -> 80%
  As we limit the accumulated percent value to 100%, the final column percent values are 60% and 40% (and not 80%).
  Now the 60% is resolved to 16.6px and the 40% is resolved to 25px and since we don't accumulate these values (see #3)
  the final table width is 25px (based on a percent value which is not even in the markup).

5. While the smaller percent values produce wider tables (see #1), during the available space distribution
  columns with smaller percent values get assigned less space.

    <tr><td style="width: 1%"></td><td style="width: 99%"></td></tr>

  This content produces a 1000px wide table due to the small (1%) percent value (see #1 #2 and #3).
  When we distribute the available space (1000px), the first cell gets only 10px (1%) while the second cell ends up with 990px (99%).

(and this is the cherry on top (not included in this patch):
 Imagine the following scenario:
   1. the accumulated column percent value > 100% (let's say 80% and 30%)
   2. as we reach the 100% while walking the columns one by one (see #4), the remaining percent value becomes 0%.
   3. In order to avoid division by 0, we pick a very small epsilon value to run the formula.
   4. Now this very small percent value produces a large resolved value (see #2) which means

    <tr><td style="width: 100%"></td></tr>

    produces a 10px wide table

    <tr><td style="width: 100%"></td><td style="width: 1%"></td></tr> <- note the 1%

    produces a very very very wide table.
)

Test: fast/layoutformattingcontext/table-with-percent-columns-only-no-content.html

* layout/formattingContexts/table/TableFormattingContext.cpp:
(WebCore::Layout::TableFormattingContext::computedIntrinsicWidthConstraints):
(WebCore::Layout::TableFormattingContext::computedPreferredWidthForColumns):
* layout/formattingContexts/table/TableGrid.h:
(WebCore::Layout::TableGrid::Column::percent const):
(WebCore::Layout::TableGrid::Column::setFixedWidth):
(WebCore::Layout::TableGrid::Column::setPercent):
* layout/formattingContexts/table/TableLayout.cpp:
(WebCore::Layout::TableFormattingContext::TableLayout::distributedHorizontalSpace):

LayoutTests:

* fast/layoutformattingcontext/table-with-percent-columns-only-no-content-expected.html: Added.
* fast/layoutformattingcontext/table-with-percent-columns-only-no-content.html: Added.


Canonical link: https://commits.webkit.org/238594@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278605 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Jul 30, 2021
https://bugs.webkit.org/show_bug.cgi?id=228047

Reviewed by Phil Pizlo.

Pre-indexed addressing means that the address is the sum of the value in the 64-bit base register
and an offset, and the address is then written back to the base register. And post-indexed
addressing means that the address is the value in the 64-bit base register, and the sum of the
address and the offset is then written back to the base register. They are relatively common for
loops to iterate over an array by increasing/decreasing a pointer into the array at each iteration.
With such an addressing mode, the instruction selector can merge the increment and access the array.

#####################################
## Pre-Index Address Mode For Load ##
#####################################

LDR Wt, [Xn, #imm]!

In B3 Reduction Strength, since we have this reduction rule:
    Turn this: Load(Add(address, offset1), offset = offset2)
    Into this: Load(address, offset = offset1 + offset2)

Then, the equivalent pattern is:
    address = Add(base, offset)
    ...
    memory = Load(base, offset)

First, we convert it to the canonical form:
    address = Add(base, offset)
    newMemory = Load(base, offset) // move the memory to just after the address
    ...
    memory = Identity(newMemory)

Next, lower to Air:
    Move %base, %address
    Move (%address, prefix(offset)), %newMemory

######################################
## Post-Index Address Mode For Load ##
######################################

LDR Wt, [Xn], #imm

Then, the equivalent pattern is:
    memory = Load(base, 0)
    ...
    address = Add(base, offset)

First, we convert it to the canonical form:
    newOffset = Constant
    newAddress = Add(base, offset)
    memory = Load(base, 0) // move the offset and address to just before the memory
    ...
    offset = Identity(newOffset)
    address = Identity(newAddress)

Next, lower to Air:
    Move %base, %newAddress
    Move (%newAddress, postfix(offset)), %memory

#############################
## Pattern Match Algorithm ##
#############################

To detect the pattern for prefix/postfix increment address is tricky due to the structure in B3 IR. The
algorithm used in this patch is to collect the first valid values (add/load), then search for any
paired value (load/add) to match all of them. In worst case, the runtime complexity is O(n^2)
when n is the number of all values.

After collecting two sets of candidates, we match the prefix incremental address first since it seems
more beneficial to the compiler (shown in the next section). And then, go for the postfix one.

##############################################
## Test for Pre/Post-Increment Address Mode ##
##############################################

Given Loop with Pre-Increment:
int64_t ldr_pre(int64_t *p) {
    int64_t res = 0;
    while (res < 10)
        res += *++p;
    return res;
}

B3 IR:
------------------------------------------------------
BB#0: ; frequency = 1.000000
    Int64 b@0 = Const64(0)
    Int64 b@2 = ArgumentReg(%x0)
    Void b@20 = Upsilon($0(b@0), ^18, WritesLocalState)
    Void b@21 = Upsilon(b@2, ^19, WritesLocalState)
    Void b@4 = Jump(Terminal)
Successors: #1
BB#1: ; frequency = 1.000000
Predecessors: #0, #2
    Int64 b@18 = Phi(ReadsLocalState)
    Int64 b@19 = Phi(ReadsLocalState)
    Int64 b@7 = Const64(10)
    Int32 b@8 = AboveEqual(b@18, $10(b@7))
    Void b@9 = Branch(b@8, Terminal)
Successors: Then:#3, Else:#2
BB#2: ; frequency = 1.000000
Predecessors: #1
    Int64 b@10 = Const64(8)
    Int64 b@11 = Add(b@19, $8(b@10))
    Int64 b@13 = Load(b@11, ControlDependent|Reads:Top)
    Int64 b@14 = Add(b@18, b@13)
    Void b@22 = Upsilon(b@14, ^18, WritesLocalState)
    Void b@23 = Upsilon(b@11, ^19, WritesLocalState)
    Void b@16 = Jump(Terminal)
Successors: #1
BB#3: ; frequency = 1.000000
Predecessors: #1
    Void b@17 = Return(b@18, Terminal)
Variables:
    Int64 var0
    Int64 var1
------------------------------------------------------

W/O Pre-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    Move $8, %x3, $8(b@12)
    Add64 $8, %x0, %x1, b@11
    Move (%x0,%x3), %x0, b@13
    Add64 %x0, %x2, %x2, b@14
    Move %x1, %x0, b@23
    Jump b@16
Successors: #1
...
------------------------------------------------------

W/ Pre-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    MoveWithIncrement64 (%x0,Pre($8)), %x2, b@13
    Add64 %x2, %x1, %x1, b@14
    Jump b@16
Successors: #1
...
------------------------------------------------------

Given Loop with Post-Increment:
int64_t ldr_pre(int64_t *p) {
    int64_t res = 0;
    while (res < 10)
        res += *p++;
    return res;
}

B3 IR:
------------------------------------------------------
BB#0: ; frequency = 1.000000
    Int64 b@0 = Const64(0)
    Int64 b@2 = ArgumentReg(%x0)
    Void b@20 = Upsilon($0(b@0), ^18, WritesLocalState)
    Void b@21 = Upsilon(b@2, ^19, WritesLocalState)
    Void b@4 = Jump(Terminal)
Successors: #1
BB#1: ; frequency = 1.000000
Predecessors: #0, #2
    Int64 b@18 = Phi(ReadsLocalState)
    Int64 b@19 = Phi(ReadsLocalState)
    Int64 b@7 = Const64(10)
    Int32 b@8 = AboveEqual(b@18, $10(b@7))
    Void b@9 = Branch(b@8, Terminal)
Successors: Then:#3, Else:#2
BB#2: ; frequency = 1.000000
Predecessors: #1
    Int64 b@10 = Load(b@19, ControlDependent|Reads:Top)
    Int64 b@11 = Add(b@18, b@10)
    Int64 b@12 = Const64(8)
    Int64 b@13 = Add(b@19, $8(b@12))
    Void b@22 = Upsilon(b@11, ^18, WritesLocalState)
    Void b@23 = Upsilon(b@13, ^19, WritesLocalState)
    Void b@16 = Jump(Terminal)
Successors: #1
BB#3: ; frequency = 1.000000
Predecessors: #1
    Void b@17 = Return(b@18, Terminal)
Variables:
    Int64 var0
    Int64 var1
------------------------------------------------------

W/O Post-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    Move (%x0), %x2, b@10
    Add64 %x2, %x1, %x1, b@11
    Add64 $8, %x0, %x0, b@13
    Jump b@16
Successors: #1
...
------------------------------------------------------

W/ Post-Increment Address Mode:
------------------------------------------------------
...
BB#2: ; frequency = 1.000000
Predecessors: #1
    MoveWithIncrement64 (%x0,Post($8)), %x2, b@10
    Add64 %x2, %x1, %x1, b@11
    Jump b@16
Successors: #1
...
------------------------------------------------------

* Sources.txt:
* assembler/AbstractMacroAssembler.h:
(JSC::AbstractMacroAssembler::PreIndexAddress::PreIndexAddress):
(JSC::AbstractMacroAssembler::PostIndexAddress::PostIndexAddress):
* assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::load64):
(JSC::MacroAssemblerARM64::load32):
(JSC::MacroAssemblerARM64::store64):
(JSC::MacroAssemblerARM64::store32):
* assembler/testmasm.cpp:
(JSC::testStorePrePostIndex32):
(JSC::testStorePrePostIndex64):
(JSC::testLoadPrePostIndex32):
(JSC::testLoadPrePostIndex64):
* b3/B3CanonicalizePrePostIncrements.cpp: Added.
(JSC::B3::canonicalizePrePostIncrements):
* b3/B3CanonicalizePrePostIncrements.h: Copied from Source/JavaScriptCore/b3/B3ValueKeyInlines.h.
* b3/B3Generate.cpp:
(JSC::B3::generateToAir):
* b3/B3LowerToAir.cpp:
* b3/B3ValueKey.h:
* b3/B3ValueKeyInlines.h:
(JSC::B3::ValueKey::ValueKey):
* b3/air/AirArg.cpp:
(JSC::B3::Air::Arg::jsHash const):
(JSC::B3::Air::Arg::dump const):
(WTF::printInternal):
* b3/air/AirArg.h:
(JSC::B3::Air::Arg::preIndex):
(JSC::B3::Air::Arg::postIndex):
(JSC::B3::Air::Arg::isPreIndex const):
(JSC::B3::Air::Arg::isPostIndex const):
(JSC::B3::Air::Arg::isMemory const):
(JSC::B3::Air::Arg::base const):
(JSC::B3::Air::Arg::offset const):
(JSC::B3::Air::Arg::isGP const):
(JSC::B3::Air::Arg::isFP const):
(JSC::B3::Air::Arg::isValidPreIndexForm):
(JSC::B3::Air::Arg::isValidPostIndexForm):
(JSC::B3::Air::Arg::isValidForm const):
(JSC::B3::Air::Arg::forEachTmpFast):
(JSC::B3::Air::Arg::forEachTmp):
(JSC::B3::Air::Arg::asPreIndexAddress const):
(JSC::B3::Air::Arg::asPostIndexAddress const):
* b3/air/AirOpcode.opcodes:
* b3/air/opcode_generator.rb:
* b3/testb3.h:
* b3/testb3_3.cpp:
(testLoadPreIndex32):
(testLoadPreIndex64):
(testLoadPostIndex32):
(testLoadPostIndex64):
(addShrTests):
* jit/ExecutableAllocator.cpp:
(JSC::jitWriteThunkGenerator):


Canonical link: https://commits.webkit.org/240125@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280493 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Aug 5, 2021
… be called more than once under WebCore::MediaControlsContextMenuProvider::contextMenuItemSelected

https://bugs.webkit.org/show_bug.cgi?id=228725
<rdar://problem/81437221>

Reviewed by Eric Carlson.

The contextmenu system used by (modern) media controls are a bit wonky in that it has to
support both macOS and iOS, which use wildly different mechanisms. The former has distinct
methods for handling when a contextmenu item is selected vs when the menu is dismissed (at
least as of r280374). The latter has a single method that handles both. Additionally, the
(modern) media controls JS expects the following from `showMediaControlsContextMenu`:
 1. `showMediaControlsContextMenu` will only `return true` if the contextmenu will be shown
 2. the callback provided to `showMediaControlsContextMenu` will always/only be invoked when
    the contextmenu is dismissed (regardless of whether an item is selected)
 3. if an item is selected, the logic for that will be handled by the `MediaControlsHost`
This patch primarily addresses #2, but also slightly adjusts the code to fix #1. It does #1
by moving the call that saves the callback further down. On iOS, #2 already works. On macOS,
it does #2 by changing from `CompletionHandler` to `Function`, allowing it to be called more
than once, with the understanding that the JS callback will not be invoked more than once.
This way, macOS can match the behavior of iOS by eagerly invoking the JS callback when a
contextmenu item is selected without waiting for the menu to actually dismiss, while still
handling the contextmenu being dismissed without an item being selected (and also not having
to worry about whether the `CompletionHandler` has already been invoked).

* Modules/mediacontrols/MediaControlsHost.h:
* Modules/mediacontrols/MediaControlsHost.cpp:
(WebCore::MediaControlsContextMenuProvider::create):
(WebCore::MediaControlsContextMenuProvider::MediaControlsContextMenuProvider):
(WebCore::MediaControlsContextMenuProvider::didDismissContextMenu):
(WebCore::MediaControlsContextMenuProvider::contextMenuCleared):
(WebCore::MediaControlsHost::showMediaControlsContextMenu):


Canonical link: https://commits.webkit.org/240278@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@280676 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Nov 9, 2021
https://bugs.webkit.org/show_bug.cgi?id=232265

Patch by Mikhail R. Gadelha <mikhail@igalia.com> on 2021-11-09
Reviewed by Saam Barati.

Follow-up from https://bugs.webkit.org/show_bug.cgi?id=232242,
this patch includes several small code changes but the patch doesn't
add/remove any feature:

1. Removed several calls to operationPutByVal*Cell* that were
only used by the 32 bit code paths due to the lack of registers.
These calls were replaced by the calls used by the 64 bit paths,
that expect EncodedJSValues
2. Because of #1, this patch removes those methods, since no one
uses them anymore.
3. Created compilePutByVal to handle all cases (similar to compileGetByVal).
4. Removed the Edge& childX from the PutByVal handling (and all methods
that expected them) in favor of getting them from node when needed.
5. Unified compileContiguousPutByVal so it could be used by both 32
and 64 bit archs.
6. Removed a lot of whitespace.

* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileContiguousPutByVal):
(JSC::DFG::SpeculativeJIT::compileDoublePutByVal):
(JSC::DFG::SpeculativeJIT::compilePutByVal):
(JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):
(JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByValForFloatTypedArray): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetByValForObjectWithString): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetByValForObjectWithSymbol): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetPrivateName): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetPrivateNameByVal): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetPrivateNameById): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByValForCellWithString): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByValForCellWithSymbol): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetByValWithThis): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutPrivateName): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutPrivateNameById): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckPrivateBrand): Deleted.
(JSC::DFG::SpeculativeJIT::compileSetPrivateBrand): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckTypeInfoFlags): Deleted.
(JSC::DFG::SpeculativeJIT::compileParseInt): Deleted.
(JSC::DFG::SpeculativeJIT::compileOverridesHasInstance): Deleted.
(JSC::DFG::SpeculativeJIT::compileInstanceOfForCells): Deleted.
(JSC::DFG::SpeculativeJIT::compileInstanceOf): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueBitNot): Deleted.
(JSC::DFG::SpeculativeJIT::compileBitwiseNot): Deleted.
(JSC::DFG::SpeculativeJIT::emitUntypedOrAnyBigIntBitOp): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueBitwiseOp): Deleted.
(JSC::DFG::SpeculativeJIT::compileBitwiseOp): Deleted.
(JSC::DFG::SpeculativeJIT::emitUntypedOrBigIntRightShiftBitOp): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueLShiftOp): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueBitRShift): Deleted.
(JSC::DFG::SpeculativeJIT::compileShiftOp): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueAdd): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueSub): Deleted.
(JSC::DFG::SpeculativeJIT::compileMathIC): Deleted.
(JSC::DFG::SpeculativeJIT::compileInstanceOfCustom): Deleted.
(JSC::DFG::SpeculativeJIT::compileIsCellWithType): Deleted.
(JSC::DFG::SpeculativeJIT::compileIsTypedArrayView): Deleted.
(JSC::DFG::SpeculativeJIT::compileToObjectOrCallObjectConstructor): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithAdd): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithAbs): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithClz32): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithDoubleUnaryOp): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithSub): Deleted.
(JSC::DFG::SpeculativeJIT::compileIncOrDec): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueNegate): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithNegate): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueMul): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithMul): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueDiv): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithDiv): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithFRound): Deleted.
(JSC::DFG::SpeculativeJIT::compileValueMod): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithMod): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithRounding): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithUnary): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithSqrt): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithMinMax): Deleted.
(JSC::DFG::compileArithPowIntegerFastPath): Deleted.
(JSC::DFG::SpeculativeJIT::compileValuePow): Deleted.
(JSC::DFG::SpeculativeJIT::compileArithPow): Deleted.
(JSC::DFG::SpeculativeJIT::compare): Deleted.
(JSC::DFG::SpeculativeJIT::compileCompareUnsigned): Deleted.
(JSC::DFG::SpeculativeJIT::compileStrictEq): Deleted.
(JSC::DFG::SpeculativeJIT::compileBooleanCompare): Deleted.
(JSC::DFG::SpeculativeJIT::compileInt32Compare): Deleted.
(JSC::DFG::SpeculativeJIT::compileDoubleCompare): Deleted.
(JSC::DFG::SpeculativeJIT::compileObjectEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileSymbolEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compilePeepHoleSymbolEquality): Deleted.
(JSC::DFG::SpeculativeJIT::emitBitwiseJSValueEquality): Deleted.
(JSC::DFG::SpeculativeJIT::emitBranchOnBitwiseJSValueEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileNotDoubleNeitherDoubleNorHeapBigIntNorStringStrictEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compilePeepHoleNotDoubleNeitherDoubleNorHeapBigIntNorStringStrictEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringToUntypedEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringIdentEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringIdentToNotStringVarEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringCompare): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringIdentCompare): Deleted.
(JSC::DFG::SpeculativeJIT::compileSameValue): Deleted.
(JSC::DFG::SpeculativeJIT::compileToBooleanString): Deleted.
(JSC::DFG::SpeculativeJIT::compileToBooleanStringOrOther): Deleted.
(JSC::DFG::SpeculativeJIT::emitStringBranch): Deleted.
(JSC::DFG::SpeculativeJIT::emitStringOrOtherBranch): Deleted.
(JSC::DFG::SpeculativeJIT::compileConstantStoragePointer): Deleted.
(JSC::DFG::SpeculativeJIT::cageTypedArrayStorage): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetIndexedPropertyStorage): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetTypedArrayByteOffset): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetByValOnDirectArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetByValOnScopedArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetScope): Deleted.
(JSC::DFG::SpeculativeJIT::compileSkipScope): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetGlobalObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetGlobalThis): Deleted.
(JSC::DFG::SpeculativeJIT::canBeRope): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetArrayLength): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckIdent): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewFunctionCommon): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewFunction): Deleted.
(JSC::DFG::SpeculativeJIT::compileSetFunctionName): Deleted.
(JSC::DFG::SpeculativeJIT::compileVarargsLength): Deleted.
(JSC::DFG::SpeculativeJIT::compileLoadVarargs): Deleted.
(JSC::DFG::SpeculativeJIT::compileForwardVarargs): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateActivation): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateDirectArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetFromArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutToArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetArgument): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateScopedArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateClonedArguments): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateArgumentsButterfly): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateRest): Deleted.
(JSC::DFG::SpeculativeJIT::compileSpread): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewArray): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewArrayWithSpread): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetRestLength): Deleted.
(JSC::DFG::SpeculativeJIT::emitPopulateSliceIndex): Deleted.
(JSC::DFG::SpeculativeJIT::compileArraySlice): Deleted.
(JSC::DFG::SpeculativeJIT::compileArrayIndexOf): Deleted.
(JSC::DFG::SpeculativeJIT::compileArrayPush): Deleted.
(JSC::DFG::SpeculativeJIT::compileNotifyWrite): Deleted.
(JSC::DFG::SpeculativeJIT::compileIsObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileTypeOfIsObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileIsCallable): Deleted.
(JSC::DFG::SpeculativeJIT::compileIsConstructor): Deleted.
(JSC::DFG::SpeculativeJIT::compileTypeOf): Deleted.
(JSC::DFG::SpeculativeJIT::emitStructureCheck): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckIsConstant): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckNotEmpty): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckStructure): Deleted.
(JSC::DFG::SpeculativeJIT::compileAllocatePropertyStorage): Deleted.
(JSC::DFG::SpeculativeJIT::compileReallocatePropertyStorage): Deleted.
(JSC::DFG::SpeculativeJIT::compileNukeStructureAndSetButterfly): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetButterfly): Deleted.
(JSC::DFG::allocateTemporaryRegistersForSnippet): Deleted.
(JSC::DFG::SpeculativeJIT::compileCallDOM): Deleted.
(JSC::DFG::SpeculativeJIT::compileCallDOMGetter): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckJSCast): Deleted.
(JSC::DFG::SpeculativeJIT::temporaryRegisterForPutByVal): Deleted.
(JSC::DFG::SpeculativeJIT::compileToStringOrCallStringConstructorOrStringValueOf): Deleted.
(JSC::DFG::getExecutable): Deleted.
(JSC::DFG::SpeculativeJIT::compileFunctionToString): Deleted.
(JSC::DFG::SpeculativeJIT::compileNumberToStringWithValidRadixConstant): Deleted.
(JSC::DFG::SpeculativeJIT::compileNumberToStringWithRadix): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewStringObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewSymbol): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewTypedArrayWithSize): Deleted.
(JSC::DFG::SpeculativeJIT::emitNewTypedArrayWithSizeInRegister): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewRegexp): Deleted.
(JSC::DFG::SpeculativeJIT::speculateCellTypeWithoutTypeFiltering): Deleted.
(JSC::DFG::SpeculativeJIT::speculateCellType): Deleted.
(JSC::DFG::SpeculativeJIT::speculateInt32): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNumber): Deleted.
(JSC::DFG::SpeculativeJIT::speculateRealNumber): Deleted.
(JSC::DFG::SpeculativeJIT::speculateDoubleRepReal): Deleted.
(JSC::DFG::SpeculativeJIT::speculateBoolean): Deleted.
(JSC::DFG::SpeculativeJIT::speculateCell): Deleted.
(JSC::DFG::SpeculativeJIT::speculateCellOrOther): Deleted.
(JSC::DFG::SpeculativeJIT::speculateObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateFunction): Deleted.
(JSC::DFG::SpeculativeJIT::speculateFinalObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateRegExpObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateArray): Deleted.
(JSC::DFG::SpeculativeJIT::speculateProxyObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateDerivedArray): Deleted.
(JSC::DFG::SpeculativeJIT::speculatePromiseObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateDateObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateMapObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateSetObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateWeakMapObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateWeakSetObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateDataViewObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateObjectOrOther): Deleted.
(JSC::DFG::SpeculativeJIT::speculateString): Deleted.
(JSC::DFG::SpeculativeJIT::speculateStringOrOther): Deleted.
(JSC::DFG::SpeculativeJIT::speculateStringIdentAndLoadStorage): Deleted.
(JSC::DFG::SpeculativeJIT::speculateStringIdent): Deleted.
(JSC::DFG::SpeculativeJIT::speculateStringObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateStringOrStringObject): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNotStringVar): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNotSymbol): Deleted.
(JSC::DFG::SpeculativeJIT::speculateSymbol): Deleted.
(JSC::DFG::SpeculativeJIT::speculateHeapBigInt): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNotCell): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNotCellNorBigInt): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNotDouble): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNeitherDoubleNorHeapBigInt): Deleted.
(JSC::DFG::SpeculativeJIT::speculateNeitherDoubleNorHeapBigIntNorString): Deleted.
(JSC::DFG::SpeculativeJIT::speculateOther): Deleted.
(JSC::DFG::SpeculativeJIT::speculateMisc): Deleted.
(JSC::DFG::SpeculativeJIT::speculate): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitchIntJump): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitchImm): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitchCharStringJump): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitchChar): Deleted.
(JSC::DFG::SpeculativeJIT::emitBinarySwitchStringRecurse): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitchStringOnString): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitchString): Deleted.
(JSC::DFG::SpeculativeJIT::emitSwitch): Deleted.
(JSC::DFG::SpeculativeJIT::addBranch): Deleted.
(JSC::DFG::SpeculativeJIT::linkBranches): Deleted.
(JSC::DFG::SpeculativeJIT::compileStoreBarrier): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutAccessorById): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutGetterSetterById): Deleted.
(JSC::DFG::SpeculativeJIT::compileResolveScope): Deleted.
(JSC::DFG::SpeculativeJIT::compileResolveScopeForHoistingFuncDeclInEval): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetGlobalVariable): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutGlobalVariable): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetDynamicVar): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutDynamicVar): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetClosureVar): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutClosureVar): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetInternalField): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutInternalField): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutAccessorByVal): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetRegExpObjectLastIndex): Deleted.
(JSC::DFG::SpeculativeJIT::compileSetRegExpObjectLastIndex): Deleted.
(JSC::DFG::SpeculativeJIT::compileRegExpExec): Deleted.
(JSC::DFG::SpeculativeJIT::compileRegExpTest): Deleted.
(JSC::DFG::SpeculativeJIT::compileStringReplace): Deleted.
(JSC::DFG::SpeculativeJIT::compileRegExpExecNonGlobalOrSticky): Deleted.
(JSC::DFG::SpeculativeJIT::compileRegExpMatchFastGlobal): Deleted.
(JSC::DFG::SpeculativeJIT::compileRegExpMatchFast): Deleted.
(JSC::DFG::SpeculativeJIT::compileLazyJSConstant): Deleted.
(JSC::DFG::SpeculativeJIT::compileMaterializeNewObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileRecordRegExpCachedResult): Deleted.
(JSC::DFG::SpeculativeJIT::compileDefineDataProperty): Deleted.
(JSC::DFG::SpeculativeJIT::compileDefineAccessorProperty): Deleted.
(JSC::DFG::SpeculativeJIT::emitAllocateButterfly): Deleted.
(JSC::DFG::SpeculativeJIT::compileNormalizeMapKey): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetMapBucketHead): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetMapBucketNext): Deleted.
(JSC::DFG::SpeculativeJIT::compileLoadKeyFromMapBucket): Deleted.
(JSC::DFG::SpeculativeJIT::compileLoadValueFromMapBucket): Deleted.
(JSC::DFG::SpeculativeJIT::compileExtractValueFromWeakMapGet): Deleted.
(JSC::DFG::SpeculativeJIT::compileThrow): Deleted.
(JSC::DFG::SpeculativeJIT::compileThrowStaticError): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorNextUpdateIndexAndMode): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorNextExtractIndex): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorNextExtractMode): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorNextUpdatePropertyName): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorHasProperty): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorInByVal): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorHasOwnProperty): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByIdFlush): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutById): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByIdDirect): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByIdWithThis): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetByOffset): Deleted.
(JSC::DFG::SpeculativeJIT::compilePutByOffset): Deleted.
(JSC::DFG::SpeculativeJIT::compileMatchStructure): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetPropertyEnumerator): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetExecutable): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetGetter): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetSetter): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetCallee): Deleted.
(JSC::DFG::SpeculativeJIT::compileSetCallee): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetArgumentCountIncludingThis): Deleted.
(JSC::DFG::SpeculativeJIT::compileSetArgumentCountIncludingThis): Deleted.
(JSC::DFG::SpeculativeJIT::compileStrCat): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewArrayBuffer): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewArrayWithSize): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewTypedArray): Deleted.
(JSC::DFG::SpeculativeJIT::compileToThis): Deleted.
(JSC::DFG::SpeculativeJIT::compileObjectKeysOrObjectGetOwnPropertyNames): Deleted.
(JSC::DFG::SpeculativeJIT::compileObjectAssign): Deleted.
(JSC::DFG::SpeculativeJIT::compileObjectCreate): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateThis): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreatePromise): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateInternalFieldObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateGenerator): Deleted.
(JSC::DFG::SpeculativeJIT::compileCreateAsyncGenerator): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewInternalFieldObjectImpl): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewGenerator): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewAsyncGenerator): Deleted.
(JSC::DFG::SpeculativeJIT::compileNewInternalFieldObject): Deleted.
(JSC::DFG::SpeculativeJIT::compileToPrimitive): Deleted.
(JSC::DFG::SpeculativeJIT::compileToPropertyKey): Deleted.
(JSC::DFG::SpeculativeJIT::compileToNumeric): Deleted.
(JSC::DFG::SpeculativeJIT::compileCallNumberConstructor): Deleted.
(JSC::DFG::SpeculativeJIT::compileLogShadowChickenPrologue): Deleted.
(JSC::DFG::SpeculativeJIT::compileLogShadowChickenTail): Deleted.
(JSC::DFG::SpeculativeJIT::compileSetAdd): Deleted.
(JSC::DFG::SpeculativeJIT::compileMapSet): Deleted.
(JSC::DFG::SpeculativeJIT::compileWeakMapGet): Deleted.
(JSC::DFG::SpeculativeJIT::compileWeakSetAdd): Deleted.
(JSC::DFG::SpeculativeJIT::compileWeakMapSet): Deleted.
(JSC::DFG::SpeculativeJIT::compileGetPrototypeOf): Deleted.
(JSC::DFG::SpeculativeJIT::compileIdentity): Deleted.
(JSC::DFG::SpeculativeJIT::compileMiscStrictEq): Deleted.
(JSC::DFG::SpeculativeJIT::emitInitializeButterfly): Deleted.
(JSC::DFG::SpeculativeJIT::compileAllocateNewArrayWithSize): Deleted.
(JSC::DFG::SpeculativeJIT::compileHasIndexedProperty): Deleted.
(JSC::DFG::SpeculativeJIT::compileExtractCatchLocal): Deleted.
(JSC::DFG::SpeculativeJIT::compileClearCatchLocals): Deleted.
(JSC::DFG::SpeculativeJIT::compileProfileType): Deleted.
(JSC::DFG::SpeculativeJIT::cachedPutById): Deleted.
(JSC::DFG::SpeculativeJIT::genericJSValueNonPeepholeCompare): Deleted.
(JSC::DFG::SpeculativeJIT::genericJSValuePeepholeBranch): Deleted.
(JSC::DFG::SpeculativeJIT::compileHeapBigIntEquality): Deleted.
(JSC::DFG::SpeculativeJIT::compileMakeRope): Deleted.
(JSC::DFG::SpeculativeJIT::compileEnumeratorGetByVal): Deleted.
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
(JSC::DFG::SpeculativeJIT::compileContiguousPutByVal): Deleted.
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* jit/AssemblyHelpers.h:
(JSC::AssemblyHelpers::branchIfEmpty):
(JSC::AssemblyHelpers::branchIfNotEmpty):


Canonical link: https://commits.webkit.org/244047@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285530 268f45cc-cd09-0410-ab3c-d52691b4dbfc
gezalore pushed a commit to gezalore/WebKit that referenced this pull request Jan 17, 2022
webkit-commit-queue pushed a commit that referenced this pull request Mar 6, 2022
…ed underneath -[UIWindow dealloc]

https://bugs.webkit.org/show_bug.cgi?id=237505
rdar://85563958

Reviewed by Tim Horton.

Source/WebKit:

It's currently possible for the web page to get permanently stuck in frozen state, due to the
`BackgroundApplication` layer tree freeze reason; this occurs when the web view is unparented from the view
hierarchy underneath the scope of UIWindow's `-dealloc` method.

During `-[UIWindow dealloc]`, the backpointer underlying the implementation of `-[UIView window]` is set to `nil`
immediately before the subclassing method hook `-willMoveToWindow:` is invoked on the view hierarchy. This means
that when `-willMoveToWindow:` is invoked, `self.window` will return `nil`. This, in turn, puts
`WKApplicationStateTrackingView` in a bad state because we bail early before resetting `_applicationStateTracker`
in the early return below, since we (erroneously) believe that we've already been unparented from the view
hierarchy, so we don't need to do anything.

```
if (!self._contentView.window || newWindow)
    return;
```

As a result, if the same web view is eventually moved back into another visible window, `-didMoveToWindow` bails
before setting up the `_applicationStateTracker` again, since it already exists from when the previous window
was still active. This means `-_applicationWillEnterForeground` is never called when the web view is
reintroduced to the view hierarchy, so `LayerTreeFreezeReason::BackgroundApplication` is never lifted.

To address this, we simply remove the debug assertion for `_applicationStateTracker`, and instead check whether
the application state tracker exists or not for the logic of the early return. Doing so also makes the early
return in `-willMoveToWindow:` consistent with the logic in one in `-didMoveToWindow`, which already consults
`_applicationStateTracker`:

```
- (void)didMoveToWindow
{
    if (!self._contentView.window || _applicationStateTracker)
        return;
```

Test: ApplicationStateTracking.WindowDeallocDoesNotPermanentlyFreezeLayerTree

* UIProcess/ios/WKApplicationStateTrackingView.mm:
(-[WKApplicationStateTrackingView willMoveToWindow:]): See above.

Tools:

Add an API test to exercise the bug. This API test is comprised of the following series of steps:
1. Create the web view and add it under window #1.
2. Post a "did enter background" notification.
3. Deallocate window #1 (thereby unparenting the web view in the process).
4. Post a "will enter foreground" notification.
5. Add the web view under window #2.
6. Load some HTML content and wait for a presentation update.

Before the fix, this test times out because the layer tree is permanently frozen after step (3), due to the
`BackgroundApplication` reason, so the presentation update in step (6) never finishes.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/ios/ApplicationStateTracking.mm: Added.
(TestWebKitAPI::TEST):



Canonical link: https://commits.webkit.org/248106@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@290875 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Mar 9, 2022
https://bugs.webkit.org/show_bug.cgi?id=237630
rdar://88690874

Reviewed by Jer Noble.

Source/WebCore:

Data cues have a start time but not an explicit duration, a data cue ends when
the next data cue from the same track starts. This means we don’t know the
duration of cue #1 until cue #2 is delivered, so when cue #1 is delivered it is
given the end time of the media file’s duration and the actual end time is updated
when cue #2 arrives.

http://webkit.org/b/229924 refactored text, audio, and video tracks to not depend
on HTMLMediaElement. Because InbandDataTextTrack could no longer access the
HTMLMediaElement to get its duration, a duration property was added to TextTrackList
that InbandDataTextTrack uses to set the duration of temporary cues.
TextTrackList.duration is set when it is created and updated when the media player
reports a duration change.

This means that if the media file’s duration is not known when the text track list
is created, and the file's duration never changes, the text track list never has a
valid duration and data cues were not added to the temporary list.

Fix this by updating TextTrackList.duration when a HTMLMediaElement reaches HAVE_METADATA.

Test: http/tests/media/hls/track-in-band-hls-metadata-cue-duration.html

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::durationChanged): Update m_textTracks.duration and post
the 'durationchange' event.
(WebCore::HTMLMediaElement::setReadyState): Call durationChanged.
(WebCore::HTMLMediaElement::mediaPlayerDurationChanged): Ditto.
* html/HTMLMediaElement.h:

* html/track/InbandDataTextTrack.cpp:
(WebCore::InbandDataTextTrack::addDataCue): Add cues to the incomplete cue map
even if the track list doesn't have duration.

LayoutTests:

* http/tests/media/hls/track-in-band-hls-metadata-cue-duration-expected.txt: Added.
* http/tests/media/hls/track-in-band-hls-metadata-cue-duration.html: Added.



Canonical link: https://commits.webkit.org/248203@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291029 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit that referenced this pull request Mar 23, 2022
    [Cocoa] metadata cue endTime may not be updated
    https://bugs.webkit.org/show_bug.cgi?id=237630
    rdar://88690874

    Reviewed by Jer Noble.

    Source/WebCore:

    Data cues have a start time but not an explicit duration, a data cue ends when
    the next data cue from the same track starts. This means we don’t know the
    duration of cue #1 until cue #2 is delivered, so when cue #1 is delivered it is
    given the end time of the media file’s duration and the actual end time is updated
    when cue #2 arrives.

    http://webkit.org/b/229924 refactored text, audio, and video tracks to not depend
    on HTMLMediaElement. Because InbandDataTextTrack could no longer access the
    HTMLMediaElement to get its duration, a duration property was added to TextTrackList
    that InbandDataTextTrack uses to set the duration of temporary cues.
    TextTrackList.duration is set when it is created and updated when the media player
    reports a duration change.

    This means that if the media file’s duration is not known when the text track list
    is created, and the file's duration never changes, the text track list never has a
    valid duration and data cues were not added to the temporary list.

    Fix this by updating TextTrackList.duration when a HTMLMediaElement reaches HAVE_METADATA.

    Test: http/tests/media/hls/track-in-band-hls-metadata-cue-duration.html

    * html/HTMLMediaElement.cpp:
    (WebCore::HTMLMediaElement::durationChanged): Update m_textTracks.duration and post
    the 'durationchange' event.
    (WebCore::HTMLMediaElement::setReadyState): Call durationChanged.
    (WebCore::HTMLMediaElement::mediaPlayerDurationChanged): Ditto.
    * html/HTMLMediaElement.h:

    * html/track/InbandDataTextTrack.cpp:
    (WebCore::InbandDataTextTrack::addDataCue): Add cues to the incomplete cue map
    even if the track list doesn't have duration.

    LayoutTests:

    * http/tests/media/hls/track-in-band-hls-metadata-cue-duration-expected.txt: Added.
    * http/tests/media/hls/track-in-band-hls-metadata-cue-duration.html: Added.

    Canonical link: https://commits.webkit.org/248203@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291029 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/245886.331@safari-613-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-613-branch@291658 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Apr 13, 2022
…with-relative-parent.html is a flaky image failure

https://bugs.webkit.org/show_bug.cgi?id=239101
<rdar://problem/91603539>

Reviewed by Antti Koivisto.

Source/WebCore:

1. Out of flow boxes are laid out independently from each other as the last step of their containing block layout.
2. However their static positions are computed during regular in-flow layout (as if their positions were static).

In order to do #1, we maintain a ListHashSet for the out-of-flow boxes and insert them at #2 (and we also have
a corresponding HashMap<ContainingBlock, ListHasSet>).

Normally this is a very simple list of descendant positioned boxes and since out-of-flow boxes don't interact with each
other, their position in the list is not important.
  e.g.
    <div id=A style="position: relative">
      <div>
        <div id=B style="position: absolute"></div>
        <div id=C style="position: absolute"></div>
      </div>
    </div>

At in-flow layout (#2), we insert B and C to "ListHashSet of A" as we come across them in DOM order and compute their static positions.
Later in the layout flow when we get to the "let's layout the out-of-flow boxes" phase (#1) we simply walk
the ListHashSet and lay out B and C (but "C and B" order would also work just fine).

However the ICB (RenderView) is a special containing block as it can hold different types of out-of-flow boxes (absolute and fixed)
and those out-of-flow boxes may have layout dependencies.
e.g.
    <body><div id=A class=absolute><div id=B class=fixed></div></div></body>

ICB's ListHasSet has both A and B, but in this case there's (static)layout dependency between these boxes.
In order to figure out the static position of B, we have to have A laid out first. In order to lay out A before B,
B has to be preceded by A in ICB's ListHasSet.

Now full layout always guarantees the correct order.
However in case of partial layout since we don't run a full #2, the ListHasSet may end up having an unexpected order.
  e.g.
   <body><div id=A class=absolute><div id=B><div id=C class=fixed></div></div></div></body>

 1. The initial (full) layout produces the following (correct) order for the ICB's ListHasSet -> AC.
 2. A subsequent partial layout (e.g. triggered by A's position change) runs an in-flow layout on the <body> which
 (re-)appends A to the ListHasSet (CA <- incorrect order). Now at this point we assume that the in-flow layout picks up B
 which eventually (re-)appends C to the ListHashSet (AC <- correct order). However since B does not need layout, we just
 stop at <body> which leaves us with an unexpected ListHashSet.
 3. As part of the ICB's out-of-flow layout, we pick C as the first box to lay out followed by A. However since C's static
 position depends on A's position, we end up using stale geometry when computing C's static position.

This patch fixes this issue by ensuring the absolute positioned boxes always come first in the ICB's ListHasSet (note
that their order is not really important -see above. What's important is that a potential (as-if-static) containing block always
comes before the fixed boxes).

Test: fast/block/fixed-inside-absolute-positioned.html

* rendering/RenderBlock.cpp:
(WebCore::PositionedDescendantsMap::addDescendant):
(WebCore::RenderBlock::insertPositionedObject):

LayoutTests:

* fast/block/fixed-inside-absolute-positioned-expected.html: Added.
* fast/block/fixed-inside-absolute-positioned.html: Added.
* platform/mac-wk1/TestExpectations:

Canonical link: https://commits.webkit.org/249597@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292817 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Apr 14, 2022
…with-relative-parent.html is a flaky image failure

https://bugs.webkit.org/show_bug.cgi?id=239101
<rdar://problem/91603539>

Reviewed by Antti Koivisto.

Source/WebCore:

1. Out of flow boxes are laid out independently from each other as the last step of their containing block layout.
2. However their static positions are computed during regular in-flow layout (as if their positions were static).

In order to do #1, we maintain a ListHashSet for the out-of-flow boxes and insert them at #2 (and we also have
a corresponding HashMap<ContainingBlock, ListHasSet>).

Normally this is a very simple list of descendant positioned boxes and since out-of-flow boxes don't interact with each
other, their position in the list is not important.
  e.g.
    <div id=A style="position: relative">
      <div>
        <div id=B style="position: absolute"></div>
        <div id=C style="position: absolute"></div>
      </div>
    </div>

At in-flow layout (#2), we insert B and C to "ListHashSet of A" as we come across them in DOM order and compute their static positions.
Later in the layout flow when we get to the "let's layout the out-of-flow boxes" phase (#1) we simply walk
the ListHashSet and lay out B and C (but "C and B" order would also work just fine).

However the ICB (RenderView) is a special containing block as it can hold different types of out-of-flow boxes (absolute and fixed)
and those out-of-flow boxes may have layout dependencies.
e.g.
    <body><div id=A class=absolute><div id=B class=fixed></div></div></body>

ICB's ListHasSet has both A and B, but in this case there's (static)layout dependency between these boxes.
In order to figure out the static position of B, we have to have A laid out first. In order to lay out A before B,
B has to be preceded by A in ICB's ListHasSet.

Now full layout always guarantees the correct order.
However in case of partial layout since we don't run a full #2, the ListHasSet may end up having an unexpected order.
  e.g.
   <body><div id=A class=absolute><div id=B><div id=C class=fixed></div></div></div></body>

 1. The initial (full) layout produces the following (correct) order for the ICB's ListHasSet -> AC.
 2. A subsequent partial layout (e.g. triggered by A's position change) runs an in-flow layout on the <body> which
 (re-)appends A to the ListHasSet (CA <- incorrect order). Now at this point we assume that the in-flow layout picks up B
 which eventually (re-)appends C to the ListHashSet (AC <- correct order). However since B does not need layout, we just
 stop at <body> which leaves us with an unexpected ListHashSet.
 3. As part of the ICB's out-of-flow layout, we pick C as the first box to lay out followed by A. However since C's static
 position depends on A's position, we end up using stale geometry when computing C's static position.

This patch fixes this issue by ensuring the absolute positioned boxes always come first in the ICB's ListHasSet (note
that their order is not really important -see above. What's important is that a potential (as-if-static) containing block always
comes before the fixed boxes).

Test: fast/block/fixed-inside-absolute-positioned.html

* rendering/RenderBlock.cpp:
(WebCore::PositionedDescendantsMap::addDescendant):
(WebCore::RenderBlock::insertPositionedObject):

LayoutTests:

* fast/block/fixed-inside-absolute-positioned-expected.html: Added.
* fast/block/fixed-inside-absolute-positioned.html: Added.
* platform/mac-wk1/TestExpectations:

Canonical link: https://commits.webkit.org/249626@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292855 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Jun 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=241856

Reviewed by Yusuke Suzuki.

1. Ruby treats numeric 0 as truthy.  However, there's a test in arm64LowerMalformedLoadStoreAddresses
   which assumes a value of 0 would be false.  As a result, we see offlineasm emit inefficient LLInt
   code like this:
    ".loc 3 821\n"        "movz x16, #0 \n"                    // LowLevelInterpreter64.asm:821
                          "add x13, x3, x16 \n"
                          "ldr x0, [x13] \n"

  ...  instead of this:
    ".loc 3 821\n"        "ldr x0, [x3] \n"                    // LowLevelInterpreter64.asm:821

   This patch fixes this.

2. offlineasm's emitARM64MoveImmediate chooses to use `movn` instead of `movz` based on whether a
   64-bit value is negative or not.  Instead, it should be making that decision based on the number of
   halfwords (16-bits) in the value that is 0xffff vs 0.  As a result, offlineasm emits code like this:
    ".loc 1 1638\n"       "movn x27, #1, lsl #48 \n"           // LowLevelInterpreter.asm:1638
                          "movk x27, #0, lsl #32 \n"
                          "movk x27, #0, lsl #16 \n"
                          "movk x27, #0 \n"

  ...  instead of this:
    ".loc 1 1638\n"       "movz x27, #65534, lsl #48 \n"       // LowLevelInterpreter.asm:1638

   This patch fixes this.

3. offlineasm is trivially assuming the range of immediate offsets for ldr/str instructions is
   [-255..4095].  However, that's only the range for byte sized load-stores.  For 32-bit, the range
   is actually [-255..16380].  For 64-bit, the range is actually [-255..32760].  As a result,
    offlineasm emits code like this:
    ".loc 1 633\n"        "movn x16, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x16 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "movz x16, #16088 \n"                // LowLevelInterpreter.asm:1519
                          "add x17, x3, x16 \n"
                          "ldr x3, [x17] \n"

  ...  instead of this:
    ".loc 1 633\n"        "movn x17, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x17 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "ldr x3, [x3, #16088] \n"            // LowLevelInterpreter.asm:1519

   This patch fixes this for 64-bit and 32-bit load-stores.  16-bit load-stores also has a wider
   range, but for now, it will continue to use the conservative range.

   This patch also introduces an `isMalformedArm64LoadAStoreAddress` so that this range check can be
   done consistently in all the places that checks for it.

4. offlineasm is eagerly emitting no-op arguments in instructions, e.g. "lsl #0", and adding 0.
   As a result, offlineasm emits code like this:
    ".loc 3 220\n"        "movz x13, #51168, lsl #0 \n"        // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13, lsl #0 \n"
                          "ldr w4, [x17, #0] \n"

  ...  instead of this:
    ".loc 3 220\n"        "movz x13, #51168 \n"                // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13 \n"
                          "ldr w4, [x17] \n"

   This unnecessary arguments are actually very common throughout the emitted LLIntAssembly.h.

   This patch removes these unnecessary arguments, which makes the emitted LLInt code more human
   readable due to less clutter.

This patch has passed the testapi and JSC stress tests with a Release build on an M1 Mac.

I also manually verified that the emitARM64MoveImmediate code is working properly by
hacking up LowLevelInterpreter64.asm to emit moves of constants of different values in
the ranges, and for load-store instructions of different sizes, and visually inspecting
the emitted code.

* Source/JavaScriptCore/offlineasm/arm64.rb:

Canonical link: https://commits.webkit.org/251771@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295766 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Jun 23, 2022
…ting layers

https://bugs.webkit.org/show_bug.cgi?id=241874

Reviewed by Simon Fraser.

addLayers stops (recursive) descending in the render tree soon after it finds a root (R) with layer.
It says that if a subtree root (R) has a layer then all layers in this subtree must have already been inserted into the layer tree at an earlier time.
(it simply assumes that any layer in the subtree is a child of (R), or some other layers in the subtree)

<div id=container>
  <div id=R>
    <div id=child>

The insertion is bottom to top; we attach
1, (child) to (R) first
2, followed by (R) to (container)
addLayers assumes that when (R) is being inserted (#2), we don't have to descend into (R)'s subtree since any renderer's layer that was inserted before (at #1) must have already been parented.

However toplayer/backdrop content is an exception where the parent layer may be outside of the subtree but still accessible. In such cases subsequent insertions (and the recursive nature of finding layer parents) could lead to double parenting where we try to insert the same layer into the layer tree multiple times.

* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::addLayers):
(WebCore::RenderElement::insertedIntoTree):
(WebCore::RenderElement::addLayers): Deleted.
* Source/WebCore/rendering/RenderElement.h:

Canonical link: https://commits.webkit.org/251772@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295767 268f45cc-cd09-0410-ab3c-d52691b4dbfc
alancoon pushed a commit that referenced this pull request Jun 23, 2022
    RenderElement::addLayers should check for dialog content before inserting layers
    https://bugs.webkit.org/show_bug.cgi?id=241874

    Reviewed by Simon Fraser.

    addLayers stops (recursive) descending in the render tree soon after it finds a root (R) with layer.
    It says that if a subtree root (R) has a layer then all layers in this subtree must have already been inserted into the layer tree at an earlier time.
    (it simply assumes that any layer in the subtree is a child of (R), or some other layers in the subtree)

    <div id=container>
      <div id=R>
        <div id=child>

    The insertion is bottom to top; we attach
    1, (child) to (R) first
    2, followed by (R) to (container)
    addLayers assumes that when (R) is being inserted (#2), we don't have to descend into (R)'s subtree since any renderer's layer that was inserted before (at #1) must have already been parented.

    However toplayer/backdrop content is an exception where the parent layer may be outside of the subtree but still accessible. In such cases subsequent insertions (and the recursive nature of finding layer parents) could lead to double parenting where we try to insert the same layer into the layer tree multiple times.

    * Source/WebCore/rendering/RenderElement.cpp:
    (WebCore::addLayers):
    (WebCore::RenderElement::insertedIntoTree):
    (WebCore::RenderElement::addLayers): Deleted.
    * Source/WebCore/rendering/RenderElement.h:

    Canonical link: https://commits.webkit.org/251772@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295767 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/245886.757@safari-613-branch
webkit-early-warning-system pushed a commit to jameshilliard/WebKit that referenced this pull request Jul 3, 2022
https://bugs.webkit.org/show_bug.cgi?id=242295

Reviewed by Michael Catanzaro.

We need to use adoptGRef when calling g_variant_get_data_as_bytes as
the return is already ref'd.

See:
https://github.com/GNOME/glib/blob/2.72.3/glib/gvariant-core.c#L975

Fixes:
==3126== 330 (120 direct, 210 indirect) bytes in 3 blocks are definitely lost in loss record 3,105 of 3,199
==3126==    at 0x48447ED: malloc (vg_replace_malloc.c:381)
==3126==    by 0xA87B2E8: g_malloc (gmem.c:106)
==3126==    by 0xA892E44: g_slice_alloc (gslice.c:1072)
==3126==    by 0xA84B005: g_bytes_new_with_free_func (gbytes.c:186)
==3126==    by 0xA84B067: g_bytes_new_take (gbytes.c:128)
==3126==    by 0xA8B934D: g_variant_ensure_serialised (gvariant-core.c:460)
==3126==    by 0xA8B958E: g_variant_get_data_as_bytes (gvariant-core.c:961)
==3126==    by 0x8765214: WebCore::KeyedEncoderGlib::finishEncoding() (KeyedEncoderGlib.cpp:139)
==3126==    by 0x53CF40E: WebKit::writeToDisk(std::unique_ptr<WebCore::KeyedEncoder, std::default_delete<WebCore::KeyedEncoder> >&&, WTF::String&&) (PersistencyUtils.cpp:53)
==3126==    by 0x545EF8C: operator() (DeviceIdHashSaltStorage.cpp:201)
==3126==    by 0x545EF8C: WTF::Detail::CallableWrapper<WebKit::DeviceIdHashSaltStorage::storeHashSaltToDisk(WebKit::DeviceIdHashSaltStorage::HashSaltForOrigin const&)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==3126==    by 0x6E52DE9: operator() (Function.h:82)
==3126==    by 0x6E52DE9: operator() (WorkQueueGeneric.cpp:70)
==3126==    by 0x6E52DE9: WTF::Detail::CallableWrapper<WTF::WorkQueueBase::dispatch(WTF::Function<void ()>&&)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==3126==    by 0x6DF490F: operator() (Function.h:82)
==3126==    by 0x6DF490F: WTF::RunLoop::performWork() (RunLoop.cpp:133)
==3126==    by 0x6E55171: WTF::RunLoop::RunLoop()::{lambda(void*)WebKit#1}::_FUN(void*) (RunLoopGLib.cpp:80)
==3126==    by 0x6E55D61: operator() (RunLoopGLib.cpp:53)
==3126==    by 0x6E55D61: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)WebKit#1}::_FUN(_GSource*, int (*)(void*), void*) (RunLoopGLib.cpp:56)
==3126==    by 0xA8723AB: g_main_dispatch (gmain.c:3381)
==3126==    by 0xA875839: g_main_context_dispatch (gmain.c:4099)
==3126==    by 0xA8759A7: g_main_context_iterate (gmain.c:4175)
==3126==    by 0xA875D41: g_main_loop_run (gmain.c:4373)
==3126==    by 0x6E5613C: WTF::RunLoop::run() (RunLoopGLib.cpp:108)
==3126==    by 0x6E52E14: operator() (WorkQueueGeneric.cpp:51)
==3126==    by 0x6E52E14: WTF::Detail::CallableWrapper<WTF::WorkQueueBase::platformInitialize(char const*, WTF::WorkQueueBase::Type, WTF::Thread::QOS)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==3126==    by 0x6DF6FD7: operator() (Function.h:82)
==3126==    by 0x6DF6FD7: WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (Threading.cpp:236)
==3126==    by 0x6E59A3F: WTF::wtfThreadEntryPoint(void*) (ThreadingPOSIX.cpp:242)
==3126==    by 0xA9D6DC2: start_thread (pthread_create.c:442)
==3126==    by 0xAA4FA0F: clone (clone.S:100)
==3126==

* Source/WebCore/platform/glib/KeyedEncoderGlib.cpp:
(WebCore::KeyedEncoderGlib::finishEncoding):

Canonical link: https://commits.webkit.org/252100@main
aperezdc pushed a commit that referenced this pull request Jul 3, 2022
…Encoding()

https://bugs.webkit.org/show_bug.cgi?id=242295

Reviewed by Michael Catanzaro.

We need to use adoptGRef when calling g_variant_get_data_as_bytes as
the return is already ref'd.

See:
https://github.com/GNOME/glib/blob/2.72.3/glib/gvariant-core.c#L975

Fixes:
==3126== 330 (120 direct, 210 indirect) bytes in 3 blocks are definitely lost in loss record 3,105 of 3,199
==3126==    at 0x48447ED: malloc (vg_replace_malloc.c:381)
==3126==    by 0xA87B2E8: g_malloc (gmem.c:106)
==3126==    by 0xA892E44: g_slice_alloc (gslice.c:1072)
==3126==    by 0xA84B005: g_bytes_new_with_free_func (gbytes.c:186)
==3126==    by 0xA84B067: g_bytes_new_take (gbytes.c:128)
==3126==    by 0xA8B934D: g_variant_ensure_serialised (gvariant-core.c:460)
==3126==    by 0xA8B958E: g_variant_get_data_as_bytes (gvariant-core.c:961)
==3126==    by 0x8765214: WebCore::KeyedEncoderGlib::finishEncoding() (KeyedEncoderGlib.cpp:139)
==3126==    by 0x53CF40E: WebKit::writeToDisk(std::unique_ptr<WebCore::KeyedEncoder, std::default_delete<WebCore::KeyedEncoder> >&&, WTF::String&&) (PersistencyUtils.cpp:53)
==3126==    by 0x545EF8C: operator() (DeviceIdHashSaltStorage.cpp:201)
==3126==    by 0x545EF8C: WTF::Detail::CallableWrapper<WebKit::DeviceIdHashSaltStorage::storeHashSaltToDisk(WebKit::DeviceIdHashSaltStorage::HashSaltForOrigin const&)::{lambda()#1}, void>::call() (Function.h:53)
==3126==    by 0x6E52DE9: operator() (Function.h:82)
==3126==    by 0x6E52DE9: operator() (WorkQueueGeneric.cpp:70)
==3126==    by 0x6E52DE9: WTF::Detail::CallableWrapper<WTF::WorkQueueBase::dispatch(WTF::Function<void ()>&&)::{lambda()#1}, void>::call() (Function.h:53)
==3126==    by 0x6DF490F: operator() (Function.h:82)
==3126==    by 0x6DF490F: WTF::RunLoop::performWork() (RunLoop.cpp:133)
==3126==    by 0x6E55171: WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) (RunLoopGLib.cpp:80)
==3126==    by 0x6E55D61: operator() (RunLoopGLib.cpp:53)
==3126==    by 0x6E55D61: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)#1}::_FUN(_GSource*, int (*)(void*), void*) (RunLoopGLib.cpp:56)
==3126==    by 0xA8723AB: g_main_dispatch (gmain.c:3381)
==3126==    by 0xA875839: g_main_context_dispatch (gmain.c:4099)
==3126==    by 0xA8759A7: g_main_context_iterate (gmain.c:4175)
==3126==    by 0xA875D41: g_main_loop_run (gmain.c:4373)
==3126==    by 0x6E5613C: WTF::RunLoop::run() (RunLoopGLib.cpp:108)
==3126==    by 0x6E52E14: operator() (WorkQueueGeneric.cpp:51)
==3126==    by 0x6E52E14: WTF::Detail::CallableWrapper<WTF::WorkQueueBase::platformInitialize(char const*, WTF::WorkQueueBase::Type, WTF::Thread::QOS)::{lambda()#1}, void>::call() (Function.h:53)
==3126==    by 0x6DF6FD7: operator() (Function.h:82)
==3126==    by 0x6DF6FD7: WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (Threading.cpp:236)
==3126==    by 0x6E59A3F: WTF::wtfThreadEntryPoint(void*) (ThreadingPOSIX.cpp:242)
==3126==    by 0xA9D6DC2: start_thread (pthread_create.c:442)
==3126==    by 0xAA4FA0F: clone (clone.S:100)
==3126==

* Source/WebCore/platform/glib/KeyedEncoderGlib.cpp:
(WebCore::KeyedEncoderGlib::finishEncoding):

Canonical link: https://commits.webkit.org/252100@main

(cherry picked from commit 025cae4)
webkit-early-warning-system pushed a commit to jameshilliard/WebKit that referenced this pull request Jul 11, 2022
…e leak

https://bugs.webkit.org/show_bug.cgi?id=242576

Reviewed by Xabier Rodriguez-Calvar.

Refactor ref counting for GstContext in GLVideoSinkGStreamer to
prevent a resource leak.

Fixes:
==196== 401 (296 direct, 105 indirect) bytes in 1 blocks are definitely lost in loss record 58,280 of 62,411
==196==    at 0x4845A83: calloc (vg_replace_malloc.c:1328)
==196==    by 0x15F58780: g_malloc0 (gmem.c:136)
==196==    by 0x161C8CBB: gst_structure_new_id_empty_with_size (gststructure.c:281)
==196==    by 0x161C8CBB: gst_structure_new_id_empty (gststructure.c:312)
==196==    by 0x161716CF: gst_context_new (gstcontext.c:178)
==196==    by 0x1122BB85: requestGLContext(char const*) (GLVideoSinkGStreamer.cpp:154)
==196==    by 0x1122BD12: setGLContext(_GstElement*, char const*) (GLVideoSinkGStreamer.cpp:173)
==196==    by 0x1122BE39: webKitGLVideoSinkChangeState(_GstElement*, GstStateChange) (GLVideoSinkGStreamer.cpp:189)
==196==    by 0x1617FA11: gst_element_change_state (gstelement.c:3083)
==196==    by 0x16180154: gst_element_set_state_func (gstelement.c:3037)
==196==    by 0x40651CE6: activate_sink (gstplaybin3.c:3805)
==196==    by 0x40651CE6: activate_sink.constprop.0 (gstplaybin3.c:3780)
==196==    by 0x40652B3E: activate_group (gstplaybin3.c:4539)
==196==    by 0x40652B3E: setup_next_source (gstplaybin3.c:4801)
==196==    by 0x406542A7: gst_play_bin3_change_state (gstplaybin3.c:5031)
==196==    by 0x1617FA11: gst_element_change_state (gstelement.c:3083)
==196==    by 0x1617FA5A: gst_element_change_state (gstelement.c:3122)
==196==    by 0x16180154: gst_element_set_state_func (gstelement.c:3037)
==196==    by 0x11257BC9: WebCore::MediaPlayerPrivateGStreamer::changePipelineState(GstState) (MediaPlayerPrivateGStreamer.cpp:924)
==196==    by 0x11258D8B: WebCore::MediaPlayerPrivateGStreamer::commitLoad() (MediaPlayerPrivateGStreamer.cpp:1184)
==196==    by 0x1125420B: WebCore::MediaPlayerPrivateGStreamer::load(WTF::String const&) (MediaPlayerPrivateGStreamer.cpp:354)
==196==    by 0x112542F4: WebCore::MediaPlayerPrivateGStreamer::load(WebCore::MediaStreamPrivate&) (MediaPlayerPrivateGStreamer.cpp:370)
==196==    by 0x148CF508: WebCore::MediaPlayer::loadWithNextMediaEngine(WebCore::MediaPlayerFactory const*) (MediaPlayer.cpp:646)
==196==    by 0x148CED64: WebCore::MediaPlayer::load(WebCore::MediaStreamPrivate&) (MediaPlayer.cpp:549)
==196==    by 0x13CF7047: WebCore::HTMLMediaElement::loadResource(WTF::URL const&, WebCore::ContentType&, WTF::String const&) (HTMLMediaElement.cpp:1599)
==196==    by 0x13CF5D70: WebCore::HTMLMediaElement::selectMediaResource()::{lambda()WebKit#1}::operator()() const (HTMLMediaElement.cpp:1413)
==196==    by 0x13D291BD: WTF::Detail::CallableWrapper<WebCore::HTMLMediaElement::selectMediaResource()::{lambda()WebKit#1}, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x131C31E7: WTF::CancellableTask::operator()() (CancellableTask.h:86)
==196==    by 0x13D2D2DD: WebCore::ActiveDOMObject::queueCancellableTaskKeepingObjectAlive<WebCore::HTMLMediaElement>(WebCore::HTMLMediaElement&, WebCore::TaskSource, WTF::TaskCancellationGroup&, WTF::Function<void ()>&&)::{lambda()WebKit#1}::operator()() (ActiveDOMObject.h:119)
==196==    by 0x13D5C88F: WTF::Detail::CallableWrapper<WebCore::ActiveDOMObject::queueCancellableTaskKeepingObjectAlive<WebCore::HTMLMediaElement>(WebCore::HTMLMediaElement&, WebCore::TaskSource, WTF::TaskCancellationGroup&, WTF::Function<void ()>&&)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x1399229B: WebCore::EventLoopFunctionDispatchTask::execute() (EventLoop.cpp:159)
==196==    by 0x13987D3A: WebCore::EventLoop::run() (EventLoop.cpp:123)
==196==    by 0x13ABF15D: WebCore::WindowEventLoop::didReachTimeToRun() (WindowEventLoop.cpp:121)
==196==    by 0x13AD46FB: void std::__invoke_impl<void, void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&>(std::__invoke_memfun_deref, void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&) (invoke.h:74)
==196==    by 0x13AD4666: std::__invoke_result<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&>::type std::__invoke<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&>(void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&) (invoke.h:96)
==196==    by 0x13AD45DC: void std::_Bind<void (WebCore::WindowEventLoop::*(WebCore::WindowEventLoop*))()>::__call<void, , 0ul>(std::tuple<>&&, std::_Index_tuple<0ul>) (functional:420)
==196==    by 0x13AD456E: void std::_Bind<void (WebCore::WindowEventLoop::*(WebCore::WindowEventLoop*))()>::operator()<, void>() (functional:503)
==196==    by 0x13AD4537: WTF::Detail::CallableWrapper<std::_Bind<void (WebCore::WindowEventLoop::*(WebCore::WindowEventLoop*))()>, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0xE23D137: WebCore::Timer::fired() (Timer.h:135)
==196==    by 0x146E59EF: WebCore::ThreadTimers::sharedTimerFiredInternal() (ThreadTimers.cpp:127)
==196==    by 0x146E52E4: WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::{lambda()WebKit#1}::operator()() const (ThreadTimers.cpp:67)
==196==    by 0x146E8407: WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x14698311: WebCore::MainThreadSharedTimer::fired() (MainThreadSharedTimer.cpp:83)
==196==    by 0x146A2E9D: void std::__invoke_impl<void, void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&>(std::__invoke_memfun_deref, void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&) (invoke.h:74)
==196==    by 0x146A2E16: std::__invoke_result<void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&>::type std::__invoke<void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&>(void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&) (invoke.h:96)
==196==    by 0x146A2D8C: void std::_Bind<void (WebCore::MainThreadSharedTimer::*(WebCore::MainThreadSharedTimer*))()>::__call<void, , 0ul>(std::tuple<>&&, std::_Index_tuple<0ul>) (functional:420)
==196==    by 0x146A2D1E: void std::_Bind<void (WebCore::MainThreadSharedTimer::*(WebCore::MainThreadSharedTimer*))()>::operator()<, void>() (functional:503)
==196==    by 0x146A2CC7: WTF::Detail::CallableWrapper<std::_Bind<void (WebCore::MainThreadSharedTimer::*(WebCore::MainThreadSharedTimer*))()>, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x146A2CE7: WTF::RunLoop::Timer<WebCore::MainThreadSharedTimer>::fired() (RunLoop.h:188)
==196==    by 0x110196A8: WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)WebKit#1}::operator()(void*) const (RunLoopGLib.cpp:177)
==196==    by 0x110196E8: WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)WebKit#1}::_FUN(void*) (RunLoopGLib.cpp:181)
==196==    by 0x11018BFA: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)WebKit#1}::operator()(_GSource*, int (*)(void*), void*) const (RunLoopGLib.cpp:53)
==196==    by 0x11018C48: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)WebKit#1}::_FUN(_GSource*, int (*)(void*), void*) (RunLoopGLib.cpp:56)
==196==    by 0x15F52293: g_main_dispatch (gmain.c:3381)
==196==    by 0x15F52293: g_main_context_dispatch (gmain.c:4099)
==196==    by 0x15F52637: g_main_context_iterate.constprop.0 (gmain.c:4175)
==196==    by 0x15F52942: g_main_loop_run (gmain.c:4373)
==196==    by 0x110192B3: WTF::RunLoop::run() (RunLoopGLib.cpp:108)
==196==    by 0xEFB8674: WebKit::AuxiliaryProcessMainBase<WebKit::WebProcess, true>::run(int, char**) (AuxiliaryProcessMain.h:70)
==196==    by 0xEFB5D26: int WebKit::AuxiliaryProcessMain<WebKit::WebProcessMainWPE>(int, char**) (AuxiliaryProcessMain.h:96)
==196==    by 0xEFB227E: WebKit::WebProcessMain(int, char**) (WebProcessMainWPE.cpp:75)
==196==    by 0x109908: main (WebProcessMain.cpp:31)
==196==

==196== 403 (88 direct, 315 indirect) bytes in 1 blocks are definitely lost in loss record 58,282 of 62,411
==196==    at 0x4840899: malloc (vg_replace_malloc.c:381)
==196==    by 0x15F58728: g_malloc (gmem.c:106)
==196==    by 0x15F710B4: g_slice_alloc (gslice.c:1072)
==196==    by 0x16171683: gst_context_new (gstcontext.c:174)
==196==    by 0x1122BC0A: requestGLContext(char const*) (GLVideoSinkGStreamer.cpp:160)
==196==    by 0x1122BD12: setGLContext(_GstElement*, char const*) (GLVideoSinkGStreamer.cpp:173)
==196==    by 0x1122BE5D: webKitGLVideoSinkChangeState(_GstElement*, GstStateChange) (GLVideoSinkGStreamer.cpp:191)
==196==    by 0x1617FA11: gst_element_change_state (gstelement.c:3083)
==196==    by 0x16180154: gst_element_set_state_func (gstelement.c:3037)
==196==    by 0x40651CE6: activate_sink (gstplaybin3.c:3805)
==196==    by 0x40651CE6: activate_sink.constprop.0 (gstplaybin3.c:3780)
==196==    by 0x40652B3E: activate_group (gstplaybin3.c:4539)
==196==    by 0x40652B3E: setup_next_source (gstplaybin3.c:4801)
==196==    by 0x406542A7: gst_play_bin3_change_state (gstplaybin3.c:5031)
==196==    by 0x1617FA11: gst_element_change_state (gstelement.c:3083)
==196==    by 0x1617FA5A: gst_element_change_state (gstelement.c:3122)
==196==    by 0x16180154: gst_element_set_state_func (gstelement.c:3037)
==196==    by 0x11257BC9: WebCore::MediaPlayerPrivateGStreamer::changePipelineState(GstState) (MediaPlayerPrivateGStreamer.cpp:924)
==196==    by 0x11258D8B: WebCore::MediaPlayerPrivateGStreamer::commitLoad() (MediaPlayerPrivateGStreamer.cpp:1184)
==196==    by 0x1125420B: WebCore::MediaPlayerPrivateGStreamer::load(WTF::String const&) (MediaPlayerPrivateGStreamer.cpp:354)
==196==    by 0x112542F4: WebCore::MediaPlayerPrivateGStreamer::load(WebCore::MediaStreamPrivate&) (MediaPlayerPrivateGStreamer.cpp:370)
==196==    by 0x148CF508: WebCore::MediaPlayer::loadWithNextMediaEngine(WebCore::MediaPlayerFactory const*) (MediaPlayer.cpp:646)
==196==    by 0x148CED64: WebCore::MediaPlayer::load(WebCore::MediaStreamPrivate&) (MediaPlayer.cpp:549)
==196==    by 0x13CF7047: WebCore::HTMLMediaElement::loadResource(WTF::URL const&, WebCore::ContentType&, WTF::String const&) (HTMLMediaElement.cpp:1599)
==196==    by 0x13CF5D70: WebCore::HTMLMediaElement::selectMediaResource()::{lambda()WebKit#1}::operator()() const (HTMLMediaElement.cpp:1413)
==196==    by 0x13D291BD: WTF::Detail::CallableWrapper<WebCore::HTMLMediaElement::selectMediaResource()::{lambda()WebKit#1}, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x131C31E7: WTF::CancellableTask::operator()() (CancellableTask.h:86)
==196==    by 0x13D2D2DD: WebCore::ActiveDOMObject::queueCancellableTaskKeepingObjectAlive<WebCore::HTMLMediaElement>(WebCore::HTMLMediaElement&, WebCore::TaskSource, WTF::TaskCancellationGroup&, WTF::Function<void ()>&&)::{lambda()WebKit#1}::operator()() (ActiveDOMObject.h:119)
==196==    by 0x13D5C88F: WTF::Detail::CallableWrapper<WebCore::ActiveDOMObject::queueCancellableTaskKeepingObjectAlive<WebCore::HTMLMediaElement>(WebCore::HTMLMediaElement&, WebCore::TaskSource, WTF::TaskCancellationGroup&, WTF::Function<void ()>&&)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x1399229B: WebCore::EventLoopFunctionDispatchTask::execute() (EventLoop.cpp:159)
==196==    by 0x13987D3A: WebCore::EventLoop::run() (EventLoop.cpp:123)
==196==    by 0x13ABF15D: WebCore::WindowEventLoop::didReachTimeToRun() (WindowEventLoop.cpp:121)
==196==    by 0x13AD46FB: void std::__invoke_impl<void, void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&>(std::__invoke_memfun_deref, void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&) (invoke.h:74)
==196==    by 0x13AD4666: std::__invoke_result<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&>::type std::__invoke<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&>(void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&) (invoke.h:96)
==196==    by 0x13AD45DC: void std::_Bind<void (WebCore::WindowEventLoop::*(WebCore::WindowEventLoop*))()>::__call<void, , 0ul>(std::tuple<>&&, std::_Index_tuple<0ul>) (functional:420)
==196==    by 0x13AD456E: void std::_Bind<void (WebCore::WindowEventLoop::*(WebCore::WindowEventLoop*))()>::operator()<, void>() (functional:503)
==196==    by 0x13AD4537: WTF::Detail::CallableWrapper<std::_Bind<void (WebCore::WindowEventLoop::*(WebCore::WindowEventLoop*))()>, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0xE23D137: WebCore::Timer::fired() (Timer.h:135)
==196==    by 0x146E59EF: WebCore::ThreadTimers::sharedTimerFiredInternal() (ThreadTimers.cpp:127)
==196==    by 0x146E52E4: WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::{lambda()WebKit#1}::operator()() const (ThreadTimers.cpp:67)
==196==    by 0x146E8407: WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::{lambda()WebKit#1}, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x14698311: WebCore::MainThreadSharedTimer::fired() (MainThreadSharedTimer.cpp:83)
==196==    by 0x146A2E9D: void std::__invoke_impl<void, void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&>(std::__invoke_memfun_deref, void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&) (invoke.h:74)
==196==    by 0x146A2E16: std::__invoke_result<void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&>::type std::__invoke<void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&>(void (WebCore::MainThreadSharedTimer::*&)(), WebCore::MainThreadSharedTimer*&) (invoke.h:96)
==196==    by 0x146A2D8C: void std::_Bind<void (WebCore::MainThreadSharedTimer::*(WebCore::MainThreadSharedTimer*))()>::__call<void, , 0ul>(std::tuple<>&&, std::_Index_tuple<0ul>) (functional:420)
==196==    by 0x146A2D1E: void std::_Bind<void (WebCore::MainThreadSharedTimer::*(WebCore::MainThreadSharedTimer*))()>::operator()<, void>() (functional:503)
==196==    by 0x146A2CC7: WTF::Detail::CallableWrapper<std::_Bind<void (WebCore::MainThreadSharedTimer::*(WebCore::MainThreadSharedTimer*))()>, void>::call() (Function.h:53)
==196==    by 0xD99E63C: WTF::Function<void ()>::operator()() const (Function.h:82)
==196==    by 0x146A2CE7: WTF::RunLoop::Timer<WebCore::MainThreadSharedTimer>::fired() (RunLoop.h:188)
==196==    by 0x110196A8: WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)WebKit#1}::operator()(void*) const (RunLoopGLib.cpp:177)
==196==    by 0x110196E8: WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)WebKit#1}::_FUN(void*) (RunLoopGLib.cpp:181)
==196==    by 0x11018BFA: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)WebKit#1}::operator()(_GSource*, int (*)(void*), void*) const (RunLoopGLib.cpp:53)
==196==    by 0x11018C48: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)WebKit#1}::_FUN(_GSource*, int (*)(void*), void*) (RunLoopGLib.cpp:56)
==196==    by 0x15F52293: g_main_dispatch (gmain.c:3381)
==196==    by 0x15F52293: g_main_context_dispatch (gmain.c:4099)
==196==    by 0x15F52637: g_main_context_iterate.constprop.0 (gmain.c:4175)
==196==    by 0x15F52942: g_main_loop_run (gmain.c:4373)
==196==    by 0x110192B3: WTF::RunLoop::run() (RunLoopGLib.cpp:108)
==196==    by 0xEFB8674: WebKit::AuxiliaryProcessMainBase<WebKit::WebProcess, true>::run(int, char**) (AuxiliaryProcessMain.h:70)
==196==    by 0xEFB5D26: int WebKit::AuxiliaryProcessMain<WebKit::WebProcessMainWPE>(int, char**) (AuxiliaryProcessMain.h:96)
==196==    by 0xEFB227E: WebKit::WebProcessMain(int, char**) (WebProcessMainWPE.cpp:75)
==196==    by 0x109908: main (WebProcessMain.cpp:31)
==196==

* Source/WebCore/platform/graphics/gstreamer/GLVideoSinkGStreamer.cpp:
(requestGLContext):
(setGLContext):

Canonical link: https://commits.webkit.org/252340@main
webkit-commit-queue pushed a commit to lopezlozoya/WebKit that referenced this pull request Feb 19, 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 WebKit#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
webkit-commit-queue pushed a commit to msaboff/WebKit that referenced this pull request Feb 21, 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, WebKit#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
webkit-commit-queue pushed a commit to msaboff/WebKit that referenced this pull request Feb 24, 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, WebKit#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
webkit-commit-queue pushed a commit to rcaliman-apple/WebKit that referenced this pull request Feb 26, 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 WebKit#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
webkit-commit-queue 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 #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
(#2) becomes non-intrusive and
(#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 #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 #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 #2).

* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout):

Canonical link: https://commits.webkit.org/293119@main
aperezdc pushed a commit that referenced this pull request Apr 6, 2025
…gi?id=290862

    Block reusing may remove floating object too early
    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 #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

Canonical link: https://commits.webkit.org/290945.158@webkitglib/2.48
SunnyM00n pushed a commit to SunnyM00n/WebKit that referenced this pull request Apr 17, 2025
    [JSC] ObjectAllocationSinking should not omit phi insertion when pointer follows to the same value
    https://bugs.webkit.org/show_bug.cgi?id=279570
    rdar://135973620

    Reviewed by Keith Miller.

    Let's consider the following FTL graph.

        BB#0
        @0 = NewObject()
        Jump WebKit#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

Canonical link: https://commits.webkit.org/283286.49@safari-7620-branch
webkit-commit-queue pushed a commit that referenced this pull request Apr 19, 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
(#2) becomes non-intrusive and
(#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 #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 #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 #2).

* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::markSiblingsWithFloatsForLayout):

Canonical link: https://commits.webkit.org/293889@main
webkit-commit-queue pushed a commit to the-chenergy/WebKit that referenced this pull request Apr 25, 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 WebKit#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
Constellation added a commit to Constellation/WebKit that referenced this pull request May 21, 2025
https://bugs.webkit.org/show_bug.cgi?id=293337
rdar://151740794

Reviewed by NOBODY (OOPS!).

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 WebKit#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):
webkit-commit-queue pushed a commit that referenced this pull request May 23, 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
Constellation added a commit to Constellation/WebKit that referenced this pull request May 23, 2025
https://bugs.webkit.org/show_bug.cgi?id=293337
rdar://151740794

Reviewed by NOBODY (OOPS!).

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 WebKit#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):
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request May 23, 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 WebKit#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
    #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
webkit-commit-queue pushed a commit that referenced this pull request Aug 5, 2025
…ht (005,007) and table-alignment-003 and 005

https://bugs.webkit.org/show_bug.cgi?id=296852

Reviewed by Antti Koivisto.

1. lastLineLogicalBaseline should return a value relative to the logical top of the box even
when the box's lines are inverted (vertical-lr).
This ensures that any baseline function simply returns the distance from the logical top, regardless of the writing mode.
2. Collapse lastLinePhysicalBaseline and lastLineLogicalBaseline into one function
and call it lastLineBaseline (and also rename firstLinePhysicalBaseline to firstLineBaseline)
3. When IFC takes a box's baseline right before running inline layout, flip it over for inverted lines when applicable (this is what we used to do at #1).

* LayoutTests/TestExpectations:
* LayoutTests/fast/text/emphasis-overlap-expected.txt:
* LayoutTests/fast/writing-mode/flipped-blocks-inline-map-local-to-container-expected.html:
* LayoutTests/fast/writing-mode/flipped-blocks-inline-map-local-to-container.html:
* Source/WebCore/layout/integration/LayoutIntegrationBoxGeometryUpdater.cpp:
(WebCore::LayoutIntegration::baselineForBox):
(WebCore::LayoutIntegration::setIntegrationBaseline):
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::firstLineBaseline const):
(WebCore::LayoutIntegration::LineLayout::lastLineBaseline const):
(WebCore::LayoutIntegration::LineLayout::baselineForLine const):
(WebCore::LayoutIntegration::LineLayout::firstLinePhysicalBaseline const): Deleted.
(WebCore::LayoutIntegration::LineLayout::lastLinePhysicalBaseline const): Deleted.
(WebCore::LayoutIntegration::LineLayout::physicalBaselineForLine const): Deleted.
(WebCore::LayoutIntegration::LineLayout::lastLineLogicalBaseline const): Deleted.
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.h:
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::firstLineBaseline const):
(WebCore::RenderBlockFlow::lastLineBaseline const):

Canonical link: https://commits.webkit.org/298236@main
webkit-commit-queue pushed a commit that referenced this pull request Aug 21, 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 #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, #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 #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
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.

2 participants