Skip to content

Commit

Permalink
🐛 Use timeOrigin for getFirstVisibleTime when page starts visible (am…
Browse files Browse the repository at this point in the history
…pproject#36273)

* Use timeOrigin for getFirstVisibleTime when page starts visible

* Update old tests

* Add tests for firstVisibleTime

* Fix global conflict

* Fix tests
  • Loading branch information
jridgewell authored and estherkim committed Nov 17, 2021
1 parent f8770c1 commit 88e3b29
Show file tree
Hide file tree
Showing 16 changed files with 128 additions and 32 deletions.
4 changes: 4 additions & 0 deletions extensions/amp-access/0.1/test/test-login-dialog.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {Services} from '#service';
import {installDocService} from '#service/ampdoc-impl';

import {FakePerformance} from '#testing/fake-dom';

import {WebLoginDialog, openLoginDialog} from '../login-dialog';

const RETURN_URL_ESC = encodeURIComponent(
Expand Down Expand Up @@ -40,6 +42,7 @@ describes.sandboxed('ViewerLoginDialog', {}, (env) => {
setInterval: () => {
throw new Error('Not allowed');
},
performance: new FakePerformance(window),
};
windowApi.document.defaultView = windowApi;
installDocService(windowApi, /* isSingleDoc */ true);
Expand Down Expand Up @@ -178,6 +181,7 @@ describes.sandboxed('WebLoginDialog', {}, (env) => {
setTimeout: (callback, t) => window.setTimeout(callback, t),
setInterval: (callback, t) => window.setInterval(callback, t),
clearInterval: (intervalId) => window.clearInterval(intervalId),
performance: new FakePerformance(window),
};
windowApi = windowObj;
windowApi.document.defaultView = windowApi;
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-experiment/0.1/test/test-variant.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {Services} from '#service';
import {AmpDocSingle} from '#service/ampdoc-impl';

import {FakePerformance} from '#testing/fake-dom';

import {allocateVariant} from '../variant';

describes.sandboxed('allocateVariant', {}, (env) => {
Expand All @@ -23,6 +25,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
nodeType: /* DOCUMENT */ 9,
body: {},
},
performance: new FakePerformance(window),
};
fakeWin.document.defaultView = fakeWin;
ampdoc = new AmpDocSingle(fakeWin);
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-experiment/1.0/test/test-variant.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {Services} from '#service';
import {AmpDocSingle} from '#service/ampdoc-impl';

import {FakePerformance} from '#testing/fake-dom';

import {allocateVariant} from '../variant';

describes.sandboxed('allocateVariant', {}, (env) => {
Expand All @@ -24,6 +26,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
nodeType: /* DOCUMENT */ 9,
body: {},
},
performance: new FakePerformance(window),
};
fakeWin.document.defaultView = fakeWin;
ampdoc = new AmpDocSingle(fakeWin);
Expand Down
19 changes: 15 additions & 4 deletions src/service/ampdoc-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -614,14 +614,25 @@ export class AmpDoc {
}

if (this.visibilityState_ != visibilityState) {
this.visibilityState_ = visibilityState;
if (visibilityState == VisibilityState.VISIBLE) {
this.lastVisibleTime_ = Date.now();
this.signals_.signal(AmpDocSignals.FIRST_VISIBLE);
this.signals_.signal(AmpDocSignals.NEXT_VISIBLE);
let visibleTime;
const {performance} = this.win;
if (this.visibilityState_ == null) {
// If the doc was initialized in the visible state, then prefer the
// timeOrigin of the document over when the JS actually initializes.
visibleTime = Math.floor(
performance.timeOrigin ?? performance.timing.navigationStart
);
} else {
visibleTime = Math.floor(performance.now());
}
this.lastVisibleTime_ = visibleTime;
this.signals_.signal(AmpDocSignals.FIRST_VISIBLE, visibleTime);
this.signals_.signal(AmpDocSignals.NEXT_VISIBLE, visibleTime);
} else {
this.signals_.reset(AmpDocSignals.NEXT_VISIBLE);
}
this.visibilityState_ = visibilityState;
this.visibilityStateHandlers_.fire();
}
}
Expand Down
8 changes: 1 addition & 7 deletions src/service/performance-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -796,13 +796,7 @@ export class Performance {
* @param {string} label
*/
mark(label) {
if (
this.win.performance &&
this.win.performance.mark &&
arguments.length == 1
) {
this.win.performance.mark(label);
}
this.win.performance.mark?.(label);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions test/unit/core/window/test-history.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {installTimerService} from '#service/timer-impl';

import {listenOncePromise} from '#utils/event-helper';

import {FakePerformance} from '#testing/fake-dom';

import {parseUrlDeprecated} from '../../../../src/url';

describes.fakeWin(
Expand Down Expand Up @@ -322,6 +324,7 @@ describes.sandboxed('Window - History', {}, (env) => {
'https://cdn.ampproject.org/c/s/www.example.com/path'
),
addEventListener: () => null,
performance: new FakePerformance(window),
};
ampdoc = new AmpDocSingle(win);
installHistoryServiceForDoc(ampdoc);
Expand Down
2 changes: 2 additions & 0 deletions test/unit/test-action.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {AmpDocSingle} from '#service/ampdoc-impl';

import {createCustomEvent} from '#utils/event-helper';

import {FakePerformance} from '#testing/fake-dom';
import {whenCalled} from '#testing/helpers/service';

/**
Expand All @@ -35,6 +36,7 @@ function actionService() {
__AMP_SERVICES: {
vsync: {obj: {}, ctor: Object},
},
performance: new FakePerformance(window),
};
return new ActionService(new AmpDocSingle(win), document);
}
Expand Down
4 changes: 4 additions & 0 deletions test/unit/test-activity.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {installViewerServiceForDoc} from '#service/viewer-impl';
import {installViewportServiceForDoc} from '#service/viewport/viewport-impl';
import {installVsyncService} from '#service/vsync-impl';

import {FakePerformance} from '#testing/fake-dom';

import {installActivityServiceForTesting} from '../../extensions/amp-analytics/0.1/activity-impl';

describes.sandboxed('Activity getTotalEngagedTime', {}, (env) => {
Expand Down Expand Up @@ -76,6 +78,7 @@ describes.sandboxed('Activity getTotalEngagedTime', {}, (env) => {
addEventListener: () => {},
removeEventListener: () => {},
Promise: window.Promise,
performance: new FakePerformance(window),
};
fakeDoc.defaultView = fakeWin;
fakeDoc.head.defaultView = fakeWin;
Expand Down Expand Up @@ -301,6 +304,7 @@ describes.sandboxed('Activity getIncrementalEngagedTime', {}, (env) => {
addEventListener: () => {},
removeEventListener: () => {},
Promise: window.Promise,
performance: new FakePerformance(window),
};
fakeDoc.defaultView = fakeWin;
fakeDoc.head.defaultView = fakeWin;
Expand Down
60 changes: 47 additions & 13 deletions test/unit/test-ampdoc.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,15 +439,27 @@ describes.sandboxed('AmpDoc.visibilityState', {}, (env) => {
addEventListener: env.sandbox.spy(),
removeEventListener: env.sandbox.spy(),
};
win = {document: doc};
win = {
document: doc,
performance: {
now: Date.now,
timeOrigin: 1,
},
};

childDoc = {
body: null,
visibilityState: 'visible',
addEventListener: env.sandbox.spy(),
removeEventListener: env.sandbox.spy(),
};
childWin = {document: childDoc};
childWin = {
document: childDoc,
performance: {
now: Date.now,
timeOrigin: 2,
},
};

top = new AmpDocSingle(win);
embedSameWindow = new AmpDocFie(win, EMBED_URL, top);
Expand Down Expand Up @@ -524,6 +536,25 @@ describes.sandboxed('AmpDoc.visibilityState', {}, (env) => {
expect(disposableService.dispose).to.be.calledOnce;
});

describe('firstVisibleTime', () => {
it('should prefer timeOrigin doc initialized to visible', () => {
win.performance.timeOrigin = 10;
top = new AmpDocSingle(win, {visibilityState: 'visible'});

expect(top.getFirstVisibleTime()).to.equal(10);
});

it('should wait for visible', () => {
top = new AmpDocSingle(win, {visibilityState: 'prerender'});

expect(top.getFirstVisibleTime()).to.equal(null);

clock.tick(100);
top.overrideVisibilityState('visible');
expect(top.getFirstVisibleTime()).to.equal(101);
});
});

it('should be visible by default', () => {
expect(top.getVisibilityState()).to.equal('visible');
expect(embedSameWindow.getVisibilityState()).to.equal('visible');
Expand All @@ -537,13 +568,13 @@ describes.sandboxed('AmpDoc.visibilityState', {}, (env) => {

expect(top.getFirstVisibleTime()).to.equal(1);
expect(embedSameWindow.getFirstVisibleTime()).to.equal(1);
expect(embedOtherWindow.getFirstVisibleTime()).to.equal(1);
expect(embedChild.getFirstVisibleTime()).to.equal(1);
expect(embedOtherWindow.getFirstVisibleTime()).to.equal(2);
expect(embedChild.getFirstVisibleTime()).to.equal(2);

expect(top.getLastVisibleTime()).to.equal(1);
expect(embedSameWindow.getLastVisibleTime()).to.equal(1);
expect(embedOtherWindow.getLastVisibleTime()).to.equal(1);
expect(embedChild.getLastVisibleTime()).to.equal(1);
expect(embedOtherWindow.getLastVisibleTime()).to.equal(2);
expect(embedChild.getLastVisibleTime()).to.equal(2);

return Promise.all([
top.whenFirstVisible(),
Expand Down Expand Up @@ -700,10 +731,10 @@ describes.sandboxed('AmpDoc.visibilityState', {}, (env) => {
expect(top.getLastVisibleTime()).to.equal(1);
expect(embedSameWindow.getFirstVisibleTime()).to.equal(1);
expect(embedSameWindow.getLastVisibleTime()).to.equal(1);
expect(embedOtherWindow.getFirstVisibleTime()).to.equal(1);
expect(embedOtherWindow.getLastVisibleTime()).to.equal(1);
expect(embedChild.getFirstVisibleTime()).to.equal(1);
expect(embedChild.getLastVisibleTime()).to.equal(1);
expect(embedOtherWindow.getFirstVisibleTime()).to.equal(2);
expect(embedOtherWindow.getLastVisibleTime()).to.equal(2);
expect(embedChild.getFirstVisibleTime()).to.equal(2);
expect(embedChild.getLastVisibleTime()).to.equal(2);

clock.tick(1);
embedOtherWindow.overrideVisibilityState('visible');
Expand All @@ -715,9 +746,9 @@ describes.sandboxed('AmpDoc.visibilityState', {}, (env) => {
expect(top.getLastVisibleTime()).to.equal(1);
expect(embedSameWindow.getFirstVisibleTime()).to.equal(1);
expect(embedSameWindow.getLastVisibleTime()).to.equal(1);
expect(embedOtherWindow.getFirstVisibleTime()).to.equal(1);
expect(embedOtherWindow.getFirstVisibleTime()).to.equal(2);
expect(embedOtherWindow.getLastVisibleTime()).to.equal(3);
expect(embedChild.getFirstVisibleTime()).to.equal(1);
expect(embedChild.getFirstVisibleTime()).to.equal(2);
expect(embedChild.getLastVisibleTime()).to.equal(3);
});

Expand Down Expand Up @@ -852,7 +883,10 @@ describes.realWin('AmpDocSingle', {}, (env) => {
addEventListener: function () {},
removeEventListener: function () {},
};
const win = {document: doc};
const win = {
document: doc,
performance: env.win.performance,
};

let bodyCallback;
env.sandbox.stub(dom, 'waitForBodyOpenPromise').callsFake(() => {
Expand Down
2 changes: 2 additions & 0 deletions test/unit/test-cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {installPlatformService} from '#service/platform-impl';
import {installTimerService} from '#service/timer-impl';
import {installViewerServiceForDoc} from '#service/viewer-impl';

import {FakePerformance} from '#testing/fake-dom';
import {macroTask} from '#testing/helpers';
import {stubServiceForDoc} from '#testing/helpers/service';

Expand Down Expand Up @@ -97,6 +98,7 @@ describes.sandboxed('cid', {}, (env) => {
clearTimeout: window.clearTimeout,
Math: window.Math,
Promise: window.Promise,
performance: new FakePerformance(window),
};
fakeWin.document.defaultView = fakeWin;
installDocService(fakeWin, /* isSingleDoc */ true);
Expand Down
3 changes: 3 additions & 0 deletions test/unit/test-custom-element-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
upgradeOrRegisterElement,
} from '#service/custom-element-registry';

import {FakePerformance} from '#testing/fake-dom';

import {BaseElement} from '../../src/base-element';
import {getImplSyncForTesting} from '../../src/custom-element';
import {ElementStub} from '../../src/element-stub';
Expand Down Expand Up @@ -278,6 +280,7 @@ describes.realWin('CustomElement register', {amp: true}, (env) => {
},
HTMLElement,
__AMP_EXTENDED_ELEMENTS: {},
performance: new FakePerformance(window),
};
doc.defaultView = win;

Expand Down
7 changes: 6 additions & 1 deletion test/unit/test-fixed-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import {installViewerServiceForDoc} from '#service/viewer-impl';
import {Animation} from '#utils/animation';
import {user} from '#utils/log';

import {FakeMutationObserver, FakeWindow} from '#testing/fake-dom';
import {
FakeMutationObserver,
FakePerformance,
FakeWindow,
} from '#testing/fake-dom';

describes.sandboxed('FixedLayer', {}, (env) => {
let parentApi;
Expand Down Expand Up @@ -150,6 +154,7 @@ describes.sandboxed('FixedLayer', {}, (env) => {
navigator: window.navigator,
location: window.location,
cookie: '',
performance: new FakePerformance(window),
},
createElement: (name) => {
return createElement(name);
Expand Down
3 changes: 3 additions & 0 deletions test/unit/test-storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {

import {dev} from '#utils/log';

import {FakePerformance} from '#testing/fake-dom';

describes.sandboxed('Storage', {}, (env) => {
let storage;
let binding;
Expand Down Expand Up @@ -37,6 +39,7 @@ describes.sandboxed('Storage', {}, (env) => {
windowApi = {
document: {},
location: 'https://acme.com/document1',
performance: new FakePerformance(window),
};
ampdoc = new AmpDocSingle(windowApi);

Expand Down
Loading

0 comments on commit 88e3b29

Please sign in to comment.