Currently, blink::HTMLElement and blink::Element both implement methods for calculating element offsets. These are named offset{Top/Left/Height/Width}ForBinding in HTMLElement, and offset{Top/Left/Height/Width} in Element. Note that HTMLElement inherits from Element, so it also has access to the non-suffixed methods.

To me, it appears that the implementation of these methods are *nearly* functionally equivalent, with two exceptions:

1. kochi@'s fix for layout object lifecycle (https://crrev.com/5c480f3117fe314b8cace4f33b84020e9e3424e2) was landed only for HTMLElement and not Element.

2. They have slightly different behavior in LayoutObject::offsetParent, due to passing 'this' versus a null pointer. See below for details on this.

So who calls each version of these methods? As the name may suggest, the offset*ForBinding methods are bindings for the V8 HTMLElement interface, so are the ones called when Javascript calls the offset* methods (e.g. https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetTop). The Element offset* methods are called in very few places: blink::ImageDocument::imageClicked and blink::SpatialNavigation::rectToAbsoluteCoordinates (static method) are the only non-test locations.

This entire thing smells of code duplication to me, and I'm hoping that we can combine them such that HTMLElement::offset*ForBinding just call their related Element::offset* methods. We would need to clear up #1 and #2 above first though.



To give a concrete walkthrough, let us compare offsetTopForBinding and offsetTop:

int HTMLElement::offsetTopForBinding() {
  Element* offsetParent = unclosedOffsetParent();
  if (LayoutBoxModelObject* layoutObject = layoutBoxModelObject())
    return adjustLayoutUnitForAbsoluteZoom(
               LayoutUnit(layoutObject->pixelSnappedOffsetTop(offsetParent)),
               layoutObject->styleRef())
        .round();
  return 0;
}

int Element::offsetTop() {
  document().updateStyleAndLayoutIgnorePendingStylesheetsForNode(this);
  if (LayoutBoxModelObject* layoutObject = layoutBoxModelObject())
    return adjustLayoutUnitForAbsoluteZoom(
               LayoutUnit(layoutObject->pixelSnappedOffsetTop(offsetParent())),
               layoutObject->styleRef())
        .round();
  return 0;
}

Apart from when the offset parent is calculated (as noted, changed by kochi@ in HTMLElement to fix lifecycle issues), the only differences are that Element::offsetTop() calls 'document().updateStyleAndLayoutIgnorePendingStylesheetsForNode(this);' at the start of its method, and they have differently named methods for getting the offset parent.

Let us look at the offset parent methods:

Element* HTMLElement::unclosedOffsetParent() {
  document().updateStyleAndLayoutIgnorePendingStylesheetsForNode(this);

  LayoutObject* layoutObject = this->layoutObject();
  if (!layoutObject)
    return nullptr;

  return layoutObject->offsetParent(this);
}

Element* Element::offsetParent() {
  document().updateStyleAndLayoutIgnorePendingStylesheetsForNode(this);

  LayoutObject* layoutObject = this->layoutObject();
  return layoutObject ? layoutObject->offsetParent() : nullptr;
}

Note that both of these call 'document().updateStyleAndLayoutIgnorePendingStylesheetsForNode(this)', so that difference between the parent offset* methods is gone. However, the call to LayoutObject::offsetParent is slightly different - HTMLElement passes 'this', whilst Element passes nullptr (default parameter value). So what impact does that have? In LayoutObject::offsetParent:

    // TODO(kochi): If |base| or |node| is nested deep in shadow roots, this
    // loop may get expensive, as isUnclosedNodeOf() can take up to O(N+M) time
    // (N and M are depths).
    if (base &&
        (node->isClosedShadowHiddenFrom(*base) ||
         (node->isInShadowTree() &&
          node->containingShadowRoot()->type() == ShadowRootType::UserAgent))) {
      // If 'position: fixed' node is found while traversing up, terminate the
      // loop and return null.
      if (ancestor->isFixedPositioned())
        return nullptr;
      continue;
    }

This is the only difference - base will be the element for HTMLElement's call, and nullptr for Element's call. Sadly, I'm not familiar enough with blink to understand the impact of this shadow tree checking.