Shifting layer position for sticky element to avoid passing unnessary variable to cc

third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp

 /*
  * Copyright (C) 2009, 2010, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 263 matching lines @@
 void CompositedLayerMapping::UpdateFilters(const ComputedStyle& style) {
   graphics_layer_->SetFilters(
       OwningLayer().CreateCompositorFilterOperationsForFilter(style));
 }
 
 void CompositedLayerMapping::UpdateBackdropFilters(const ComputedStyle& style) {
   graphics_layer_->SetBackdropFilters(
       OwningLayer().CreateCompositorFilterOperationsForBackdropFilter(style));
 }
 
+bool CompositedLayerMapping::UsesCompositedStickyPosition() const {
+  return GetLayoutObject().Style()->GetPosition() == EPosition::kSticky &&
+         (owning_layer_.AncestorOverflowLayer()->IsRootLayer()
+              ? GetLayoutObject().View()->GetFrameView()->IsScrollable()
+              : owning_layer_.AncestorOverflowLayer()
+                    ->NeedsCompositedScrolling());
+}
+
 void CompositedLayerMapping::UpdateStickyConstraints(
     const ComputedStyle& style) {
-  bool sticky = style.GetPosition() == EPosition::kSticky;
   const PaintLayer* ancestor_overflow_layer =
       owning_layer_.AncestorOverflowLayer();
-  // TODO(flackr): Do we still need this?
-  if (sticky) {
-    if (!ancestor_overflow_layer->IsRootLayer()) {
-      sticky = ancestor_overflow_layer->NeedsCompositedScrolling();
-    } else {
-      sticky = GetLayoutObject().View()->GetFrameView()->IsScrollable();
-    }
-  }
-
   WebLayerStickyPositionConstraint web_constraint;
-  if (sticky) {
+  if (UsesCompositedStickyPosition()) {

[trchen, 2017/06/09 20:20:36]

nits: Prefer shorter branch first, early out first.

if (!UsesCompositedStickyPosition()) {
 
graphics_layer_->SetStickyPositionConstraint(WebLayerStickyPositionConstraint());
  return;
}

Then we can unident 50 lines of code. :)

[chrishtr, 2017/06/09 20:28:07]

Is this if statement just an optimization? If so, is it necessary? What if
we just always compute the constraints?

[yigu, 2017/06/09 20:52:39]

It seems that we call UpdateStickyConstraints unconditionally so we need this
check to exclude non-sticky elements. How about we do something like
if (UsesCompositedStickyPosition())
  UpdateStickyConstraints(...);
That way we don't need this condition within UpdateStickyConstraints any more.
It's just we don't SetStickyPositionConstraints for non-sticky element's
graphics_layer neither but I think that's OK? WDYT?

[chrishtr, 2017/06/09 20:58:29]

I would like to get rid of the UsesCompositedStickyPosition() to reduce
complexity if perf
doesn't matter, so how about just checking if owning_layer_ has sticky
constraints at all?

[flackr, 2017/06/10 00:25:05]

We would still have to check UsesCompositedStickyPosition() in
UpdateMainGraphicsLayerGeometry in case the ancestor scroller is not composited,
because if the scroller isn't known about on the compositor it can't do the
sticky offset correctly. Even worse, if we set constraints for something with a
non composited scroller I think on the compositor we'll just use the next
scroller up because that will be the nearest ancestor scroller the compositor
knows about. And now that we don't pass the main thread offset over we won't
know that the element has already been offset for sticky position.

     const StickyConstraintsMap& constraints_map =
         ancestor_overflow_layer->GetScrollableArea()->GetStickyConstraintsMap();
     const StickyPositionScrollingConstraints& constraints =
         constraints_map.at(&owning_layer_);
 
     web_constraint.is_sticky = true;
     web_constraint.is_anchored_left =
         constraints.GetAnchorEdges() &
         StickyPositionScrollingConstraints::kAnchorEdgeLeft;
     web_constraint.is_anchored_right =
@@ skipping 811 matching lines @@
   UpdateScrollParent(ScrollParent());
   RegisterScrollingLayers();
 
   UpdateCompositingReasons();
 }
 
 void CompositedLayerMapping::UpdateMainGraphicsLayerGeometry(
     const IntRect& relative_compositing_bounds,
     const IntRect& local_compositing_bounds,
     const IntPoint& graphics_layer_parent_location) {
-  graphics_layer_->SetPosition(FloatPoint(
-      relative_compositing_bounds.Location() - graphics_layer_parent_location));
-  graphics_layer_->SetOffsetFromLayoutObject(
-      ToIntSize(local_compositing_bounds.Location()));
-  // Find the layout offset of the unshifted sticky box within its parent
-  // composited layer. This information is used by the compositor side to
-  // compute the additional offset required to keep the element stuck under
-  // compositor scrolling.
-  FloatSize main_thread_sticky_offset;
-  if (GetLayoutObject().Style()->GetPosition() == EPosition::kSticky) {
+  // Find and remove the offset applied for sticky position if the compositor
+  // will shift the layer for sticky position to avoid offsetting the layer
+  // twice.
+  FloatSize offset_for_sticky_position;
+  if (UsesCompositedStickyPosition()) {
     const StickyConstraintsMap& constraints_map =
         owning_layer_.AncestorOverflowLayer()
             ->GetScrollableArea()
             ->GetStickyConstraintsMap();
     const StickyPositionScrollingConstraints& constraints =
         constraints_map.at(&owning_layer_);
 
-    main_thread_sticky_offset =
+    offset_for_sticky_position =
         constraints.GetOffsetForStickyPosition(constraints_map);
   }
-  graphics_layer_->SetOffsetForStickyPosition(
-      RoundedIntSize(main_thread_sticky_offset));
+  graphics_layer_->SetPosition(
+      FloatPoint(relative_compositing_bounds.Location() -
+                 graphics_layer_parent_location) -
+      offset_for_sticky_position);
+  graphics_layer_->SetOffsetFromLayoutObject(
+      ToIntSize(local_compositing_bounds.Location()));
 
   FloatSize old_size = graphics_layer_->Size();
   const FloatSize contents_size(relative_compositing_bounds.Size());
   if (old_size != contents_size)
     graphics_layer_->SetSize(contents_size);
 
   // m_graphicsLayer is the corresponding GraphicsLayer for this PaintLayer and
   // its non-compositing descendants. So, the visibility flag for
   // m_graphicsLayer should be true if there are any non-compositing visible
   // layers.
@@ skipping 2477 matching lines @@
   } else if (graphics_layer == decoration_outline_layer_.get()) {
     name = "Decoration Layer";
   } else {
     NOTREACHED();
   }
 
   return name;
 }
 
 }  // namespace blink