Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor inline layout nesting #30289

Merged
merged 1 commit into from Sep 5, 2023

Conversation

mrobinson
Copy link
Member

This makes the nesting architecture of inline layout a lot simpler. Now
PartialInlineBoxFragment and InlineNestingLevelState are replaced by
two structs:

  • InlineContainerState
  • InlineBoxContainerState

InlineContainerState holds state for the root of the inline formatting
context, while InlineBoxContainerState holds state for inline boxes.
InlineBoxContainerState has an InlineContainerState as the first
element, thus "extends" it.

Now the inline iterators are stack variables directly in the layout()
method. They are no longer stored in the state. This avoids the weird
nesting of state and instead relies on a normal vector to hold the
stack of state.

Co-authored-by: Mukilan Thiyagarajan mukilan@igalia.com


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because they should not change behavior.

@mrobinson
Copy link
Member Author

@bors-servo try=wpt-2020

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

🔨 Triggering try run (#6073243247) with platform=linux and layout=2020

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

Test results for linux-wpt-layout-2020 from try job (#6073243247):

Flaky unexpected result (20)
  • FAIL [expected PASS] /_mozilla/css/dirty_viewport.html (#13731)
  • OK /_mozilla/mozilla/task_queue_throttling.any.html (#22519)
    • FAIL [expected PASS] subtest: Throttling the performance timeline task queue. assert_true: expected true got false
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected PASS] /css/CSS2/css1/c534-bgreps-002.xht
  • TIMEOUT [expected PASS] /css/css-color/animation/opacity-animation-ending-correctly-002.html (#29216)
  • TIMEOUT [expected PASS] /css/css-text/white-space/control-chars-010.html (#24297)
  • TIMEOUT [expected OK] /dom/events/webkit-transition-end-event.html (#26497)
    • TIMEOUT [expected FAIL] subtest: webkitTransitionEnd event listener should trigger for an animation Test timed out
    • NOTRUN [expected PASS] subtest: webkitTransitionEnd event listener should not trigger if an unprefixed listener also exists
    • NOTRUN [expected PASS] subtest: webkitTransitionEnd event listener should not trigger if an unprefixed event handler also exists
    • NOTRUN [expected FAIL] subtest: event types for prefixed and unprefixed transitionend event listeners should be named appropriately
    • NOTRUN [expected PASS] subtest: webkitTransitionEnd event listener is case sensitive
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin.window.html (#29049)
    • FAIL [expected PASS] subtest: Same-origin navigation started from unload handler must be ignored assert_equals: expected "?pass" but got "?fail"
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/overlapping-navigations-and-traversals/nav-cancelation-2.sub.html (#29738)
    • TIMEOUT [expected FAIL] subtest: grandparent cancels a pending navigation in a cross-origin grandchild Test timed out
  • OK /html/browsers/browsing-the-web/read-media/pageload-image-in-popup.html (#23849)
    • PASS [expected FAIL] subtest: The document for a standalone media file should have one child in the body.
  • OK /html/browsers/history/the-history-interface/traverse_the_history_2.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-valid.html (#28259)
    • TIMEOUT [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with URL fragments should be skipped. Test timed out
  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • FAIL [expected TIMEOUT] subtest: Area element should support autofocus promise_test: Unhandled rejection with value: object "TypeError: w.document.querySelector(...) is null"
  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • PASS [expected FAIL] subtest: multipart/form-data: single quote in value (normal form)
  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: characters not in encoding in name and value (formdata event)
  • OK /html/webappapis/animation-frames/callback-cross-realm-report-exception.html
    • FAIL [expected PASS] subtest: requestAnimationFrame() reports the exception from its callback in the callback's global object assert_array_equals: lengths differ, expected array ["frame1"] length 1, got [] length 0
  • ERROR /resource-timing/content-type-parsing.html (#29131)
    • FAIL [expected TIMEOUT] subtest: mime-type 16 : text/html;charset=�gbk assert_equals: expected (string) "text/html" but got (undefined) undefined
    • TIMEOUT [expected NOTRUN] subtest: mime-type 17 : text/html;charset= gbk Test timed out
  • TIMEOUT [expected OK] /webmessaging/with-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank Test timed out
  • ERROR [expected OK] /workers/semantics/run-a-worker/003.html (#22765)
Stable unexpected results that are known to be intermittent (24)
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • FAIL [expected PASS] subtest: Matching font-weight: '430' should prefer '400 425' over '350 399' assert_equals: Unexpected font on test element expected 487 but got 532
    • FAIL [expected PASS] subtest: Matching font-weight: '500' should prefer '500' over '450 460' assert_equals: Unexpected font on test element expected 487 but got 532
    • PASS [expected FAIL] subtest: Matching font-weight: '500' should prefer '400' over '350 399'
    • PASS [expected FAIL] subtest: Matching font-weight: '500' should prefer '501 550' over '502 560'
    • FAIL [expected PASS] subtest: Matching font-weight: '501' should prefer '450 460' over '390 410' assert_equals: Unexpected font on test element expected 487 but got 532
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '200 300' over '400'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '400' over '450 460'
    • PASS [expected FAIL] subtest: Matching font-stretch: '110%' should prefer '50% 80%' over '60% 70%'
    • PASS [expected FAIL] subtest: Matching font-stretch: '90%' should prefer '50% 80%' over '60% 70%'
    • PASS [expected FAIL] subtest: Matching font-stretch: '90%' should prefer '110% 140%' over '120% 130%'
    • And 16 more unexpected results...
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • PASS [expected FAIL] subtest: sec-fetch-site - Not sent to non-trustworthy same-origin destination, no attributes
  • TIMEOUT /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src=''
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-load-as-html.xhtml (#29070)
    • TIMEOUT [expected PASS] subtest: javascript: URL navigation to a string must create a HTML document using the correct properties Test timed out
  • OK /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html (#28066)
    • FAIL [expected PASS] subtest: D83D DE0D set in href="" targeting a frame and clicked assert_equals: expected "😍" but got ""
    • FAIL [expected PASS] subtest: DE0D 0041 set in href="" targeting a frame and clicked assert_equals: expected "\ufffdA" but got ""
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
    • FAIL [expected PASS] subtest: Cross-origin navigation started from unload handler must be ignored promise_test: Unhandled rejection with value: object "SecurityError: The operation is insecure."
  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
  • OK /html/browsers/history/the-location-interface/location-protocol-setter.html (#20839)
    • PASS [expected FAIL] subtest: Equivalent tests for data URL and srcdoc <iframe>s
  • OK /html/browsers/the-window-object/open-close/creating_browsing_context_test_01.html (#29046)
  • OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-top.html (#28259)
    • FAIL [expected TIMEOUT] subtest: Autofocus elements in top-level browsing context's documents with "top" fragments should work. assert_not_equals: got disallowed value Element node <body></body>
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK /html/semantics/embedded-content/the-img-element/non-active-document.html (#21544)
    • PASS [expected FAIL] subtest: createHTMLDocument
    • PASS [expected FAIL] subtest: <template>
  • OK /html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-quirks-mode.html (#21666)
  • OK /html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-standards-mode.html (#21666)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e38 50w, /images/green-16x16.png?e38 51w" sizes="(min-width:calc(0)) 1px"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e45 50w, /images/green-16x16.png?e45 51w" sizes="(min-width:-1px) 1px, 100vw"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e58 50w, /images/green-16x16.png?e58 51w" sizes="(min-width:0) or (unknown-mf-name) 1px"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e59 50w, /images/green-16x16.png?e59 51w" sizes="(min-width:0) or (min-width:unknown-mf-value) 1px"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e60 50w, /images/green-16x16.png?e60 51w" sizes="(min-width:0) or (min-width:-1px) 1px"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e61 50w, /images/green-16x16.png?e61 51w" sizes="(min-width:0) or (unknown &quot;general-enclosed&quot;) 1px"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e62 50w, /images/green-16x16.png?e62 51w" sizes="(min-width:0) or unknown-general-enclosed(foo) 1px"> ref sizes="1px" (standards mode)
    • PASS [expected FAIL] subtest: <img srcset="/images/green-1x1.png?e106 50w, /images/green-16x16.png?e106 51w" sizes="(min-width:0) or (unknown-general-enclosed !) 1px"> ref sizes="1px" (standards mode)
  • OK /html/semantics/forms/form-submission-0/form-submit-iframe-then-location-navigate.html (#29634)
    • PASS [expected FAIL] subtest: Verifies that location navigations take precedence when following form submissions.
  • OK [expected TIMEOUT] /html/semantics/forms/form-submission-0/reparent-form-during-planned-navigation-task.html (#29724)
    • PASS [expected TIMEOUT] subtest: reparent-form-during-planned-navigation-task
  • TIMEOUT /html/webappapis/scripting/events/compile-event-handler-settings-objects.html (#24246)
    • PASS [expected FAIL] subtest: The entry settings object while executing the compiled callback via Web IDL's invoke must be that of the node document
  • OK [expected TIMEOUT] /html/webappapis/scripting/processing-model-2/integration-with-the-javascript-job-queue/promise-job-entry-different-function-realm.html (#25805)
  • PASS [expected CRASH] /streams/readable-streams/crashtests/strategy-worker-terminate.html (#30124)
  • OK [expected TIMEOUT] /webmessaging/without-ports/017.html (#24486)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank
  • TIMEOUT [expected OK] /webmessaging/without-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out

@github-actions
Copy link

github-actions bot commented Sep 4, 2023

✨ Try run (#6073243247) succeeded.

This makes the nesting architecture of inline layout a lot simpler. Now
PartialInlineBoxFragment and InlineNestingLevelState are replaced by
two structs:

 - InlineContainerState
 - InlineBoxContainerState

InlineContainerState holds state for the root of the inline formatting
context, while InlineBoxContainerState holds state for inline boxes.
InlineBoxContainerState has an InlineContainerState as the first
element, thus "extends" it.

Now the inline iterators are stack variables directly in the `layout()`
method. They are no longer stored in the state. This avoids the weird
nesting of state and instead relies on a normal vector to hold the
stack of state.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
@mrobinson
Copy link
Member Author

Thanks for the review, @mukilan. I will land this now.

@mrobinson mrobinson added this pull request to the merge queue Sep 5, 2023
Merged via the queue into servo:master with commit 577baea Sep 5, 2023
10 checks passed
@mrobinson mrobinson deleted the refactor-inline-layout branch September 5, 2023 19:20
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.

None yet

2 participants