Refactor to remove need for infiniteIntRect when computing rects in pre-paint

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"
@@ skipping 87 matching lines @@
 
 // Returns whether |a| is an ancestor of or equal to |b|.
 static bool isAncestorOfOrEqualTo(const ClipPaintPropertyNode* a,
                                   const ClipPaintPropertyNode* b) {
   while (b && b != a) {
     b = b->parent();
   }
   return b == a;
 }
 
-ClipRect PrePaintTreeWalk::clipRectForContext(
+FloatClipRect PrePaintTreeWalk::clipRectForContext(
     const PaintPropertyTreeBuilderContext::ContainingBlockContext& context,
     const EffectPaintPropertyNode* effect,
     const PropertyTreeState& ancestorState,
     const LayoutPoint& ancestorPaintOffset,
     bool& hasClip) {
   // Only return a non-infinite clip if clips differ, or the "ancestor" state is
   // actually an ancestor clip. This ensures no accuracy issues due to
   // transforms applied to infinite rects.
   if (isAncestorOfOrEqualTo(context.clip, ancestorState.clip()))

[wkorman, 2017/02/16 21:45:32]

Do we have existing tests that cover this logic? PrePaintTreeWalkTest looks like
it mainly has clip caching and subsequence related tests.

[chrishtr, 2017/02/16 23:48:32]

Yes, but they are layout tests. The situation happens when a composited
element has an out of flow descendant that is not clipped.
sourceToDestinationClipRect actually does "handle" this situation now
(falling back to an infinite rect),
but in my original patch I added it to produce results that are the same
as before. Perhaps I should try to remove this.

-    return ClipRect(LayoutRect(LayoutRect::infiniteIntRect()));
+    return FloatClipRect();
 
   hasClip = true;
 
   PropertyTreeState localState(context.transform, context.clip, effect);
 
-  // TODO(chrishtr): remove need for this.
-  LayoutRect localRect(LayoutRect::infiniteIntRect());
+  FloatClipRect rect(
+      m_geometryMapper.sourceToDestinationClipRect(localState, ancestorState));
 
-  LayoutRect rect(m_geometryMapper
-                      .sourceToDestinationVisualRect(FloatRect(localRect),
-                                                     localState, ancestorState)
-                      .rect());
-  rect.moveBy(-ancestorPaintOffset);
+  rect.moveBy(-FloatPoint(ancestorPaintOffset));
   return rect;
 }
 
 void PrePaintTreeWalk::invalidatePaintLayerOptimizationsIfNeeded(
     const LayoutObject& object,
     const PaintLayer& ancestorTransformedOrRootPaintLayer,
     PaintPropertyTreeBuilderContext& context) {
   if (!object.hasLayer())
     return;
 
@@ skipping 132 matching lines @@
           roundedIntPoint(context.treeBuilderContext.current.paintOffset);
       walk(*toFrameView(widget), context);
     }
     // TODO(pdr): Investigate RemoteFrameView (crbug.com/579281).
   }
 
   object.getMutableForPainting().clearPaintFlags();
 }
 
 }  // namespace blink