Generalize visible descendant dirty bits to prepare for more properties.

third_party/WebKit/Source/core/paint/PaintLayer.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights
  * reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@web.de>
@@ skipping 115 matching lines @@
       groupedMapping(nullptr) {}
 
 PaintLayerRareData::~PaintLayerRareData() {}
 
 PaintLayer::PaintLayer(LayoutBoxModelObject* layoutObject)
     : m_hasSelfPaintingLayerDescendant(false),
       m_hasSelfPaintingLayerDescendantDirty(false),
       m_isRootLayer(layoutObject->isLayoutView()),
       m_isVisibleContentDirty(true),
       m_hasVisibleContent(false),
-      m_isVisibleDescendantDirty(false),
+      m_needsDescendantDependentFlagsUpdate(false),
       m_hasVisibleDescendant(false),
 #if DCHECK_IS_ON()
       m_needsPositionUpdate(true),
 #endif
       m_is3DTransformedDescendantDirty(true),
       m_has3DTransformedDescendant(false),
       m_containsDirtyOverlayScrollbars(false),
       m_needsAncestorDependentCompositingInputsUpdate(true),
       m_needsDescendantDependentCompositingInputsUpdate(true),
       m_childNeedsCompositingInputsUpdate(true),
@@ skipping 157 matching lines @@
 
   // FIXME(400589): We would like to do this in
   // PaintLayerScrollableArea::updateAfterLayout, but it depends on the size
   // computed by updateLayerPosition.
   if (m_scrollableArea) {
     if (ScrollAnimatorBase* scrollAnimator =
             m_scrollableArea->existingScrollAnimator())
       scrollAnimator->updateAfterLayout();
   }
 
-  // FIXME: We should be able to remove this call because we don't care about
-  // any descendant-dependent flags, but code somewhere else is reading these
-  // flags and depending on us to update them.
-  updateDescendantDependentFlags();

[chrishtr, 2016/12/01 16:13:37]

This is not needed because it will happen later during the compositing update
step.
I also verified that there are no callers outside of compositing or later in the
lifecycle that need any of these flags. See the attached bug, which links to 
a doc that investigates all call sites.

-
   for (PaintLayer* child = firstChild(); child; child = child->nextSibling())
     child->updateLayerPositionRecursive();
 }
 
 void PaintLayer::updateHasSelfPaintingLayerDescendant() const {
   DCHECK(m_hasSelfPaintingLayerDescendantDirty);
 
   m_hasSelfPaintingLayerDescendant = false;
 
   for (PaintLayer* child = firstChild(); child; child = child->nextSibling()) {
@@ skipping 305 matching lines @@
   // invalidation code structure.
   layoutObject.mapToVisualRectInAncestorSpace(&paintInvalidationContainer,
                                               rect);
 
   mapRectInPaintInvalidationContainerToBacking(paintInvalidationContainer,
                                                rect);
 }
 
 void PaintLayer::dirtyVisibleContentStatus() {
   m_isVisibleContentDirty = true;
-  if (parent())
-    parent()->dirtyAncestorChainVisibleDescendantStatus();
+  markAncestorChainForDescendantDependentFlagsUpdate();

[Xianzhu, 2016/12/01 17:59:32]

This will also set m_needsDescendantDependentFlagsUpdate for |this|. Is this
expected?

[chrishtr, 2016/12/01 18:05:22]

It's intended yes. It's because visible contents updated for |this| in
updateDescendantDependentFlags.

   // Non-self-painting layers paint into their ancestor layer, and count as part
   // of the "visible contents" of the parent, so we need to dirty it.
   if (!isSelfPaintingLayer())
     parent()->dirtyVisibleContentStatus();
 }
 
-void PaintLayer::potentiallyDirtyVisibleContentStatus(EVisibility visibility) {
-  if (m_isVisibleContentDirty)
-    return;
-  if (hasVisibleContent() == (visibility == EVisibility::Visible))

[chrishtr, 2016/12/01 16:13:37]

This is cleanup for what looks like unnecessary complexity. abarth added this
code in this patch: https://codereview.chromium.org/453113002
which was trying to avoid reading dirty state, by adding a
potentiallyDirtyVisibleContentStatus method which bailed if the value was dirty.
The code in LayoutObject that is the call site has since been simplified to
avoid conditionally dirtying visible content state based on current visible
state, so we can now just remove this code and unconditionally dirty the
state in LayoutObject.

-    return;
-  dirtyVisibleContentStatus();
-}
-
-void PaintLayer::dirtyAncestorChainVisibleDescendantStatus() {
+void PaintLayer::markAncestorChainForDescendantDependentFlagsUpdate() {
   for (PaintLayer* layer = this; layer; layer = layer->parent()) {
-    if (layer->m_isVisibleDescendantDirty)
+    if (layer->m_needsDescendantDependentFlagsUpdate)
       break;
-    layer->m_isVisibleDescendantDirty = true;
+    layer->m_needsDescendantDependentFlagsUpdate = true;
   }
 }
 
 // FIXME: this is quite brute-force. We could be more efficient if we were to
 // track state and update it as appropriate as changes are made in the layout
 // tree.
 void PaintLayer::updateScrollingStateAfterCompositingChange() {
   TRACE_EVENT0("blink",
                "PaintLayer::updateScrollingStateAfterCompositingChange");
   m_isAllScrollingContentComposited = true;
@@ skipping 12 matching lines @@
     } else if (!child->stackingNode()->isStackingContext()) {
       // If the child is composited, but not a stacking context, it may paint
       // negative z-index descendants into an ancestor's GraphicsLayer.
       m_isAllScrollingContentComposited = false;
       return;
     }
   }
 }
 
 void PaintLayer::updateDescendantDependentFlags() {
-  if (m_isVisibleDescendantDirty) {
+  if (m_needsDescendantDependentFlagsUpdate) {
     m_hasVisibleDescendant = false;
 
     for (PaintLayer* child = firstChild(); child;
          child = child->nextSibling()) {
       child->updateDescendantDependentFlags();
 
       if (child->m_hasVisibleContent || child->m_hasVisibleDescendant)
         m_hasVisibleDescendant = true;
     }
 
-    m_isVisibleDescendantDirty = false;
+    m_needsDescendantDependentFlagsUpdate = false;
   }
 
   if (m_isVisibleContentDirty) {
     bool previouslyHasVisibleContent = m_hasVisibleContent;
     if (layoutObject()->style()->visibility() == EVisibility::Visible) {
       m_hasVisibleContent = true;
     } else {
       // layer may be hidden but still have some visible content, check for this
       m_hasVisibleContent = false;
       LayoutObject* r = layoutObject()->slowFirstChild();
@@ skipping 551 matching lines @@
     // building up generated content layers. This is ok, since the lists will
     // start off dirty in that case anyway.
     child->stackingNode()->dirtyStackingContextZOrderLists();
   }
 
   // Non-self-painting children paint into this layer, so the visible contents
   // status of this layer is affected.
   if (!child->isSelfPaintingLayer())
     dirtyVisibleContentStatus();
 
-  dirtyAncestorChainVisibleDescendantStatus();
+  markAncestorChainForDescendantDependentFlagsUpdate();
   dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
 
   child->setNeedsRepaint();
-
-  child->updateDescendantDependentFlags();

[chrishtr, 2016/12/01 16:13:37]

Same here - not necessary.

 }
 
 PaintLayer* PaintLayer::removeChild(PaintLayer* oldChild) {
   if (oldChild->previousSibling())
     oldChild->previousSibling()->setNextSibling(oldChild->nextSibling());
   if (oldChild->nextSibling())
     oldChild->nextSibling()->setPreviousSibling(oldChild->previousSibling());
 
   if (m_first == oldChild)
     m_first = oldChild->nextSibling();
@@ skipping 18 matching lines @@
   oldChild->setPreviousSibling(0);
   oldChild->setNextSibling(0);
   oldChild->m_parent = 0;
 
   // Remove any ancestor overflow layers which descended into the removed child.
   if (oldChild->ancestorOverflowLayer())
     oldChild->removeAncestorOverflowLayer(oldChild->ancestorOverflowLayer());
 
   dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
 
-  oldChild->updateDescendantDependentFlags();
-
   if (oldChild->m_hasVisibleContent || oldChild->m_hasVisibleDescendant)
-    dirtyAncestorChainVisibleDescendantStatus();
+    markAncestorChainForDescendantDependentFlagsUpdate();
 
   if (oldChild->enclosingPaginationLayer())
     oldChild->clearPaginationRecursive();
 
   setNeedsRepaint();
 
   return oldChild;
 }
 
 void PaintLayer::removeOnlyThisLayerAfterStyleChange() {
@@ skipping 1661 matching lines @@
 
   m_stackingNode->styleDidChange(oldStyle);
 
   if (m_scrollableArea)
     m_scrollableArea->updateAfterStyleChange(oldStyle);
 
   // Overlay scrollbars can make this layer self-painting so we need
   // to recompute the bit once scrollbars have been updated.
   updateSelfPaintingLayer();
 
-  updateDescendantDependentFlags();

[chrishtr, 2016/12/01 16:13:37]

Same here - not necessary.

-
   updateTransform(oldStyle, layoutObject()->styleRef());
   updateFilters(oldStyle, layoutObject()->styleRef());
   updateClipPath(oldStyle, layoutObject()->styleRef());
 
   setNeedsCompositingInputsUpdate();
 }
 
 bool PaintLayer::scrollsOverflow() const {
   if (PaintLayerScrollableArea* scrollableArea = this->getScrollableArea())
     return scrollableArea->scrollsOverflow();
@@ skipping 245 matching lines @@
 }
 
 void showLayerTree(const blink::LayoutObject* layoutObject) {
   if (!layoutObject) {
     LOG(INFO) << "Cannot showLayerTree. Root is (nil)";
     return;
   }
   showLayerTree(layoutObject->enclosingLayer());
 }
 #endif