Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove visibilityState from viewer #24566

Merged
merged 6 commits into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions ads/google/a4a/test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function setupForAdTesting(fixture) {
// functions.
function noopMethods(
impl,
doc,
ampdoc,
sandbox,
pageLayoutBox = {
top: 11,
Expand All @@ -81,7 +81,7 @@ function noopMethods(
impl.element.build = noop;
impl.element.getPlaceholder = noop;
impl.element.createPlaceholder = noop;
sandbox.stub(impl, 'getAmpDoc').returns(doc);
sandbox.stub(impl, 'getAmpDoc').returns(ampdoc);
sandbox.stub(impl, 'getPageLayoutBox').returns(pageLayoutBox);
}

Expand Down Expand Up @@ -331,7 +331,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
return fixture.addElement(elem).then(() => {
return googleAdUrl(impl, '', 0, [], []).then(url1 => {
expect(url1).to.match(/ady=11/);
Expand All @@ -352,7 +352,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
const getRect = () => {
return {'width': 100, 'height': 200};
};
Expand Down Expand Up @@ -385,7 +385,7 @@ describe('Google A4A utils', () => {
'data-experiment-id': '123,456',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
return fixture.addElement(elem).then(() => {
return googleAdUrl(impl, '', 0, {}, ['789', '098']).then(url1 => {
expect(url1).to.match(/eid=123%2C456%2C789%2C098/);
Expand All @@ -405,7 +405,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
impl.win.AMP_CONFIG = {type: 'production'};
impl.win.location.hash = 'foo,deid=123456,654321,bar';
return fixture.addElement(elem).then(() => {
Expand All @@ -427,7 +427,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
impl.win.gaGlobal = {cid: 'foo', hid: 'bar'};
return fixture.addElement(elem).then(() => {
return googleAdUrl(impl, '', 0, [], []).then(url => {
Expand All @@ -449,7 +449,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
const createElementStub = sandbox.stub(
impl.win.document,
'createElement'
Expand Down Expand Up @@ -478,7 +478,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
const createElementStub = sandbox.stub(
impl.win.document,
'createElement'
Expand All @@ -505,7 +505,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
impl.win.SVGElement = undefined;
const createElementStub = sandbox.stub(
impl.win.document,
Expand Down Expand Up @@ -535,7 +535,7 @@ describe('Google A4A utils', () => {
'height': '50',
});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
sandbox
.stub(Services.viewerForDoc(impl.getAmpDoc()), 'getReferrerUrl')
.returns(new Promise(() => {}));
Expand Down Expand Up @@ -564,7 +564,7 @@ describe('Google A4A utils', () => {
doc.win = fixture.win;
const elem = createElementWithAttributes(doc, 'amp-a4a', {});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox);
noopMethods(impl, fixture.ampdoc, sandbox);
return fixture.addElement(elem).then(() => {
return googleAdUrl(impl, '', Date.now(), [], []).then(url => {
expect(url).to.match(/[&?]bdt=[1-9][0-9]*[&$]/);
Expand All @@ -580,7 +580,7 @@ describe('Google A4A utils', () => {
doc.win = fixture.win;
const elem = createElementWithAttributes(doc, 'amp-a4a', {});
const impl = new MockA4AImpl(elem);
noopMethods(impl, doc, sandbox, {
noopMethods(impl, fixture.ampdoc, sandbox, {
top: 0,
left: 0,
right: 0,
Expand Down Expand Up @@ -902,6 +902,7 @@ describe('Google A4A utils', () => {
describe('variables for amp-analytics', () => {
let a4a;
let sandbox;
let ampdoc;

beforeEach(() => {
sandbox = sinon.sandbox;
Expand All @@ -913,7 +914,8 @@ describe('Google A4A utils', () => {
'type': 'adsense',
'data-amp-slot-index': '4',
});
element.getAmpDoc = () => fixture.doc;
ampdoc = fixture.ampdoc;
element.getAmpDoc = () => ampdoc;
a4a = new MockA4AImpl(element);
});
});
Expand Down Expand Up @@ -957,9 +959,7 @@ describe('Google A4A utils', () => {
});

it('should include viewer lastVisibleTime', () => {
const getLastVisibleTime = () => 300;
const viewerStub = sandbox.stub(Services, 'viewerForDoc');
viewerStub.returns({getLastVisibleTime});
sandbox.stub(ampdoc, 'getLastVisibleTime').returns(300);

const vars = getCsiAmpAnalyticsVariables('trigger', a4a, null);
expect(vars['viewerLastVisibleTime']).to.be.a('number');
Expand Down
2 changes: 1 addition & 1 deletion ads/google/a4a/traffic-experiments.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const SINGLE_PASS_EXPERIMENT_IDS = {
*/
export function extractUrlExperimentId(win, element) {
const expParam =
Services.viewerForDoc(element).getParam('exp') ||
Services.ampdoc(element).getParam('exp') ||
parseQueryString(win.location.search)['exp'];
if (!expParam) {
return null;
Expand Down
5 changes: 2 additions & 3 deletions ads/google/a4a/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ export function googlePageParameters(a4a, startTime) {
const viewport = Services.viewportForDoc(ampDoc);
const viewportRect = viewport.getRect();
const viewportSize = viewport.getSize();
const visibilityState = Services.viewerForDoc(ampDoc).getVisibilityState();
const visibilityState = ampDoc.getVisibilityState();
return {
'is_amp': a4a.isXhrAllowed()
? AmpAdImplementation.AMP_AD_XHR_TO_IFRAME_OR_AMP
Expand Down Expand Up @@ -626,12 +626,11 @@ export function getCsiAmpAnalyticsConfig() {
export function getCsiAmpAnalyticsVariables(analyticsTrigger, a4a, qqid) {
const {win} = a4a;
const ampdoc = a4a.getAmpDoc();
const viewer = Services.viewerForDoc(ampdoc);
const navStart = getNavigationTiming(win, 'navigationStart');
const vars = /** @type {!JsonObject} */ ({
'correlator': getCorrelator(win, ampdoc),
'slotId': a4a.element.getAttribute('data-amp-slot-index'),
'viewerLastVisibleTime': viewer.getLastVisibleTime() - navStart,
'viewerLastVisibleTime': ampdoc.getLastVisibleTime() - navStart,
});
if (qqid) {
vars['qqid'] = qqid;
Expand Down
5 changes: 3 additions & 2 deletions builtins/amp-pixel.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ export class AmpPixel extends BaseElement {
return;
}
// Trigger, but only when visible.
const viewer = Services.viewerForDoc(this.getAmpDoc());
viewer.whenFirstVisible().then(this.trigger_.bind(this));
this.getAmpDoc()
.whenFirstVisible()
.then(this.trigger_.bind(this));
}

/**
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ export class AmpA4A extends AMP.BaseElement {
this.uiHandler = new AMP.AmpAdUIHandler(this);

const verifier = signatureVerifierFor(this.win);
this.keysetPromise_ = Services.viewerForDoc(this.getAmpDoc())
this.keysetPromise_ = this.getAmpDoc()
.whenFirstVisible()
.then(() => {
this.getSigningServiceNames().forEach(signingServiceName => {
Expand Down Expand Up @@ -632,7 +632,7 @@ export class AmpA4A extends AMP.BaseElement {
// - Rendering fails => return false
// - Chain cancelled => don't return; drop error
// - Uncaught error otherwise => don't return; percolate error up
this.adPromise_ = Services.viewerForDoc(this.getAmpDoc())
this.adPromise_ = this.getAmpDoc()
.whenFirstVisible()
.then(() => {
checkStillCurrent();
Expand Down Expand Up @@ -997,7 +997,7 @@ export class AmpA4A extends AMP.BaseElement {
this.isRelayoutNeededFlag = true;
this.getResource().layoutCanceled();
// Only Require relayout after page visible
Services.viewerForDoc(this.getAmpDoc())
this.getAmpDoc()
.whenNextVisible()
.then(() => {
Services.ownersForDoc(this.getAmpDoc())./*OK*/ requireLayout(
Expand Down
3 changes: 1 addition & 2 deletions extensions/amp-a4a/0.1/amp-ad-network-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

import {FailureType, RecoveryModeType} from './amp-ad-type-defs';
import {Services} from '../../../src/services';
import {dev, devAssert} from '../../../src/log';
import {isLayoutSizeDefined} from '../../../src/layout';
import {map} from '../../../src/utils/object';
Expand Down Expand Up @@ -150,7 +149,7 @@ export class AmpAdNetworkBase extends AMP.BaseElement {
* @private
*/
sendRequest_() {
this.adResponsePromise_ = Services.viewerForDoc(this.getAmpDoc())
this.adResponsePromise_ = this.getAmpDoc()
.whenFirstVisible()
.then(() => {
const url = this.getRequestUrl();
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-a4a/0.1/test/test-amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -2544,7 +2544,7 @@ describe('amp-a4a', () => {
const a4aElement = createA4aElement(fixture.doc);
const a4a = new MockA4AImpl(a4aElement);
a4a.adPromise_ = Promise.resolve();
a4a.getAmpDoc = () => a4a.win.document;
a4a.getAmpDoc = () => fixture.ampdoc;
a4a.getResource = () => {
return {
layoutCanceled: () => {},
Expand Down Expand Up @@ -2583,7 +2583,7 @@ describe('amp-a4a', () => {
const a4aElement = createA4aElement(fixture.doc);
const a4a = new MockA4AImpl(a4aElement);
a4a.adPromise_ = Promise.resolve();
a4a.getAmpDoc = () => a4a.win.document;
a4a.getAmpDoc = () => fixture.ampdoc;
a4a.getResource = () => {
return {
layoutCanceled: () => {},
Expand Down Expand Up @@ -2622,7 +2622,7 @@ describe('amp-a4a', () => {
const a4aElement = createA4aElement(fixture.doc);
const a4a = new MockA4AImpl(a4aElement);
a4a.adPromise_ = null;
a4a.getAmpDoc = () => a4a.win.document;
a4a.getAmpDoc = () => fixture.ampdoc;
a4a.getResource = () => {
return {
layoutCanceled: () => {},
Expand Down
5 changes: 1 addition & 4 deletions extensions/amp-access/0.1/amp-access-server-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,6 @@ export class AccessServerJwtAdapter {
/** @private @const */
this.clientAdapter_ = new AccessClientAdapter(ampdoc, configJson, context);

/** @private @const {!../../../src/service/viewer-interface.ViewerInterface} */
this.viewer_ = Services.viewerForDoc(ampdoc);

/** @const @private {!../../../src/service/xhr-impl.Xhr} */
this.xhr_ = Services.xhrFor(ampdoc.win);

Expand All @@ -116,7 +113,7 @@ export class AccessServerJwtAdapter {
this.isProxyOrigin_ = isProxyOrigin(ampdoc.win.location) || isInExperiment;

const serviceUrlOverride = isInExperiment
? this.viewer_.getParam('serverAccessService')
? ampdoc.getParam('serverAccessService')
: null;

/** @private @const {string} */
Expand Down
5 changes: 1 addition & 4 deletions extensions/amp-access/0.1/amp-access-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ export class AccessServerAdapter {
/** @private @const */
this.clientAdapter_ = new AccessClientAdapter(ampdoc, configJson, context);

/** @private @const {!../../../src/service/viewer-interface.ViewerInterface} */
this.viewer_ = Services.viewerForDoc(ampdoc);

/** @const @protected {!../../../src/service/xhr-impl.Xhr} */
this.xhr_ = Services.xhrFor(ampdoc.win);

Expand All @@ -99,7 +96,7 @@ export class AccessServerAdapter {
this.isProxyOrigin_ = isProxyOrigin(ampdoc.win.location) || isInExperiment;

const serviceUrlOverride = isInExperiment
? this.viewer_.getParam('serverAccessService')
? ampdoc.getParam('serverAccessService')
: null;

/** @private @const {string} */
Expand Down
12 changes: 6 additions & 6 deletions extensions/amp-access/0.1/amp-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ export class AccessService {
runAuthorization_(opt_disableFallback) {
this.toggleTopClass_('amp-access-loading', true);

const authorizations = this.viewer_.whenFirstVisible().then(() => {
const authorizations = this.ampdoc.whenFirstVisible().then(() => {
return Promise.all(
this.sources_.map(source => this.runOneAuthorization_(source))
);
Expand Down Expand Up @@ -559,11 +559,11 @@ export class AccessService {
}
this.reportViewPromise_ = null;
this.ampdoc.whenReady().then(() => {
if (this.viewer_.isVisible()) {
if (this.ampdoc.isVisible()) {
this.reportWhenViewed_(timeToView);
}
this.viewer_.onVisibilityChanged(() => {
if (this.viewer_.isVisible()) {
this.ampdoc.onVisibilityChanged(() => {
if (this.ampdoc.isVisible()) {
this.reportWhenViewed_(timeToView);
}
});
Expand Down Expand Up @@ -624,8 +624,8 @@ export class AccessService {
return new Promise((resolve, reject) => {
// 1. Document becomes invisible again: cancel.
unlistenSet.push(
this.viewer_.onVisibilityChanged(() => {
if (!this.viewer_.isVisible()) {
this.ampdoc.onVisibilityChanged(() => {
if (!this.ampdoc.isVisible()) {
reject(cancellation());
}
})
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-access/0.1/login-dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const RETURN_URL_REGEX = new RegExp('RETURN_URL');
*/
export function createLoginDialog(ampdoc, urlOrPromise) {
const viewer = Services.viewerForDoc(ampdoc);
const overrideDialog = parseInt(viewer.getParam('dialog'), 10);
const overrideDialog = parseInt(ampdoc.getParam('dialog'), 10);
if (overrideDialog) {
return new ViewerLoginDialog(viewer, urlOrPromise);
}
Expand Down
Loading