RenderLayer::hasVisibleDescendant should ASSERT(!m_visibleDescendantStatusDirty)

Source/core/rendering/RenderBox.cpp

Index: Source/core/rendering/RenderBox.cpp
diff --git a/Source/core/rendering/RenderBox.cpp b/Source/core/rendering/RenderBox.cpp
index 27493a3937ab0ba065ba04fb60ebe4dfb9c73ffa..cdd213e02dcf7b937e36df0debaece7edacee76b 100644
--- a/Source/core/rendering/RenderBox.cpp
+++ b/Source/core/rendering/RenderBox.cpp
@@ -1963,8 +1963,12 @@ void RenderBox::deleteLineBoxWrapper()
 
 LayoutRect RenderBox::clippedOverflowRectForPaintInvalidation(const RenderLayerModelObject* paintInvalidationContainer, const PaintInvalidationState* paintInvalidationState) const
 {
-    if (style()->visibility() != VISIBLE && enclosingLayer()->subtreeIsInvisible())
-        return LayoutRect();
+    if (style()->visibility() != VISIBLE) {
+        RenderLayer* layer = enclosingLayer();
+        layer->updateDescendantDependentFlags();

[skobes, 2014/08/08 20:43:18]

Why is this method, which doesn't seem like it should have side effects,
responsible for performing the update?

It seems like we should either make hasVisibleDescendant() do the update
automatically instead of asserting, or there should be some well-defined point
where it is updated.

[abarth-chromium, 2014/08/08 21:24:39]

That's a good question.  The answer is slightly involved:

1) There is a defined time for clean out this state, which is just prior to
entering the InCompositingUpdate document lifecycle state.
2) Some folks need to query this information earlier (e.g., during layout), and
those call sites are responsible for cleaning the subtree they need to query.

I agree with you that we should make hasVisibleDescendant automatically clear
out the state (as well as ASSERT that we haven't yet reached
InCompositingUpdate).  I originally tried that approach, but the patch quickly
spiraled out of control and I was failing tests for reasons that were difficult
for me to understand.

The approach I've taking in this CL (and I hope to continue) is to clean up this
system in small, controlled CLs.  Once we've got the system maintaining its
dirty bits with reasonable invariants, I agree with you that we should switch to
an "automatic cleaning" design.

If you prefer, I can try the automatic cleaning approach again and attempt to
break that change down into small, controlled pieces.  I understand the system
better now and might be able to make that work.

Please let me know which approach you'd prefer.

Thanks!

[skobes, 2014/08/08 21:45:01]

Got it... I'm fine with the approach here as long as there is a plan to get into
a better state.  Can you add a comment?

+        if (layer->subtreeIsInvisible())
+            return LayoutRect();
+    }
 
     LayoutRect r = visualOverflowRect();
     mapRectToPaintInvalidationBacking(paintInvalidationContainer, r, false /*fixed*/, paintInvalidationState);