<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[201201] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/201201">201201</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2016-05-19 16:54:32 -0700 (Thu, 19 May 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Cleanup RenderObject::container()
https://bugs.webkit.org/show_bug.cgi?id=157914

Reviewed by David Hyatt.

1. Now we have a dedicated method for the optional, repaintContainerSkipped branch. The container finding
logic is moved to a static inline function so the compiler can optimize out the repaintContainerSkipped branches, when
container() is called instead of container(repaintContainer, isRepaintContainerSkipped).
2. Use helper functions like canContainAbsolutelyPositionedObjects()
3. Remove stale comments.

No behaviour change.

* rendering/RenderBox.cpp:
(WebCore::RenderBox::mapLocalToContainer):
(WebCore::RenderBox::pushMappingToContainer):
(WebCore::RenderBox::computeRectForRepaint):
* rendering/RenderInline.cpp:
(WebCore::RenderInline::computeRectForRepaint):
(WebCore::RenderInline::mapLocalToContainer):
(WebCore::RenderInline::pushMappingToContainer):
* rendering/RenderObject.cpp:
(WebCore::containerForElement):
(WebCore::RenderObject::container):
* rendering/RenderObject.h:
* rendering/RenderText.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBoxcpp">trunk/Source/WebCore/rendering/RenderBox.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderInlinecpp">trunk/Source/WebCore/rendering/RenderInline.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderObjectcpp">trunk/Source/WebCore/rendering/RenderObject.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderObjecth">trunk/Source/WebCore/rendering/RenderObject.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderTexth">trunk/Source/WebCore/rendering/RenderText.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (201200 => 201201)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2016-05-19 23:48:01 UTC (rev 201200)
+++ trunk/Source/WebCore/ChangeLog        2016-05-19 23:54:32 UTC (rev 201201)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2016-05-19  Zalan Bujtas  &lt;zalan@apple.com&gt;
+
+        Cleanup RenderObject::container()
+        https://bugs.webkit.org/show_bug.cgi?id=157914
+
+        Reviewed by David Hyatt.
+
+        1. Now we have a dedicated method for the optional, repaintContainerSkipped branch. The container finding
+        logic is moved to a static inline function so the compiler can optimize out the repaintContainerSkipped branches, when
+        container() is called instead of container(repaintContainer, isRepaintContainerSkipped).
+        2. Use helper functions like canContainAbsolutelyPositionedObjects()
+        3. Remove stale comments.
+
+        No behaviour change.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::mapLocalToContainer):
+        (WebCore::RenderBox::pushMappingToContainer):
+        (WebCore::RenderBox::computeRectForRepaint):
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::computeRectForRepaint):
+        (WebCore::RenderInline::mapLocalToContainer):
+        (WebCore::RenderInline::pushMappingToContainer):
+        * rendering/RenderObject.cpp:
+        (WebCore::containerForElement):
+        (WebCore::RenderObject::container):
+        * rendering/RenderObject.h:
+        * rendering/RenderText.h:
+
</ins><span class="cx"> 2016-05-16  Enrica Casucci  &lt;enrica@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         No candidate punctuation when typing punctuation in WK2 text field.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (201200 => 201201)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderBox.cpp        2016-05-19 23:48:01 UTC (rev 201200)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp        2016-05-19 23:54:32 UTC (rev 201201)
</span><span class="lines">@@ -2004,7 +2004,7 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     bool containerSkipped;
</span><del>-    RenderElement* container = this-&gt;container(repaintContainer, &amp;containerSkipped);
</del><ins>+    RenderElement* container = this-&gt;container(repaintContainer, containerSkipped);
</ins><span class="cx">     if (!container)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="lines">@@ -2052,7 +2052,7 @@
</span><span class="cx">     ASSERT(ancestorToStopAt != this);
</span><span class="cx"> 
</span><span class="cx">     bool ancestorSkipped;
</span><del>-    RenderElement* container = this-&gt;container(ancestorToStopAt, &amp;ancestorSkipped);
</del><ins>+    RenderElement* container = this-&gt;container(ancestorToStopAt, ancestorSkipped);
</ins><span class="cx">     if (!container)
</span><span class="cx">         return nullptr;
</span><span class="cx"> 
</span><span class="lines">@@ -2241,7 +2241,7 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     bool containerSkipped;
</span><del>-    auto* renderer = container(repaintContainer, &amp;containerSkipped);
</del><ins>+    auto* renderer = container(repaintContainer, containerSkipped);
</ins><span class="cx">     if (!renderer)
</span><span class="cx">         return adjustedRect;
</span><span class="cx">     
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderInlinecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (201200 => 201201)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderInline.cpp        2016-05-19 23:48:01 UTC (rev 201200)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp        2016-05-19 23:54:32 UTC (rev 201201)
</span><span class="lines">@@ -1278,7 +1278,7 @@
</span><span class="cx">         return adjustedRect;
</span><span class="cx"> 
</span><span class="cx">     bool containerSkipped;
</span><del>-    RenderElement* container = this-&gt;container(repaintContainer, &amp;containerSkipped);
</del><ins>+    RenderElement* container = this-&gt;container(repaintContainer, containerSkipped);
</ins><span class="cx">     if (!container)
</span><span class="cx">         return adjustedRect;
</span><span class="cx"> 
</span><span class="lines">@@ -1342,7 +1342,7 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     bool containerSkipped;
</span><del>-    RenderElement* container = this-&gt;container(repaintContainer, &amp;containerSkipped);
</del><ins>+    RenderElement* container = this-&gt;container(repaintContainer, containerSkipped);
</ins><span class="cx">     if (!container)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="lines">@@ -1380,7 +1380,7 @@
</span><span class="cx">     ASSERT(ancestorToStopAt != this);
</span><span class="cx"> 
</span><span class="cx">     bool ancestorSkipped;
</span><del>-    RenderElement* container = this-&gt;container(ancestorToStopAt, &amp;ancestorSkipped);
</del><ins>+    RenderElement* container = this-&gt;container(ancestorToStopAt, ancestorSkipped);
</ins><span class="cx">     if (!container)
</span><span class="cx">         return nullptr;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (201200 => 201201)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderObject.cpp        2016-05-19 23:48:01 UTC (rev 201200)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp        2016-05-19 23:54:32 UTC (rev 201201)
</span><span class="lines">@@ -1396,71 +1396,36 @@
</span><span class="cx">     return style().hasEntirelyFixedBackground();
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-RenderElement* RenderObject::container(const RenderLayerModelObject* repaintContainer, bool* repaintContainerSkipped) const
</del><ins>+static inline RenderElement* containerForElement(const RenderObject&amp; renderer, const RenderLayerModelObject* repaintContainer, bool* repaintContainerSkipped)
</ins><span class="cx"> {
</span><del>-    if (repaintContainerSkipped)
-        *repaintContainerSkipped = false;
-
</del><span class="cx">     // This method is extremely similar to containingBlock(), but with a few notable
</span><span class="cx">     // exceptions.
</span><del>-    // (1) It can be used on orphaned subtrees, i.e., it can be called safely even when
-    // the object is not part of the primary document subtree yet.
-    // (2) For normal flow elements, it just returns the parent.
-    // (3) For absolute positioned elements, it will return a relative positioned inline.
-    // containingBlock() simply skips relpositioned inlines and lets an enclosing block handle
-    // the layout of the positioned object.  This does mean that computePositionedLogicalWidth and
-    // computePositionedLogicalHeight have to use container().
-    auto o = parent();
-
-    if (isText())
-        return o;
-
-    EPosition pos = style().position();
-    if (pos == FixedPosition) {
-        // container() can be called on an object that is not in the
-        // tree yet.  We don't call view() since it will assert if it
-        // can't get back to the canvas.  Instead we just walk as high up
-        // as we can.  If we're in the tree, we'll get the root.  If we
-        // aren't we'll get the root of our little subtree (most likely
-        // we'll just return nullptr).
-        // FIXME: The definition of view() has changed to not crawl up the render tree.  It might
-        // be safe now to use it.
-        // FIXME: share code with containingBlockForFixedPosition().
-        while (o &amp;&amp; o-&gt;parent() &amp;&amp; !(o-&gt;hasTransform() &amp;&amp; o-&gt;isRenderBlock())) {
-            // foreignObject is the containing block for its contents.
-            if (o-&gt;isSVGForeignObject())
-                break;
-
-            // The render flow thread is the top most containing block
-            // for the fixed positioned elements.
-            if (o-&gt;isOutOfFlowRenderFlowThread())
-                break;
-
-            if (repaintContainerSkipped &amp;&amp; o == repaintContainer)
-                *repaintContainerSkipped = true;
-
-            o = o-&gt;parent();
-        }
-    } else if (pos == AbsolutePosition) {
-        // Same goes here.  We technically just want our containing block, but
-        // we may not have one if we're part of an uninstalled subtree.  We'll
-        // climb as high as we can though.
-        // FIXME: share code with isContainingBlockCandidateForAbsolutelyPositionedObject().
-        // FIXME: hasTransformRelatedProperty() includes preserves3D() check, but this may need to change: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27566
-        while (o &amp;&amp; o-&gt;style().position() == StaticPosition &amp;&amp; !o-&gt;isRenderView() &amp;&amp; !(o-&gt;hasTransformRelatedProperty() &amp;&amp; o-&gt;isRenderBlock())) {
-            if (o-&gt;isSVGForeignObject()) // foreignObject is the containing block for contents inside it
-                break;
-
-            if (repaintContainerSkipped &amp;&amp; o == repaintContainer)
-                *repaintContainerSkipped = true;
-
-            o = o-&gt;parent();
-        }
</del><ins>+    // (1) For normal flow elements, it just returns the parent.
+    // (2) For absolute positioned elements, it will return a relative positioned inline, while
+    // containingBlock() skips to the non-anonymous containing block.
+    // This does mean that computePositionedLogicalWidth and computePositionedLogicalHeight have to use container().
+    EPosition pos = renderer.style().position();
+    auto* parent = renderer.parent();
+    if (is&lt;RenderText&gt;(renderer) || (pos != FixedPosition &amp;&amp; pos != AbsolutePosition))
+        return parent;
+    for (; parent &amp;&amp; (pos == AbsolutePosition ? !parent-&gt;canContainAbsolutelyPositionedObjects() : !parent-&gt;canContainFixedPositionObjects()); parent = parent-&gt;parent()) {
+        if (repaintContainerSkipped &amp;&amp; repaintContainer == parent)
+            *repaintContainerSkipped = true;
</ins><span class="cx">     }
</span><ins>+    return parent;
+}
</ins><span class="cx"> 
</span><del>-    return o;
</del><ins>+RenderElement* RenderObject::container() const
+{
+    return containerForElement(*this, nullptr, nullptr);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+RenderElement* RenderObject::container(const RenderLayerModelObject* repaintContainer, bool&amp; repaintContainerSkipped) const
+{
+    repaintContainerSkipped = false;
+    return containerForElement(*this, repaintContainer, &amp;repaintContainerSkipped);
+}
+
</ins><span class="cx"> bool RenderObject::isSelectionBorder() const
</span><span class="cx"> {
</span><span class="cx">     SelectionState st = selectionState();
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderObjecth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderObject.h (201200 => 201201)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderObject.h        2016-05-19 23:48:01 UTC (rev 201200)
+++ trunk/Source/WebCore/rendering/RenderObject.h        2016-05-19 23:54:32 UTC (rev 201201)
</span><span class="lines">@@ -571,7 +571,8 @@
</span><span class="cx">     // Returns the object containing this one. Can be different from parent for positioned elements.
</span><span class="cx">     // If repaintContainer and repaintContainerSkipped are not null, on return *repaintContainerSkipped
</span><span class="cx">     // is true if the renderer returned is an ancestor of repaintContainer.
</span><del>-    RenderElement* container(const RenderLayerModelObject* repaintContainer = nullptr, bool* repaintContainerSkipped = nullptr) const;
</del><ins>+    RenderElement* container() const;
+    RenderElement* container(const RenderLayerModelObject* repaintContainer, bool&amp; repaintContainerSkipped) const;
</ins><span class="cx"> 
</span><span class="cx">     RenderBoxModelObject* offsetParent() const;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderTexth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderText.h (201200 => 201201)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderText.h        2016-05-19 23:48:01 UTC (rev 201200)
+++ trunk/Source/WebCore/rendering/RenderText.h        2016-05-19 23:54:32 UTC (rev 201201)
</span><span class="lines">@@ -205,6 +205,8 @@
</span><span class="cx">     LayoutRect collectSelectionRectsForLineBoxes(const RenderLayerModelObject* repaintContainer, bool clipToVisibleContent, Vector&lt;LayoutRect&gt;*);
</span><span class="cx"> 
</span><span class="cx">     void node() const = delete;
</span><ins>+    void container() const = delete; // Use parent() instead.
+    void container(const RenderLayerModelObject&amp;, bool&amp;) const = delete; // Use parent() instead.
</ins><span class="cx"> 
</span><span class="cx">     // We put the bitfield first to minimize padding on 64-bit.
</span><span class="cx">     unsigned m_hasBreakableChar : 1; // Whether or not we can be broken into multiple lines.
</span></span></pre>
</div>
</div>

</body>
</html>