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.

Sign up for GitHub

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

Jump to bottom

This ensures that the viewport anchor layer will be updated when updating compositing layers upon style change #5658

Merged
merged 1 commit into from Oct 26, 2022
Merged

This ensures that the viewport anchor layer will be updated when updating compositing layers upon style change #5658

merged 1 commit into from Oct 26, 2022

Conversation

arunsundarapple
Copy link
Contributor

@arunsundarapple arunsundarapple commented Oct 21, 2022 •
edited by webkit-early-warning-system

67c0ccb

This ensures that the viewport anchor layer will be updated when updating compositing layers upon style change
https://bugs.webkit.org/show_bug.cgi?id=246890
rdar://99568474

Reviewed by Simon Fraser.

* LayoutTests/compositing/scrolling/fixed-style-with-rotation.html: Added.
* LayoutTests/compositing/scrolling/sticky-style-with-rotation.html: Added.
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::layerStyleChanged):

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

d964e19

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
✅ 🧪 api-ios   🧪 api-mac ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk1
✅ 🛠 tv-sim ✅ 🧪 mac-wk2
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@arunsundarapple arunsundarapple changed the title This ensures that the viewport anchor layer will be updated when updating compositing layers upon style change Fix assertion failure due to viewport anchor layer not being updated when updating style Oct 21, 2022
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 8a3be4d)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ❌ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 api-mac ✅ 🧪 api-gtk
✅ 🛠 tv ❌ 🧪 mac-wk1
✅ 🛠 tv-sim ❌ 🧪 mac-wk2
✅ 🛠 watch ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

smfr
smfr reviewed Oct 21, 2022
View reviewed changes
LayoutTests/compositing/scrolling/fixed-style-with-rotation.html Outdated
<script>
onload = () => {
document.body.offsetTop;
document.body.style.position = 'sticky';
Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

LayoutTests/compositing/scrolling/fixed-style-with-rotation.html
@@ -0,0 +1,16 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to have these tests run as "text" tests. Add:

if (window.testRunner)
  testRunner.dumpAsText();

and you also need to include the resulting .txt files in your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference, unless a test case requires invalid HTML to reproduce a bug, it's usually best to structure documents like this:

<!DOCTYPE html>
<html>
<head>
<script>[as needed]</script>
<style>[as needed]</style>
</head>
<body>
[content goes here, but can also include script tags]
</body>
</html>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you ! I will keep this in mind.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 22, 2022
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash d964e19)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
✅ 🧪 api-ios   🧪 api-mac ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk1
✅ 🛠 tv-sim ✅ 🧪 mac-wk2
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@smfr smfr added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Oct 26, 2022
@arunsundarapple @smfr
This ensures that the viewport anchor layer will be updated when upda…
67c0ccb 
…ting compositing layers upon style change

https://bugs.webkit.org/show_bug.cgi?id=246890
rdar://99568474

Reviewed by Simon Fraser.

* LayoutTests/compositing/scrolling/fixed-style-with-rotation.html: Added.
* LayoutTests/compositing/scrolling/sticky-style-with-rotation.html: Added.
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::layerStyleChanged):

Canonical link: https://commits.webkit.org/256025@main
@webkit-commit-queue webkit-commit-queue changed the title Fix assertion failure due to viewport anchor layer not being updated when updating style This ensures that the viewport anchor layer will be updated when updating compositing layers upon style change Oct 26, 2022
@webkit-commit-queue
Copy link
Collaborator

Committed 256025@main (67c0ccb): https://commits.webkit.org/256025@main

Reviewed commits have been landed. Closing PR #5658 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 67c0ccb into WebKit:main Oct 26, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddkilzer ddkilzer ddkilzer left review comments

@smfr smfr smfr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@arunsundarapple @webkit-early-warning-system @webkit-commit-queue @smfr @ddkilzer @webkit-ews-buildbot