Skip to content

Commit e9abd3b

Browse files
author
Dima Voytenko
authored
Remove onBodyAvailable from doc-state service. Start deprecation. (ampproject#22580)
* Remove onBodyAvailable from doc-state service. Start deprecation. * Switch amp-story to use viewer visibility * cleanup * rename documentState to globalDocumentState * cleanup * fixing tests * why is ampdoc tests failing? * test fixes * fixing tests
1 parent b42dbb5 commit e9abd3b

24 files changed

+76
-113
lines changed

build-system/tasks/presubmit-checks.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ const forbiddenTerms = {
204204
message: privateServiceFactory,
205205
whitelist: ['src/service/crypto-impl.js', 'src/runtime.js'],
206206
},
207-
'installDocumentStateService': {
207+
'installGlobalDocumentStateService': {
208208
message: privateServiceFactory,
209209
whitelist: ['src/service/document-state.js', 'src/runtime.js'],
210210
},
@@ -411,6 +411,15 @@ const forbiddenTerms = {
411411
'extensions/amp-consent/0.1/consent-state-manager.js',
412412
],
413413
},
414+
// Global documentState service.
415+
'globalDocumentStateFor': {
416+
message: 'Global document API. In the process of being deprecated.',
417+
whitelist: [
418+
'src/services.js',
419+
'src/service/viewer-impl.js',
420+
'src/service/vsync-impl.js',
421+
],
422+
},
414423
'getBaseCid': {
415424
message: requiresReviewPrivacy,
416425
whitelist: ['src/service/cid-impl.js', 'src/service/viewer-impl.js'],

extensions/amp-analytics/0.1/test/test-visibility-manager.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1052,7 +1052,7 @@ describes.realWin('VisibilityManager integrated', {amp: true}, env => {
10521052
eventResolver2 = resolve;
10531053
});
10541054

1055-
const docState = Services.documentStateFor(win);
1055+
const docState = Services.globalDocumentStateFor(win);
10561056
sandbox.stub(docState, 'isHidden').callsFake(() => false);
10571057
sandbox.stub(viewer, 'getFirstVisibleTime').callsFake(() => startTime);
10581058

extensions/amp-story-auto-ads/0.1/test/test-amp-story-auto-ads.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
import * as analytics from '../../../../src/analytics';
1717
import {AmpStoryAutoAds} from '../amp-story-auto-ads';
18+
import {Services} from '../../../../src/services';
1819
import {macroTask} from '../../../../testing/yield';
1920

2021
const NOOP = () => {};
@@ -34,6 +35,8 @@ describes.realWin(
3435

3536
beforeEach(() => {
3637
win = env.win;
38+
const viewer = Services.viewerForDoc(env.ampdoc);
39+
sandbox.stub(Services, 'viewerForDoc').returns(viewer);
3740
adElement = win.document.createElement('amp-story-auto-ads');
3841
storyElement = win.document.createElement('amp-story');
3942
win.document.body.appendChild(storyElement);

extensions/amp-story/1.0/amp-story.js

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,8 @@ export class AmpStory extends AMP.BaseElement {
341341
/** @private @const {!../../../src/service/platform-impl.Platform} */
342342
this.platform_ = Services.platformFor(this.win);
343343

344-
/** @private @const {!../../../src/service/document-state.DocumentState} */
345-
this.documentState_ = Services.documentStateFor(this.win);
344+
/** @private @const {!../../../src/service/viewer-impl.Viewer} */
345+
this.viewer_ = Services.viewerForDoc(this.element);
346346

347347
/**
348348
* Store the current paused state, to make sure the story does not play on
@@ -751,9 +751,7 @@ export class AmpStory extends AMP.BaseElement {
751751
}
752752
});
753753

754-
// TODO(#16795): Remove once the runtime triggers pause/resume callbacks
755-
// on document visibility change (eg: user switches tab).
756-
this.documentState_.onVisibilityChanged(() => this.onVisibilityChanged_());
754+
this.viewer_.onVisibilityChanged(() => this.onVisibilityChanged_());
757755

758756
this.getViewport().onResize(debounce(this.win, () => this.onResize(), 300));
759757
this.installGestureRecognizers_();
@@ -767,11 +765,9 @@ export class AmpStory extends AMP.BaseElement {
767765
// If the story is within a viewer that enabled the swipe capability, this
768766
// disables the navigation education overlay on the X axis to enable the
769767
// swipe to the next story feature.
770-
const viewerService = Services.viewerForDoc(this.element);
771-
const swipeRecognizer =
772-
viewerService && viewerService.hasCapability('swipe')
773-
? SwipeYRecognizer
774-
: SwipeXYRecognizer;
768+
const swipeRecognizer = this.viewer_.hasCapability('swipe')
769+
? SwipeYRecognizer
770+
: SwipeXYRecognizer;
775771

776772
// Shows "tap to navigate" hint when swiping.
777773
gestures.onGesture(swipeRecognizer, gesture => {
@@ -956,7 +952,7 @@ export class AmpStory extends AMP.BaseElement {
956952
// Preloads and prerenders the share menu.
957953
this.shareMenu_.build();
958954

959-
const infoDialog = Services.viewerForDoc(this.element).isEmbedded()
955+
const infoDialog = this.viewer_.isEmbedded()
960956
? new InfoDialog(this.win, this.element)
961957
: null;
962958
if (infoDialog) {
@@ -976,7 +972,7 @@ export class AmpStory extends AMP.BaseElement {
976972
// Story is being prerendered: resolve the layoutCallback when the first
977973
// page is built. Other pages will only build if the document becomes
978974
// visible.
979-
if (!Services.viewerForDoc(this.element).hasBeenVisible()) {
975+
if (!this.viewer_.hasBeenVisible()) {
980976
return whenUpgradedToCustomElement(firstPageEl).then(() =>
981977
firstPageEl.whenBuilt()
982978
);
@@ -1711,9 +1707,7 @@ export class AmpStory extends AMP.BaseElement {
17111707
* @private
17121708
*/
17131709
onVisibilityChanged_() {
1714-
this.documentState_.isHidden()
1715-
? this.pauseCallback()
1716-
: this.resumeCallback();
1710+
this.viewer_.isVisible() ? this.resumeCallback() : this.pauseCallback();
17171711
}
17181712

17191713
/**

extensions/amp-story/1.0/test/test-amp-story.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ describes.realWin(
9696
.stub(viewer, 'hasCapability')
9797
.withArgs('swipe')
9898
.returns(hasSwipeCapability);
99+
sandbox.stub(Services, 'viewerForDoc').returns(viewer);
99100

100101
const storeService = new AmpStoryStoreService(win);
101102
registerServiceBuilder(win, 'story-store', () => storeService);
@@ -688,9 +689,9 @@ describes.realWin(
688689
it('should pause the story when tab becomes inactive', () => {
689690
createPages(story.element, 2, ['cover', 'page-1']);
690691

691-
sandbox.stub(story.documentState_, 'isHidden').returns(true);
692+
sandbox.stub(story.viewer_, 'isVisible').returns(false);
692693
const onVisibilityChangedStub = sandbox.stub(
693-
story.documentState_,
694+
story.viewer_,
694695
'onVisibilityChanged'
695696
);
696697

@@ -710,9 +711,9 @@ describes.realWin(
710711
it('should play the story when tab becomes active', () => {
711712
createPages(story.element, 2, ['cover', 'page-1']);
712713

713-
sandbox.stub(story.documentState_, 'isHidden').returns(false);
714+
sandbox.stub(story.viewer_, 'isVisible').returns(true);
714715
const onVisibilityChangedStub = sandbox.stub(
715-
story.documentState_,
716+
story.viewer_,
716717
'onVisibilityChanged'
717718
);
718719

extensions/amp-story/1.0/test/test-full-bleed-animations.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {AmpStory} from '../amp-story';
2222
import {AmpStoryPage} from '../amp-story-page';
2323
import {AmpStoryStoreService} from '../amp-story-store-service';
2424
import {LocalizationService} from '../../../../src/service/localization';
25+
import {Services} from '../../../../src/services';
2526
import {
2627
calculateTargetScalingFactor,
2728
targetFitsWithinPage,
@@ -47,6 +48,9 @@ describes.realWin(
4748

4849
sandbox.stub(win.history, 'replaceState');
4950

51+
const viewer = Services.viewerForDoc(env.ampdoc);
52+
sandbox.stub(Services, 'viewerForDoc').returns(viewer);
53+
5054
const storeService = new AmpStoryStoreService(win);
5155
registerServiceBuilder(win, 'story-store', () => storeService);
5256

extensions/amp-story/1.0/test/test-live-story-manager.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import {AmpStory} from '../amp-story';
1818
import {AmpStoryPage} from '../amp-story-page';
1919
import {LiveStoryManager} from '../live-story-manager';
20+
import {Services} from '../../../../src/services';
2021
import {addAttributesToElement} from '../../../../src/dom';
2122

2223
describes.realWin('LiveStoryManager', {amp: true}, env => {
@@ -47,6 +48,8 @@ describes.realWin('LiveStoryManager', {amp: true}, env => {
4748

4849
beforeEach(() => {
4950
win = env.win;
51+
const viewer = Services.viewerForDoc(env.ampdoc);
52+
sandbox.stub(Services, 'viewerForDoc').returns(viewer);
5053
storyEl = win.document.createElement('amp-story');
5154
addAttributesToElement(storyEl, {
5255
'id': 'testStory',

src/dom.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ export function waitForChildPromise(parent, checkFunc) {
9090

9191
/**
9292
* Waits for document's body to be available and ready.
93-
* Will be deprecated soon; use {@link AmpDoc#waitForBodyOpen} or
94-
* @{link DocumentState#onBodyAvailable} instead.
9593
* @param {!Document} doc
9694
* @param {function()} callback
9795
*/

src/runtime.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import {
5858
import {installCidService} from './service/cid-impl';
5959
import {installCryptoService} from './service/crypto-impl';
6060
import {installDocumentInfoServiceForDoc} from './service/document-info-impl';
61-
import {installDocumentStateService} from './service/document-state';
61+
import {installGlobalDocumentStateService} from './service/document-state';
6262
import {installGlobalNavigationHandlerForDoc} from './service/navigation';
6363
import {installGlobalSubmitListenerForDoc} from './document-submit';
6464
import {installHiddenObserverForDoc} from './service/hidden-observer-impl';
@@ -100,7 +100,7 @@ const TAG = 'runtime';
100100
export function installRuntimeServices(global) {
101101
installCryptoService(global);
102102
installBatchedXhrService(global);
103-
installDocumentStateService(global);
103+
installGlobalDocumentStateService(global);
104104
installPlatformService(global);
105105
installTemplatesService(global);
106106
installTimerService(global);

src/service/document-state.js

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import {Observable} from '../observable';
1818
import {getVendorJsPropertyName} from '../style';
1919
import {registerServiceBuilder} from '../service';
20-
import {waitForChild} from '../dom';
2120

2221
/**
2322
*/
@@ -70,9 +69,6 @@ export class DocumentState {
7069
this.boundOnVisibilityChanged_
7170
);
7271
}
73-
74-
/** @private {?Observable} */
75-
this.bodyAvailableObservable_ = null;
7672
}
7773

7874
/**
@@ -112,41 +108,11 @@ export class DocumentState {
112108
onVisibilityChanged_() {
113109
this.visibilityObservable_.fire();
114110
}
115-
116-
/**
117-
* If body is already available, callback is called synchronously and null
118-
* is returned.
119-
* @param {function()} handler
120-
* @return {?UnlistenDef}
121-
*/
122-
onBodyAvailable(handler) {
123-
const doc = this.document_;
124-
if (doc.body) {
125-
handler();
126-
return null;
127-
}
128-
if (!this.bodyAvailableObservable_) {
129-
this.bodyAvailableObservable_ = new Observable();
130-
waitForChild(
131-
doc.documentElement,
132-
() => !!doc.body,
133-
this.onBodyAvailable_.bind(this)
134-
);
135-
}
136-
return this.bodyAvailableObservable_.add(handler);
137-
}
138-
139-
/** @private */
140-
onBodyAvailable_() {
141-
this.bodyAvailableObservable_.fire();
142-
this.bodyAvailableObservable_.removeAll();
143-
this.bodyAvailableObservable_ = null;
144-
}
145111
}
146112

147113
/**
148114
* @param {!Window} window
149115
*/
150-
export function installDocumentStateService(window) {
116+
export function installGlobalDocumentStateService(window) {
151117
registerServiceBuilder(window, 'documentState', DocumentState);
152118
}

0 commit comments

Comments
 (0)