Title: [288127] trunk/Source/WebCore
Revision
288127
Author
simon.fra...@apple.com
Date
2022-01-18 10:44:19 -0800 (Tue, 18 Jan 2022)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (288126 => 288127)


--- trunk/Source/WebCore/ChangeLog	2022-01-18 18:42:25 UTC (rev 288126)
+++ trunk/Source/WebCore/ChangeLog	2022-01-18 18:44:19 UTC (rev 288127)
@@ -1,3 +1,29 @@
+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-18  Youenn Fablet  <you...@apple.com>
 
         Reduce failure timer in CoreAudioSharedUnit in the case we only render audio samples

Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (288126 => 288127)


--- trunk/Source/WebCore/rendering/RenderElement.cpp	2022-01-18 18:42:25 UTC (rev 288126)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp	2022-01-18 18:44:19 UTC (rev 288127)
@@ -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: trunk/Source/WebCore/rendering/RenderElement.h (288126 => 288127)


--- trunk/Source/WebCore/rendering/RenderElement.h	2022-01-18 18:42:25 UTC (rev 288126)
+++ trunk/Source/WebCore/rendering/RenderElement.h	2022-01-18 18:44:19 UTC (rev 288127)
@@ -111,7 +111,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: trunk/Source/WebCore/rendering/RenderLayer.cpp (288126 => 288127)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2022-01-18 18:42:25 UTC (rev 288126)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2022-01-18 18:44:19 UTC (rev 288127)
@@ -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