[iOS WK2] Pages often blank on first load if page loaded by typing the URL
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Apr 2014 06:13:58 +0000 (06:13 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Apr 2014 06:13:58 +0000 (06:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131665

Reviewed by Tim Horton.

The document overlay-related code in RemoteLayerTreeDrawingArea::setRootCompositingLayer()
was triggering a compositing layer flush when called with a null rootLayer, which happens
for pages going into the page cache. This would trigger a layer flush that would clobber
the root layer for the visible page, resulting in missing content.

Also, rebuildCompositingLayerTree() is called recursively and the m_documentOverlayRootLayer
was being added to (and then removed from) every single compositing layers.

Fix both these by changing to a pull model, where RenderLayerCompositor requests
the overlay layer via ChromeClient, and gets it at the end of every flush,
adding to the children of the root layer.

Source/WebCore:
* WebCore.exp.in:
* page/ChromeClient.h:
(WebCore::ChromeClient::documentOverlayLayerForFrame):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::RenderLayerCompositor):
(WebCore::RenderLayerCompositor::flushPendingLayerChanges): Put visibleRect
into a variable for ease of debugging.
(WebCore::RenderLayerCompositor::updateCompositingLayers): Asser
that we're not in the page cache (this would have caught the bug).
(WebCore::RenderLayerCompositor::appendOverlayLayers):
(WebCore::RenderLayerCompositor::rebuildCompositingLayerTree):
(WebCore::RenderLayerCompositor::setDocumentOverlayRootLayer): Deleted.
* rendering/RenderLayerCompositor.h:

Source/WebKit2:
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::documentOverlayLayerForFrame):
* WebProcess/WebCoreSupport/WebChromeClient.h:
* WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::setRootCompositingLayer):

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

Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/page/ChromeClient.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.h
Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm
Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

index 281639c..d0512a7 100644 (file)
@@ -1,3 +1,36 @@
+2014-04-14  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Pages often blank on first load if page loaded by typing the URL
+        https://bugs.webkit.org/show_bug.cgi?id=131665
+
+        Reviewed by Tim Horton.
+
+        The document overlay-related code in RemoteLayerTreeDrawingArea::setRootCompositingLayer()
+        was triggering a compositing layer flush when called with a null rootLayer, which happens
+        for pages going into the page cache. This would trigger a layer flush that would clobber
+        the root layer for the visible page, resulting in missing content.
+        
+        Also, rebuildCompositingLayerTree() is called recursively and the m_documentOverlayRootLayer
+        was being added to (and then removed from) every single compositing layers.
+        
+        Fix both these by changing to a pull model, where RenderLayerCompositor requests
+        the overlay layer via ChromeClient, and gets it at the end of every flush,
+        adding to the children of the root layer.
+
+        * WebCore.exp.in:
+        * page/ChromeClient.h:
+        (WebCore::ChromeClient::documentOverlayLayerForFrame):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::RenderLayerCompositor):
+        (WebCore::RenderLayerCompositor::flushPendingLayerChanges): Put visibleRect
+        into a variable for ease of debugging.
+        (WebCore::RenderLayerCompositor::updateCompositingLayers): Asser
+        that we're not in the page cache (this would have caught the bug).
+        (WebCore::RenderLayerCompositor::appendOverlayLayers):
+        (WebCore::RenderLayerCompositor::rebuildCompositingLayerTree):
+        (WebCore::RenderLayerCompositor::setDocumentOverlayRootLayer): Deleted.
+        * rendering/RenderLayerCompositor.h:
+
 2014-04-14  Jon Honeycutt  <jhoneycutt@apple.com>
 
         Assertion failure under FEImage::determineAbsolutePaintRect()
index 3d68ea4..4feebca 100644 (file)
@@ -871,7 +871,6 @@ __ZN7WebCore21NetworkStorageSession25switchToNewTestingSessionEv
 __ZN7WebCore21NetworkStorageSession28createPrivateBrowsingSessionERKN3WTF6StringE
 __ZN7WebCore21PlatformKeyboardEvent24disambiguateKeyDownEventENS_13PlatformEvent4TypeEb
 __ZN7WebCore21RemoteCommandListener6createERNS_27RemoteCommandListenerClientE
-__ZN7WebCore21RenderLayerCompositor27setDocumentOverlayRootLayerEPNS_13GraphicsLayerE
 __ZN7WebCore21ResourceLoadScheduler20servePendingRequestsENS_20ResourceLoadPriorityE
 __ZN7WebCore21ResourceLoadScheduler20servePendingRequestsEPNS0_15HostInformationENS_20ResourceLoadPriorityE
 __ZN7WebCore21ResourceLoadScheduler21resumePendingRequestsEv
index 67b5954..290e810 100644 (file)
@@ -290,6 +290,8 @@ public:
     // Returns whether or not the client can render the composited layer,
     // regardless of the settings.
     virtual bool allowsAcceleratedCompositing() const { return true; }
+    // Supply a layer that will added as an overlay over other document layers (scrolling with the document).
+    virtual GraphicsLayer* documentOverlayLayerForFrame(Frame&) { return nullptr; }
 
     enum CompositingTrigger {
         ThreeDTransformTrigger = 1 << 0,
index dffeba3..5141a7f 100644 (file)
@@ -255,7 +255,6 @@ RenderLayerCompositor::RenderLayerCompositor(RenderView& renderView)
     , m_isTrackingRepaints(false)
     , m_layersWithTiledBackingCount(0)
     , m_rootLayerAttachment(RootLayerUnattached)
-    , m_documentOverlayRootLayer(nullptr)
     , m_layerFlushTimer(this, &RenderLayerCompositor::layerFlushTimerFired)
     , m_layerFlushThrottlingEnabled(page() && page()->progress().isMainLoadProgressing())
     , m_layerFlushThrottlingTemporarilyDisabledForInteraction(false)
@@ -430,7 +429,8 @@ void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
 #if PLATFORM(IOS)
         double horizontalMargin = defaultTileWidth / pageScaleFactor();
         double verticalMargin = defaultTileHeight / pageScaleFactor();
-        rootLayer->flushCompositingState(frameView.computeCoverageRect(horizontalMargin, verticalMargin));
+        FloatRect visibleRect = frameView.computeCoverageRect(horizontalMargin, verticalMargin);
+        rootLayer->flushCompositingState(visibleRect);
 #else
         // Having a m_clipLayer indicates that we're doing scrolling via GraphicsLayers.
         IntRect visibleRect = m_clipLayer ? IntRect(IntPoint(), frameView.contentsSize()) : frameView.visibleContentRect();
@@ -597,6 +597,8 @@ bool RenderLayerCompositor::hasAnyAdditionalCompositedLayers(const RenderLayer&
 void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)
 {
     m_updateCompositingLayersTimer.stop();
+
+    ASSERT(!m_renderView.document().inPageCache());
     
     // Compositing layers will be updated in Document::implicitClose() if suppressed here.
     if (!m_renderView.document().visualUpdatesAllowed())
@@ -689,6 +691,7 @@ void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
 
         // Host the document layer in the RenderView's root layer.
         if (isFullUpdate) {
+            appendOverlayLayers(childList);
             // Even when childList is empty, don't drop out of compositing mode if there are
             // composited layers that we didn't hit in our traversal (e.g. because of visibility:hidden).
             if (childList.isEmpty() && !hasAnyAdditionalCompositedLayers(*updateRoot))
@@ -721,6 +724,17 @@ void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
     InspectorInstrumentation::layerTreeDidChange(page());
 }
 
+void RenderLayerCompositor::appendOverlayLayers(Vector<GraphicsLayer*>& childList)
+{
+    Frame& frame = m_renderView.frameView().frame();
+    Page* page = frame.page();
+    if (!page)
+        return;
+
+    if (GraphicsLayer* overlayLayer = page->chrome().client().documentOverlayLayerForFrame(frame))
+        childList.append(overlayLayer);
+}
+
 void RenderLayerCompositor::layerBecameNonComposited(const RenderLayer& layer)
 {
     // Inform the inspector that the given RenderLayer was destroyed.
@@ -1461,9 +1475,6 @@ void RenderLayerCompositor::rebuildCompositingLayerTree(RenderLayer& layer, Vect
 
         childLayersOfEnclosingLayer.append(layerBacking->childForSuperlayers());
     }
-
-    if (m_documentOverlayRootLayer)
-        childLayersOfEnclosingLayer.append(m_documentOverlayRootLayer);
 }
 
 void RenderLayerCompositor::rebuildRegionCompositingLayerTree(RenderNamedFlowFragment* region, Vector<GraphicsLayer*>& childList, int depth)
@@ -3819,13 +3830,4 @@ void RenderLayerCompositor::paintRelatedMilestonesTimerFired(Timer<RenderLayerCo
     m_renderView.frameView().firePaintRelatedMilestonesIfNeeded();
 }
 
-void RenderLayerCompositor::setDocumentOverlayRootLayer(GraphicsLayer* documentOverlayRootLayer)
-{
-    if (m_documentOverlayRootLayer)
-        m_documentOverlayRootLayer->removeFromParent();
-    m_documentOverlayRootLayer = documentOverlayRootLayer;
-    setCompositingLayersNeedRebuild(true);
-    scheduleCompositingLayerUpdate();
-}
-
 } // namespace WebCore
index 208206a..e32d109 100644 (file)
@@ -304,8 +304,6 @@ public:
     void setRootExtendedBackgroundColor(const Color&);
     Color rootExtendedBackgroundColor() const { return m_rootExtendedBackgroundColor; }
 
-    void setDocumentOverlayRootLayer(GraphicsLayer*);
-
 private:
     class OverlapMap;
 
@@ -366,6 +364,7 @@ private:
     bool layerHas3DContent(const RenderLayer&) const;
     bool isRunningAcceleratedTransformAnimation(RenderLayerModelObject&) const;
 
+    void appendOverlayLayers(Vector<GraphicsLayer*>&);
     bool hasAnyAdditionalCompositedLayers(const RenderLayer& rootLayer) const;
 
     void ensureRootLayer();
@@ -508,8 +507,6 @@ private:
     std::unique_ptr<GraphicsLayer> m_layerForFooter;
 #endif
 
-    GraphicsLayer* m_documentOverlayRootLayer;
-
     std::unique_ptr<GraphicsLayerUpdater> m_layerUpdater; // Updates tiled layer visible area periodically while animations are running.
 
     Timer<RenderLayerCompositor> m_layerFlushTimer;
index 6f1caa4..e1b81c5 100644 (file)
@@ -1,3 +1,28 @@
+2014-04-14  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Pages often blank on first load if page loaded by typing the URL
+        https://bugs.webkit.org/show_bug.cgi?id=131665
+
+        Reviewed by Tim Horton.
+
+        The document overlay-related code in RemoteLayerTreeDrawingArea::setRootCompositingLayer()
+        was triggering a compositing layer flush when called with a null rootLayer, which happens
+        for pages going into the page cache. This would trigger a layer flush that would clobber
+        the root layer for the visible page, resulting in missing content.
+        
+        Also, rebuildCompositingLayerTree() is called recursively and the m_documentOverlayRootLayer
+        was being added to (and then removed from) every single compositing layers.
+        
+        Fix both these by changing to a pull model, where RenderLayerCompositor requests
+        the overlay layer via ChromeClient, and gets it at the end of every flush,
+        adding to the children of the root layer.
+
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::documentOverlayLayerForFrame):
+        * WebProcess/WebCoreSupport/WebChromeClient.h:
+        * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
+        (WebKit::RemoteLayerTreeDrawingArea::setRootCompositingLayer):
+
 2014-04-14  Tim Horton  <timothy_horton@apple.com>
 
         Fix the 32-bit build.
index f8c2d95..daf418f 100644 (file)
@@ -771,6 +771,14 @@ void WebChromeClient::attachRootGraphicsLayer(Frame*, GraphicsLayer* layer)
         m_page->exitAcceleratedCompositingMode();
 }
 
+GraphicsLayer* WebChromeClient::documentOverlayLayerForFrame(Frame& frame)
+{
+    if (&frame == &m_page->corePage()->mainFrame())
+        return m_page->pageOverlayController().documentOverlayRootLayer();
+
+    return nullptr;
+}
+
 void WebChromeClient::setNeedsOneShotDrawingSynchronization()
 {
     notImplemented();
index 0099652..e72d129 100644 (file)
@@ -202,6 +202,7 @@ private:
     virtual void attachRootGraphicsLayer(WebCore::Frame*, WebCore::GraphicsLayer*) override;
     virtual void setNeedsOneShotDrawingSynchronization() override;
     virtual void scheduleCompositingLayerFlush() override;
+    virtual WebCore::GraphicsLayer* documentOverlayLayerForFrame(WebCore::Frame&) override;
 
     virtual CompositingTriggerFlags allowedCompositingTriggers() const
     {
index 1adc5a5..1d0cd8f 100644 (file)
@@ -94,8 +94,6 @@ void RemoteLayerTreeDrawingArea::setRootCompositingLayer(GraphicsLayer* rootLaye
         children.append(m_webPage->pageOverlayController().viewOverlayRootLayer());
     }
     m_rootLayer->setChildren(children);
-
-    m_webPage->mainFrameView()->renderView()->compositor().setDocumentOverlayRootLayer(m_webPage->pageOverlayController().documentOverlayRootLayer());
 }
 
 void RemoteLayerTreeDrawingArea::updateGeometry(const IntSize& viewSize, const IntSize& layerPosition)
index d06d413..aa50ce1 100644 (file)
@@ -449,8 +449,6 @@ void TiledCoreAnimationDrawingArea::setRootCompositingLayer(CALayer *layer)
 
     [m_hostingLayer setSublayers:layer ? @[ layer, m_webPage->pageOverlayController().viewOverlayRootLayer()->platformLayer() ] : @[ ]];
 
-    m_webPage->mainFrameView()->renderView()->compositor().setDocumentOverlayRootLayer(m_webPage->pageOverlayController().documentOverlayRootLayer());
-
     bool hadRootLayer = !!m_rootLayer;
     m_rootLayer = layer;
     [m_rootLayer setSublayerTransform:m_transform];