Make PrePaintTreeWalkContext::treeBuilderContext a unique_ptr

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/PrePaintTreeWalk.h"
 
 #include "core/dom/DocumentLifecycle.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/layout/LayoutMultiColumnSpannerPlaceholder.h"
 #include "core/layout/LayoutPart.h"
 #include "core/layout/LayoutView.h"
 #include "core/paint/PaintLayer.h"
 
 namespace blink {
 
 struct PrePaintTreeWalkContext {
   PrePaintTreeWalkContext()
-      : paintInvalidatorContext(treeBuilderContext),
+      : treeBuilderContext(
+            WTF::wrapUnique(new PaintPropertyTreeBuilderContext)),
+        paintInvalidatorContext(*treeBuilderContext),
         ancestorOverflowPaintLayer(nullptr),
         ancestorTransformedOrRootPaintLayer(nullptr) {}
   PrePaintTreeWalkContext(const PrePaintTreeWalkContext& parentContext)
-      : treeBuilderContext(parentContext.treeBuilderContext),
-        paintInvalidatorContext(treeBuilderContext,
+      : treeBuilderContext(WTF::wrapUnique(new PaintPropertyTreeBuilderContext(
+            *parentContext.treeBuilderContext))),
+        paintInvalidatorContext(*treeBuilderContext,
                                 parentContext.paintInvalidatorContext),
         ancestorOverflowPaintLayer(parentContext.ancestorOverflowPaintLayer),
         ancestorTransformedOrRootPaintLayer(
             parentContext.ancestorTransformedOrRootPaintLayer) {}
 
-  PaintPropertyTreeBuilderContext treeBuilderContext;
+  // PaintPropertyTreeBuilderContext is large and can lead to stack overflows

[pdr., 2017/03/10 20:52:39]

I think we may still have the issues in https://crbug.com/698653 with deep
recursion + PaintInvalidatorContext. Can you make both treeBuilderContext and
paintInvalidatorContext heap-allocated in this patch?

[Xianzhu, 2017/03/10 21:54:48]

I think 96 bytes is already small enough. It's smaller than
PaintInvalidationState (168 bytes) which has been used for years. According to
the data in the web-platform-team discussion about stack overflow, most stack
overflows on m56 and m57 happened during layout and paint. None of paint
invalidation functions is in the top 20 functions where stack overflow occurred.

+  // when recursion is deep so this context object is allocated on the heap.
+  // See: https://crbug.com/698653.
+  std::unique_ptr<PaintPropertyTreeBuilderContext> treeBuilderContext;
+
   PaintInvalidatorContext paintInvalidatorContext;
 
   // The ancestor in the PaintLayer tree which has overflow clip, or
   // is the root layer. Note that it is tree ancestor, not containing
   // block or stacking ancestor.
   PaintLayer* ancestorOverflowPaintLayer;
   PaintLayer* ancestorTransformedOrRootPaintLayer;
 };
 
 void PrePaintTreeWalk::walk(FrameView& rootFrame) {
   DCHECK(rootFrame.frame().document()->lifecycle().state() ==
          DocumentLifecycle::InPrePaint);
 
   PrePaintTreeWalkContext initialContext;
-  initialContext.treeBuilderContext =
-      m_propertyTreeBuilder.setupInitialContext();
+  m_propertyTreeBuilder.setupInitialContext(*initialContext.treeBuilderContext);
   initialContext.ancestorTransformedOrRootPaintLayer =
       rootFrame.layoutView()->layer();
 
   // GeometryMapper depends on paint properties.
   if (rootFrame.needsPaintPropertyUpdate() ||
       (rootFrame.layoutView() &&
        !shouldEndWalkBefore(*rootFrame.layoutView(), initialContext)))
     m_geometryMapper.clearCache();
 
   walk(rootFrame, initialContext);
   m_paintInvalidator.processPendingDelayedPaintInvalidations();
 }
 
 void PrePaintTreeWalk::walk(FrameView& frameView,
                             const PrePaintTreeWalkContext& parentContext) {
   if (frameView.shouldThrottleRendering()) {
     // Skip the throttled frame. Will update it when it becomes unthrottled.
     return;
   }
 
   PrePaintTreeWalkContext context(parentContext);
   // ancestorOverflowLayer does not cross frame boundaries.
   context.ancestorOverflowPaintLayer = nullptr;
-  m_propertyTreeBuilder.updateProperties(frameView, context.treeBuilderContext);
+  m_propertyTreeBuilder.updateProperties(frameView,
+                                         *context.treeBuilderContext);
   m_paintInvalidator.invalidatePaintIfNeeded(frameView,
                                              context.paintInvalidatorContext);
 
   if (LayoutView* view = frameView.layoutView()) {
     walk(*view, context);
 #if DCHECK_IS_ON()
     view->assertSubtreeClearedPaintInvalidationFlags();
 #endif
   }
   frameView.clearNeedsPaintPropertyUpdate();
@@ skipping 64 matching lines @@
   if (object.styleRef().hasTransform() ||
       &object == context.paintInvalidatorContext.paintInvalidationContainer) {
     context.ancestorTransformedOrRootPaintLayer = &paintLayer;
   }
 
   const ObjectPaintProperties& ancestorPaintProperties =
       *context.ancestorTransformedOrRootPaintLayer->layoutObject()
            .paintProperties();
   PropertyTreeState ancestorState =
       *ancestorPaintProperties.localBorderBoxProperties();
-  const EffectPaintPropertyNode* effect =
-      context.treeBuilderContext.currentEffect;
 
 #ifdef CHECK_CLIP_RECTS
   ShouldRespectOverflowClipType respectOverflowClip = RespectOverflowClip;
 #endif
   if (context.ancestorTransformedOrRootPaintLayer->compositingState() ==
           PaintsIntoOwnBacking &&
       ancestorPaintProperties.overflowClip()) {
     ancestorState.setClip(ancestorPaintProperties.overflowClip());
 #ifdef CHECK_CLIP_RECTS
     respectOverflowClip = IgnoreOverflowClip;
 #endif
   }
 
 #ifdef CHECK_CLIP_RECTS
   ClipRects& oldClipRects =
       paintLayer.clipper(PaintLayer::DoNotUseGeometryMapper)
           .paintingClipRects(&ancestorTransformedOrRootPaintLayer,
                              respectOverflowClip, LayoutSize());
 #endif
 
   bool hasClip = false;
   RefPtr<ClipRects> clipRects = ClipRects::create();
   const LayoutPoint& ancestorPaintOffset =
       context.ancestorTransformedOrRootPaintLayer->layoutObject().paintOffset();
 
   FloatClipRect clipRect;
-  computeClipRectForContext(context.treeBuilderContext.current, effect,
+  const EffectPaintPropertyNode* effect =
+      context.treeBuilderContext->currentEffect;
+  computeClipRectForContext(context.treeBuilderContext->current, effect,
                             ancestorState, ancestorPaintOffset, hasClip,
                             clipRect);
   clipRects->setOverflowClipRect(clipRect);
 #ifdef CHECK_CLIP_RECTS
   CHECK(!hasClip ||
         clipRects->overflowClipRect() == oldClipRects.overflowClipRect())
       << "rect= " << clipRects->overflowClipRect().toString();
 #endif
 
-  computeClipRectForContext(context.treeBuilderContext.fixedPosition, effect,
+  computeClipRectForContext(context.treeBuilderContext->fixedPosition, effect,
                             ancestorState, ancestorPaintOffset, hasClip,
                             clipRect);
   clipRects->setFixedClipRect(clipRect);
 #ifdef CHECK_CLIP_RECTS
   CHECK(hasClip || clipRects->fixedClipRect() == oldClipRects.fixedClipRect())
       << " fixed=" << clipRects->fixedClipRect().toString();
 #endif
 
-  computeClipRectForContext(context.treeBuilderContext.absolutePosition, effect,
-                            ancestorState, ancestorPaintOffset, hasClip,
+  computeClipRectForContext(context.treeBuilderContext->absolutePosition,
+                            effect, ancestorState, ancestorPaintOffset, hasClip,
                             clipRect);
   clipRects->setPosClipRect(clipRect);
 #ifdef CHECK_CLIP_RECTS
   CHECK(!hasClip || clipRects->posClipRect() == oldClipRects.posClipRect())
       << " abs=" << clipRects->posClipRect().toString();
 #endif
 
   ClipRects* previousClipRects = paintLayer.previousPaintingClipRects();
 
   if (!previousClipRects || *clipRects != *previousClipRects) {
     paintLayer.setNeedsRepaint();
     paintLayer.setPreviousPaintPhaseDescendantOutlinesEmpty(false);
     paintLayer.setPreviousPaintPhaseFloatEmpty(false);
     paintLayer.setPreviousPaintPhaseDescendantBlockBackgroundsEmpty(false);
     // All subsequences which are contained below this paintLayer must also
     // be checked.
     context.paintInvalidatorContext.forcedSubtreeInvalidationFlags |=
         PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate;
   }
 
   paintLayer.setPreviousPaintingClipRects(*clipRects);
 }
 
 bool PrePaintTreeWalk::shouldEndWalkBefore(
     const LayoutObject& object,
     const PrePaintTreeWalkContext& context) {
   return (
       !object.needsPaintPropertyUpdate() &&
       !object.descendantNeedsPaintPropertyUpdate() &&
-      !context.treeBuilderContext.forceSubtreeUpdate &&
+      !context.treeBuilderContext->forceSubtreeUpdate &&
       !context.paintInvalidatorContext.forcedSubtreeInvalidationFlags &&
       !object
            .shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState());
 }
 
 void PrePaintTreeWalk::walk(const LayoutObject& object,
                             const PrePaintTreeWalkContext& parentContext) {
   if (shouldEndWalkBefore(object, parentContext))
     return;
 
-  // PrePaintTreeWalkContext is large and can lead to stack overflows when
-  // recursion is deep so this context object is allocated on the heap.
-  // See: https://crbug.com/698653.
-  std::unique_ptr<PrePaintTreeWalkContext> context =
-      WTF::wrapUnique(new PrePaintTreeWalkContext(parentContext));
+  PrePaintTreeWalkContext context(parentContext);
 
   // This must happen before updatePropertiesForSelf, because the latter reads
   // some of the state computed here.
-  updateAuxiliaryObjectProperties(object, *context);
+  updateAuxiliaryObjectProperties(object, context);
 
   m_propertyTreeBuilder.updatePropertiesForSelf(object,
-                                                context->treeBuilderContext);
+                                                *context.treeBuilderContext);
   m_paintInvalidator.invalidatePaintIfNeeded(object,
-                                             context->paintInvalidatorContext);
+                                             context.paintInvalidatorContext);
   m_propertyTreeBuilder.updatePropertiesForChildren(
-      object, context->treeBuilderContext);
+      object, *context.treeBuilderContext);
 
-  invalidatePaintLayerOptimizationsIfNeeded(object, *context);
+  invalidatePaintLayerOptimizationsIfNeeded(object, context);
 
   for (const LayoutObject* child = object.slowFirstChild(); child;
        child = child->nextSibling()) {
     if (child->isLayoutMultiColumnSpannerPlaceholder()) {
       child->getMutableForPainting().clearPaintFlags();
       continue;
     }
-    walk(*child, *context);
+    walk(*child, context);
   }
 
   if (object.isLayoutPart()) {
     const LayoutPart& layoutPart = toLayoutPart(object);
     FrameViewBase* frameViewBase = layoutPart.frameViewBase();
     if (frameViewBase && frameViewBase->isFrameView()) {
-      context->treeBuilderContext.current.paintOffset +=
+      context.treeBuilderContext->current.paintOffset +=
           layoutPart.replacedContentRect().location() -
           frameViewBase->frameRect().location();
-      context->treeBuilderContext.current.paintOffset =
-          roundedIntPoint(context->treeBuilderContext.current.paintOffset);
-      walk(*toFrameView(frameViewBase), *context);
+      context.treeBuilderContext->current.paintOffset =
+          roundedIntPoint(context.treeBuilderContext->current.paintOffset);
+      walk(*toFrameView(frameViewBase), context);
     }
     // TODO(pdr): Investigate RemoteFrameView (crbug.com/579281).
   }
 
   object.getMutableForPainting().clearPaintFlags();
 }
 
 }  // namespace blink