Handle nested position:sticky elements correctly (main thread)

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

 // Copyright 2014 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/layout/compositing/CompositingReasonFinder.h"
 
 #include "core/CSSPropertyNames.h"
 #include "core/dom/Document.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/Settings.h"
 #include "core/layout/LayoutView.h"
 #include "core/page/Page.h"
 #include "core/paint/PaintLayer.h"
 
 namespace blink {
 
+namespace {
+// TODO(smcgruer): Extract to common helper function.
+StickyPositionScrollingConstraints* stickyConstraintsForLayoutObject(
+    const LayoutBoxModelObject* o) {

[flackr, 2017/02/22 22:40:15]

Can we pass through the reference type instead of a pointer?

[smcgruer, 2017/02/23 20:14:17]

Used the original PaintLayer instead, and pulled this out of a helper method
since its not very long anymore...

+  if (!o || !o->layer()->ancestorOverflowLayer())

[flackr, 2017/02/22 22:40:15]

o should never be null afaict. Since we're calling this from
CompositingReasonFinder I think that ancestorOverflowLayer should never be null
either should it?

[smcgruer, 2017/02/23 20:14:17]

Done.

+    return nullptr;
+
+  PaintLayerScrollableArea* scrollableArea =
+      o->layer()->ancestorOverflowLayer()->getScrollableArea();
+  auto it = scrollableArea->stickyConstraintsMap().find(o->layer());
+  if (it == scrollableArea->stickyConstraintsMap().end())
+    return nullptr;

[flackr, 2017/02/22 22:40:15]

Can this happen? We only call this function for sticky position objects with
clean compositing inputs right?

[smcgruer, 2017/02/23 20:14:17]

Done.

+
+  return &it->value;
+}
+}  // namespace
+
 CompositingReasonFinder::CompositingReasonFinder(LayoutView& layoutView)
     : m_layoutView(layoutView),
       m_compositingTriggers(
           static_cast<CompositingTriggerFlags>(AllCompositingTriggers)) {
   updateTriggers();
 }
 
 void CompositingReasonFinder::updateTriggers() {
   m_compositingTriggers = 0;
 
@@ skipping 197 matching lines @@
        !layer->backgroundIsKnownToBeOpaqueInRect(
            LayoutRect(layer->boundingBoxForCompositing())) ||
        layer->compositesWithTransform() || layer->compositesWithOpacity())) {
     return false;
   }
   // Don't promote fixed position elements that are descendants of a non-view
   // container, e.g. transformed elements.  They will stay fixed wrt the
   // container rather than the enclosing frame.
   if (layer->sticksToViewport())
     return m_layoutView.frameView()->isScrollable();
-  return layer->layoutObject().style()->position() == EPosition::kSticky &&
-         layer->ancestorOverflowLayer()->scrollsOverflow();
+
+  if (layer->layoutObject().style()->position() != EPosition::kSticky)
+    return false;
+
+  // Don't promote nested sticky elements; the compositor can't handle them.
+  // TODO(smcgruer): Add cc nested sticky support (http://crbug.com/672710)
+  StickyPositionScrollingConstraints* constraints =
+      stickyConstraintsForLayoutObject(&layer->layoutObject());
+  return layer->ancestorOverflowLayer()->scrollsOverflow() &&
+         !constraints->hasAncestorStickyElement();

[flackr, 2017/02/22 22:40:15]

stickyConstraintsForLayoutObject can return null but we do not check for null.
Can we remove the null returns from stickyConstraintsForLayoutObject or do we
need to check for null here?

[smcgruer, 2017/02/23 20:14:17]

Done.

 }
 
 }  // namespace blink