Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit aa28e48

Browse files
NarretzNarretz
committed
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. Closes #14066
1 parent f31586d commit aa28e48

File tree

8 files changed

+101
-50
lines changed

8 files changed

+101
-50
lines changed

‎src/AngularPublic.js

+2
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
$CacheFactoryProvider,
6565
$ControllerProvider,
6666
$DocumentProvider,
67+
$$IsDocumentHiddenProvider,
6768
$ExceptionHandlerProvider,
6869
$FilterProvider,
6970
$$ForceReflowProvider,
@@ -226,6 +227,7 @@ function publishExternalAPI(angular) {
226227
$cacheFactory: $CacheFactoryProvider,
227228
$controller: $ControllerProvider,
228229
$document: $DocumentProvider,
230+
$$isDocumentHidden: $$IsDocumentHiddenProvider,
229231
$exceptionHandler: $ExceptionHandlerProvider,
230232
$filter: $FilterProvider,
231233
$$forceReflow: $$ForceReflowProvider,

‎src/ng/animateRunner.js

+3-7
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ var $$AnimateAsyncRunFactoryProvider = function() {
2828
};
2929

3030
var $$AnimateRunnerFactoryProvider = function() {
31-
this.$get = ['$q', '$sniffer', '$$animateAsyncRun', '$document', '$timeout',
32-
function($q, $sniffer, $$animateAsyncRun, $document, $timeout) {
31+
this.$get = ['$q', '$sniffer', '$$animateAsyncRun', '$$isDocumentHidden', '$timeout',
32+
function($q, $sniffer, $$animateAsyncRun, $$isDocumentHidden, $timeout) {
3333

3434
var INITIAL_STATE = 0;
3535
var DONE_PENDING_STATE = 1;
@@ -81,11 +81,7 @@ var $$AnimateRunnerFactoryProvider = function() {
8181

8282
this._doneCallbacks = [];
8383
this._tick = function(fn) {
84-
var doc = $document[0];
85-
86-
// the document may not be ready or attached
87-
// to the module for some internal tests
88-
if (doc && doc.hidden) {
84+
if ($$isDocumentHidden()) {
8985
timeoutTick(fn);
9086
} else {
9187
rafTick(fn);

‎src/ng/document.js

+26
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,29 @@ function $DocumentProvider() {
3030
return jqLite(window.document);
3131
}];
3232
}
33+
34+
35+
/**
36+
* @private
37+
* Listens for document visibility change and makes the current status accessible.
38+
*/
39+
function $$IsDocumentHiddenProvider() {
40+
this.$get = ['$document', '$rootScope', function($document, $rootScope) {
41+
var doc = $document[0];
42+
var hidden = doc && doc.hidden;
43+
44+
$document.on('visibilitychange', changeListener);
45+
46+
$rootScope.$on('$destroy', function() {
47+
$document.off('visibilitychange', changeListener);
48+
});
49+
50+
function changeListener() {
51+
hidden = doc.hidden;
52+
}
53+
54+
return function() {
55+
return hidden;
56+
};
57+
}];
58+
}

‎src/ngAnimate/animateQueue.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
9797

9898
this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap',
9999
'$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow',
100+
'$$isDocumentHidden',
100101
function($$rAF, $rootScope, $rootElement, $document, $$HashMap,
101-
$$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow) {
102+
$$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow,
103+
$$isDocumentHidden) {
102104

103105
var activeAnimationsLookup = new $$HashMap();
104106
var disabledElementsLookup = new $$HashMap();
@@ -331,7 +333,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
331333

332334
var isStructural = ['enter', 'move', 'leave'].indexOf(event) >= 0;
333335

334-
var documentHidden = $document[0].hidden;
336+
var documentHidden = $$isDocumentHidden();
335337

336338
// this is a hard disable of all animations for the application or on
337339
// the element itself, therefore there is no need to continue further

‎test/ng/animateRunnerSpec.js

+5-6
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,13 @@ describe("$$AnimateRunner", function() {
163163
}));
164164

165165
it("should use timeouts to trigger async operations when the document is hidden", function() {
166-
var doc;
166+
var hidden = true;
167167

168168
module(function($provide) {
169-
doc = jqLite({
170-
body: document.body,
171-
hidden: true
169+
170+
$provide.value('$$isDocumentHidden', function() {
171+
return hidden;
172172
});
173-
$provide.value('$document', doc);
174173
});
175174

176175
inject(function($$AnimateRunner, $rootScope, $$rAF, $timeout) {
@@ -184,7 +183,7 @@ describe("$$AnimateRunner", function() {
184183
$timeout.flush();
185184
expect(spy).toHaveBeenCalled();
186185

187-
doc[0].hidden = false;
186+
hidden = false;
188187

189188
spy = jasmine.createSpy();
190189
runner = new $$AnimateRunner();

‎test/ng/compileSpec.js

+21-22
Original file line numberDiff line numberDiff line change
@@ -6087,22 +6087,22 @@ describe('$compile', function() {
60876087
});
60886088

60896089
inject(function($compile, $rootScope) {
6090-
expect(jqLiteCacheSize()).toEqual(0);
6090+
var cacheSize = jqLiteCacheSize();
60916091

60926092
element = $compile('<div><div ng-repeat="x in xs" ng-if="x==1">{{x}}</div></div>')($rootScope);
6093-
expect(jqLiteCacheSize()).toEqual(1);
6093+
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
60946094

60956095
$rootScope.$apply('xs = [0,1]');
6096-
expect(jqLiteCacheSize()).toEqual(2);
6096+
expect(jqLiteCacheSize()).toEqual(cacheSize + 2);
60976097

60986098
$rootScope.$apply('xs = [0]');
6099-
expect(jqLiteCacheSize()).toEqual(1);
6099+
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
61006100

61016101
$rootScope.$apply('xs = []');
6102-
expect(jqLiteCacheSize()).toEqual(1);
6102+
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
61036103

61046104
element.remove();
6105-
expect(jqLiteCacheSize()).toEqual(0);
6105+
expect(jqLiteCacheSize()).toEqual(cacheSize + 0);
61066106
});
61076107
});
61086108

@@ -6119,22 +6119,22 @@ describe('$compile', function() {
61196119
});
61206120

61216121
inject(function($compile, $rootScope) {
6122-
expect(jqLiteCacheSize()).toEqual(0);
6122+
var cacheSize = jqLiteCacheSize();
61236123

61246124
element = $compile('<div><div ng-repeat="x in xs" ng-if="x==1">{{x}}</div></div>')($rootScope);
6125-
expect(jqLiteCacheSize()).toEqual(0);
6125+
expect(jqLiteCacheSize()).toEqual(cacheSize);
61266126

61276127
$rootScope.$apply('xs = [0,1]');
6128-
expect(jqLiteCacheSize()).toEqual(0);
6128+
expect(jqLiteCacheSize()).toEqual(cacheSize);
61296129

61306130
$rootScope.$apply('xs = [0]');
6131-
expect(jqLiteCacheSize()).toEqual(0);
6131+
expect(jqLiteCacheSize()).toEqual(cacheSize);
61326132

61336133
$rootScope.$apply('xs = []');
6134-
expect(jqLiteCacheSize()).toEqual(0);
6134+
expect(jqLiteCacheSize()).toEqual(cacheSize);
61356135

61366136
element.remove();
6137-
expect(jqLiteCacheSize()).toEqual(0);
6137+
expect(jqLiteCacheSize()).toEqual(cacheSize);
61386138
});
61396139
});
61406140

@@ -6150,26 +6150,26 @@ describe('$compile', function() {
61506150
});
61516151

61526152
inject(function($compile, $rootScope) {
6153-
expect(jqLiteCacheSize()).toEqual(0);
6153+
var cacheSize = jqLiteCacheSize();
61546154
element = $compile('<div><div ng-repeat="x in xs" ng-if="val">{{x}}</div></div>')($rootScope);
61556155

61566156
$rootScope.$apply('xs = [0,1]');
61576157
// At this point we have a bunch of comment placeholders but no real transcluded elements
61586158
// So the cache only contains the root element's data
6159-
expect(jqLiteCacheSize()).toEqual(1);
6159+
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
61606160

61616161
$rootScope.$apply('val = true');
61626162
// Now we have two concrete transcluded elements plus some comments so two more cache items
6163-
expect(jqLiteCacheSize()).toEqual(3);
6163+
expect(jqLiteCacheSize()).toEqual(cacheSize + 3);
61646164

61656165
$rootScope.$apply('val = false');
61666166
// Once again we only have comments so no transcluded elements and the cache is back to just
61676167
// the root element
6168-
expect(jqLiteCacheSize()).toEqual(1);
6168+
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
61696169

61706170
element.remove();
61716171
// Now we've even removed the root element along with its cache
6172-
expect(jqLiteCacheSize()).toEqual(0);
6172+
expect(jqLiteCacheSize()).toEqual(cacheSize + 0);
61736173
});
61746174
});
61756175

@@ -6206,6 +6206,7 @@ describe('$compile', function() {
62066206
});
62076207

62086208
inject(function($compile, $rootScope, $httpBackend, $timeout, $templateCache) {
6209+
var cacheSize = jqLiteCacheSize();
62096210
$httpBackend.whenGET('red.html').respond('<p>red.html</p>');
62106211
var template = $compile(
62116212
'<div ng-controller="Leak">' +
@@ -6220,7 +6221,7 @@ describe('$compile', function() {
62206221
$timeout.flush();
62216222
$httpBackend.flush();
62226223
expect(linkFn).not.toHaveBeenCalled();
6223-
expect(jqLiteCacheSize()).toEqual(2);
6224+
expect(jqLiteCacheSize()).toEqual(cacheSize + 2);
62246225

62256226
$templateCache.removeAll();
62266227
var destroyedScope = $rootScope.$new();
@@ -6983,9 +6984,7 @@ describe('$compile', function() {
69836984

69846985
it('should not leak memory with nested transclusion', function() {
69856986
inject(function($compile, $rootScope) {
6986-
var size;
6987-
6988-
expect(jqLiteCacheSize()).toEqual(0);
6987+
var size, initialSize = jqLiteCacheSize();
69896988

69906989
element = jqLite('<div><ul><li ng-repeat="n in nums">{{n}} => <i ng-if="0 === n%2">Even</i><i ng-if="1 === n%2">Odd</i></li></ul></div>');
69916990
$compile(element)($rootScope.$new());
@@ -6999,7 +6998,7 @@ describe('$compile', function() {
69996998
expect(jqLiteCacheSize()).toEqual(size);
70006999

70017000
element.remove();
7002-
expect(jqLiteCacheSize()).toEqual(0);
7001+
expect(jqLiteCacheSize()).toEqual(initialSize);
70037002
});
70047003
});
70057004
});

‎test/ng/documentSpec.js

+28
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,31 @@ describe('$document', function() {
2727
});
2828
});
2929
});
30+
31+
32+
describe('$$isDocumentHidden', function() {
33+
it('should return false by default', inject(function($$isDocumentHidden, $document) {
34+
expect($$isDocumentHidden()).toBeFalsy(); // undefined in browsers that don't support visibility
35+
}));
36+
37+
it('should listen on the visibilitychange event', function() {
38+
var spy = spyOn(document, 'addEventListener').andCallThrough();
39+
40+
inject(function($$isDocumentHidden, $document) {
41+
expect(spy.calls.mostRecent.args[0]).toBe('visibilitychange');
42+
expect(spy.calls.mostRecent.args[1]).toEqual(jasmine.any(Function));
43+
expect($$isDocumentHidden()).toBeFalsy(); // undefined in browsers that don't support visibility
44+
});
45+
46+
});
47+
48+
it('should remove the listener when the $rootScope is destroyed', function() {
49+
var spy = spyOn(document, 'removeEventListener').andCallThrough();
50+
51+
inject(function($$isDocumentHidden, $rootScope) {
52+
$rootScope.$destroy();
53+
expect(spy.calls.mostRecent.args[0]).toBe('visibilitychange');
54+
expect(spy.calls.mostRecent.args[1]).toEqual(jasmine.any(Function));
55+
});
56+
});
57+
});

‎test/ngAnimate/animateSpec.js

+12-13
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,12 @@ describe("animations", function() {
156156
}));
157157

158158
it("should skip animations entirely if the document is hidden", function() {
159-
var doc;
159+
var hidden = true;
160160

161161
module(function($provide) {
162-
doc = jqLite({
163-
body: document.body,
164-
hidden: true
162+
$provide.value('$$isDocumentHidden', function() {
163+
return hidden;
165164
});
166-
$provide.value('$document', doc);
167165
});
168166

169167
inject(function($animate, $rootScope) {
@@ -172,7 +170,7 @@ describe("animations", function() {
172170
expect(capturedAnimation).toBeFalsy();
173171
expect(element[0].parentNode).toEqual(parent[0]);
174172

175-
doc[0].hidden = false;
173+
hidden = false;
176174

177175
$animate.leave(element);
178176
$rootScope.$digest();
@@ -2271,18 +2269,19 @@ describe("animations", function() {
22712269

22722270

22732271
describe('because the document is hidden', function() {
2274-
beforeEach(module(function($provide) {
2275-
var doc = jqLite({
2276-
body: document.body,
2277-
hidden: true
2272+
var hidden = true;
2273+
2274+
beforeEach(function() {
2275+
module(function($provide) {
2276+
$provide.value('$$isDocumentHidden', function() {
2277+
return hidden;
2278+
});
22782279
});
2279-
$provide.value('$document', doc);
2280-
}));
2280+
});
22812281

22822282
it('should trigger callbacks for an enter animation',
22832283
inject(function($animate, $rootScope, $rootElement, $document) {
22842284

2285-
var callbackTriggered = false;
22862285
var spy = jasmine.createSpy();
22872286
$animate.on('enter', jqLite($document[0].body), spy);
22882287

0 commit comments

Comments
 (0)