Skip to content

Conversation

@smfr
Copy link
Contributor

@smfr smfr commented Dec 22, 2022

bf82da4

Add a "post rendering frame" hook
https://bugs.webkit.org/show_bug.cgi?id=249807
rdar://103648840

Reviewed by Chris Dumez and Ryosuke Niwa.

We need to add a Page-level "post rendering frame" hook so we can do things like hook up
postRequestAnimationFrame() (webkit.org/b/249798), possibly run the WindowEventLoop
(webkit.org/b/249684) and correctly implement Inspectors "frame end" event (webkit.org/b/249796).

For Core Animation-driven drawing clients (TiledCoreAnimationDrawingArea, WebKitLegacy) we want this
post-rendering frame hook to run after CA's runloop observer returns, since this callback may run
arbitrary script at some point and we need to avoid any possible re-entrancy issues. So we use a
second CFRunLoopObserver, ordered as CoreAnimationCommit+2 (after InspectorFrameEnd).

For RemoteLayerTreeDrawingArea, we can just call the entry point after
didCompleteRenderingUpdateDisplay().

* Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp:
(WebCore::InspectorTimelineAgent::internalStart):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::didCompleteRenderingFrame):
* Source/WebCore/page/Page.h:
* Source/WebCore/platform/cf/RunLoopObserver.cpp:
(WebCore::RunLoopObserver::schedule):
* Source/WebCore/platform/cf/RunLoopObserver.h: Modernize terminology: LayerFlush -> RenderingUpdate and add PostRenderingUpdate.
* Source/WebKit/WebProcess/WebPage/DrawingArea.cpp:
(WebKit::DrawingArea::didCompleteRenderingFrame):
* Source/WebKit/WebProcess/WebPage/DrawingArea.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::updateRendering):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didCompleteRenderingFrame):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea):
(WebKit::TiledCoreAnimationDrawingArea::~TiledCoreAnimationDrawingArea):
(WebKit::TiledCoreAnimationDrawingArea::setLayerTreeStateIsFrozen):
(WebKit::TiledCoreAnimationDrawingArea::dispatchAfterEnsuringUpdatedScrollPosition):
(WebKit::TiledCoreAnimationDrawingArea::didCompleteRenderingUpdateDisplay):
(WebKit::TiledCoreAnimationDrawingArea::scheduleRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::invalidateRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::renderingUpdateRunLoopCallback):
(WebKit::TiledCoreAnimationDrawingArea::schedulePostRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::invalidatePostRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::postRenderingUpdateRunLoopCallback):
(WebKit::TiledCoreAnimationDrawingArea::updateRenderingRunLoopCallback): Deleted.
* Source/WebKitLegacy/mac/WebView/WebView.mm:
(-[WebView _didCompleteRenderingUpdateDisplay]):
* Source/WebKitLegacy/mac/WebView/WebViewData.mm:
(WebViewLayerFlushScheduler::WebViewLayerFlushScheduler):

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

118d365

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

@smfr smfr self-assigned this Dec 22, 2022
@smfr smfr added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Dec 22, 2022
@smfr smfr force-pushed the eng/Add-a-post-rendering-frame-hook branch from e5a810d to 7203718 Compare December 22, 2022 22:33
@smfr smfr requested a review from rniwa December 23, 2022 00:34
Copy link
Member

Choose a reason for hiding this comment

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

Can we add an assertion that this is back to the main/web thread?

Copy link
Member

Choose a reason for hiding this comment

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

It would be ideal to all these updating per WindowEventLoop object. e.g. cross-site contents in a single Page may not share EventLoop object (this is especially true once site isolation is implemented).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, but I'd rather do that later. It should be pretty clean; the Page updateRendering code is in a good state.

Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We don't need to specify the whole type nowadays:

RefPtr protectedFlushController { m_flushController };

@smfr smfr force-pushed the eng/Add-a-post-rendering-frame-hook branch from 7203718 to 118d365 Compare December 23, 2022 18:27
@smfr smfr added the merge-queue Applied to send a pull request to merge-queue label Dec 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=249807
rdar://103648840

Reviewed by Chris Dumez and Ryosuke Niwa.

We need to add a Page-level "post rendering frame" hook so we can do things like hook up
postRequestAnimationFrame() (webkit.org/b/249798), possibly run the WindowEventLoop
(webkit.org/b/249684) and correctly implement Inspectors "frame end" event (webkit.org/b/249796).

For Core Animation-driven drawing clients (TiledCoreAnimationDrawingArea, WebKitLegacy) we want this
post-rendering frame hook to run after CA's runloop observer returns, since this callback may run
arbitrary script at some point and we need to avoid any possible re-entrancy issues. So we use a
second CFRunLoopObserver, ordered as CoreAnimationCommit+2 (after InspectorFrameEnd).

For RemoteLayerTreeDrawingArea, we can just call the entry point after
didCompleteRenderingUpdateDisplay().

* Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp:
(WebCore::InspectorTimelineAgent::internalStart):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::didCompleteRenderingFrame):
* Source/WebCore/page/Page.h:
* Source/WebCore/platform/cf/RunLoopObserver.cpp:
(WebCore::RunLoopObserver::schedule):
* Source/WebCore/platform/cf/RunLoopObserver.h: Modernize terminology: LayerFlush -> RenderingUpdate and add PostRenderingUpdate.
* Source/WebKit/WebProcess/WebPage/DrawingArea.cpp:
(WebKit::DrawingArea::didCompleteRenderingFrame):
* Source/WebKit/WebProcess/WebPage/DrawingArea.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::updateRendering):
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didCompleteRenderingFrame):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea):
(WebKit::TiledCoreAnimationDrawingArea::~TiledCoreAnimationDrawingArea):
(WebKit::TiledCoreAnimationDrawingArea::setLayerTreeStateIsFrozen):
(WebKit::TiledCoreAnimationDrawingArea::dispatchAfterEnsuringUpdatedScrollPosition):
(WebKit::TiledCoreAnimationDrawingArea::didCompleteRenderingUpdateDisplay):
(WebKit::TiledCoreAnimationDrawingArea::scheduleRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::invalidateRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::renderingUpdateRunLoopCallback):
(WebKit::TiledCoreAnimationDrawingArea::schedulePostRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::invalidatePostRenderingUpdateRunLoopObserver):
(WebKit::TiledCoreAnimationDrawingArea::postRenderingUpdateRunLoopCallback):
(WebKit::TiledCoreAnimationDrawingArea::updateRenderingRunLoopCallback): Deleted.
* Source/WebKitLegacy/mac/WebView/WebView.mm:
(-[WebView _didCompleteRenderingUpdateDisplay]):
* Source/WebKitLegacy/mac/WebView/WebViewData.mm:
(WebViewLayerFlushScheduler::WebViewLayerFlushScheduler):

Canonical link: https://commits.webkit.org/258311@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Add-a-post-rendering-frame-hook branch from 118d365 to bf82da4 Compare December 23, 2022 23:58
@webkit-commit-queue
Copy link
Collaborator

Committed 258311@main (bf82da4): https://commits.webkit.org/258311@main

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

@webkit-commit-queue webkit-commit-queue merged commit bf82da4 into WebKit:main Dec 23, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 23, 2022
@smfr smfr deleted the eng/Add-a-post-rendering-frame-hook branch January 9, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants