-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
markAllBackingStoreVolatileFromTimer can stop the volatility timer too soon and not mark all surfaces as volatile. #20014
markAllBackingStoreVolatileFromTimer can stop the volatility timer too soon and not mark all surfaces as volatile. #20014
Conversation
EWS run on current version of this PR (hash ee92e02)
|
LOG_WITH_STREAM(RemoteLayerBuffers, stream << "RemoteLayerWithRemoteRenderingBackingStoreCollection::markBackingStoreVolatileAfterReachabilityChange - succeeded " << succeeded); | ||
if (!succeeded && weakThis) | ||
weakThis->scheduleVolatilityTimer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some risk that we never succeed and keep scheduling this timer forever?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that risk is existing, the volatility timer is repeating, and it's only the successful marking of all surfaces that explicitly stops it.
Commit message contains (OOPS!) and no reviewer found, blocking PR #20014 |
…o soon and not mark all surfaces as volatile. https://bugs.webkit.org/show_bug.cgi?id=264229 <rdar://116711724> Reviewed by Simon Fraser. RemoteLayerWithRemoteRenderingBackingStoreCollection::markAllBackingStoreVolatileFromTimer stops the volatility timer if the operation is successful, since there should be no further backing stores to mark volatile. This is an async operation though, and it's possible for markBackingStoreVolatileAfterReachabilityChange to have been called in the interim and there are now new backing stores that aren't yet volatile. This change ensures that we reschedule the volatility timer if any markBackingStoreVolatileAfterReachabilityChange don't succesfully mark the relevant surfaces as volatile. * Source/WebKit/Shared/RemoteLayerTree/RemoteLayerWithRemoteRenderingBackingStoreCollection.mm: (WebKit::RemoteLayerWithRemoteRenderingBackingStoreCollection::markBackingStoreVolatileAfterReachabilityChange): Canonical link: https://commits.webkit.org/270486@main
ee92e02
to
a6666b9
Compare
Committed 270486@main (a6666b9): https://commits.webkit.org/270486@main Reviewed commits have been landed. Closing PR #20014 and removing active labels. |
a6666b9
ee92e02