address comments

Author: mmalerba

src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts

@@ -176,6 +176,9 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
    *     pixels).
    */
   updateBufferSize(minBufferPx: number, maxBufferPx: number) {
+    if (maxBufferPx < minBufferPx) {
+      throw('CDK virtual scroll: maxBufferPx must be greater than or equal to minBufferPx');
+    }
     this._minBufferPx = minBufferPx;
     this._maxBufferPx = maxBufferPx;
   }

src/cdk-experimental/scrolling/virtual-scroll-viewport.spec.ts

@@ -40,6 +40,12 @@ describe('CdkVirtualScrollViewport', () => {
           'should render 4 items to fill 200px space based on 50px estimate from first item');
     }));
 
+    it('should throw if maxBufferPx is less than minBufferPx', fakeAsync(() => {
+      testComponent.minBufferPx = 100;
+      testComponent.maxBufferPx = 99;
+      expect(() => finishInit(fixture)).toThrow();
+    }));
+
     // TODO(mmalerba): Add test that it corrects the initial render if it didn't render enough,
     // once it actually does that.
   });

src/cdk/scrolling/fixed-size-virtual-scroll.ts

@@ -67,6 +67,9 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
    * @param maxBufferPx The amount of buffer (in pixels) to render when rendering more.
    */
   updateItemAndBufferSize(itemSize: number, minBufferPx: number, maxBufferPx: number) {
+    if (maxBufferPx < minBufferPx) {
+      throw Error('CDK virtual scroll: maxBufferPx must be greater than or equal to minBufferPx');
+    }
     this._itemSize = itemSize;
     this._minBufferPx = minBufferPx;
     this._maxBufferPx = maxBufferPx;
@@ -119,30 +122,31 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
 
     const scrollOffset = this._viewport.measureScrollOffset();
     const firstVisibleIndex = scrollOffset / this._itemSize;
-    const range = {...this._viewport.getRenderedRange()};
+    const renderedRange = this._viewport.getRenderedRange();
+    const newRange = {start: renderedRange.start, end: renderedRange.end};
+    const viewportSize = this._viewport.getViewportSize();
+    const dataLength = this._viewport.getDataLength();
 
-    const startBuffer = scrollOffset - range.start * this._itemSize;
-    if (startBuffer < this._minBufferPx && range.start != 0) {
+    const startBuffer = scrollOffset - newRange.start * this._itemSize;
+    if (startBuffer < this._minBufferPx && newRange.start != 0) {
       const expandStart = Math.ceil((this._maxBufferPx - startBuffer) / this._itemSize);
-      range.start = Math.max(0, range.start - expandStart);
-      range.end = Math.min(this._viewport.getDataLength(),
-          Math.ceil(firstVisibleIndex +
-              (this._viewport.getViewportSize() + this._minBufferPx) / this._itemSize));
+      newRange.start = Math.max(0, newRange.start - expandStart);
+      newRange.end = Math.min(dataLength,
+          Math.ceil(firstVisibleIndex + (viewportSize + this._minBufferPx) / this._itemSize));
     } else {
-      const endBuffer =
-          range.end * this._itemSize - (scrollOffset + this._viewport.getViewportSize());
-      if (endBuffer < this._minBufferPx && range.end != this._viewport.getDataLength()) {
+      const endBuffer = newRange.end * this._itemSize - (scrollOffset + viewportSize);
+      if (endBuffer < this._minBufferPx && newRange.end != dataLength) {
         const expandEnd = Math.ceil((this._maxBufferPx - endBuffer) / this._itemSize);
         if (expandEnd > 0) {
-          range.end = Math.min(this._viewport.getDataLength(), range.end + expandEnd);
-          range.start = Math.max(0,
+          newRange.end = Math.min(dataLength, newRange.end + expandEnd);
+          newRange.start = Math.max(0,
               Math.floor(firstVisibleIndex - this._minBufferPx / this._itemSize));
         }
       }
     }
 
-    this._viewport.setRenderedRange(range);
-    this._viewport.setRenderedContentOffset(this._itemSize * range.start);
+    this._viewport.setRenderedRange(newRange);
+    this._viewport.setRenderedContentOffset(this._itemSize * newRange.start);
     this._scrolledIndexChange.next(Math.floor(firstVisibleIndex));
   }
 }

src/cdk/scrolling/scrolling.md

@@ -94,7 +94,7 @@ that it allows for better performance, since items do not need to be measured as
 </cdk-virtual-scroll-viewport>
 ```
 
-The fixed size strategy also supports setting a couple buffer parameters that determine how much
+The fixed size strategy also supports setting a couple of buffer parameters that determine how much
 extra content is rendered beyond what is visible in the viewport. The first of these parameters is
 `minBufferPx`. The `minBufferPx` is the minimum amount of content buffer (in pixels) that the
 viewport must render. If the viewport ever detects that there is less buffered content it will

src/cdk/scrolling/virtual-scroll-viewport.spec.ts

@@ -576,7 +576,13 @@ describe('CdkVirtualScrollViewport', () => {
       flush();
 
       expect(viewport.getRenderedRange())
-          .toEqual({start: 0, end: 8}, 'should not render 2 more buffer items');
+          .toEqual({start: 0, end: 8}, 'should render 2 more buffer items');
+    }));
+
+    it('should throw if maxBufferPx is less than minBufferPx', fakeAsync(() => {
+      testComponent.minBufferPx = 100;
+      testComponent.maxBufferPx = 99;
+      expect(() => finishInit(fixture)).toThrow();
     }));
   });
 });