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

Commit d4b05dd

Browse files
committed
revert: perf(ngAnimate): listen for document visibility changes
This reverts commit b377d6b. This commit introduces dependency on $document during the injector bootstrap. This is usually fine, except for cases when $document is mocked out in existing test suites. Since we don't provide an out of box mock, people have created their owns that don't sufficiently mock all apis. In these environments this change results in errors like $document.on is not a function, etc.
1 parent 59802bc commit d4b05dd

File tree

8 files changed

+50
-102
lines changed

8 files changed

+50
-102
lines changed

src/AngularPublic.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@
6565
$ControllerProvider,
6666
$DateProvider,
6767
$DocumentProvider,
68-
$$IsDocumentHiddenProvider,
6968
$ExceptionHandlerProvider,
7069
$FilterProvider,
7170
$$ForceReflowProvider,
@@ -228,7 +227,6 @@ function publishExternalAPI(angular) {
228227
$cacheFactory: $CacheFactoryProvider,
229228
$controller: $ControllerProvider,
230229
$document: $DocumentProvider,
231-
$$isDocumentHidden: $$IsDocumentHiddenProvider,
232230
$exceptionHandler: $ExceptionHandlerProvider,
233231
$filter: $FilterProvider,
234232
$$forceReflow: $$ForceReflowProvider,

src/ng/animateRunner.js

Lines changed: 7 additions & 3 deletions
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', '$$isDocumentHidden', '$timeout',
32-
function($q, $sniffer, $$animateAsyncRun, $$isDocumentHidden, $timeout) {
31+
this.$get = ['$q', '$sniffer', '$$animateAsyncRun', '$document', '$timeout',
32+
function($q, $sniffer, $$animateAsyncRun, $document, $timeout) {
3333

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

8282
this._doneCallbacks = [];
8383
this._tick = function(fn) {
84-
if ($$isDocumentHidden()) {
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) {
8589
timeoutTick(fn);
8690
} else {
8791
rafTick(fn);

src/ng/document.js

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,3 @@ 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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
102102

103103
this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap',
104104
'$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow',
105-
'$$isDocumentHidden',
106105
function($$rAF, $rootScope, $rootElement, $document, $$HashMap,
107-
$$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow,
108-
$$isDocumentHidden) {
106+
$$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow) {
109107

110108
var activeAnimationsLookup = new $$HashMap();
111109
var disabledElementsLookup = new $$HashMap();
@@ -369,7 +367,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
369367

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

372-
var documentHidden = $$isDocumentHidden();
370+
var documentHidden = $document[0].hidden;
373371

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

test/ng/animateRunnerSpec.js

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

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

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

175176
inject(function($$AnimateRunner, $rootScope, $$rAF, $timeout) {
@@ -183,7 +184,7 @@ describe("$$AnimateRunner", function() {
183184
$timeout.flush();
184185
expect(spy).toHaveBeenCalled();
185186

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

188189
spy = jasmine.createSpy();
189190
runner = new $$AnimateRunner();

test/ng/compileSpec.js

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7420,22 +7420,22 @@ describe('$compile', function() {
74207420
});
74217421

74227422
inject(function($compile, $rootScope) {
7423-
var cacheSize = jqLiteCacheSize();
7423+
expect(jqLiteCacheSize()).toEqual(0);
74247424

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

74287428
$rootScope.$apply('xs = [0,1]');
7429-
expect(jqLiteCacheSize()).toEqual(cacheSize + 2);
7429+
expect(jqLiteCacheSize()).toEqual(2);
74307430

74317431
$rootScope.$apply('xs = [0]');
7432-
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
7432+
expect(jqLiteCacheSize()).toEqual(1);
74337433

74347434
$rootScope.$apply('xs = []');
7435-
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
7435+
expect(jqLiteCacheSize()).toEqual(1);
74367436

74377437
element.remove();
7438-
expect(jqLiteCacheSize()).toEqual(cacheSize + 0);
7438+
expect(jqLiteCacheSize()).toEqual(0);
74397439
});
74407440
});
74417441

@@ -7452,22 +7452,22 @@ describe('$compile', function() {
74527452
});
74537453

74547454
inject(function($compile, $rootScope) {
7455-
var cacheSize = jqLiteCacheSize();
7455+
expect(jqLiteCacheSize()).toEqual(0);
74567456

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

74607460
$rootScope.$apply('xs = [0,1]');
7461-
expect(jqLiteCacheSize()).toEqual(cacheSize);
7461+
expect(jqLiteCacheSize()).toEqual(0);
74627462

74637463
$rootScope.$apply('xs = [0]');
7464-
expect(jqLiteCacheSize()).toEqual(cacheSize);
7464+
expect(jqLiteCacheSize()).toEqual(0);
74657465

74667466
$rootScope.$apply('xs = []');
7467-
expect(jqLiteCacheSize()).toEqual(cacheSize);
7467+
expect(jqLiteCacheSize()).toEqual(0);
74687468

74697469
element.remove();
7470-
expect(jqLiteCacheSize()).toEqual(cacheSize);
7470+
expect(jqLiteCacheSize()).toEqual(0);
74717471
});
74727472
});
74737473

@@ -7483,26 +7483,26 @@ describe('$compile', function() {
74837483
});
74847484

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

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

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

74987498
$rootScope.$apply('val = false');
74997499
// Once again we only have comments so no transcluded elements and the cache is back to just
75007500
// the root element
7501-
expect(jqLiteCacheSize()).toEqual(cacheSize + 1);
7501+
expect(jqLiteCacheSize()).toEqual(1);
75027502

75037503
element.remove();
75047504
// Now we've even removed the root element along with its cache
7505-
expect(jqLiteCacheSize()).toEqual(cacheSize + 0);
7505+
expect(jqLiteCacheSize()).toEqual(0);
75067506
});
75077507
});
75087508

@@ -7539,7 +7539,6 @@ describe('$compile', function() {
75397539
});
75407540

75417541
inject(function($compile, $rootScope, $httpBackend, $timeout, $templateCache) {
7542-
var cacheSize = jqLiteCacheSize();
75437542
$httpBackend.whenGET('red.html').respond('<p>red.html</p>');
75447543
var template = $compile(
75457544
'<div ng-controller="Leak">' +
@@ -7554,7 +7553,7 @@ describe('$compile', function() {
75547553
$timeout.flush();
75557554
$httpBackend.flush();
75567555
expect(linkFn).not.toHaveBeenCalled();
7557-
expect(jqLiteCacheSize()).toEqual(cacheSize + 2);
7556+
expect(jqLiteCacheSize()).toEqual(2);
75587557

75597558
$templateCache.removeAll();
75607559
var destroyedScope = $rootScope.$new();
@@ -8337,7 +8336,9 @@ describe('$compile', function() {
83378336

83388337
it('should not leak memory with nested transclusion', function() {
83398338
inject(function($compile, $rootScope) {
8340-
var size, initialSize = jqLiteCacheSize();
8339+
var size;
8340+
8341+
expect(jqLiteCacheSize()).toEqual(0);
83418342

83428343
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>');
83438344
$compile(element)($rootScope.$new());
@@ -8351,7 +8352,7 @@ describe('$compile', function() {
83518352
expect(jqLiteCacheSize()).toEqual(size);
83528353

83538354
element.remove();
8354-
expect(jqLiteCacheSize()).toEqual(initialSize);
8355+
expect(jqLiteCacheSize()).toEqual(0);
83558356
});
83568357
});
83578358
});

test/ng/documentSpec.js

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

test/ngAnimate/animateSpec.js

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,14 @@ describe("animations", function() {
157157
}));
158158

159159
it("should skip animations entirely if the document is hidden", function() {
160-
var hidden = true;
160+
var doc;
161161

162162
module(function($provide) {
163-
$provide.value('$$isDocumentHidden', function() {
164-
return hidden;
163+
doc = jqLite({
164+
body: document.body,
165+
hidden: true
165166
});
167+
$provide.value('$document', doc);
166168
});
167169

168170
inject(function($animate, $rootScope) {
@@ -171,7 +173,7 @@ describe("animations", function() {
171173
expect(capturedAnimation).toBeFalsy();
172174
expect(element[0].parentNode).toEqual(parent[0]);
173175

174-
hidden = false;
176+
doc[0].hidden = false;
175177

176178
$animate.leave(element);
177179
$rootScope.$digest();
@@ -2514,19 +2516,18 @@ describe("animations", function() {
25142516

25152517

25162518
describe('because the document is hidden', function() {
2517-
var hidden = true;
2518-
2519-
beforeEach(function() {
2520-
module(function($provide) {
2521-
$provide.value('$$isDocumentHidden', function() {
2522-
return hidden;
2523-
});
2519+
beforeEach(module(function($provide) {
2520+
var doc = jqLite({
2521+
body: document.body,
2522+
hidden: true
25242523
});
2525-
});
2524+
$provide.value('$document', doc);
2525+
}));
25262526

25272527
it('should trigger callbacks for an enter animation',
25282528
inject(function($animate, $rootScope, $rootElement, $document) {
25292529

2530+
var callbackTriggered = false;
25302531
var spy = jasmine.createSpy();
25312532
$animate.on('enter', jqLite($document[0].body), spy);
25322533

0 commit comments

Comments
 (0)