perf(scroll-dispatcher): avoid triggering change detection on scroll (#…

…3687)

* Avoids triggering change detection when listening to global scroll events in the `ScrollDispatcher`.
* Avoids triggering change detection when scrolling inside of `Scrollable` instances.
* Switches a `ScrollDispatcher` test to use spies, instead of toggling booleans.

src/lib/core/overlay/position/connected-position-strategy.spec.ts

@@ -534,7 +534,9 @@ describe('ConnectedPositionStrategy', () => {
           fakeElementRef,
           {originX: 'start', originY: 'bottom'},
           {overlayX: 'start', overlayY: 'top'});
-      strategy.withScrollableContainers([new Scrollable(new FakeElementRef(scrollable), null)]);
+
+      strategy.withScrollableContainers([
+          new Scrollable(new FakeElementRef(scrollable), null, null, null)]);
 
       positionChangeHandler = jasmine.createSpy('positionChangeHandler');
       onPositionChangeSubscription = strategy.onPositionChange.subscribe(positionChangeHandler);

src/lib/core/overlay/scroll/scroll-dispatcher.spec.ts

@@ -41,32 +41,54 @@ describe('Scroll Dispatcher', () => {
 
     it('should notify through the directive and service that a scroll event occurred',
         fakeAsync(() => {
-      let hasDirectiveScrollNotified = false;
       // Listen for notifications from scroll directive
-      let scrollable = fixture.componentInstance.scrollable;
-      scrollable.elementScrolled().subscribe(() => { hasDirectiveScrollNotified = true; });
+      const scrollable = fixture.componentInstance.scrollable;
+      const directiveSpy = jasmine.createSpy('directive scroll callback');
+      scrollable.elementScrolled().subscribe(directiveSpy);
 
       // Listen for notifications from scroll service with a throttle of 100ms
       const throttleTime = 100;
-      let hasServiceScrollNotified = false;
-      scroll.scrolled(throttleTime, () => { hasServiceScrollNotified = true; });
+      const serviceSpy = jasmine.createSpy('service scroll callback');
+      scroll.scrolled(throttleTime, serviceSpy);
 
       // Emit a scroll event from the scrolling element in our component.
       // This event should be picked up by the scrollable directive and notify.
       // The notification should be picked up by the service.
       dispatchFakeEvent(fixture.componentInstance.scrollingElement.nativeElement, 'scroll');
 
       // The scrollable directive should have notified the service immediately.
-      expect(hasDirectiveScrollNotified).toBe(true);
+      expect(directiveSpy).toHaveBeenCalled();
 
       // Verify that the throttle is used, the service should wait for the throttle time until
       // sending the notification.
-      expect(hasServiceScrollNotified).toBe(false);
+      expect(serviceSpy).not.toHaveBeenCalled();
 
       // After the throttle time, the notification should be sent.
       tick(throttleTime);
-      expect(hasServiceScrollNotified).toBe(true);
+      expect(serviceSpy).toHaveBeenCalled();
     }));
+
+    it('should not execute the global events in the Angular zone', () => {
+      const spy = jasmine.createSpy('zone unstable callback');
+      const subscription = fixture.ngZone.onUnstable.subscribe(spy);
+
+      scroll.scrolled(0, () => {});
+      dispatchFakeEvent(document, 'scroll');
+      dispatchFakeEvent(window, 'resize');
+
+      expect(spy).not.toHaveBeenCalled();
+      subscription.unsubscribe();
+    });
+
+    it('should not execute the scrollable events in the Angular zone', () => {
+      const spy = jasmine.createSpy('zone unstable callback');
+      const subscription = fixture.ngZone.onUnstable.subscribe(spy);
+
+      dispatchFakeEvent(fixture.componentInstance.scrollingElement.nativeElement, 'scroll');
+
+      expect(spy).not.toHaveBeenCalled();
+      subscription.unsubscribe();
+    });
   });
 
   describe('Nested scrollables', () => {

src/lib/core/overlay/scroll/scroll-dispatcher.ts

@@ -1,4 +1,4 @@
-import {Injectable, ElementRef, Optional, SkipSelf} from '@angular/core';
+import {Injectable, ElementRef, Optional, SkipSelf, NgZone} from '@angular/core';
 import {Scrollable} from './scrollable';
 import {Subject} from 'rxjs/Subject';
 import {Observable} from 'rxjs/Observable';
@@ -17,6 +17,8 @@ export const DEFAULT_SCROLL_TIME = 20;
  */
 @Injectable()
 export class ScrollDispatcher {
+  constructor(private _ngZone: NgZone) { }
+
   /** Subject for notifying that a registered scrollable reference element has been scrolled. */
   _scrolled: Subject<void> = new Subject<void>();
 
@@ -69,10 +71,12 @@ export class ScrollDispatcher {
     this._scrolledCount++;
 
     if (!this._globalSubscription) {
-      this._globalSubscription = Observable.merge(
-        Observable.fromEvent(window.document, 'scroll'),
-        Observable.fromEvent(window, 'resize')
-      ).subscribe(() => this._notify());
+      this._globalSubscription = this._ngZone.runOutsideAngular(() => {
+        return Observable.merge(
+          Observable.fromEvent(window.document, 'scroll'),
+          Observable.fromEvent(window, 'resize')
+        ).subscribe(() => this._notify());
+      });
     }
 
     // Note that we need to do the subscribing from here, in order to be able to remove
@@ -118,13 +122,14 @@ export class ScrollDispatcher {
   }
 }
 
-export function SCROLL_DISPATCHER_PROVIDER_FACTORY(parentDispatcher: ScrollDispatcher) {
-  return parentDispatcher || new ScrollDispatcher();
+export function SCROLL_DISPATCHER_PROVIDER_FACTORY(parentDispatcher: ScrollDispatcher,
+                                                   ngZone: NgZone) {
+  return parentDispatcher || new ScrollDispatcher(ngZone);
 }
 
 export const SCROLL_DISPATCHER_PROVIDER = {
   // If there is already a ScrollDispatcher available, use that. Otherwise, provide a new one.
   provide: ScrollDispatcher,
-  deps: [[new Optional(), new SkipSelf(), ScrollDispatcher]],
+  deps: [[new Optional(), new SkipSelf(), ScrollDispatcher], NgZone],
   useFactory: SCROLL_DISPATCHER_PROVIDER_FACTORY
 };

src/lib/core/overlay/scroll/scrollable.ts

@@ -1,5 +1,6 @@
-import {Directive, ElementRef, OnInit, OnDestroy} from '@angular/core';
+import {Directive, ElementRef, OnInit, OnDestroy, NgZone, Renderer} from '@angular/core';
 import {Observable} from 'rxjs/Observable';
+import {Subject} from 'rxjs/Subject';
 import {ScrollDispatcher} from './scroll-dispatcher';
 import 'rxjs/add/observable/fromEvent';
 
@@ -13,22 +14,38 @@ import 'rxjs/add/observable/fromEvent';
   selector: '[cdk-scrollable]'
 })
 export class Scrollable implements OnInit, OnDestroy {
+  private _elementScrolled: Subject<Event> = new Subject();
+  private _scrollListener: Function;
+
   constructor(private _elementRef: ElementRef,
-              private _scroll: ScrollDispatcher) {}
+              private _scroll: ScrollDispatcher,
+              private _ngZone: NgZone,
+              private _renderer: Renderer) {}
 
   ngOnInit() {
+    this._scrollListener = this._ngZone.runOutsideAngular(() => {
+      return this._renderer.listen(this.getElementRef().nativeElement, 'scroll', (event: Event) => {
+        this._elementScrolled.next(event);
+      });
+    });
+
     this._scroll.register(this);
   }
 
   ngOnDestroy() {
     this._scroll.deregister(this);
+
+    if (this._scrollListener) {
+      this._scrollListener();
+      this._scrollListener = null;
+    }
   }
 
   /**
    * Returns observable that emits when a scroll event is fired on the host element.
    */
   elementScrolled(): Observable<any> {
-    return Observable.fromEvent(this._elementRef.nativeElement, 'scroll');
+    return this._elementScrolled.asObservable();
   }
 
   getElementRef(): ElementRef {

src/lib/core/testing/dispatch-events.ts

@@ -5,7 +5,7 @@ import {
 } from './event-objects';
 
 /** Shorthand to dispatch a fake event on a specified node. */
-export function dispatchFakeEvent(node: Node, type: string) {
+export function dispatchFakeEvent(node: Node | Window, type: string) {
   node.dispatchEvent(createFakeEvent(type));
 }