Some simple cleanup in PaintPropertyTreeBuilder.

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.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/PaintPropertyTreeBuilder.h"
 
 #include <memory>
 #include "core/dom/DOMNodeIds.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
@@ skipping 732 matching lines @@
   } else {
     PropertyTreeState local_border_box =
         PropertyTreeState(context.current.transform, context.current.clip,
                           context.current_effect);
     object.GetMutableForPainting().SetLocalBorderBoxProperties(
         local_border_box);
   }
 }
 
 static bool NeedsScrollbarPaintOffset(const LayoutObject& object) {
-  if (object.IsBoxModelObject()) {
-    if (auto* area = ToLayoutBoxModelObject(object).GetScrollableArea()) {
-      if (area->HorizontalScrollbar() || area->VerticalScrollbar())
-        return true;
-    }
+  if (!object.IsBoxModelObject())
+    return false;
+  if (auto* area = ToLayoutBoxModelObject(object).GetScrollableArea()) {
+    if (area->HorizontalScrollbar() || area->VerticalScrollbar())
+      return true;
   }
   return false;
 }
 
 // TODO(trchen): Remove this once we bake the paint offset into frameRect.
 void PaintPropertyTreeBuilder::UpdateScrollbarPaintOffset(
     const LayoutObject& object,
     ObjectPaintProperties& properties,
     PaintPropertyTreeBuilderFragmentContext& context,
     bool& force_subtree_update) {
@@ skipping 140 matching lines @@
   // reasons at frame boundaries, so we can early-out when not at a LayoutView.
   // TODO(pdr): Need to find a solution to the style-related main thread
   // scrolling reasons such as opacity and transform which violate this.
   if (!object.IsLayoutView())
     return ancestor_reasons;
   return GetMainThreadScrollingReasons(*object.GetFrameView(),
                                        ancestor_reasons);
 }
 
 static bool NeedsScrollTranslation(const LayoutObject& object) {
-  if (object.HasOverflowClip()) {
-    const LayoutBox& box = ToLayoutBox(object);
-    auto* scrollable_area = box.GetScrollableArea();
-    IntSize scroll_offset = box.ScrolledContentOffset();
-    if (!scroll_offset.IsZero() || scrollable_area->ScrollsOverflow())
-      return true;
-  }
-  return false;
+  if (!object.HasOverflowClip())
+    return false;
+  const LayoutBox& box = ToLayoutBox(object);
+  auto* scrollable_area = box.GetScrollableArea();
+  IntSize scroll_offset = box.ScrolledContentOffset();
+  return !scroll_offset.IsZero() || scrollable_area->ScrollsOverflow();

[wkorman, 2017/05/18 23:47:22]

Can scrollable area ever be null? Should we null- or dcheck-it?

[chrishtr, 2017/05/19 00:21:35]

No, because HasOverflowClip ensures it. I don't know about DCHECK,
other callsites do the same thing.

 }
 
 void PaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation(
     const LayoutObject& object,
     ObjectPaintProperties& properties,
     PaintPropertyTreeBuilderFragmentContext& context,
     bool& force_subtree_update) {
   if (object.NeedsPaintPropertyUpdate() || force_subtree_update) {
     if (NeedsScrollTranslation(object)) {
       const LayoutBox& box = ToLayoutBox(object);
@@ skipping 34 matching lines @@
     }
   }
 
   if (properties.ScrollTranslation()) {
     context.current.transform = properties.ScrollTranslation();
     context.current.scroll = context.current.transform->ScrollNode();
     context.current.should_flatten_inherited_transform = false;
   }
 }
 
-static bool NeedsCssClipFixedPosition(const LayoutObject& object) {
-  return !object.IsLayoutView() && !object.CanContainFixedPositionObjects() &&

[wkorman, 2017/05/18 23:47:21]

NeedsCssClip is obviously redundant with the one callsite of this method, but
how are you sure re: the other two?

[chrishtr, 2017/05/19 00:21:35]

The logic in UpdatePaintProperties looks like:

bool needs_paint_properties = ... || NeedsCssClip() || ... ||
NeedsCssClipFixedPosition() || ...;

which is expanded as:

bool needs_paint_properties = ... || NeedsCssClip() || ... ||
(!object.IsLayoutView() && !object.CanContainFixedPositionObjects() &&
NeedsCssClip()) || ...;

Then

NeedsCssClip() || (!object.IsLayoutView() &&
!object.CanContainFixedPositionObjects() && NeedsCssClip())

can be reduced to

NeedsCssClip().

[wkorman, 2017/05/19 00:32:30]

Ah. LOL.

-         NeedsCssClip(object);
-}
-
 void PaintPropertyTreeBuilder::UpdateOutOfFlowContext(
     const LayoutObject& object,
     PaintPropertyTreeBuilderFragmentContext& context,
     bool& force_subtree_update) {
   if (!object.IsBoxModelObject() && !object.PaintProperties())
     return;
 
   if (object.IsLayoutBlock())
     context.paint_offset_for_float = context.current.paint_offset;
 
@@ skipping 168 matching lines @@
 
 void PaintPropertyTreeBuilder::UpdatePaintProperties(
     const LayoutObject& object,
     PaintPropertyTreeBuilderContext& full_context) {
   bool needs_paint_properties =
       NeedsPaintOffsetTranslation(object) || NeedsTransform(object) ||
       NeedsEffect(object) || NeedsTransformForNonRootSVG(object) ||
       NeedsFilter(object) || NeedsCssClip(object) ||
       NeedsScrollbarPaintOffset(object) || NeedsOverflowClip(object) ||
       NeedsPerspective(object) || NeedsSVGLocalToBorderBoxTransform(object) ||
-      NeedsScrollTranslation(object) || NeedsCssClipFixedPosition(object);
+      NeedsScrollTranslation(object);
+
   bool had_paint_properties = object.PaintProperties();
 
   if (needs_paint_properties && !had_paint_properties) {
     ObjectPaintProperties& paint_properties =
         object.GetMutableForPainting().EnsurePaintProperties();
     if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
       paint_properties.SetCompositorElementId(
           CreateDomNodeBasedCompositorElementId(object));
     }
   } else if (!needs_paint_properties && had_paint_properties) {
@@ skipping 92 matching lines @@
                            context.force_subtree_update);
 
     context.force_subtree_update |= object.SubtreeNeedsPaintPropertyUpdate();
   }
 
   if (object.CanContainAbsolutePositionObjects())
     context.container_for_absolute_position = &object;
 }
 
 }  // namespace blink