Commit 317a8e0e authored by alancutter's avatar alancutter Committed by Commit bot

Notify Blink to suppress frame requests during BeginMainFrame

This change fixes a performance regression where main thread frames were
being continuously requested during composited animations.
This is a temporary workaround that tells Blink when BeginMainFrame is
active and it should not be requesting new frames for visual updates.

BUG=704763

Review-Url: https://codereview.chromium.org/2791223002
Cr-Commit-Position: refs/heads/master@{#462022}
parent dffcf95a
......@@ -941,6 +941,7 @@ void RenderWidget::RequestScheduleAnimation() {
void RenderWidget::UpdateVisualState() {
GetWebWidget()->updateAllLifecyclePhases();
GetWebWidget()->setSuppressFrameRequestsWorkaroundFor704763Only(false);
if (time_to_first_active_paint_recorded_)
return;
......@@ -958,6 +959,8 @@ void RenderWidget::UpdateVisualState() {
void RenderWidget::WillBeginCompositorFrame() {
TRACE_EVENT0("gpu", "RenderWidget::willBeginCompositorFrame");
GetWebWidget()->setSuppressFrameRequestsWorkaroundFor704763Only(true);
// The UpdateTextInputState can result in further layout and possibly
// enable GPU acceleration so they need to be called before any painting
// is done.
......
......@@ -73,10 +73,21 @@ void PageAnimator::serviceScriptedAnimations(
}
}
void PageAnimator::setSuppressFrameRequestsWorkaroundFor704763Only(
bool suppressFrameRequests) {
// If we are enabling the suppression and it was already enabled then we must
// have missed disabling it at the end of a previous frame.
DCHECK(!m_suppressFrameRequestsWorkaroundFor704763Only ||
!suppressFrameRequests);
m_suppressFrameRequestsWorkaroundFor704763Only = suppressFrameRequests;
}
DISABLE_CFI_PERF
void PageAnimator::scheduleVisualUpdate(LocalFrame* frame) {
if (m_servicingAnimations || m_updatingLayoutAndStyleForPainting)
if (m_servicingAnimations || m_updatingLayoutAndStyleForPainting ||
m_suppressFrameRequestsWorkaroundFor704763Only) {
return;
}
m_page->chromeClient().scheduleAnimation(frame->view());
}
......
......@@ -23,6 +23,12 @@ class CORE_EXPORT PageAnimator final : public GarbageCollected<PageAnimator> {
bool isServicingAnimations() const { return m_servicingAnimations; }
// TODO(alancutter): Remove the need for this by implementing frame request
// suppression logic at the BeginMainFrame level. This is a temporary
// workaround to fix a perf regression.
// DO NOT use this outside of crbug.com/704763.
void setSuppressFrameRequestsWorkaroundFor704763Only(bool);
// See documents of methods with the same names in FrameView class.
void updateAllLifecyclePhases(LocalFrame& rootFrame);
AnimationClock& clock() { return m_animationClock; }
......@@ -33,6 +39,7 @@ class CORE_EXPORT PageAnimator final : public GarbageCollected<PageAnimator> {
Member<Page> m_page;
bool m_servicingAnimations;
bool m_updatingLayoutAndStyleForPainting;
bool m_suppressFrameRequestsWorkaroundFor704763Only = false;
AnimationClock m_animationClock;
};
......
......@@ -232,6 +232,11 @@ void WebFrameWidgetImpl::didExitFullscreen() {
view()->didExitFullscreen();
}
void WebFrameWidgetImpl::setSuppressFrameRequestsWorkaroundFor704763Only(
bool suppressFrameRequests) {
page()->animator().setSuppressFrameRequestsWorkaroundFor704763Only(
suppressFrameRequests);
}
void WebFrameWidgetImpl::beginFrame(double lastFrameTimeMonotonic) {
TRACE_EVENT1("blink", "WebFrameWidgetImpl::beginFrame", "frameTime",
lastFrameTimeMonotonic);
......
......@@ -80,6 +80,7 @@ class WebFrameWidgetImpl final
void resizeVisualViewport(const WebSize&) override;
void didEnterFullscreen() override;
void didExitFullscreen() override;
void setSuppressFrameRequestsWorkaroundFor704763Only(bool) final;
void beginFrame(double lastFrameTimeMonotonic) override;
void updateAllLifecyclePhases() override;
void paint(WebCanvas*, const WebRect&) override;
......
......@@ -395,6 +395,13 @@ void WebPagePopupImpl::initializeLayerTreeView() {
}
}
void WebPagePopupImpl::setSuppressFrameRequestsWorkaroundFor704763Only(
bool suppressFrameRequests) {
if (!m_page)
return;
m_page->animator().setSuppressFrameRequestsWorkaroundFor704763Only(
suppressFrameRequests);
}
void WebPagePopupImpl::beginFrame(double lastFrameTimeMonotonic) {
if (!m_page)
return;
......
......@@ -76,6 +76,7 @@ class WebPagePopupImpl final : public WebPagePopup,
private:
// WebWidget functions
void setSuppressFrameRequestsWorkaroundFor704763Only(bool) final;
void beginFrame(double lastFrameTimeMonotonic) override;
void updateAllLifecyclePhases() override;
void willCloseLayerTreeView() override;
......
......@@ -57,6 +57,11 @@ void WebViewFrameWidget::didExitFullscreen() {
return m_webView->didExitFullscreen();
}
void WebViewFrameWidget::setSuppressFrameRequestsWorkaroundFor704763Only(
bool suppressFrameRequests) {
return m_webView->setSuppressFrameRequestsWorkaroundFor704763Only(
suppressFrameRequests);
}
void WebViewFrameWidget::beginFrame(double lastFrameTimeMonotonic) {
return m_webView->beginFrame(lastFrameTimeMonotonic);
}
......
......@@ -48,6 +48,7 @@ class WebViewFrameWidget : public WebFrameWidgetBase {
void resizeVisualViewport(const WebSize&) override;
void didEnterFullscreen() override;
void didExitFullscreen() override;
void setSuppressFrameRequestsWorkaroundFor704763Only(bool) final;
void beginFrame(double lastFrameTimeMonotonic) override;
void updateAllLifecyclePhases() override;
void paint(WebCanvas*, const WebRect& viewPort) override;
......
......@@ -2004,6 +2004,11 @@ void WebViewImpl::didUpdateFullscreenSize() {
m_fullscreenController->updateSize();
}
void WebViewImpl::setSuppressFrameRequestsWorkaroundFor704763Only(
bool suppressFrameRequests) {
m_page->animator().setSuppressFrameRequestsWorkaroundFor704763Only(
suppressFrameRequests);
}
void WebViewImpl::beginFrame(double lastFrameTimeMonotonic) {
TRACE_EVENT1("blink", "WebViewImpl::beginFrame", "frameTime",
lastFrameTimeMonotonic);
......
......@@ -129,6 +129,7 @@ class WEB_EXPORT WebViewImpl final
void didEnterFullscreen() override;
void didExitFullscreen() override;
void setSuppressFrameRequestsWorkaroundFor704763Only(bool) override;
void beginFrame(double lastFrameTimeMonotonic) override;
void updateAllLifecyclePhases() override;
......
......@@ -76,6 +76,9 @@ class WebWidget {
virtual void didEnterFullscreen() {}
virtual void didExitFullscreen() {}
// TODO(crbug.com/704763): Remove the need for this.
virtual void setSuppressFrameRequestsWorkaroundFor704763Only(bool) {}
// Called to update imperative animation state. This should be called before
// paint, although the client can rate-limit these calls.
// |lastFrameTimeMonotonic| is in seconds.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment