REGRESSION (r244182): RenderingUpdate should not be scheduled for invisible pages
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 May 2019 19:31:06 +0000 (19:31 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 May 2019 19:31:06 +0000 (19:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197451

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2019-05-01
Reviewed by Simon Fraser.

Before r244182, some web pages never need to schedule a RenderingUpdate.
Only pages with rAF callbacks, web animations, intersection and resize
observers needed to do so. After r244182, all pages have to schedule a
RenderingUpdate when a page rendering update is required.

When Safari opens, it create a 'blank' web page. The blank page will not
be visible unless the user selects to show the 'Empty page' in the new
tab. Although the blank page is not visible, the loader needs to resolveStyle()
which requires to scheduleLayerFlushNow().

We need to optimize this case: calling scheduleLayerFlushNow() for invisible
pages. We do that by checking if the page is visible before scheduling
the RenderingUpdate.

Also we need to change or get rid of scheduleLayerFlushNow() since its name
has become confusing. It suggests that it is going to schedule flushing
the layer 'now'. But after r244182, it does scheduleRenderingUpdate() first.
And when it fires, scheduleCompositingLayerFlush() will be called.

* page/RenderingUpdateScheduler.cpp:
(WebCore::RenderingUpdateScheduler::scheduleRenderingUpdate):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::scheduleLayerFlush):
(WebCore::RenderLayerCompositor::didChangeVisibleRect):
(WebCore::RenderLayerCompositor::frameViewDidScroll):
(WebCore::RenderLayerCompositor::attachRootLayer):
(WebCore::RenderLayerCompositor::setLayerFlushThrottlingEnabled):
(WebCore::RenderLayerCompositor::layerFlushTimerFired):
(WebCore::RenderLayerCompositor::scheduleLayerFlushNow): Deleted.
* rendering/RenderLayerCompositor.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244837 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/RenderingUpdateScheduler.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index 0fa54ab..7cff018 100644 (file)
@@ -1,3 +1,41 @@
+2019-05-01  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION (r244182): RenderingUpdate should not be scheduled for invisible pages
+        https://bugs.webkit.org/show_bug.cgi?id=197451
+
+        Reviewed by Simon Fraser.
+
+        Before r244182, some web pages never need to schedule a RenderingUpdate.
+        Only pages with rAF callbacks, web animations, intersection and resize 
+        observers needed to do so. After r244182, all pages have to schedule a
+        RenderingUpdate when a page rendering update is required.
+
+        When Safari opens, it create a 'blank' web page. The blank page will not
+        be visible unless the user selects to show the 'Empty page' in the new
+        tab. Although the blank page is not visible, the loader needs to resolveStyle()
+        which requires to scheduleLayerFlushNow(). 
+
+        We need to optimize this case: calling scheduleLayerFlushNow() for invisible
+        pages. We do that by checking if the page is visible before scheduling
+        the RenderingUpdate.
+
+        Also we need to change or get rid of scheduleLayerFlushNow() since its name
+        has become confusing. It suggests that it is going to schedule flushing
+        the layer 'now'. But after r244182, it does scheduleRenderingUpdate() first.
+        And when it fires, scheduleCompositingLayerFlush() will be called.
+
+        * page/RenderingUpdateScheduler.cpp:
+        (WebCore::RenderingUpdateScheduler::scheduleRenderingUpdate):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::scheduleLayerFlush):
+        (WebCore::RenderLayerCompositor::didChangeVisibleRect):
+        (WebCore::RenderLayerCompositor::frameViewDidScroll):
+        (WebCore::RenderLayerCompositor::attachRootLayer):
+        (WebCore::RenderLayerCompositor::setLayerFlushThrottlingEnabled):
+        (WebCore::RenderLayerCompositor::layerFlushTimerFired):
+        (WebCore::RenderLayerCompositor::scheduleLayerFlushNow): Deleted.
+        * rendering/RenderLayerCompositor.h:
+
 2019-05-01  Darin Adler  <darin@apple.com>
 
         WebKit has too much of its own UTF-8 code and should rely more on ICU's UTF-8 support
index 610176d..02083b8 100644 (file)
@@ -47,6 +47,12 @@ void RenderingUpdateScheduler::scheduleRenderingUpdate()
     if (isScheduled())
         return;
 
+    // Optimize the case when an invisible page wants just to schedule layer flush.
+    if (!m_page.isVisible()) {
+        scheduleCompositingLayerFlush();
+        return;
+    }
+
     tracePoint(ScheduleRenderingUpdate);
 
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
index bdf89dc..0306e0c 100644 (file)
@@ -453,12 +453,6 @@ void RenderLayerCompositor::notifyFlushRequired(const GraphicsLayer* layer)
     scheduleLayerFlush(layer->canThrottleLayerFlush());
 }
 
-void RenderLayerCompositor::scheduleLayerFlushNow()
-{
-    m_hasPendingLayerFlush = false;
-    page().renderingUpdateScheduler().scheduleRenderingUpdate();
-}
-
 void RenderLayerCompositor::scheduleLayerFlush(bool canThrottle)
 {
     ASSERT(!m_flushingLayers);
@@ -466,11 +460,12 @@ void RenderLayerCompositor::scheduleLayerFlush(bool canThrottle)
     if (canThrottle)
         startInitialLayerFlushTimerIfNeeded();
 
-    if (canThrottle && isThrottlingLayerFlushes()) {
+    if (canThrottle && isThrottlingLayerFlushes())
         m_hasPendingLayerFlush = true;
-        return;
+    else {
+        m_hasPendingLayerFlush = false;
+        page().renderingUpdateScheduler().scheduleRenderingUpdate();
     }
-    scheduleLayerFlushNow();
 }
 
 FloatRect RenderLayerCompositor::visibleRectForLayerFlushing() const
@@ -607,7 +602,7 @@ void RenderLayerCompositor::didChangeVisibleRect()
     bool requiresFlush = rootLayer->visibleRectChangeRequiresFlush(visibleRect);
     LOG_WITH_STREAM(Compositing, stream << "RenderLayerCompositor::didChangeVisibleRect " << visibleRect << " requiresFlush " << requiresFlush);
     if (requiresFlush)
-        scheduleLayerFlushNow();
+        scheduleLayerFlush();
 }
 
 void RenderLayerCompositor::notifyFlushBeforeDisplayRefresh(const GraphicsLayer*)
@@ -1911,7 +1906,7 @@ void RenderLayerCompositor::frameViewDidScroll()
     // it will also manage updating the scroll layer position.
     if (hasCoordinatedScrolling()) {
         // We have to schedule a flush in order for the main TiledBacking to update its tile coverage.
-        scheduleLayerFlushNow();
+        scheduleLayerFlush();
         return;
     }
 
@@ -3802,7 +3797,7 @@ void RenderLayerCompositor::attachRootLayer(RootLayerAttachment attachment)
     rootLayerAttachmentChanged();
     
     if (m_shouldFlushOnReattach) {
-        scheduleLayerFlushNow();
+        scheduleLayerFlush();
         m_shouldFlushOnReattach = false;
     }
 }
@@ -4374,7 +4369,7 @@ void RenderLayerCompositor::setLayerFlushThrottlingEnabled(bool enabled)
     m_layerFlushTimer.stop();
     if (!m_hasPendingLayerFlush)
         return;
-    scheduleLayerFlushNow();
+    scheduleLayerFlush();
 }
 
 void RenderLayerCompositor::disableLayerFlushThrottlingTemporarilyForInteraction()
@@ -4417,7 +4412,7 @@ void RenderLayerCompositor::layerFlushTimerFired()
 {
     if (!m_hasPendingLayerFlush)
         return;
-    scheduleLayerFlushNow();
+    scheduleLayerFlush();
 }
 
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
index 1e652a2..a5f94a0 100644 (file)
@@ -168,7 +168,7 @@ public:
 
     // GraphicsLayers buffer state, which gets pushed to the underlying platform layers
     // at specific times.
-    void scheduleLayerFlush(bool canThrottle);
+    void scheduleLayerFlush(bool canThrottle = false);
     void flushPendingLayerChanges(bool isFlushRoot = true);
 
     // Called when the GraphicsLayer for the given RenderLayer has flushed changes inside of flushPendingLayerChanges().
@@ -526,7 +526,6 @@ private:
 
     bool shouldCompositeOverflowControls() const;
 
-    void scheduleLayerFlushNow();
     bool isThrottlingLayerFlushes() const;
     void startInitialLayerFlushTimerIfNeeded();
     void startLayerFlushTimerIfNeeded();