Skip to content

Commit

Permalink
Support visibility API in the ampdoc (#23799)
Browse files Browse the repository at this point in the history 
* Support visibility API in the ampdoc

* fixing integr tests

* review fixes

* nits

* fix preconnect

* test fixes

* wip

* test fixes

* test fixes
  • Loading branch information
Dima Voytenko authored Aug 19, 2019
1 parent 2fa7684 commit 0c5e03f
Show file tree
Hide file tree
Showing 22 changed files with 1,077 additions and 484 deletions.
16 changes: 8 additions & 8 deletions build-system/tasks/presubmit-checks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,6 @@ const forbiddenTerms = {
'src/service/viewer-impl.js',
],
},
'setViewerVisibilityState': {
message: privateServiceFactory,
whitelist: [
'src/runtime.js',
'src/service/core-services.js',
'src/service/viewer-impl.js',
],
},
'installViewportServiceForDoc': {
message: privateServiceFactory,
whitelist: [
Expand Down Expand Up @@ -607,6 +599,14 @@ const forbiddenTerms = {
'src/service/resources-impl.js',
],
},
'overrideVisibilityState': {
message: 'overrideVisibilityState is a restricted API.',
whitelist: [
'src/runtime.js',
'src/service/ampdoc-impl.js',
'src/service/viewer-impl.js',
],
},
'(win|Win)(dow)?(\\(\\))?\\.open\\W': {
message: 'Use dom.openWindowDialog',
whitelist: ['src/dom.js'],
Expand Down
19 changes: 10 additions & 9 deletions extensions/amp-a4a/0.1/test/test-amp-a4a.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ import {
assignAdUrlToError,
protectFunctionWrapper,
} from '../amp-a4a';
import {
AmpDoc,
installDocService,
updateFieModeForTesting,
} from '../../../../src/service/ampdoc-impl';
import {CONSENT_POLICY_STATE} from '../../../../src/consent-state';
import {Extensions} from '../../../../src/service/extensions-impl';
import {FetchMock, networkFailure} from './fetch-mock';
Expand All @@ -49,7 +54,6 @@ import {
} from '../../../../ads/google/a4a/traffic-experiments';
import {Services} from '../../../../src/services';
import {Signals} from '../../../../src/utils/signals';
import {Viewer} from '../../../../src/service/viewer-impl';
import {cancellation} from '../../../../src/error';
import {createElementWithAttributes} from '../../../../src/dom';
import {createIframePromise} from '../../../../testing/iframe';
Expand All @@ -59,10 +63,7 @@ import {
incrementLoadingAds,
is3pThrottled,
} from '../../../amp-ad/0.1/concurrent-load';
import {
installDocService,
updateFieModeForTesting,
} from '../../../../src/service/ampdoc-impl';

import {layoutRectLtwh} from '../../../../src/layout-rect';
import {resetScheduledElementForTesting} from '../../../../src/service/custom-element-registry';
import {data as testFragments} from './testdata/test_fragments';
Expand All @@ -84,7 +85,7 @@ describe('amp-a4a', () => {
let sandbox;
let fetchMock;
let getSigningServiceNamesMock;
let viewerWhenVisibleMock;
let whenVisibleMock;
let adResponse;
let onCreativeRenderSpy;
let getResourceStub;
Expand All @@ -98,8 +99,8 @@ describe('amp-a4a', () => {
);
onCreativeRenderSpy = sandbox.spy(AmpA4A.prototype, 'onCreativeRender');
getSigningServiceNamesMock.returns(['google']);
viewerWhenVisibleMock = sandbox.stub(Viewer.prototype, 'whenFirstVisible');
viewerWhenVisibleMock.returns(Promise.resolve());
whenVisibleMock = sandbox.stub(AmpDoc.prototype, 'whenFirstVisible');
whenVisibleMock.returns(Promise.resolve());
getResourceStub = sandbox.stub(AmpA4A.prototype, 'getResource');
getResourceStub.returns({
getUpgradeDelayMs: () => 12345,
Expand Down Expand Up @@ -2322,7 +2323,7 @@ describe('amp-a4a', () => {
return createIframePromise().then(fixture => {
setupForAdTesting(fixture);
let whenFirstVisibleResolve = null;
viewerWhenVisibleMock.returns(
whenVisibleMock.returns(
new Promise(resolve => {
whenFirstVisibleResolve = resolve;
})
Expand Down
5 changes: 1 addition & 4 deletions extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {Services} from '../../../../src/services';
import {adConfig} from '../../../../ads/_config';
import {createElementWithAttributes} from '../../../../src/dom';
import {macroTask} from '../../../../testing/yield';
import {stubService} from '../../../../testing/test-helper';
import {user} from '../../../../src/log';

function createAmpAd(win, attachToAmpdoc = false, ampdoc) {
Expand Down Expand Up @@ -79,9 +78,7 @@ describes.realWin(
win.document.body.appendChild(ad3p.element);
ad3p.buildCallback();
// Turn the doc to visible so prefetch will be proceeded.
stubService(sandbox, win, 'viewer', 'whenFirstVisible').returns(
whenFirstVisible
);
sandbox.stub(env.ampdoc, 'whenFirstVisible').returns(whenFirstVisible);
});

afterEach(() => {
Expand Down
16 changes: 8 additions & 8 deletions extensions/amp-analytics/0.1/test/test-visibility-manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, env => {
viewer = win.services.viewer.obj;
sandbox.stub(viewer, 'getFirstVisibleTime').callsFake(() => 1);
viewport = win.services.viewport.obj;
startVisibilityHandlerCount = viewer.visibilityObservable_.getHandlerCount();
startVisibilityHandlerCount = getVisibilityHandlerCount();

root = new VisibilityManagerForDoc(ampdoc);

Expand All @@ -94,6 +94,10 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, env => {
});
});

function getVisibilityHandlerCount() {
return ampdoc.visibilityStateHandlers_.getHandlerCount();
}

it('should initialize correctly backgrounded', () => {
viewer.setVisibilityState_(VisibilityState.HIDDEN);
root = new VisibilityManagerForDoc(ampdoc);
Expand Down Expand Up @@ -164,9 +168,7 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, env => {
});

it('should switch visibility based on viewer for main doc', () => {
expect(viewer.visibilityObservable_.getHandlerCount()).equal(
startVisibilityHandlerCount + 1
);
expect(getVisibilityHandlerCount()).equal(startVisibilityHandlerCount + 1);
expect(root.getRootVisibility()).to.equal(1);

// Go prerender.
Expand Down Expand Up @@ -338,9 +340,7 @@ describes.fakeWin('VisibilityManagerForDoc', {amp: true}, env => {
expect(otherUnsubscribes.callCount).to.equal(2);

// Viewer and viewport have been unsubscribed.
expect(viewer.visibilityObservable_.getHandlerCount()).equal(
startVisibilityHandlerCount
);
expect(getVisibilityHandlerCount()).equal(startVisibilityHandlerCount);

// Intersection observer disconnected.
expect(inOb.disconnected).to.be.true;
Expand Down Expand Up @@ -841,7 +841,7 @@ describes.realWin(
expect(root.getRootVisibility()).to.equal(0);
});

it('should initialize correctly foregrounded', () => {
it('should initialize correctly in foreground', () => {
expect(root.parent).to.equal(parentRoot);
expect(root.ampdoc).to.equal(ampdoc);
expect(root.getStartTime()).to.equal(embed.getStartTime());
Expand Down
50 changes: 14 additions & 36 deletions extensions/amp-install-serviceworker/0.1/test/test-amp-install-serviceworker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
} from '../../../../src/url';
import {loadPromise} from '../../../../src/event-helper';
import {
registerServiceBuilder,
registerServiceBuilderForDoc,
resetServiceForTesting,
} from '../../../../src/service';
Expand Down Expand Up @@ -55,7 +54,6 @@ describes.realWin(
let container;
let ampdoc;
let maybeInstallUrlRewriteStub;
let whenVisible;

beforeEach(() => {
doc = env.win.document;
Expand Down Expand Up @@ -94,13 +92,9 @@ describes.realWin(
},
},
};
whenVisible = Promise.resolve();
registerServiceBuilder(implementation.win, 'viewer', function() {
return {
whenFirstVisible: () => whenVisible,
isVisible: () => true,
};
});
const whenVisible = Promise.resolve();
sandbox.stub(ampdoc, 'whenFirstVisible').returns(whenVisible);
sandbox.stub(ampdoc, 'isVisible').returns(true);
implementation.buildCallback();
expect(calledSrc).to.be.undefined;
return Promise.all([whenVisible, loadPromise(implementation.win)]).then(
Expand Down Expand Up @@ -137,13 +131,9 @@ describes.realWin(
},
},
};
whenVisible = Promise.resolve();
registerServiceBuilder(implementation.win, 'viewer', function() {
return {
whenFirstVisible: () => whenVisible,
isVisible: () => true,
};
});
const whenVisible = Promise.resolve();
sandbox.stub(ampdoc, 'whenFirstVisible').returns(whenVisible);
sandbox.stub(ampdoc, 'isVisible').returns(true);
implementation.buildCallback();
expect(calledSrc).to.be.undefined;
return Promise.all([whenVisible, loadPromise(implementation.win)]).then(
Expand Down Expand Up @@ -207,13 +197,9 @@ describes.realWin(
},
},
};
whenVisible = Promise.resolve();
registerServiceBuilder(implementation.win, 'viewer', function() {
return {
whenFirstVisible: () => whenVisible,
isVisible: () => true,
};
});
const whenVisible = Promise.resolve();
sandbox.stub(ampdoc, 'whenFirstVisible').returns(whenVisible);
sandbox.stub(ampdoc, 'isVisible').returns(true);
implementation.buildCallback();
return Promise.all([whenVisible, loadPromise(implementation.win)]).then(
() => {
Expand Down Expand Up @@ -279,13 +265,9 @@ describes.realWin(
},
},
};
whenVisible = Promise.resolve();
registerServiceBuilder(implementation.win, 'viewer', function() {
return {
whenFirstVisible: () => whenVisible,
isVisible: () => true,
};
});
const whenVisible = Promise.resolve();
sandbox.stub(ampdoc, 'whenFirstVisible').returns(whenVisible);
sandbox.stub(ampdoc, 'isVisible').returns(true);
implementation.buildCallback();
return Promise.all([whenVisible, loadPromise(implementation.win)]).then(
() => {
Expand Down Expand Up @@ -418,12 +400,8 @@ describes.realWin(
};
});
whenVisible = Promise.resolve();
registerServiceBuilder(win, 'viewer', function() {
return {
whenFirstVisible: () => whenVisible,
isVisible: () => true,
};
});
sandbox.stub(ampdoc, 'whenFirstVisible').returns(whenVisible);
sandbox.stub(ampdoc, 'isVisible').returns(true);
});

function testIframe(callCount = 1) {
Expand Down
57 changes: 34 additions & 23 deletions extensions/amp-skimlinks/0.1/test/test-amp-skimlinks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ describes.fakeWin(
},
},
env => {
let ampSkimlinks, helpers;
let ampSkimlinks, helpers, ampdoc;

beforeEach(() => {
ampdoc = env.ampdoc;
helpers = helpersFactory(env);
ampSkimlinks = helpers.createAmpSkimlinks({
'publisher-code': 'pubIdXdomainId',
Expand All @@ -44,6 +45,10 @@ describes.fakeWin(
env.sandbox.restore();
});

function nextMicroTask() {
return Promise.resolve().then(() => Promise.resolve());
}

describe('skimOptions', () => {
it('Should raise an error if publisher-code is missing', () => {
ampSkimlinks = helpers.createAmpSkimlinks();
Expand Down Expand Up @@ -293,29 +298,32 @@ describes.fakeWin(
});

it('Should send the impression tracking if visible', () => {
return ampSkimlinks.onPageScanned_().then(() => {
const stub = ampSkimlinks.trackingService_.sendImpressionTracking;
expect(stub.calledOnce).to.be.true;
});
return ampSkimlinks
.onPageScanned_()
.then(nextMicroTask)
.then(() => {
const stub = ampSkimlinks.trackingService_.sendImpressionTracking;
expect(stub.calledOnce).to.be.true;
});
});

it('Should wait until visible to send the impression tracking', () => {
const isVisibleDefer = new Deferred();
const fakeViewer = {
whenFirstVisible: env.sandbox
.stub()
.returns(isVisibleDefer.promise),
};
helpers.mockServiceGetter('viewerForDoc', fakeViewer);

return ampSkimlinks.onPageScanned_().then(() => {
const stub = ampSkimlinks.trackingService_.sendImpressionTracking;
expect(stub.called).to.be.false;
isVisibleDefer.resolve();
return isVisibleDefer.promise.then(() => {
expect(stub.calledOnce).to.be.true;
sandbox
.stub(ampdoc, 'whenFirstVisible')
.returns(isVisibleDefer.promise);

return ampSkimlinks
.onPageScanned_()
.then(nextMicroTask)
.then(() => {
const stub = ampSkimlinks.trackingService_.sendImpressionTracking;
expect(stub.called).to.be.false;
isVisibleDefer.resolve();
return isVisibleDefer.promise.then(() => {
expect(stub.calledOnce).to.be.true;
});
});
});
});

it('Should update tracking info with the guid', () => {
Expand Down Expand Up @@ -350,10 +358,13 @@ describes.fakeWin(
});

it('Should send the impression tracking if visible', () => {
return ampSkimlinks.onPageScanned_().then(() => {
const stub = ampSkimlinks.trackingService_.sendImpressionTracking;
expect(stub.calledOnce).to.be.true;
});
return ampSkimlinks
.onPageScanned_()
.then(nextMicroTask)
.then(() => {
const stub = ampSkimlinks.trackingService_.sendImpressionTracking;
expect(stub.calledOnce).to.be.true;
});
});

it('Should wait until visible to send the impression tracking', () => {
Expand Down
12 changes: 9 additions & 3 deletions extensions/amp-smartlinks/0.1/test/test-amp-smartlinks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,15 @@ describes.fakeWin(
env.sandbox.spy(ampSmartlinks, 'getLinkmateOptions_');
env.sandbox.stub(xhr, 'fetchJson');

return ampSmartlinks.buildCallback().then(() => {
expect(ampSmartlinks.getLinkmateOptions_.calledOnce).to.be.true;
});
return (
ampSmartlinks
.buildCallback()
// Skip microtask.
.then(() => Promise.resolve())
.then(() => {
expect(ampSmartlinks.getLinkmateOptions_.calledOnce).to.be.true;
})
);
});
});

Expand Down
Loading

0 comments on commit 0c5e03f

Please sign in to comment.