Title: [288569] branches/safari-613-branch/Source/WebCore
Revision
288569
Author
repst...@apple.com
Date
2022-01-25 11:38:32 -0800 (Tue, 25 Jan 2022)

Log Message

Cherry-pick r288127. rdar://problem/87785288

    Clean up some code around RenderElement::addLayers()
    https://bugs.webkit.org/show_bug.cgi?id=235272

    Reviewed by Darin Adler.

    The code that looks for the next layer via render tree traversal is tricky and
    hard to understand. Do some initial cleanup prior to fixing it for top layer.

    First, use std::optional<> in the static addLayers() to make the beforeChild
    finding easier to understand (no longer need a null newObject as the signal that
    you've tried to look).

    Second, use references in findNextLayer() and rename 'startPoint' to make its
    purpose more clear.

    * rendering/RenderElement.cpp:
    (WebCore::addLayers):
    (WebCore::RenderElement::addLayers):
    (WebCore::RenderElement::findNextLayer const):
    (WebCore::RenderElement::findNextLayer): Deleted.
    * rendering/RenderElement.h:
    * rendering/RenderLayer.cpp:
    (WebCore::RenderLayer::insertOnlyThisLayer):

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

Modified Paths

Diff

Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (288568 => 288569)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-01-25 19:38:29 UTC (rev 288568)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-01-25 19:38:32 UTC (rev 288569)
@@ -1,5 +1,62 @@
 2022-01-25  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r288127. rdar://problem/87785288
+
+    Clean up some code around RenderElement::addLayers()
+    https://bugs.webkit.org/show_bug.cgi?id=235272
+    
+    Reviewed by Darin Adler.
+    
+    The code that looks for the next layer via render tree traversal is tricky and
+    hard to understand. Do some initial cleanup prior to fixing it for top layer.
+    
+    First, use std::optional<> in the static addLayers() to make the beforeChild
+    finding easier to understand (no longer need a null newObject as the signal that
+    you've tried to look).
+    
+    Second, use references in findNextLayer() and rename 'startPoint' to make its
+    purpose more clear.
+    
+    * rendering/RenderElement.cpp:
+    (WebCore::addLayers):
+    (WebCore::RenderElement::addLayers):
+    (WebCore::RenderElement::findNextLayer const):
+    (WebCore::RenderElement::findNextLayer): Deleted.
+    * rendering/RenderElement.h:
+    * rendering/RenderLayer.cpp:
+    (WebCore::RenderLayer::insertOnlyThisLayer):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288127 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-01-18  Simon Fraser  <simon.fra...@apple.com>
+
+            Clean up some code around RenderElement::addLayers()
+            https://bugs.webkit.org/show_bug.cgi?id=235272
+
+            Reviewed by Darin Adler.
+
+            The code that looks for the next layer via render tree traversal is tricky and
+            hard to understand. Do some initial cleanup prior to fixing it for top layer.
+
+            First, use std::optional<> in the static addLayers() to make the beforeChild
+            finding easier to understand (no longer need a null newObject as the signal that
+            you've tried to look).
+
+            Second, use references in findNextLayer() and rename 'startPoint' to make its
+            purpose more clear.
+
+            * rendering/RenderElement.cpp:
+            (WebCore::addLayers):
+            (WebCore::RenderElement::addLayers):
+            (WebCore::RenderElement::findNextLayer const):
+            (WebCore::RenderElement::findNextLayer): Deleted.
+            * rendering/RenderElement.h:
+            * rendering/RenderLayer.cpp:
+            (WebCore::RenderLayer::insertOnlyThisLayer):
+
+2022-01-25  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r288059. rdar://problem/87785288
 
     Make a function that returns the ordered list of top layer RenderLayers

Modified: branches/safari-613-branch/Source/WebCore/rendering/RenderElement.cpp (288568 => 288569)


--- branches/safari-613-branch/Source/WebCore/rendering/RenderElement.cpp	2022-01-25 19:38:29 UTC (rev 288568)
+++ branches/safari-613-branch/Source/WebCore/rendering/RenderElement.cpp	2022-01-25 19:38:32 UTC (rev 288569)
@@ -634,22 +634,18 @@
     return RenderPtr<RenderObject>(&renderer);
 }
 
-static void addLayers(RenderElement& renderer, RenderLayer* parentLayer, RenderElement*& newObject, RenderLayer*& beforeChild)
+static void addLayers(const RenderElement& addedRenderer, RenderElement& currentRenderer, RenderLayer& parentLayer, std::optional<RenderLayer*>& beforeChild)
 {
-    if (renderer.hasLayer()) {
-        if (!beforeChild && newObject) {
-            // We need to figure out the layer that follows newObject. We only do
-            // this the first time we find a child layer, and then we update the
-            // pointer values for newObject and beforeChild used by everyone else.
-            beforeChild = newObject->parent()->findNextLayer(parentLayer, newObject);
-            newObject = nullptr;
-        }
-        parentLayer->addChild(*downcast<RenderLayerModelObject>(renderer).layer(), beforeChild);
+    if (currentRenderer.hasLayer()) {
+        if (!beforeChild.has_value())
+            beforeChild = addedRenderer.parent()->findNextLayer(parentLayer, &addedRenderer);
+
+        parentLayer.addChild(*downcast<RenderLayerModelObject>(currentRenderer).layer(), beforeChild.value());
         return;
     }
 
-    for (auto& child : childrenOfType<RenderElement>(renderer))
-        addLayers(child, parentLayer, newObject, beforeChild);
+    for (auto& child : childrenOfType<RenderElement>(currentRenderer))
+        addLayers(addedRenderer, child, parentLayer, beforeChild);
 }
 
 void RenderElement::addLayers(RenderLayer* parentLayer)
@@ -657,9 +653,8 @@
     if (!parentLayer)
         return;
 
-    RenderElement* renderer = this;
-    RenderLayer* beforeChild = nullptr;
-    WebCore::addLayers(*this, parentLayer, renderer, beforeChild);
+    std::optional<RenderLayer*> beforeChild;
+    WebCore::addLayers(*this, *this, *parentLayer, beforeChild);
 }
 
 void RenderElement::removeLayers(RenderLayer* parentLayer)
@@ -691,25 +686,20 @@
         child.moveLayers(oldParent, newParent);
 }
 
-RenderLayer* RenderElement::findNextLayer(RenderLayer* parentLayer, RenderObject* startPoint, bool checkParent)
+RenderLayer* RenderElement::findNextLayer(RenderLayer& parentLayer, const RenderObject* siblingToTraverseFrom, bool checkParent) const
 {
-    // Error check the parent layer passed in. If it's null, we can't find anything.
-    if (!parentLayer)
-        return nullptr;
-
     // Step 1: If our layer is a child of the desired parent, then return our layer.
-    RenderLayer* ourLayer = hasLayer() ? downcast<RenderLayerModelObject>(*this).layer() : nullptr;
-    if (ourLayer && ourLayer->parent() == parentLayer)
+    auto* ourLayer = hasLayer() ? downcast<RenderLayerModelObject>(*this).layer() : nullptr;
+    if (ourLayer && ourLayer->parent() == &parentLayer)
         return ourLayer;
 
     // Step 2: If we don't have a layer, or our layer is the desired parent, then descend
     // into our siblings trying to find the next layer whose parent is the desired parent.
-    if (!ourLayer || ourLayer == parentLayer) {
-        for (RenderObject* child = startPoint ? startPoint->nextSibling() : firstChild(); child; child = child->nextSibling()) {
+    if (!ourLayer || ourLayer == &parentLayer) {
+        for (auto* child = siblingToTraverseFrom ? siblingToTraverseFrom->nextSibling() : firstChild(); child; child = child->nextSibling()) {
             if (!is<RenderElement>(*child))
                 continue;
-            RenderLayer* nextLayer = downcast<RenderElement>(*child).findNextLayer(parentLayer, nullptr, false);
-            if (nextLayer)
+            if (auto* nextLayer = downcast<RenderElement>(*child).findNextLayer(parentLayer, nullptr, false))
                 return nextLayer;
         }
     }
@@ -716,7 +706,7 @@
 
     // Step 3: If our layer is the desired parent layer, then we're finished. We didn't
     // find anything.
-    if (parentLayer == ourLayer)
+    if (ourLayer == &parentLayer)
         return nullptr;
 
     // Step 4: If |checkParent| is set, climb up to our parent and check its siblings that

Modified: branches/safari-613-branch/Source/WebCore/rendering/RenderElement.h (288568 => 288569)


--- branches/safari-613-branch/Source/WebCore/rendering/RenderElement.h	2022-01-25 19:38:29 UTC (rev 288568)
+++ branches/safari-613-branch/Source/WebCore/rendering/RenderElement.h	2022-01-25 19:38:32 UTC (rev 288569)
@@ -110,7 +110,7 @@
     void addLayers(RenderLayer* parentLayer);
     void removeLayers(RenderLayer* parentLayer);
     void moveLayers(RenderLayer* oldParent, RenderLayer& newParent);
-    RenderLayer* findNextLayer(RenderLayer* parentLayer, RenderObject* startPoint, bool checkParent = true);
+    RenderLayer* findNextLayer(RenderLayer& parentLayer, const RenderObject* siblingToTraverseFrom, bool checkParent = true) const;
 
     virtual void dirtyLinesFromChangedChild(RenderObject&) { }
 

Modified: branches/safari-613-branch/Source/WebCore/rendering/RenderLayer.cpp (288568 => 288569)


--- branches/safari-613-branch/Source/WebCore/rendering/RenderLayer.cpp	2022-01-25 19:38:29 UTC (rev 288568)
+++ branches/safari-613-branch/Source/WebCore/rendering/RenderLayer.cpp	2022-01-25 19:38:32 UTC (rev 288569)
@@ -483,9 +483,11 @@
     if (!m_parent && renderer().parent()) {
         // We need to connect ourselves when our renderer() has a parent.
         // Find our enclosingLayer and add ourselves.
-        RenderLayer* parentLayer = renderer().parent()->enclosingLayer();
-        ASSERT(parentLayer);
-        RenderLayer* beforeChild = parentLayer->reflectionLayer() != this ? renderer().parent()->findNextLayer(parentLayer, &renderer()) : nullptr;
+        auto* parentLayer = renderer().parent()->enclosingLayer();
+        if (!parentLayer)
+            return;
+
+        auto* beforeChild = parentLayer->reflectionLayer() != this ? renderer().parent()->findNextLayer(*parentLayer, &renderer()) : nullptr;
         parentLayer->addChild(*this, beforeChild);
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to