Use scrollTop instead of getBoundingClientRect to control VirtualList rendering

src/components/shared/VirtualList.js

@@ -36,6 +36,7 @@
 import * as React from 'react';
 import classNames from 'classnames';
 import range from 'array-range';
+import { getResizeObserverWrapper } from 'firefox-profiler/utils/resize-observer-wrapper';
 
 import type { CssPixels } from 'firefox-profiler/types';
 
@@ -154,21 +155,6 @@ type VirtualListInnerProps<Item> = {|
 class VirtualListInner<Item> extends React.PureComponent<
   VirtualListInnerProps<Item>
 > {
-  _container: ?HTMLElement;
-
-  _takeContainerRef = (element: ?HTMLDivElement) => {
-    this._container = element;
-  };
-
-  /* This method is used by users of this component. */
-  /* eslint-disable-next-line react/no-unused-class-component-methods */
-  getBoundingClientRect() {
-    if (this._container) {
-      return this._container.getBoundingClientRect();
-    }
-    return new DOMRect(0, 0, 0, 0);
-  }
-
   render() {
     const {
       itemHeight,
@@ -194,7 +180,6 @@ class VirtualListInner<Item> extends React.PureComponent<
     return (
       <div
         className={className}
-        ref={this._takeContainerRef}
         // Add padding to list height to account for overlay scrollbars.
         style={{
           height: `${(items.length + 1) * itemHeight}px`,
@@ -261,21 +246,19 @@ type VirtualListProps<Item> = {|
   +ariaActiveDescendant?: null | string,
 |};
 
-type Geometry = {
-  // getBoundingClientRect in the Flow definitions is wrong, and labels the return values
-  // as a ClientRect, and not a DOMRect. https://github.com/facebook/flow/issues/5475
-  //
-  // Account for that here:
-  outerRect: DOMRect | ClientRect,
-  innerRectY: CssPixels,
-};
+type VirtualListState = {|
+  // This value is updated from the scroll event.
+  scrollTop: CssPixels,
+  // This is updated from a resize observer.
+  containerHeight: CssPixels,
+|};
 
 export class VirtualList<Item> extends React.PureComponent<
-  VirtualListProps<Item>
+  VirtualListProps<Item>,
+  VirtualListState
 > {
   _container: {| current: HTMLDivElement | null |} = React.createRef();
-  _inner: {| current: VirtualListInner<Item> | null |} = React.createRef();
-  _geometry: ?Geometry;
+  state = { scrollTop: 0, containerHeight: 0 };
 
   componentDidMount() {
     document.addEventListener('copy', this._onCopy, false);
@@ -285,8 +268,8 @@ export class VirtualList<Item> extends React.PureComponent<
         'The container was assumed to exist while mounting The VirtualList.'
       );
     }
-    container.addEventListener('scroll', this._onScroll);
-    this._onScroll(); // for initial size
+
+    getResizeObserverWrapper().subscribe(container, this._resizeListener);
   }
 
   componentWillUnmount() {
@@ -297,12 +280,18 @@ export class VirtualList<Item> extends React.PureComponent<
         'The container was assumed to exist while unmounting The VirtualList.'
       );
     }
-    container.removeEventListener('scroll', this._onScroll);
+    getResizeObserverWrapper().unsubscribe(container, this._resizeListener);
   }
 
-  _onScroll = () => {
-    this._geometry = this._queryGeometry();
-    this.forceUpdate();
+  // The listener is only called when the document is visible.
+  _resizeListener = (contentRect: DOMRectReadOnly) => {
+    this.setState({ containerHeight: contentRect.height });
+  };
+
+  _onScroll = (event: SyntheticEvent<HTMLElement>) => {
+    this.setState({
+      scrollTop: event.currentTarget.scrollTop,
+    });
   };
 
   _onCopy = (event: ClipboardEvent) => {
@@ -312,29 +301,14 @@ export class VirtualList<Item> extends React.PureComponent<
     }
   };
 
-  _queryGeometry(): Geometry | void {
-    const container = this._container.current;
-    const inner = this._inner.current;
-    if (!container || !inner) {
-      return undefined;
-    }
-    const outerRect = container.getBoundingClientRect();
-    const innerRectY = inner.getBoundingClientRect().top;
-    return { outerRect, innerRectY };
-  }
-
   computeVisibleRange() {
     const { itemHeight, disableOverscan } = this.props;
-    if (!this._geometry) {
-      return { visibleRangeStart: 0, visibleRangeEnd: 100 };
-    }
-    const { outerRect, innerRectY } = this._geometry;
+    const { scrollTop, containerHeight } = this.state;
     const overscan = disableOverscan ? 0 : 25;
     const chunkSize = 16;
-    let visibleRangeStart =
-      Math.floor((outerRect.top - innerRectY) / itemHeight) - overscan;
+    let visibleRangeStart = Math.floor(scrollTop / itemHeight) - overscan;
     let visibleRangeEnd =
-      Math.ceil((outerRect.bottom - innerRectY) / itemHeight) + overscan;
+      Math.ceil((scrollTop + containerHeight) / itemHeight) + overscan;
     if (!disableOverscan) {
       visibleRangeStart = Math.floor(visibleRangeStart / chunkSize) * chunkSize;
       visibleRangeEnd = Math.ceil(visibleRangeEnd / chunkSize) * chunkSize;
@@ -502,6 +476,7 @@ export class VirtualList<Item> extends React.PureComponent<
         role={ariaRole}
         aria-label={ariaLabel}
         aria-activedescendant={ariaActiveDescendant}
+        onScroll={this._onScroll}
       >
         <div
           className={`${className}InnerWrapper`}
@@ -523,7 +498,6 @@ export class VirtualList<Item> extends React.PureComponent<
               containerWidth={containerWidth}
               forceRender={forceRender}
               key={columnIndex}
-              ref={columnIndex === 0 ? this._inner : undefined}
             />
           ))}
         </div>

src/components/shared/WithSize.js

@@ -44,7 +44,6 @@ export function withSize<
   // See: https://github.com/firefox-devtools/profiler/issues/3062
   // eslint-disable-next-line flowtype/no-existential-type
   return class WithSizeWrapper extends React.PureComponent<*, State> {
-    _dirtySize: DOMRectReadOnly | null = null;
     state = { width: 0, height: 0 };
     _container: HTMLElement | null;
 
@@ -55,37 +54,15 @@ export function withSize<
       }
       this._container = container;
       getResizeObserverWrapper().subscribe(container, this._resizeListener);
-      window.addEventListener(
-        'visibilitychange',
-        this._visibilityChangeListener
-      );
     }
 
-    // The size is only updated when the document is visible.
-    // In other cases resizing is registered in _dirtySize.
+    // The listener is only called when the document is visible.
     _resizeListener = (contentRect: DOMRectReadOnly) => {
       const container = this._container;
       if (!container) {
         return;
       }
-      if (document.hidden) {
-        this._dirtySize = contentRect;
-      } else {
-        this._updateSize(container, contentRect);
-      }
-    };
-
-    // If resizing was registered when the document wasn't visible,
-    // the size will be updated when the document becomes visible
-    _visibilityChangeListener = () => {
-      const container = this._container;
-      if (!container) {
-        return;
-      }
-      if (!document.hidden && this._dirtySize) {
-        this._updateSize(container, this._dirtySize);
-        this._dirtySize = null;
-      }
+      this._updateSize(container, contentRect);
     };
 
     componentWillUnmount() {
@@ -94,10 +71,6 @@ export function withSize<
         getResizeObserverWrapper().unsubscribe(container, this._resizeListener);
       }
 
-      window.removeEventListener(
-        'visibilitychange',
-        this._visibilityChangeListener
-      );
       this._container = null;
     }
 

src/test/components/ProfileCallTreeView.test.js

@@ -44,13 +44,14 @@ import {
 } from '../fixtures/profiles/processed-profile';
 import { createGeckoProfile } from '../fixtures/profiles/gecko-profile';
 import { autoMockElementSize } from '../fixtures/mocks/element-size';
+import { triggerResizeObservers } from '../fixtures/mocks/resize-observer';
 
 import type { Profile } from 'firefox-profiler/types';
 
 autoMockCanvasContext();
 
 // This makes the bounding box large enough so that we don't trigger
-// VirtualList's virtualization. We assert this above.
+// VirtualList's virtualization. We assert this below.
 autoMockElementSize({ width: 1000, height: 2000 });
 
 describe('calltree/ProfileCallTreeView', function () {
@@ -381,6 +382,9 @@ describe('calltree/ProfileCallTreeView navigation keys', () => {
       </Provider>
     );
 
+    // This automatically uses the bounding box set in autoMockElementSize.
+    triggerResizeObservers();
+
     // Assert that we used a large enough bounding box to include all children.
     const renderedRows = container.querySelectorAll(
       '.treeViewRow.treeViewRowScrolledColumns'

src/utils/resize-observer-wrapper.js

@@ -17,21 +17,56 @@ export type ResizeObserverWrapper = {|
 function createResizeObserverWrapper() {
   // This keeps the list of callbacks for each observed element.
   const callbacks: Map<Element, Set<ResizeObserverCallback>> = new Map();
-  const resizeObserver = new ResizeObserver((entries) => {
+  // This keeps the list of changes while the tab is hidden.
+  const dirtyChanges: Map<Element, DOMRectReadOnly> = new Map();
+
+  let _resizeObserver = null;
+
+  function notifyListenersForElement(element: Element, rect: DOMRectReadOnly) {
+    const callbacksForElement = callbacks.get(element);
+    if (callbacksForElement) {
+      callbacksForElement.forEach((callback) => callback(rect));
+    }
+  }
+
+  function resizeObserverCallback(entries) {
     for (const entry of entries) {
-      const callbacksForElement = callbacks.get(entry.target);
-      if (callbacksForElement) {
-        callbacksForElement.forEach((callback) => callback(entry.contentRect));
+      if (document.hidden) {
+        dirtyChanges.set(entry.target, entry.contentRect);
+      } else {
+        notifyListenersForElement(entry.target, entry.contentRect);
       }
     }
-  });
+  }
+
+  function visibilityChangeListener() {
+    if (!document.hidden) {
+      dirtyChanges.forEach((rect, element) =>
+        notifyListenersForElement(element, rect)
+      );
+      dirtyChanges.clear();
+    }
+  }
+
+  function getResizeObserver() {
+    if (!_resizeObserver) {
+      _resizeObserver = new ResizeObserver(resizeObserverCallback);
+      window.addEventListener('visibilitychange', visibilityChangeListener);
+    }
+    return _resizeObserver;
+  }
+
+  function stopResizeObserver() {
+    _resizeObserver = null;
+    window.removeEventListener('visibilitychange', visibilityChangeListener);
+  }
 
   return {
     subscribe(element: HTMLElement, callback: ResizeObserverCallback) {
       const callbacksForElement = callbacks.get(element) ?? new Set();
       callbacks.set(element, callbacksForElement);
       callbacksForElement.add(callback);
-      resizeObserver.observe(element);
+      getResizeObserver().observe(element);
     },
     unsubscribe(element: HTMLElement, callback: ResizeObserverCallback) {
       const callbacksForElement = callbacks.get(element);
@@ -44,12 +79,12 @@ function createResizeObserverWrapper() {
         }
         if (callbacksForElement.size === 0) {
           callbacks.delete(element);
-          resizeObserver.unobserve(element);
+          getResizeObserver().unobserve(element);
         }
         if (callbacks.size === 0) {
           // It's important to clean this up properly so that tests are behaving
           // as expected.
-          _resizeObserverWrapper = null;
+          stopResizeObserver();
         }
       } else {
         console.warn(