Force compositing inputs to be clean for getBoundingClientRect

third_party/WebKit/Source/core/dom/Element.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc.
  * All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
@@ skipping 1107 matching lines @@
   return rect;
 }
 
 void Element::clientQuads(Vector<FloatQuad>& quads) {
   document().updateStyleAndLayoutIgnorePendingStylesheetsForNode(this);
 
   LayoutObject* elementLayoutObject = layoutObject();
   if (!elementLayoutObject)
     return;
 
+  FrameView* view = document().view();
+  if (UNLIKELY(view && elementLayoutObject->enclosingBoxModelObject() &&
+               elementLayoutObject->enclosingBoxModelObject()
+                   ->isInStickySubtree())) {
+    view->updateLifecycleToCompositingCleanPlusScrolling();

[chrishtr, 2017/01/19 21:29:19]

Hmm. I think this will be incorrect if layout/style state is dirty and updating
layout/style will
result in a change to isInStickySubtree(). Also, on reflection, the logic here
to avoid
updating to compositing clean plus scrolling only has a significant effect if
the developer
dirtied style/layout, then called clientQuads before yielding, because otherwise
Blink would update
the entire lifecycle before coming back to script.

Now I'm not sure if this is important to optimize. And also detecting whether
there is any stick
positioned ancestor is not so easy without extra code to store more dirty bits
(your CL incurs a
tree walk up to the root every time clientQuads is called).

Now I think we should just land this patch without the conditional here.

[smcgruer, 2017/01/20 01:40:41]

I think those are all fair points. I'm happy to return this to the unconditional
approach.

Any thoughts on measuring any possible performance impact?

[flackr, 2017/01/20 02:19:41]

We could certainly measure how often we actually don't already have fresh
values, and/or how long it takes to update, but even with microbenchmark data
it's not clear to me if we would know the overall impact. I think we can rely on
telemetry pagesets catching if this regresses performance overall in the wild,
although I agree with Chris that it's likely going to be clean most of the time
and repeated calls will only incur the cost once unless they dirty something
which is a known bad pattern.

[smcgruer, 2017/01/20 02:32:29]

Ack, uploaded new PS that takes the code back to PS2.

+  }
+
   if (isSVGElement() && !elementLayoutObject->isSVGRoot()) {
     // Get the bounding rectangle from the SVG model.
     if (toSVGElement(this)->isSVGGraphicsElement())
       quads.push_back(elementLayoutObject->localToAbsoluteQuad(
           elementLayoutObject->objectBoundingBox()));
     return;
   }
 
   // FIXME: Handle table/inline-table with a caption.
   if (elementLayoutObject->isBoxModelObject() || elementLayoutObject->isBR())
@@ skipping 2992 matching lines @@
 }
 
 DEFINE_TRACE_WRAPPERS(Element) {
   if (hasRareData()) {
     visitor->traceWrappers(elementRareData());
   }
   ContainerNode::traceWrappers(visitor);
 }
 
 }  // namespace blink