Skip to content

Commit

Permalink
remove premeasuredRect flows
Browse files Browse the repository at this point in the history
  • Loading branch information
samouri committed Mar 22, 2021
1 parent 1df6793 commit 11355e5
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 49 deletions.
28 changes: 4 additions & 24 deletions src/service/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,6 @@ export class Resource {
// TODO(#30620): remove isInViewport_ and whenWithinViewport.
/** @const @private {boolean} */
this.isInViewport_ = false;

/**
* A client rect that was "premeasured" by an IntersectionObserver.
* @private {?ClientRect}
*/
this.premeasuredRect_ = null;
}

/**
Expand Down Expand Up @@ -427,19 +421,11 @@ export class Resource {
return this.element.getUpgradeDelayMs();
}

/** Removes the premeasured rect, likely forcing a manual measure. */
invalidatePremeasurementAndRequestMeasure() {
this.premeasuredRect_ = null;
this.requestMeasure();
}

/**
* Measures the resource's boundaries. An upgraded element will be
* transitioned to the "ready for layout" state.
* @param {boolean} usePremeasuredRect If true, consumes the previously
* premeasured ClientRect instead of calling getBoundingClientRect().
*/
measure(usePremeasuredRect = false) {
measure() {
// Check if the element is ready to be measured.
// Placeholders are special. They are technically "owned" by parent AMP
// elements, sized by parents, but laid out independently. This means
Expand Down Expand Up @@ -470,12 +456,7 @@ export class Resource {
this.isMeasureRequested_ = false;

const oldBox = this.layoutBox_;
if (usePremeasuredRect) {
this.computeMeasurements_(devAssert(this.premeasuredRect_));
} else {
this.computeMeasurements_();
}
this.premeasuredRect_ = null;
this.computeMeasurements_();
const newBox = this.layoutBox_;

// Note that "left" doesn't affect readiness for the layout.
Expand Down Expand Up @@ -521,12 +502,11 @@ export class Resource {

/**
* Computes the current layout box and position-fixed state of the element.
* @param {!ClientRect=} opt_premeasuredRect
* @private
*/
computeMeasurements_(opt_premeasuredRect) {
computeMeasurements_() {
const viewport = Services.viewportForDoc(this.element);
this.layoutBox_ = viewport.getLayoutRect(this.element, opt_premeasuredRect);
this.layoutBox_ = viewport.getLayoutRect(this.element);

// Calculate whether the element is currently is or in `position:fixed`.
let isFixed = false;
Expand Down
5 changes: 2 additions & 3 deletions src/service/resources-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1010,13 +1010,12 @@ export class ResourcesImpl {
* Always returns true unless the resource was previously displayed but is
* not displayed now (i.e. the resource should be unloaded).
* @param {!Resource} r
* @param {boolean} usePremeasuredRect
* @return {boolean}
* @private
*/
measureResource_(r, usePremeasuredRect = false) {
measureResource_(r) {
const wasDisplayed = r.isDisplayed();
r.measure(usePremeasuredRect);
r.measure();
return !(wasDisplayed && !r.isDisplayed());
}

Expand Down
8 changes: 1 addition & 7 deletions src/service/viewport/viewport-binding-def.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,9 @@ export class ViewportBindingDef {
* pass in, if they cached these values and would like to avoid
* remeasure. Requires appropriate updating the values on scroll.
* @param {number=} unusedScrollTop Same comment as above.
* @param {!ClientRect=} unusedPremeasuredRect
* @return {!../../layout-rect.LayoutRectDef}
*/
getLayoutRect(
unusedEl,
unusedScrollLeft,
unusedScrollTop,
unusedPremeasuredRect
) {}
getLayoutRect(unusedEl, unusedScrollLeft, unusedScrollTop) {}

/**
* Returns the client rect of the current window.
Expand Down
4 changes: 2 additions & 2 deletions src/service/viewport/viewport-binding-ios-embed-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ export class ViewportBindingIosEmbedWrapper_ {
contentHeightChanged() {}

/** @override */
getLayoutRect(el, opt_scrollLeft, opt_scrollTop, opt_premeasuredRect) {
const b = opt_premeasuredRect || el./*OK*/ getBoundingClientRect();
getLayoutRect(el, opt_scrollLeft, opt_scrollTop) {
const b = el./*OK*/ getBoundingClientRect();
const scrollTop =
opt_scrollTop != undefined ? opt_scrollTop : this.getScrollTop();
const scrollLeft =
Expand Down
4 changes: 2 additions & 2 deletions src/service/viewport/viewport-binding-natural.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ export class ViewportBindingNatural_ {
}

/** @override */
getLayoutRect(el, opt_scrollLeft, opt_scrollTop, opt_premeasuredRect) {
const b = opt_premeasuredRect || el./*OK*/ getBoundingClientRect();
getLayoutRect(el, opt_scrollLeft, opt_scrollTop) {
const b = el./*OK*/ getBoundingClientRect();
const scrollTop =
opt_scrollTop != undefined ? opt_scrollTop : this.getScrollTop();
const scrollLeft =
Expand Down
11 changes: 3 additions & 8 deletions src/service/viewport/viewport-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,14 +354,14 @@ export class ViewportImpl {
}

/** @override */
getLayoutRect(el, opt_premeasuredRect) {
getLayoutRect(el) {
const scrollLeft = this.getScrollLeft();
const scrollTop = this.getScrollTop();

// Go up the window hierarchy through friendly iframes.
const frameElement = getParentWindowFrameElement(el, this.ampdoc.win);
if (frameElement) {
const b = this.binding_.getLayoutRect(el, 0, 0, opt_premeasuredRect);
const b = this.binding_.getLayoutRect(el, 0, 0);
const c = this.binding_.getLayoutRect(
frameElement,
scrollLeft,
Expand All @@ -375,12 +375,7 @@ export class ViewportImpl {
);
}

return this.binding_.getLayoutRect(
el,
scrollLeft,
scrollTop,
opt_premeasuredRect
);
return this.binding_.getLayoutRect(el, scrollLeft, scrollTop);
}

/** @override */
Expand Down
4 changes: 1 addition & 3 deletions src/service/viewport/viewport-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,9 @@ export class ViewportInterface extends Disposable {
* Note that this function should be called in vsync measure. Please consider
* using `getClientRectAsync` instead.
* @param {!Element} el
* @param {!ClientRect=} opt_premeasuredRect If provided, use this
* premeasured ClientRect instead of calling getBoundingClientRect.
* @return {!../../layout-rect.LayoutRectDef}
*/
getLayoutRect(el, opt_premeasuredRect) {}
getLayoutRect(el) {}

/**
* Returns the clientRect of the element.
Expand Down

0 comments on commit 11355e5

Please sign in to comment.