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

Move Page::renderingUpdateCompleted call to outside loop in Page::finalizeRenderingUpdate #14183

Conversation

achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented May 22, 2023

8730f01

Move Page::renderingUpdateCompleted call to outside loop in Page::finalizeRenderingUpdate
https://bugs.webkit.org/show_bug.cgi?id=257127
rdar://109664043

Reviewed by Tim Horton.

This does not change behavior with site isolation off because a page will only ever have
one root frame, which is always the main frame.  A root frame is a local frame that does
not have a local parent, which means its layer is a root drawing layer in this process
that will be composited in the UI process.

This change is needed as a step towards drawing multiple iframes in the same process,
in which case a Page would have multiple root frames.  It only needs to call renderingUpdateCompleted
once and if it doesn't then assertions and crashes happen.

This change is also needed to fix some crashes from #14025
but this change is completely unrelated so I'm doing it in a separate PR.

* Source/WebCore/page/Page.cpp:
(WebCore::Page::finalizeRenderingUpdate):
(WebCore::Page::finalizeRenderingUpdateForRootFrame):

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

69d66f3

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt   πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  watch
❌ πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim

@achristensen07 achristensen07 self-assigned this May 22, 2023
@achristensen07 achristensen07 added the WebKit Process Model Bugs related to WebKit's multi-process architecture label May 22, 2023
@achristensen07 achristensen07 added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 22, 2023
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Move-PagerenderingUpdateCompleted-call-to-outside-loop-in-PagefinalizeRenderingUpdate branch from 69d66f3 to 5d3c118 Compare May 22, 2023 17:32
…alizeRenderingUpdate

https://bugs.webkit.org/show_bug.cgi?id=257127
rdar://109664043

Reviewed by Tim Horton.

This does not change behavior with site isolation off because a page will only ever have
one root frame, which is always the main frame.  A root frame is a local frame that does
not have a local parent, which means its layer is a root drawing layer in this process
that will be composited in the UI process.

This change is needed as a step towards drawing multiple iframes in the same process,
in which case a Page would have multiple root frames.  It only needs to call renderingUpdateCompleted
once and if it doesn't then assertions and crashes happen.

This change is also needed to fix some crashes from WebKit#14025
but this change is completely unrelated so I'm doing it in a separate PR.

* Source/WebCore/page/Page.cpp:
(WebCore::Page::finalizeRenderingUpdate):
(WebCore::Page::finalizeRenderingUpdateForRootFrame):

Canonical link: https://commits.webkit.org/264330@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Move-PagerenderingUpdateCompleted-call-to-outside-loop-in-PagefinalizeRenderingUpdate branch from 5d3c118 to 8730f01 Compare May 22, 2023 17:35
@webkit-commit-queue
Copy link
Collaborator

Committed 264330@main (8730f01): https://commits.webkit.org/264330@main

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

@webkit-commit-queue webkit-commit-queue merged commit 8730f01 into WebKit:main May 22, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Process Model Bugs related to WebKit's multi-process architecture
Projects
None yet
4 participants