ProcessSwap.PageOverlayLayerPersistence fails on iOS and in debug builds
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 Feb 2019 02:38:02 +0000 (02:38 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 Feb 2019 02:38:02 +0000 (02:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194963

Reviewed by Dean Jackson.

Source/WebCore:

Tested by existing failing API test.

* page/Page.cpp:
(WebCore::Page::installedPageOverlaysChanged): Deleted.
* page/Page.h:
(WebCore::Page::pageOverlayController):
* page/PageOverlayController.cpp:
(WebCore::PageOverlayController::installedPageOverlaysChanged):
(WebCore::PageOverlayController::detachViewOverlayLayers):
(WebCore::PageOverlayController::installPageOverlay):
(WebCore::PageOverlayController::uninstallPageOverlay):
(WebCore::PageOverlayController::willDetachRootLayer): Deleted.
* page/PageOverlayController.h:
As intended by r240940, move installedPageOverlaysChanged to PageOverlayController.
Also, make it ignore isInWindow state; otherwise, if you install a overlay
and then come into window, nothing installs the root layer. There is no
need for this code to follow in-window state manually anymore since
the DrawingArea and RenderLayerCompositor just hook the layers up when needed.

Make some methods private, and make detachViewOverlayLayers only touch
*view* overlays, so that we don't detach the document-relative root
layer when you drop to having no view overlays. This maintains
existing behavior because nothing was calling PageOverlayController::detachViewOverlayLayers.

Now there are no callers of willDetachRootLayer, so remove it.

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Do a `contains` check instead of `equals`, because in debug builds we
put the GraphicsLayer pointer in a prefix.

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

Source/WebCore/ChangeLog
Source/WebCore/page/Page.cpp
Source/WebCore/page/Page.h
Source/WebCore/page/PageOverlayController.cpp
Source/WebCore/page/PageOverlayController.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index 8892a61..b3caad6 100644 (file)
@@ -1,3 +1,36 @@
+2019-02-22  Tim Horton  <timothy_horton@apple.com>
+
+        ProcessSwap.PageOverlayLayerPersistence fails on iOS and in debug builds
+        https://bugs.webkit.org/show_bug.cgi?id=194963
+
+        Reviewed by Dean Jackson.
+
+        Tested by existing failing API test.
+
+        * page/Page.cpp:
+        (WebCore::Page::installedPageOverlaysChanged): Deleted.
+        * page/Page.h:
+        (WebCore::Page::pageOverlayController):
+        * page/PageOverlayController.cpp:
+        (WebCore::PageOverlayController::installedPageOverlaysChanged):
+        (WebCore::PageOverlayController::detachViewOverlayLayers):
+        (WebCore::PageOverlayController::installPageOverlay):
+        (WebCore::PageOverlayController::uninstallPageOverlay):
+        (WebCore::PageOverlayController::willDetachRootLayer): Deleted.
+        * page/PageOverlayController.h:
+        As intended by r240940, move installedPageOverlaysChanged to PageOverlayController.
+        Also, make it ignore isInWindow state; otherwise, if you install a overlay
+        and then come into window, nothing installs the root layer. There is no
+        need for this code to follow in-window state manually anymore since
+        the DrawingArea and RenderLayerCompositor just hook the layers up when needed.
+
+        Make some methods private, and make detachViewOverlayLayers only touch
+        *view* overlays, so that we don't detach the document-relative root
+        layer when you drop to having no view overlays. This maintains
+        existing behavior because nothing was calling PageOverlayController::detachViewOverlayLayers.
+
+        Now there are no callers of willDetachRootLayer, so remove it.
+
 2019-02-22  Andy Estes  <aestes@apple.com>
 
         [iOS] Break a reference cycle between PreviewLoader and ResourceLoader
index b02688a..c9e87a2 100644 (file)
@@ -2641,19 +2641,6 @@ void Page::appearanceDidChange()
     }
 }
 
-void Page::installedPageOverlaysChanged()
-{
-    if (isInWindow()) {
-        if (pageOverlayController().hasViewOverlays())
-            chrome().client().attachViewOverlayGraphicsLayer(&pageOverlayController().layerWithViewOverlays());
-        else
-            chrome().client().attachViewOverlayGraphicsLayer(nullptr);
-    }
-
-    if (auto* frameView = mainFrame().view())
-        frameView->setNeedsCompositingConfigurationUpdate();
-}
-
 void Page::setUnobscuredSafeAreaInsets(const FloatBoxExtent& insets)
 {
     if (m_unobscuredSafeAreaInsets == insets)
index 9464a8d..29d28c8 100644 (file)
@@ -413,8 +413,6 @@ public:
 
     WheelEventDeltaFilter* wheelEventDeltaFilter() { return m_recentWheelEventDeltaFilter.get(); }
     PageOverlayController& pageOverlayController() { return *m_pageOverlayController; }
-    
-    void installedPageOverlaysChanged();
 
 #if PLATFORM(MAC)
 #if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
index 86a6852..580694c 100644 (file)
@@ -65,6 +65,19 @@ void PageOverlayController::createRootLayersIfNeeded()
     m_viewOverlayRootLayer->setName("View overlay container");
 }
 
+void PageOverlayController::installedPageOverlaysChanged()
+{
+    if (hasViewOverlays())
+        attachViewOverlayLayers();
+    else
+        detachViewOverlayLayers();
+
+    if (auto* frameView = m_page.mainFrame().view())
+        frameView->setNeedsCompositingConfigurationUpdate();
+
+    updateForceSynchronousScrollLayerPositionUpdates();
+}
+
 bool PageOverlayController::hasDocumentOverlays() const
 {
     for (const auto& overlay : m_pageOverlays) {
@@ -92,7 +105,6 @@ void PageOverlayController::attachViewOverlayLayers()
 void PageOverlayController::detachViewOverlayLayers()
 {
     m_page.chrome().client().attachViewOverlayGraphicsLayer(nullptr);
-    willDetachRootLayer();
 }
 
 GraphicsLayer* PageOverlayController::documentOverlayRootLayer() const
@@ -192,8 +204,6 @@ void PageOverlayController::installPageOverlay(PageOverlay& overlay, PageOverlay
     auto& rawLayer = layer.get();
     m_overlayGraphicsLayers.set(&overlay, WTFMove(layer));
 
-    updateForceSynchronousScrollLayerPositionUpdates();
-
     overlay.setPage(&m_page);
 
     if (FrameView* frameView = m_page.mainFrame().view())
@@ -204,7 +214,7 @@ void PageOverlayController::installPageOverlay(PageOverlay& overlay, PageOverlay
     if (fadeMode == PageOverlay::FadeMode::Fade)
         overlay.startFadeInAnimation();
 
-    m_page.installedPageOverlaysChanged();
+    installedPageOverlaysChanged();
 }
 
 void PageOverlayController::uninstallPageOverlay(PageOverlay& overlay, PageOverlay::FadeMode fadeMode)
@@ -222,8 +232,7 @@ void PageOverlayController::uninstallPageOverlay(PageOverlay& overlay, PageOverl
     bool removed = m_pageOverlays.removeFirst(&overlay);
     ASSERT_UNUSED(removed, removed);
 
-    updateForceSynchronousScrollLayerPositionUpdates();
-    m_page.installedPageOverlaysChanged();
+    installedPageOverlaysChanged();
 }
 
 void PageOverlayController::updateForceSynchronousScrollLayerPositionUpdates()
@@ -272,14 +281,6 @@ GraphicsLayer& PageOverlayController::layerForOverlay(PageOverlay& overlay) cons
     return *m_overlayGraphicsLayers.get(&overlay);
 }
 
-void PageOverlayController::willDetachRootLayer()
-{
-    GraphicsLayer::unparentAndClear(m_documentOverlayRootLayer);
-    GraphicsLayer::unparentAndClear(m_viewOverlayRootLayer);
-
-    m_initialized = false;
-}
-
 void PageOverlayController::didChangeViewSize()
 {
     for (auto& overlayAndLayer : m_overlayGraphicsLayers) {
index e61482b..827b7a1 100644 (file)
@@ -47,9 +47,6 @@ public:
     bool hasDocumentOverlays() const;
     bool hasViewOverlays() const;
 
-    void attachViewOverlayLayers();
-    void detachViewOverlayLayers();
-
     GraphicsLayer& layerWithDocumentOverlays();
     GraphicsLayer& layerWithViewOverlays();
 
@@ -87,7 +84,9 @@ private:
     WEBCORE_EXPORT GraphicsLayer* documentOverlayRootLayer() const;
     WEBCORE_EXPORT GraphicsLayer* viewOverlayRootLayer() const;
 
-    void willDetachRootLayer();
+    void installedPageOverlaysChanged();
+    void attachViewOverlayLayers();
+    void detachViewOverlayLayers();
 
     void updateSettingsForLayer(GraphicsLayer&);
     void updateForceSynchronousScrollLayerPositionUpdates();
index c17f5b4..ee90c7f 100644 (file)
@@ -1,3 +1,14 @@
+2019-02-22  Tim Horton  <timothy_horton@apple.com>
+
+        ProcessSwap.PageOverlayLayerPersistence fails on iOS and in debug builds
+        https://bugs.webkit.org/show_bug.cgi?id=194963
+
+        Reviewed by Dean Jackson.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+        Do a `contains` check instead of `equals`, because in debug builds we
+        put the GraphicsLayer pointer in a prefix.
+
 2019-02-22  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS] Callout menu overlaps in-page controls when editing a comment in github.com's issue tracker
index 819259e..163a270 100644 (file)
@@ -5287,7 +5287,7 @@ static bool hasOverlay(CALayer *layer)
 {
     __block bool hasViewOverlay = false;
     traverseLayerTree(layer, ^(CALayer *layer) {
-        if ([layer.name isEqualToString:@"View overlay container"])
+        if ([layer.name containsString:@"View overlay container"])
             hasViewOverlay = true;
     });
     return hasViewOverlay;