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

hideContentUntilPendingUpdate callbacks can be processed too late after the transaction and cause flickering. #20268

Merged
merged 1 commit into from
Nov 13, 2023
Merged

hideContentUntilPendingUpdate callbacks can be processed too late after the transaction and cause flickering. #20268

merged 1 commit into from
Nov 13, 2023

Conversation

mattwoodrow
Copy link
Contributor

@mattwoodrow mattwoodrow commented Nov 9, 2023 •
edited by webkit-early-warning-system
Loading

923ed51

hideContentUntilPendingUpdate callbacks can be processed too late after the transaction and cause flickering.
https://bugs.webkit.org/show_bug.cgi?id=264531
<rdar://118083889>

Reviewed by Tim Horton.

The callbacks for DispatchAfterEnsuringDrawing get processed at the end of the transaction, after we've already told
the client that we've committed the transaction. In some cases, this can cause them to be included as a separate CA
commit, and cause flickering.

This adds a pre-transaction check for a callback for hideContentUntilPendingUpdate, and clears the reply id, so that we
re-attach the root layer as part of the main commit.

* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTreeTransaction):
(WebKit::RemoteLayerTreeDrawingAreaProxy::hideContentUntilPendingUpdate):

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

70747d9

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

@mattwoodrow mattwoodrow self-assigned this Nov 9, 2023
@mattwoodrow mattwoodrow added the Layout and Rendering For bugs with layout and rendering of Web pages. label Nov 9, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 9, 2023 •
edited
Loading

EWS run on current version of this PR (hash 70747d9)

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

hortont424
hortont424 approved these changes Nov 10, 2023
View reviewed changes
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
// set the root node during the same CA transaction.
for (auto& callbackID : layerTreeTransaction.callbackIDs()) {
if (callbackID == m_replyForUnhidingContent) {
m_replyForUnhidingContent = AsyncReplyID { };
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird that clearing something causes something else to be processed (I think the code before 359bfd9 was a bit clearer), maybe we can give it a name that is clear that it's something we're waiting for? (so that "clearing it" is obviously "no longer waiting"?)

@hortont424
Copy link
Contributor

I get how this could fix a transient flash, but my understanding was we also had reports of permanent blankness, and I don't understand how this could fix those?

@mattwoodrow mattwoodrow added the merge-queue Applied to send a pull request to merge-queue label Nov 13, 2023
@mattwoodrow
hideContentUntilPendingUpdate callbacks can be processed too late aft…
923ed51 
…er the transaction and cause flickering.

https://bugs.webkit.org/show_bug.cgi?id=264531
<rdar://118083889>

Reviewed by Tim Horton.

The callbacks for DispatchAfterEnsuringDrawing get processed at the end of the transaction, after we've already told
the client that we've committed the transaction. In some cases, this can cause them to be included as a separate CA
commit, and cause flickering.

This adds a pre-transaction check for a callback for hideContentUntilPendingUpdate, and clears the reply id, so that we
re-attach the root layer as part of the main commit.

* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTreeTransaction):
(WebKit::RemoteLayerTreeDrawingAreaProxy::hideContentUntilPendingUpdate):

Canonical link: https://commits.webkit.org/270672@main
@webkit-commit-queue
Copy link
Collaborator

Committed 270672@main (923ed51): https://commits.webkit.org/270672@main

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

@webkit-commit-queue webkit-commit-queue merged commit 923ed51 into WebKit:main Nov 13, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hortont424 hortont424 hortont424 approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

Assignees

@mattwoodrow mattwoodrow

Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
Milestone
No milestone
4 participants
@mattwoodrow @webkit-early-warning-system @hortont424 @webkit-commit-queue