Unify WebContent log prefix and subsystem by pvollan · Pull Request #1 · pvollan/WebKit

pvollan · 2025-05-01T17:08:52Z

78ea1c437eab7fe9a3d7e9f111fffe281ebda54b

Unify WebContent log prefix and subsystem
https://bugs.webkit.org/show_bug.cgi?id=292381
rdar://150453365

Reviewed by NOBODY (OOPS!).

We should use the same prefix and subsystem for WebContent logs that are being forwarded to the UI process through
the log hook, as well as for those that are being directly sent over CoreIPC. We have not migrated all WebKit logs
to the efficient version yet, so there are still some that are being picked up by the log hook.

* Source/WebKit/Scripts/generate-derived-log-sources.py:
(generate_message_receiver_implementations_file):
* Source/WebKit/Shared/LogStream.cpp:
(WebKit::LogStream::logOnBehalfOfWebContent):

https://bugs.webkit.org/show_bug.cgi?id=292382 rdar://150453365 Reviewed by NOBODY (OOPS!). We should use the same prefix and subsystem for WebContent logs that are being forwarded to the UI process through the log hook, as well as for those that are being directly sent over CoreIPC. We have not migrated all WebKit logs to the efficient version yet, so there are still some that are being picked up by the log hook. * Source/WebKit/Scripts/generate-derived-log-sources.py: (generate_message_receiver_implementations_file): * Source/WebKit/Shared/LogStream.cpp: (WebKit::LogStream::logOnBehalfOfWebContent):

aproskuryakov · 2025-05-01T17:52:19Z

Not sure what happened here, but this PR is made against a fork, not against the real WebKit.

pvollan · 2025-05-01T18:13:10Z

Not sure what happened here, but this PR is made against a fork, not against the real WebKit.

Ah, yes, the remote was incorrectly set.

Thanks for catching!

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

@x

https://bugs.webkit.org/show_bug.cgi?id=293337 rdar://151740794 Reviewed by Yijia Huang and Justin Michaud. The current MovHintRemoval's analysis looks weird. We should just do liveness analysis globally and use this information for MovHint removal. 1. "Use" is a node which may exit. When exit happens, we should keep all use of live locals at this bytecode exit location alive. 2. "Def" is MovHint. We kill the locals here. And doing fixpoint analysis and using this information to remove MovHint. Also, pruning Availability in OSRAvailabilityAnalysisPhase via bytecode liveness is wrong: they need to keep live nodes from DFG for example. 0: PutHint @x, PROP(@y) 1: OSR exit point #1 (here, loc0 is not alive) 2: -- Pruning happens -- 3: MovHint @x, loc0 4: OSR exit point #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

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

…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

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

@0

https://bugs.webkit.org/show_bug.cgi?id=299504 rdar://161294228 Reviewed by Yijia Huang. Previous one 300327@main worked for the same basic block's load, but it didn't work for the load in dominators. This patch updates CSE rules to make immutable load elimination work with dominators' load. Like, BB#0 @0: Load(@x, immutable) @1: CCall(...) # potentially clobber everything Branch ... #1, #2 BB#1 @2: CCall(...) # potentially clobber everything Jump WebKit#3 BB#2 @3: CCall(...) # potentially clobber everything Jump WebKit#3 BB#3 @4: Load(@x, immutable) ... Then @4 should be replaced with Identity(@0) as dominator BB#0 is having immutable load @0 matching to @4. Tests: Source/JavaScriptCore/b3/testb3_1.cpp Source/JavaScriptCore/b3/testb3_8.cpp * Source/JavaScriptCore/b3/B3EliminateCommonSubexpressions.cpp: * Source/JavaScriptCore/b3/testb3.h: * Source/JavaScriptCore/b3/testb3_1.cpp: (run): * Source/JavaScriptCore/b3/testb3_8.cpp: (testLoadImmutableDominated): (testLoadImmutableNonDominated): Canonical link: https://commits.webkit.org/300562@main

…on-in-child.html is failing https://bugs.webkit.org/show_bug.cgi?id=299628 rdar://161203486 Reviewed by Basuke Suzuki. Suppose we have a mainframe and an iframe and this series of navigations happens: 1. iframe fragment navigates to "/#a" 2. main frame fragment navigates to "/#1" 3. main frame fragment navigates to "/#2" 4. main frame fragment navigates to "/WebKit#3" 5. iframe goes back 6. iframe fragment navigates to "/#b" After Step 5, the UI Process b/f list should be: A) mainframe - URL - ItemID A ** iframe - URL - ItemID A B) mainframe - URL - ItemID B ** iframe - URL/#a - ItemID B C) mainframe - URL/#1 - ItemID C ** iframe - URL/#a - ItemID C D) mainframe - URL/#2 - ItemID D ** iframe - URL/#a - ItemID D E) mainframe - URL/WebKit#3 - ItemID E ** iframe - URL/#a - ItemID E The mainframe's Navigation object's m_entries should be: A) mainframe - URL - ItemID A C) mainframe - URL/#1 - ItemID C D) mainframe - URL/#2 - ItemID D E) mainframe - URL/WebKit#3 - ItemID E <--- current index The iframe's Navigation object's m_entries should be: A) ** iframe - URL - ItemID A <--- current index E) ** iframe - URL/#a - ItemID E According to this layout test, after Step 6: The mainframe's Navigation object's m_entries should be: A) mainframe - URL - ItemID A <--- current index The iframe's Navigation object's m_entries should be: A) ** iframe - URL - ItemID A F) ** iframe - URL/#b - ItemID F <--- current index So when a subframe has a PUSH same-document navigation and disposes of any forward entries, any parent frame must do the same. This test was failing because the parent frame was not disposing of its forward entries. To fix this, we add a new function to recusively dispose of all forward entries in any parent frames when a subframe has a PUSH same-document navigation. We use the ItemID to determine what entry must stay and then dispose of any entries that come after that one. * LayoutTests/imported/w3c/web-platform-tests/navigation-api/per-entry-events/dispose-for-navigation-in-child-expected.txt: * Source/WebCore/page/LocalDOMWindowProperty.cpp: (WebCore::LocalDOMWindowProperty::protectedFrame const): * Source/WebCore/page/LocalDOMWindowProperty.h: * Source/WebCore/page/Navigation.cpp: (WebCore::Navigation::updateNavigationEntry): (WebCore::Navigation::disposeOfForwardEntriesInParents): Call recursivelyDisposeOfForwardEntriesInParents on the main frame, which will traverse down the frame tree, and for each frame until we reach the subframe that actually navigated, dispose of any forward entries. (WebCore::Navigation::recursivelyDisposeOfForwardEntriesInParents): (WebCore::Navigation::updateForNavigation): This is called for same-document navigations. If it's a PUSH navigation, call disposeOfForwardEntriesInParents. The ItemID that we keep in these parent frames is the current ItemID right before this PUSH operation happens. * Source/WebCore/page/Navigation.h: Canonical link: https://commits.webkit.org/300721@main

rdar://161694748 https://bugs.webkit.org/show_bug.cgi?id=299920 Reviewed by Ryosuke Niwa. Prior to this patch, DocumentInlines.h was the most expensive header used in WebCore; it is included 303 times in a WebCore Unified build, and on this machine, each include of that file too the compile on average 1.5s to parse, for a total of 8m CPU minutes of time spent parsing this header. To diagnose why this header is so expensive, I commented out all the implementations from the body of this header and ran a compile, collecting a list of all the errors encountered. I then collected the counts of each error message, and grouped liked errors together. It became clear that most uses of DocumentInlines.h were to pull in a very narrow set of inlined methods, but to get those few methods had to include a very large and very heavy header. To address this, DocumentInlines.h was broken up into separate headers, each which includes only Document.h and one or two other headers. Usually the second header was required to define the return type of the method, and the type definition is required because the return type is a CheckedPtr or RefPtr. However because the return value is typically used at the call site, that return value header would have been included already, meaning this style of header has zero effective additional cost. The files were created in descending order of the number of uses: - DocumentView.h - 187 - DocumentPage.h - 175 - DocumentQuirks.h - 84 - DocumentSecurityOrigin.h - 54 - DocumentResourceLoader.h - 49 - DocumentMarkers.h - 27 - DocumentEventLoop.h - 26 - DocumentSettingsValues.h - 23 - DocumentWindow.h - 19 The remaining grab-bag of inlined methods had less than 10 uses each and were left in DocumentInlines.h. After this patch, DocumentInlines.h is used in just over 100 implementation files and no headers. None of the new Document*.h headers or DocumentInlines.h appear in the top 100 most expensive header files. DocumentInlines.h dropped from the #1 most expensive header taking 8m to parse to WebKit#389 and 5.7s of parsing. The most expensive new header is DocumentPage.h at WebKit#124 and 25s of parsing. Canonical link: https://commits.webkit.org/300989@main

…are trimmed https://bugs.webkit.org/show_bug.cgi?id=301794 Reviewed by Antti Koivisto. 1. In 'TextBoxPainter::collectDecoratingBoxesForBackgroundPainting', make 'decoratingBoxLocation' represent the actual position of the decorating box itself, not the position of the text content inside it (the inline box). (While these typically align, in cases with trimmed sides, the inline box can be offset while the text content remains in its original position.) 2. Make sure that 'underlineOffsetForTextBoxPainting' consistently returns the underline position relative to the decorating inline box - not the text _inside_ the decorating inline box (see #1). Test: fast/inline/text-box-trim-with-underline2.html * LayoutTests/fast/inline/text-box-trim-with-underline2-expected.html: Added. * LayoutTests/fast/inline/text-box-trim-with-underline2.html: Added. * Source/WebCore/rendering/TextBoxPainter.cpp: (WebCore::TextBoxPainter::collectDecoratingBoxesForBackgroundPainting): * Source/WebCore/style/InlineTextBoxStyle.cpp: (WebCore::textBoxEdgeAdjustmentForUnderline): (WebCore::underlineOffsetForTextBoxPainting): * Source/WebCore/style/InlineTextBoxStyle.h: Canonical link: https://commits.webkit.org/302435@main

https://bugs.webkit.org/show_bug.cgi?id=304917 rdar://167529315 Reviewed by Marcus Plutowski. This patch implements chain of compare with ARM64 ccmp / ccmn. Let's say, if (x0 == 0 && x1 == 1) { // target-block } Then this will be compiled via LLVM into wasm. And this will create BitAnd(Equal(a, 0), Equal(b, 1)). Then ARM64 ccmp can handle it as follows. cmp x0, #0 ccmp x1, #1, #0, eq // cmp x1, #1 when flag is eq and override, otherwise set #0 to flag b.eq target-block This reduces small weird basic blocks and reduces prediction miss, and reduces code size. We introduce CompareOnFlags, CompareConditionallyOnFlags, and BranchOnFlags Air opcodes. All of them are annotated as /effects since this relies on the current flag, and they are not producing register output while it has side effects. This ensures that we are not removing these instructions, and we are not hoisting etc. randomly. We introduced V8's compare chain detection mechanism and using it for chained cmp code generation. Tests: Source/JavaScriptCore/b3/testb3_1.cpp Source/JavaScriptCore/b3/testb3_8.cpp * Source/JavaScriptCore/assembler/MacroAssemblerARM64.h: (JSC::MacroAssemblerARM64::compareOnFlags32): (JSC::MacroAssemblerARM64::compareOnFlags64): (JSC::MacroAssemblerARM64::compareOnFlagsFloat): (JSC::MacroAssemblerARM64::compareOnFlagsDouble): (JSC::MacroAssemblerARM64::compareConditionallyOnFlags32): (JSC::MacroAssemblerARM64::compareConditionallyOnFlags64): (JSC::MacroAssemblerARM64::compareConditionallyOnFlagsFloat): (JSC::MacroAssemblerARM64::compareConditionallyOnFlagsDouble): (JSC::MacroAssemblerARM64::branchOnFlags): * Source/JavaScriptCore/b3/B3LowerToAir.cpp: * Source/JavaScriptCore/b3/air/AirOpcode.opcodes: * Source/JavaScriptCore/b3/air/AirOptimizeBlockOrder.cpp: (JSC::B3::Air::optimizeBlockOrder): * Source/JavaScriptCore/b3/testb3.h: * Source/JavaScriptCore/b3/testb3_1.cpp: (run): * Source/JavaScriptCore/b3/testb3_8.cpp: (testCCmpAnd32): (testCCmpAnd64): (testCCmpOr32): (testCCmpOr64): (testCCmpAndAnd32): (testCCmpOrOr32): (testCCmpAndOr32): (testCCmnAnd32WithNegativeImm): (testCCmnAnd64WithNegativeImm): (testCCmpWithLargePositiveImm): (testCCmpWithLargeNegativeImm): (testCCmpSmartOperandOrdering32): (testCCmpSmartOperandOrdering64): (testCCmpOperandCommutation32): (testCCmpOperandCommutation64): (testCCmpCombinedOptimizations): (testCCmpZeroRegisterOptimization32): (testCCmpZeroRegisterOptimization64): (testCCmpMixedAndOr32): (testCCmpMixedOrAnd32): (testCCmpNegatedAnd32): (testCCmpNegatedOr32): (testCCmpMixedWidth32And64): (testCCmpMixedWidth64And32): Canonical link: https://commits.webkit.org/305493@main

https://bugs.webkit.org/show_bug.cgi?id=312598 Reviewed by Antti Koivisto. In quirks mode, when an element has a percentage height inside a flex item, the browser needs to figure out what that percentage resolves against. The quirks spec (section 3.5) has a special algorithm for this. It walks up the containing block chain, skipping auto-height block containers, looking for an ancestor with a definite height. Step 4 says to stop the walk when the ancestor is "not a block container" - flex containers are not block containers, so the walk stops there. The problem was twofold: 1. Resolution: once the walk stopped at the flex container, we called availableLogicalHeightForPercentageComputation() on it. This returned nullopt because the flex container has auto height. But after cross-axis sizing, the flex container's used content height IS known. We now fall back to contentBoxLogicalHeight() in this case, scoped to isInCrossAxisStretchLayout() so we only use it when the value is final. 2. Relayout timing: the resolution above only works during the stretch relayout pass (when isInCrossAxisStretchLayout() is true). The stretch relayout check in applyStretchAlignmentToFlexItem used block->hasPercentHeightDescendants(), which asks the flex item directly "do you have percent-height descendants registered on you?" In quirks mode, the answer is no since the percent height descendant is registered on the flex container (see #1) Using flexItemHasPercentHeightDescendants() instead asks the flex container "do you have percent-height descendants that are under this flex item?" - which correctly returns true, so the stretch relayout fires. Note: the deeply nested case (multiple wrapper divs between the flex item and the percentage-height descendant) still does not work because the stretch relayout only marks the flex item dirty (MarkOnlyThis), and layout does not propagate through the clean intermediate ancestors down to the descendant. That is a separate issue. Test: fast/flexbox/quirks-percentage-height-in-flex-item.html * LayoutTests/fast/flexbox/quirks-percentage-height-in-flex-item-expected.html: Added. * LayoutTests/fast/flexbox/quirks-percentage-height-in-flex-item.html: Added. * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::computePercentageLogicalHeightGeneric const): * Source/WebCore/rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::applyStretchAlignmentToFlexItem): * Source/WebCore/rendering/RenderFlexibleBox.h: Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (WebCore::RenderFlexibleBox::isInCrossAxisStretchLayout const): Canonical link: https://commits.webkit.org/311581@main

https://bugs.webkit.org/show_bug.cgi?id=313498 rdar://175714384 Reviewed by Sihui Liu. In Document::isSecureContext, WebKit walks the frame tree to check if all of a frame's ancestors are "secure". It does this to gate access to powerful web APIs such as navigator.geolocation. For each ancestor, we call Document::isDocumentSecure which performs checks to see if the frame is potentially trustworthy. Below is the implementation. It does the following: 1. If the document is sandboxed, it checks if the document's URL is trustworthy 2. Otherwise, check if the document's security origin is trustworthy. ``` static inline bool isDocumentSecure(const Document& document) { if (document.isSandboxed(SandboxFlag::Origin)) return isURLPotentiallyTrustworthy(document.url()); return document.securityOrigin().isPotentiallyTrustworthy(); } ``` With site isolation enabled, it is possible for some of the document's ancestors to be RemoteFrames in different processes. Currently, the code in Document::isSecureContext, only handles LocalFrames and silently skips any RemoteFrame ancestors. This patch handles the RemoteFrame case by adding a fallback when an ancestor frame can't be cast to a LocalFrame. Since we can't get the document of the RemoteFrame, we can't call Document::isDocumentSecure like the LocalFrame case. Instead, this patch directly calls isPotentiallyTrustworthy on the RemoteFrame's security origin. This is #2 from the description of Document::isDocumentSecure earlier in this commit message. I chose to skip #1 since we don't have the full URL or sandbox flags of the remote frame. Also, only checking the RemoteFrame's security origin is more conservative since in the worst case we would treat a frame as insecure (and block requests) where the pre site isolation case would treat it as secure. This patches fixes imported/w3c/web-platform-tests/secure-contexts/basic-popup-and-iframe-tests.html with site isolation enabled. * LayoutTests/platform/ios-site-isolation/TestExpectations: * LayoutTests/platform/mac-site-isolation/TestExpectations: * Source/WebCore/dom/Document.cpp: (WebCore::Document::isSecureContext const): Canonical link: https://commits.webkit.org/312199@main

pvollan self-assigned this May 1, 2025

pvollan force-pushed the eng/Unify-WebContent-log-prefix-and-subsystem branch from 78ea1c4 to 8767ed4 Compare May 1, 2025 17:23

pvollan closed this May 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify WebContent log prefix and subsystem#1

Unify WebContent log prefix and subsystem#1
pvollan wants to merge 1 commit intomainfrom
eng/Unify-WebContent-log-prefix-and-subsystem

pvollan commented May 1, 2025

Uh oh!

aproskuryakov commented May 1, 2025

Uh oh!

pvollan commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pvollan commented May 1, 2025

78ea1c437eab7fe9a3d7e9f111fffe281ebda54b

Uh oh!

aproskuryakov commented May 1, 2025

Uh oh!

pvollan commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants