Changeset 247012 in webkit

Timestamp:

Jul 1, 2019 11:09:27 AM (5 years ago)

Author:

Antti Koivisto

Message:

REGRESSION(r240047): Overflow scrollers on WK1 fail to update their content size when it changes
​https://bugs.webkit.org/show_bug.cgi?id=199360
<rdar://problem/51643386>

Reviewed by Simon Fraser.

r240047 replaced didCommitChangesForLayer() mechanism by a more narrow didChangePlatformLayerForLayer.
Unfortunately on WK1 we relied on scroll layers being invalidated after every size (and scrollbar) change.
Without this invalidation we don't call WebChromeClientIOS::addOrUpdateScrollingLayer and the UIKit delegate
that resizes the UIScrollView content.

Fix by removing the scroll layer invalidation mechanism from LegacyWebKitScrollingLayerCoordinator completely and instead
simply update all scroll layers after commit. The UIKit delegate doesn't do any significant work if nothing changes,
this was not a very meaninful optimization.

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::updateScrollCoordinatedLayersAfterFlush):

Update all scroll layers after flush (similar to viewport constrained layers).

(WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
(WebCore::LegacyWebKitScrollingLayerCoordinator::updateScrollingLayer):
(WebCore::LegacyWebKitScrollingLayerCoordinator::addScrollingLayer):
(WebCore::LegacyWebKitScrollingLayerCoordinator::removeScrollingLayer):
(WebCore::LegacyWebKitScrollingLayerCoordinator::registerScrollingLayersNeedingUpdate): Deleted.
(WebCore::LegacyWebKitScrollingLayerCoordinator::didChangePlatformLayerForLayer): Deleted.

• rendering/RenderLayerCompositor.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (6 diffs)
• rendering/RenderLayerCompositor.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-01  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION(r240047): Overflow scrollers on WK1 fail to update their content size when it changes
+        https://bugs.webkit.org/show_bug.cgi?id=199360
+        <rdar://problem/51643386>
+
+        Reviewed by Simon Fraser.
+
+        r240047 replaced didCommitChangesForLayer() mechanism by a more narrow didChangePlatformLayerForLayer.
+        Unfortunately on WK1 we relied on scroll layers being invalidated after every size (and scrollbar) change.
+        Without this invalidation we don't call WebChromeClientIOS::addOrUpdateScrollingLayer and the UIKit delegate
+        that resizes the UIScrollView content.
+
+        Fix by removing the scroll layer invalidation mechanism from LegacyWebKitScrollingLayerCoordinator completely and instead
+        simply update all scroll layers after commit. The UIKit delegate doesn't do any significant work if nothing changes,
+        this was not a very meaninful optimization.
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateScrollCoordinatedLayersAfterFlush):
+
+        Update all scroll layers after flush (similar to viewport constrained layers).
+
+        (WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::updateScrollingLayer):
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::addScrollingLayer):
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::removeScrollingLayer):
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::registerScrollingLayersNeedingUpdate): Deleted.
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::didChangePlatformLayerForLayer): Deleted.
+        * rendering/RenderLayerCompositor.h:
+
 2019-07-01  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -558 +558 @@
     if (m_legacyScrollingLayerCoordinator) {
         m_legacyScrollingLayerCoordinator->registerAllViewportConstrainedLayers(*this);
-        m_legacyScrollingLayerCoordinator->registerScrollingLayersNeedingUpdate();
+        m_legacyScrollingLayerCoordinator->registerAllScrollingLayers();
     }
 }
@@ -565 +565 @@
 void RenderLayerCompositor::didChangePlatformLayerForLayer(RenderLayer& layer, const GraphicsLayer*)
 {
-#if PLATFORM(IOS_FAMILY)
-    if (m_legacyScrollingLayerCoordinator)
-        m_legacyScrollingLayerCoordinator->didChangePlatformLayerForLayer(layer);
-#endif
-
     auto* scrollingCoordinator = this->scrollingCoordinator();
     if (!scrollingCoordinator)
@@ -4778 +4773 @@
     bool allowHorizontalScrollbar = !layer.horizontalScrollbarHiddenByStyle();
     bool allowVerticalScrollbar = !layer.verticalScrollbarHiddenByStyle();
+
     m_chromeClient.addOrUpdateScrollingLayer(layer.renderer().element(), backing->scrollContainerLayer()->platformLayer(), backing->scrolledContentsLayer()->platformLayer(),
         layer.reachableTotalContentsSize(), allowHorizontalScrollbar, allowVerticalScrollbar);
@@ -4786 +4782 @@
     for (auto* layer : m_scrollingLayers)
         updateScrollingLayer(*layer);
-}
-
-void LegacyWebKitScrollingLayerCoordinator::registerScrollingLayersNeedingUpdate()
-{
-    for (auto* layer : m_scrollingLayersNeedingUpdate)
-        updateScrollingLayer(*layer);
-    
-    m_scrollingLayersNeedingUpdate.clear();
 }
 
@@ -4808 +4796 @@
 {
     m_scrollingLayers.add(&layer);
-    m_scrollingLayersNeedingUpdate.add(&layer);
 }
 
 void LegacyWebKitScrollingLayerCoordinator::removeScrollingLayer(RenderLayer& layer, RenderLayerBacking& backing)
 {
-    m_scrollingLayersNeedingUpdate.remove(&layer);
     if (m_scrollingLayers.remove(&layer)) {
         auto* scrollContainerLayer = backing.scrollContainerLayer()->platformLayer();
@@ -4839 +4825 @@
 }
 
-void LegacyWebKitScrollingLayerCoordinator::didChangePlatformLayerForLayer(RenderLayer& layer)
-{
-    if (m_scrollingLayers.contains(&layer))
-        m_scrollingLayersNeedingUpdate.add(&layer);
-}
-
 #endif
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.h

@@ -119 +119 @@
     
     void registerAllScrollingLayers();
-    void registerScrollingLayersNeedingUpdate();
     void unregisterAllScrollingLayers();
     
@@ -128 +127 @@
     void removeViewportConstrainedLayer(RenderLayer&);
 
-    void didChangePlatformLayerForLayer(RenderLayer&);
-
     void removeLayer(RenderLayer&);
 
@@ -140 +137 @@
     HashSet<RenderLayer*> m_viewportConstrainedLayers;
 
-    HashSet<RenderLayer*> m_scrollingLayersNeedingUpdate;
     const bool m_coordinateViewportConstrainedLayers;
 };