Some simple cleanup in PaintPropertyTreeBuilder.

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

Index: third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
index 31fdb1eea2b78234f9d7f06d50430cec34ee2c23..e2aeadc4a06421c6b512b6a91b42cc032f37a4ce 100644
--- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
@@ -750,11 +750,11 @@ void PaintPropertyTreeBuilder::UpdateLocalBorderBoxContext(
 }
 
 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;
 }
@@ -915,14 +915,12 @@ static MainThreadScrollingReasons GetMainThreadScrollingReasons(
 }
 
 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(
@@ -977,11 +975,6 @@ void PaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation(
   }
 }
 
-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,
@@ -1170,7 +1163,8 @@ void PaintPropertyTreeBuilder::UpdatePaintProperties(
       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) {