From 32d48cf9de5f01e0cf63ce7afd56686dfa34b31f Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 26 Apr 2016 14:01:09 -0700 Subject: [PATCH 1/2] perf($animate): listen for document visibility changes perf(ngAnimate): listen for document visibility changes Accessing the document for the hidden state is costly for platforms like Electron. Instead, listen for visibilitychange and store the state. (#14071) Closes #14066 --- src/AngularPublic.js | 2 ++ src/ng/animateRunner.js | 10 +++----- src/ng/document.js | 26 +++++++++++++++++++++ src/ngAnimate/animateQueue.js | 6 +++-- test/ng/animateRunnerSpec.js | 11 ++++----- test/ng/compileSpec.js | 43 +++++++++++++++++------------------ test/ng/documentSpec.js | 26 +++++++++++++++++++++ test/ngAnimate/animateSpec.js | 25 ++++++++++---------- 8 files changed, 99 insertions(+), 50 deletions(-) diff --git a/src/AngularPublic.js b/src/AngularPublic.js index 20c35f52f658..43fbe048c08e 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -65,6 +65,7 @@ $ControllerProvider, $DateProvider, $DocumentProvider, + $$IsDocumentHiddenProvider, $ExceptionHandlerProvider, $FilterProvider, $$ForceReflowProvider, @@ -227,6 +228,7 @@ function publishExternalAPI(angular) { $cacheFactory: $CacheFactoryProvider, $controller: $ControllerProvider, $document: $DocumentProvider, + $$isDocumentHidden: $$IsDocumentHiddenProvider, $exceptionHandler: $ExceptionHandlerProvider, $filter: $FilterProvider, $$forceReflow: $$ForceReflowProvider, diff --git a/src/ng/animateRunner.js b/src/ng/animateRunner.js index c1552faa18a2..adc052775501 100644 --- a/src/ng/animateRunner.js +++ b/src/ng/animateRunner.js @@ -28,8 +28,8 @@ var $$AnimateAsyncRunFactoryProvider = function() { }; var $$AnimateRunnerFactoryProvider = function() { - this.$get = ['$q', '$sniffer', '$$animateAsyncRun', '$document', '$timeout', - function($q, $sniffer, $$animateAsyncRun, $document, $timeout) { + this.$get = ['$q', '$sniffer', '$$animateAsyncRun', '$$isDocumentHidden', '$timeout', + function($q, $sniffer, $$animateAsyncRun, $$isDocumentHidden, $timeout) { var INITIAL_STATE = 0; var DONE_PENDING_STATE = 1; @@ -81,11 +81,7 @@ var $$AnimateRunnerFactoryProvider = function() { this._doneCallbacks = []; this._tick = function(fn) { - var doc = $document[0]; - - // the document may not be ready or attached - // to the module for some internal tests - if (doc && doc.hidden) { + if ($$isDocumentHidden()) { timeoutTick(fn); } else { rafTick(fn); diff --git a/src/ng/document.js b/src/ng/document.js index fcab252bf9ac..27f9a33951e8 100644 --- a/src/ng/document.js +++ b/src/ng/document.js @@ -30,3 +30,29 @@ function $DocumentProvider() { return jqLite(window.document); }]; } + + +/** + * @private + * Listens for document visibility change and makes the current status accessible. + */ +function $$IsDocumentHiddenProvider() { + this.$get = ['$document', '$rootScope', function($document, $rootScope) { + var doc = $document[0]; + var hidden = doc && doc.hidden; + + $document.on('visibilitychange', changeListener); + + $rootScope.$on('$destroy', function() { + $document.off('visibilitychange', changeListener); + }); + + function changeListener() { + hidden = doc.hidden; + } + + return function() { + return hidden; + }; + }]; +} diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index cda4c8746c7d..537aafafc341 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -102,8 +102,10 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap', '$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow', + '$$isDocumentHidden', function($$rAF, $rootScope, $rootElement, $document, $$HashMap, - $$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow) { + $$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow, + $$isDocumentHidden) { var activeAnimationsLookup = new $$HashMap(); var disabledElementsLookup = new $$HashMap(); @@ -367,7 +369,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { var isStructural = ['enter', 'move', 'leave'].indexOf(event) >= 0; - var documentHidden = $document[0].hidden; + var documentHidden = $$isDocumentHidden(); // this is a hard disable of all animations for the application or on // the element itself, therefore there is no need to continue further diff --git a/test/ng/animateRunnerSpec.js b/test/ng/animateRunnerSpec.js index 622da76ff22b..38d658cfec35 100644 --- a/test/ng/animateRunnerSpec.js +++ b/test/ng/animateRunnerSpec.js @@ -163,14 +163,13 @@ describe("$$AnimateRunner", function() { })); it("should use timeouts to trigger async operations when the document is hidden", function() { - var doc; + var hidden = true; module(function($provide) { - doc = jqLite({ - body: document.body, - hidden: true + + $provide.value('$$isDocumentHidden', function() { + return hidden; }); - $provide.value('$document', doc); }); inject(function($$AnimateRunner, $rootScope, $$rAF, $timeout) { @@ -184,7 +183,7 @@ describe("$$AnimateRunner", function() { $timeout.flush(); expect(spy).toHaveBeenCalled(); - doc[0].hidden = false; + hidden = false; spy = jasmine.createSpy(); runner = new $$AnimateRunner(); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 4117cbf709ef..eb66063d7ed5 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -7213,22 +7213,22 @@ describe('$compile', function() { }); inject(function($compile, $rootScope) { - expect(jqLiteCacheSize()).toEqual(0); + var cacheSize = jqLiteCacheSize(); element = $compile('
{{x}}
')($rootScope); - expect(jqLiteCacheSize()).toEqual(1); + expect(jqLiteCacheSize()).toEqual(cacheSize + 1); $rootScope.$apply('xs = [0,1]'); - expect(jqLiteCacheSize()).toEqual(2); + expect(jqLiteCacheSize()).toEqual(cacheSize + 2); $rootScope.$apply('xs = [0]'); - expect(jqLiteCacheSize()).toEqual(1); + expect(jqLiteCacheSize()).toEqual(cacheSize + 1); $rootScope.$apply('xs = []'); - expect(jqLiteCacheSize()).toEqual(1); + expect(jqLiteCacheSize()).toEqual(cacheSize + 1); element.remove(); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize + 0); }); }); @@ -7245,22 +7245,22 @@ describe('$compile', function() { }); inject(function($compile, $rootScope) { - expect(jqLiteCacheSize()).toEqual(0); + var cacheSize = jqLiteCacheSize(); element = $compile('
{{x}}
')($rootScope); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize); $rootScope.$apply('xs = [0,1]'); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize); $rootScope.$apply('xs = [0]'); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize); $rootScope.$apply('xs = []'); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize); element.remove(); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize); }); }); @@ -7276,26 +7276,26 @@ describe('$compile', function() { }); inject(function($compile, $rootScope) { - expect(jqLiteCacheSize()).toEqual(0); + var cacheSize = jqLiteCacheSize(); element = $compile('
{{x}}
')($rootScope); $rootScope.$apply('xs = [0,1]'); // At this point we have a bunch of comment placeholders but no real transcluded elements // So the cache only contains the root element's data - expect(jqLiteCacheSize()).toEqual(1); + expect(jqLiteCacheSize()).toEqual(cacheSize + 1); $rootScope.$apply('val = true'); // Now we have two concrete transcluded elements plus some comments so two more cache items - expect(jqLiteCacheSize()).toEqual(3); + expect(jqLiteCacheSize()).toEqual(cacheSize + 3); $rootScope.$apply('val = false'); // Once again we only have comments so no transcluded elements and the cache is back to just // the root element - expect(jqLiteCacheSize()).toEqual(1); + expect(jqLiteCacheSize()).toEqual(cacheSize + 1); element.remove(); // Now we've even removed the root element along with its cache - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(cacheSize + 0); }); }); @@ -7332,6 +7332,7 @@ describe('$compile', function() { }); inject(function($compile, $rootScope, $httpBackend, $timeout, $templateCache) { + var cacheSize = jqLiteCacheSize(); $httpBackend.whenGET('red.html').respond('

red.html

'); var template = $compile( '
' + @@ -7346,7 +7347,7 @@ describe('$compile', function() { $timeout.flush(); $httpBackend.flush(); expect(linkFn).not.toHaveBeenCalled(); - expect(jqLiteCacheSize()).toEqual(2); + expect(jqLiteCacheSize()).toEqual(cacheSize + 2); $templateCache.removeAll(); var destroyedScope = $rootScope.$new(); @@ -8129,9 +8130,7 @@ describe('$compile', function() { it('should not leak memory with nested transclusion', function() { inject(function($compile, $rootScope) { - var size; - - expect(jqLiteCacheSize()).toEqual(0); + var size, initialSize = jqLiteCacheSize(); element = jqLite('
  • {{n}} => EvenOdd
'); $compile(element)($rootScope.$new()); @@ -8145,7 +8144,7 @@ describe('$compile', function() { expect(jqLiteCacheSize()).toEqual(size); element.remove(); - expect(jqLiteCacheSize()).toEqual(0); + expect(jqLiteCacheSize()).toEqual(initialSize); }); }); }); diff --git a/test/ng/documentSpec.js b/test/ng/documentSpec.js index 3fbca1d7a048..349d12453913 100644 --- a/test/ng/documentSpec.js +++ b/test/ng/documentSpec.js @@ -27,3 +27,29 @@ describe('$document', function() { }); }); }); + + +describe('$$isDocumentHidden', function() { + it('should listen on the visibilitychange event', function() { + var doc; + + var spy = spyOn(document, 'addEventListener').and.callThrough(); + + inject(function($$isDocumentHidden, $document) { + expect(spy.calls.mostRecent().args[0]).toBe('visibilitychange'); + expect(spy.calls.mostRecent().args[1]).toEqual(jasmine.any(Function)); + expect($$isDocumentHidden()).toBeFalsy(); // undefined in browsers that don't support visibility + }); + + }); + + it('should remove the listener when the $rootScope is destroyed', function() { + var spy = spyOn(document, 'removeEventListener').and.callThrough(); + + inject(function($$isDocumentHidden, $rootScope) { + $rootScope.$destroy(); + expect(spy.calls.mostRecent().args[0]).toBe('visibilitychange'); + expect(spy.calls.mostRecent().args[1]).toEqual(jasmine.any(Function)); + }); + }); +}); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 2d1d89b256d5..6bd18d456b3f 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -157,14 +157,12 @@ describe("animations", function() { })); it("should skip animations entirely if the document is hidden", function() { - var doc; + var hidden = true; module(function($provide) { - doc = jqLite({ - body: document.body, - hidden: true + $provide.value('$$isDocumentHidden', function() { + return hidden; }); - $provide.value('$document', doc); }); inject(function($animate, $rootScope) { @@ -173,7 +171,7 @@ describe("animations", function() { expect(capturedAnimation).toBeFalsy(); expect(element[0].parentNode).toEqual(parent[0]); - doc[0].hidden = false; + hidden = false; $animate.leave(element); $rootScope.$digest(); @@ -2503,18 +2501,19 @@ describe("animations", function() { describe('because the document is hidden', function() { - beforeEach(module(function($provide) { - var doc = jqLite({ - body: document.body, - hidden: true + var hidden = true; + + beforeEach(function() { + module(function($provide) { + $provide.value('$$isDocumentHidden', function() { + return hidden; + }); }); - $provide.value('$document', doc); - })); + }); it('should trigger callbacks for an enter animation', inject(function($animate, $rootScope, $rootElement, $document) { - var callbackTriggered = false; var spy = jasmine.createSpy(); $animate.on('enter', jqLite($document[0].body), spy); From 59f822a1b269a14a73c75ee1e56a808563267c46 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Fri, 6 May 2016 13:40:38 +0200 Subject: [PATCH 2/2] fixup --- test/ng/documentSpec.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/ng/documentSpec.js b/test/ng/documentSpec.js index 349d12453913..5a6a8acc9fe5 100644 --- a/test/ng/documentSpec.js +++ b/test/ng/documentSpec.js @@ -30,15 +30,18 @@ describe('$document', function() { describe('$$isDocumentHidden', function() { + + it('should return false by default', inject(function($$isDocumentHidden, $document) { + expect($$isDocumentHidden()).toBeFalsy(); // undefined in browsers that don't support visibility + })); + it('should listen on the visibilitychange event', function() { - var doc; var spy = spyOn(document, 'addEventListener').and.callThrough(); inject(function($$isDocumentHidden, $document) { expect(spy.calls.mostRecent().args[0]).toBe('visibilitychange'); expect(spy.calls.mostRecent().args[1]).toEqual(jasmine.any(Function)); - expect($$isDocumentHidden()).toBeFalsy(); // undefined in browsers that don't support visibility }); });