[webkit-reviews] review denied: [Bug 4377] visibility:hidden can't be overriden in child for positioned element : [Attachment 4026] updated patch

[bugzilla-request-daemon at opendarwin.org]

Dave Hyatt <hyatt at apple.com> has denied Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 4377: visibility:hidden can't be overriden in child for positioned element
http://bugzilla.opendarwin.org/show_bug.cgi?id=4377

Attachment 4026: updated patch
http://bugzilla.opendarwin.org/attachment.cgi?id=4026&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
+    if (style()->visibility() != VISIBLE &&
newChild->style()->visibility()==VISIBLE && !newChild->layer()) {

Style nit with the above, needs spaces before and after the ==.  

+    if (!object->firstChild() && object->style()) {
+	 m_visibilityKnown = true;
+	 m_hasVisibleContent = object->style()->visibility() == VISIBLE;
+    }

This seems like a needlessly aggressive optimization to me.  checkVisibility()
will quickly determine the same thing, right?

Your change to collectLayers results in the entire layer tree being walked now.
 The old code would not descend into child layers if they weren't visible.  You
could further optimize by propagating bits up the layer tree (i.e., rather than
just a "hasVisibleContent" in this layer bit, you could have a
"hasVisibleContent" bit for the layer or its descendant layers.