Skip to content

layout: Preserve cached intrinsic inline sizes in more cases#41160

Merged
Loirooriol merged 1 commit intoservo:mainfrom
Loirooriol:intrinsic-contributions
Dec 11, 2025
Merged

layout: Preserve cached intrinsic inline sizes in more cases#41160
Loirooriol merged 1 commit intoservo:mainfrom
Loirooriol:intrinsic-contributions

Conversation

@Loirooriol
Copy link
Copy Markdown
Contributor

If a box has layout damage, but the intrinsic contribution of some ancestor doesn't depend on its contents, then we don't need to clear the cached intrinsic sizes of further ancestors.

Testing: No behavior change, just an optimization. It would be good to have unit tests that check we don't do unnecessary work, but leaving this for the future.

@Loirooriol Loirooriol added A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 T-linux-wpt Do a try run of the WPT labels Dec 9, 2025
@github-actions github-actions bot removed the T-linux-wpt Do a try run of the WPT label Dec 9, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2025

🔨 Triggering try run (#20070885797) for Linux (WPT)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2025

Test results for linux-wpt from try job (#20070885797):

Flaky unexpected result (33)
  • TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
    • TIMEOUT [expected PASS] subtest: Fetching a blob URL immediately before revoking it works in <script> tags.

      Test timed out
      

  • OK /IndexedDB/idbfactory_open.any.html
    • FAIL [expected PASS] subtest: Calling open() with version argument 1.5 should not throw.

      assert_equals: version expected 1 but got 9007199254740991
      

  • OK /_mozilla/css/offset_properties_inline.html (#40543)
    • FAIL [expected PASS] subtest: offsetTop

      assert_equals: offsetTop of #inline-1 should be 0. expected 0 but got -1
      

    • FAIL [expected PASS] subtest: offsetLeft

      assert_equals: offsetLeft of #inline-2 should be 40. expected 40 but got 25
      

  • OK /_mozilla/webxr/create_session.https.html
    • FAIL [expected PASS] subtest: create_session

      can't access property "simulateDeviceConnection", navigator.xr.test is undefined
      

  • OK /_mozilla/webxr/obtain_frame.https.html
    • FAIL [expected PASS] subtest: obtain_frame

      promise_test: Unhandled rejection with value: object "TypeError: can't access property "simulateDeviceConnection", navigator.xr.test is undefined"
      

  • ERROR [expected TIMEOUT] /_mozilla/webxr/sessionavailable.https.html
  • FAIL [expected PASS] /css/css-backgrounds/background-size-042.html
  • FAIL [expected PASS] /css/css-backgrounds/border-image-repeat-space-9.html
  • OK /css/css-fonts/generic-family-keywords-002.html (#40929)
    • PASS [expected FAIL] subtest: font-family: -webkit-serif treated as <font-family>, not <generic-name>
    • PASS [expected FAIL] subtest: font-family: -webkit-sans-serif treated as <font-family>, not <generic-name>
    • PASS [expected FAIL] subtest: font-family: -webkit-cursive treated as <font-family>, not <generic-name>
    • PASS [expected FAIL] subtest: font-family: -webkit-fantasy treated as <font-family>, not <generic-name>
    • PASS [expected FAIL] subtest: font-family: -webkit-monospace treated as <font-family>, not <generic-name>
    • PASS [expected FAIL] subtest: font-family: -webkit-system-ui treated as <font-family>, not <generic-name>
    • PASS [expected FAIL] subtest: font-family: -webkit-math treated as <font-family>, not <generic-name>
    • FAIL [expected PASS] subtest: font-family: -webkit-generic(fangsong) treated as <font-family>, not <generic-name>

      assert_equals: expected 50 but got 30
      

    • FAIL [expected PASS] subtest: font-family: -webkit-generic(kai) treated as <font-family>, not <generic-name>

      assert_equals: expected 50 but got 30
      

    • FAIL [expected PASS] subtest: font-family: -webkit-generic(khmer-mul) treated as <font-family>, not <generic-name>

      assert_equals: expected 50 but got 30
      

    • And 12 more unexpected results...
  • OK /custom-elements/form-associated/ElementInternals-setFormValue.html (#29174)
    • PASS [expected FAIL] subtest: Single value - empty name exists
    • PASS [expected FAIL] subtest: Single value - Non-empty name exists
  • OK [expected TIMEOUT] /fetch/api/redirect/redirect-keepalive.https.any.html (#32153)
    • PASS [expected TIMEOUT] subtest: [keepalive][iframe][load] mixed content redirect; setting up
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • FAIL [expected PASS] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'

      assert_unreached: load should not be fired Reached unreachable code
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • FAIL [expected PASS] subtest: load event does not fire on window.open('about:blank')

      assert_unreached: load should not be fired Reached unreachable code
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • PASS [expected FAIL] subtest: aElement.click() before the load event must NOT replace
  • OK /html/browsers/history/the-history-interface/traverse_the_history_3.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK /html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/allow-scripts-flag-changing-1.html (#39694)
    • PASS [expected FAIL] subtest: Meta refresh is blocked by the allow-scripts sandbox flag at its creation time, not when refresh comes due
  • CRASH [expected OK] /html/semantics/forms/form-submission-0/form-submit-iframe-then-location-navigate.html (#29634)
  • OK /html/semantics/forms/form-submission-0/jsurl-form-submit.tentative.html (#36489)
    • PASS [expected FAIL] subtest: Verifies that form submissions scheduled inside javascript: urls take precedence over the javascript: url's return value.
  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • FAIL [expected PASS] subtest: multipart/form-data: Basic test (formdata event)

      assert_equals: expected "\r\nContent-Disposition: form-data; name=\"basic\"\r\n\r\ntest\r\n--\r\n" but got ""
      

    • PASS [expected FAIL] subtest: multipart/form-data: 0x00 in value (formdata event)
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • FAIL [expected PASS] subtest: text/plain: Basic test (formdata event)

      assert_equals: expected "basic=test\r\n" but got ""
      

    • PASS [expected FAIL] subtest: text/plain: 0x00 in value (formdata event)
  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • FAIL [expected PASS] subtest: application/x-www-form-urlencoded: Basic File test (normal form)

      assert_equals: expected "basic=file-test.txt" but got ""
      

    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: \r\n in name (normal form)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: \r\n in value (formdata event)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: double quote in value (formdata event)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: non-ASCII in filename (normal form)
  • TIMEOUT [expected ERROR] /html/semantics/links/links-created-by-a-and-area-elements/target_blank_implicit_noopener_base.html (#40347)
  • OK /html/semantics/scripting-1/the-script-element/module/dynamic-import/blob-url.any.html (#33948)
    • FAIL [expected PASS] subtest: Revoking a blob URL immediately after calling import will not fail

      promise_test: Unhandled rejection with value: object "TypeError: Dynamic import failed"
      

  • OK [expected ERROR] /html/user-activation/no-activation-thru-escape-key.html (#40343)
  • OK /resource-timing/buffer-full-add-then-clear.html (#40819)
    • PASS [expected FAIL] subtest: Test that if the buffer is cleared after entries were added to the secondary buffer, those entries make it into the primary one
  • CRASH [expected OK] /trusted-types/get-trusted-types-compliant-attribute-value.html
  • CRASH [expected OK] /trusted-types/set-event-handlers-content-attributes.tentative.html
  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?01-05 (#38975)
    • PASS [expected TIMEOUT] subtest: Navigate a window via anchor with javascript:-urls in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ default policy in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via anchor with javascript:-urls in enforcing mode.
  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?06-10 (#37920)
    • PASS [expected TIMEOUT] subtest: Navigate a frame via anchor with javascript:-urls w/ default policy in report-only mode.
    • FAIL [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in enforcing mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
      

    • FAIL [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in report-only mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""
      

  • TIMEOUT /trusted-types/trusted-types-navigation.html?26-30 (#38807)
    • PASS [expected TIMEOUT] subtest: Navigate a window via form-submission with javascript:-urls in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a window via form-submission with javascript:-urls w/ default policy in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls in enforcing mode.
    • TIMEOUT [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in enforcing mode.

      Test timed out
      

  • TIMEOUT /trusted-types/trusted-types-navigation.html?31-35 (#38034)
    • TIMEOUT [expected PASS] subtest: Navigate a frame via form-submission with javascript:-urls in report-only mode.

      Test timed out
      

    • NOTRUN [expected TIMEOUT] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in report-only mode.
  • CRASH [expected TIMEOUT] /wasm/webapi/empty-body.any.worker.html
Stable unexpected results that are known to be intermittent (18)
  • OK /IndexedDB/idbobjectstore_getAll.any.html (#39276)
    • PASS [expected FAIL] subtest: Get all values with transaction.commit()
  • OK /IndexedDB/idbobjectstore_getAll.any.worker.html (#39400)
    • PASS [expected FAIL] subtest: Get all values with transaction.commit()
  • FAIL [expected PASS] /_mozilla/mozilla/sslfail.html (#10760)
  • TIMEOUT [expected OK] /_mozilla/mozilla/window_resize_event.html (#36741)
    • TIMEOUT [expected PASS] subtest: Popup onresize event fires after resizeTo

      Test timed out
      

  • OK /css/css-cascade/layer-font-face-override.html (#35935)
    • FAIL [expected PASS] subtest: @font-face override update with appended sheet 2

      assert_equals: expected "80px" but got "41.45px"
      

  • OK /css/css-fonts/generic-family-keywords-001.html (#37467)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(kai)
    • FAIL [expected PASS] subtest: @font-face matching for quoted and unquoted generic(khmer-mul)

      assert_equals: quoted generic(khmer-mul) matches  @font-face rule expected 50 but got 30
      

  • OK /fetch/content-length/api-and-duplicate-headers.any.html (#35873)
    • FAIL [expected PASS] subtest: fetch() and duplicate Content-Length/Content-Type headers

      promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
      

  • OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
    • PASS [expected FAIL] subtest: sec-fetch-user
    • FAIL [expected PASS] subtest: sec-fetch-storage-access - Cross-site

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

  • OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
    • FAIL [expected PASS] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-origin destination

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy cross-site destination
  • OK /fetch/metadata/generated/element-img-environment-change.https.sub.html (#30111)
    • FAIL [expected PASS] subtest: sec-fetch-site - Cross-site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • FAIL [expected PASS] subtest: sec-fetch-site - Same-Origin -> Same-Site -> Same-Origin redirect, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • PASS [expected FAIL] subtest: sec-fetch-site - Cross-Site -> Same Origin, no attributes
    • FAIL [expected PASS] subtest: sec-fetch-site - Cross-Site -> Same-Site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • PASS [expected FAIL] subtest: sec-fetch-site - Cross-Site -> Cross-Site, no attributes
    • FAIL [expected PASS] subtest: sec-fetch-site - Same-Origin -> Same Origin, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • PASS [expected FAIL] subtest: sec-fetch-site - Same-Origin -> Same-Site, no attributes
    • PASS [expected FAIL] subtest: sec-fetch-site - Same-Site -> Cross-Site, no attributes
    • PASS [expected FAIL] subtest: sec-fetch-mode - attributes: crossorigin
    • FAIL [expected PASS] subtest: sec-fetch-mode - attributes: crossorigin=anonymous

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • And 3 more unexpected results...
  • OK /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • PASS [expected FAIL] subtest: sec-fetch-site - Not sent to non-trustworthy same-site destination, no attributes
    • PASS [expected FAIL] subtest: sec-fetch-site - Not sent to non-trustworthy cross-site destination, no attributes
    • FAIL [expected PASS] subtest: sec-fetch-mode - Not sent to non-trustworthy same-origin destination, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • PASS [expected FAIL] subtest: sec-fetch-mode - Not sent to non-trustworthy same-site destination, no attributes
    • PASS [expected FAIL] subtest: sec-fetch-mode - Not sent to non-trustworthy cross-site destination, no attributes
    • PASS [expected FAIL] subtest: sec-fetch-dest - Not sent to non-trustworthy cross-site destination, no attributes
  • ERROR /fetch/metadata/generated/serviceworker.https.sub.html (#36247)
    • FAIL [expected PASS] subtest: sec-fetch-site - Same origin, no options - registration

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

  • ERROR [expected OK] /focus/focus-event-after-switching-iframes.sub.html (#40368)
  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • PASS [expected NOTRUN] subtest: Non-HTMLElement should not support autofocus
    • TIMEOUT [expected FAIL] subtest: Host element with delegatesFocus should support autofocus

      Test timed out
      

    • NOTRUN [expected FAIL] subtest: Host element with delegatesFocus including no focusable descendants should be skipped
    • NOTRUN [expected FAIL] subtest: Area element should support autofocus
  • OK /navigation-timing/test-navigation-type-reload.html (#33334)
    • PASS [expected FAIL] subtest: Reload domComplete > Original domComplete
    • PASS [expected FAIL] subtest: Reload domContentLoadedEventStart > Original domContentLoadedEventStart
    • PASS [expected FAIL] subtest: Reload domInteractive > Original domInteractive
    • PASS [expected FAIL] subtest: Reload fetchStart > Original fetchStart
    • PASS [expected FAIL] subtest: Reload loadEventEnd > Original loadEventEnd
    • PASS [expected FAIL] subtest: Reload loadEventStart > Original loadEventStart
  • OK /preload/preload-error.sub.html (#37177)
    • PASS [expected FAIL] subtest: 404 (fetch): main
  • TIMEOUT [expected OK] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.html (#29053)
    • TIMEOUT [expected PASS] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe

      Test timed out
      

  • OK [expected ERROR] /webxr/render_state_update.https.html (#27535)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2025

✨ Try run (#20070885797) succeeded.

@Loirooriol Loirooriol force-pushed the intrinsic-contributions branch from fc1aa02 to 3d3475b Compare December 10, 2025 00:57
@Loirooriol
Copy link
Copy Markdown
Contributor Author

Example:

<!DOCTYPE html>
<style>:root { width: max-content; border: solid; }</style>
<section>
  <main style="width: 100px; background: cyan">
    <article>
      <div id="target" style="height: 100px"></div>
    </article>
  </main>
</section>
<script>requestAnimationFrame(() => target.style.width = "200px");</script>

We will no longer clear the cached intrinsic size for <section>, <body> and <html>.
We will still clear it for <div>, <article> and <main>.

@Loirooriol Loirooriol marked this pull request as ready for review December 10, 2025 11:16
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 10, 2025
@Loirooriol Loirooriol force-pushed the intrinsic-contributions branch from 3d3475b to 0c6fc1b Compare December 10, 2025 14:43
Copy link
Copy Markdown
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but the new logic is pretty hairy so I suggested some comments. Feel free to make them clearer / more correct as you like.

let outer_inline_content_sizes_depend_on_content = Cell::new(false);
node.with_each_layout_box_base_including_pseudos(|base| {
base.clear_fragments();
if original_element_damage.contains(RestyleDamage::RELAYOUT) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if original_element_damage.contains(RestyleDamage::RELAYOUT) {
// If the node itself has damage, we must clear both cached inline sizes and also
// any cached layout results.
if original_element_damage.contains(RestyleDamage::RELAYOUT) {

*base.cached_layout_result.borrow_mut() = None;
*base.cached_inline_content_size.borrow_mut() = None;
} else if damage_from_children.contains(RestyleDamage::RELAYOUT) {
*base.cached_layout_result.borrow_mut() = None;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*base.cached_layout_result.borrow_mut() = None;
// If any children had layout damage, we never reuse the cached layout result
// as the descendants will need to change.
*base.cached_layout_result.borrow_mut() = None;

*base.cached_inline_content_size.borrow_mut() = None;
} else if damage_from_children.contains(RestyleDamage::RELAYOUT) {
*base.cached_layout_result.borrow_mut() = None;
if !damage_from_children.contains(LayoutDamage::recompute_inline_content_sizes()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !damage_from_children.contains(LayoutDamage::recompute_inline_content_sizes()) {
// This can happen when a descendent doesn't have layout damage and doesn't depend
// on content size -- for instance when it is has a fixed inline size.
if !damage_from_children.contains(LayoutDamage::recompute_inline_content_sizes()) {

}
*base.cached_inline_content_size.borrow_mut() = None;
}
if !base.base_fragment_info.is_anonymous() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably also use an explanatory comment.

Comment on lines +194 to +196
if outer_inline_content_sizes_depend_on_content.get() {
damage_for_parent.insert(LayoutDamage::recompute_inline_content_sizes())
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if outer_inline_content_sizes_depend_on_content.get() {
damage_for_parent.insert(LayoutDamage::recompute_inline_content_sizes())
}
// Only clear the cached inline content sizes on ancestors, when this node's inline content
// sizes actually depend on content, otherwise the ancestors don't on the descendant's
// content either.
if outer_inline_content_sizes_depend_on_content.get() {
damage_for_parent.insert(LayoutDamage::recompute_inline_content_sizes())
}

/// Recollect the box children for this element, because some of the them will be
/// rebuilt.
const RECOLLECT_BOX_TREE_CHILDREN = 0b0111_1111_1111 << 4;
/// Clear the cached inline content sizes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Clear the cached inline content sizes.
/// Clear the cached inline content sizes and recompute them during the next layout.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 10, 2025
If a box has layout damage, but the intrinsic contribution of some
ancestor doesn't depend on its contents, then we don't need to clear
the cached intrinsic sizes of further ancestors.

Signed-off-by: Oriol Brufau <[email protected]>
Co-authored-by: Martin Robinson <[email protected]>
@Loirooriol Loirooriol force-pushed the intrinsic-contributions branch from 0c6fc1b to 4300a5d Compare December 11, 2025 00:24
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 11, 2025
@Loirooriol Loirooriol enabled auto-merge December 11, 2025 00:25
@Loirooriol Loirooriol added this pull request to the merge queue Dec 11, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 11, 2025
Merged via the queue into servo:main with commit b285923 Dec 11, 2025
32 checks passed
@Loirooriol Loirooriol deleted the intrinsic-contributions branch December 11, 2025 01:24
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants