🐛 Use timeOrigin for getFirstVisibleTime when page starts visible (#3…

…6273)

* Use timeOrigin for getFirstVisibleTime when page starts visible

* Update old tests

* Add tests for firstVisibleTime

* Fix global conflict

* Fix tests

extensions/amp-access/0.1/test/test-login-dialog.js

@@ -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(
@@ -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);
@@ -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;

extensions/amp-experiment/0.1/test/test-variant.js

@@ -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) => {
@@ -23,6 +25,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
         nodeType: /* DOCUMENT */ 9,
         body: {},
       },
+      performance: new FakePerformance(window),
     };
     fakeWin.document.defaultView = fakeWin;
     ampdoc = new AmpDocSingle(fakeWin);

extensions/amp-experiment/1.0/test/test-variant.js

@@ -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) => {
@@ -24,6 +26,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
         nodeType: /* DOCUMENT */ 9,
         body: {},
       },
+      performance: new FakePerformance(window),
     };
     fakeWin.document.defaultView = fakeWin;
     ampdoc = new AmpDocSingle(fakeWin);

src/service/ampdoc-impl.js

@@ -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();
     }
   }

src/service/performance-impl.js

@@ -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);
   }
 
   /**

test/unit/core/window/test-history.js

@@ -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(
@@ -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);

test/unit/test-action.js

@@ -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';
 
 /**
@@ -35,6 +36,7 @@ function actionService() {
     __AMP_SERVICES: {
       vsync: {obj: {}, ctor: Object},
     },
+    performance: new FakePerformance(window),
   };
   return new ActionService(new AmpDocSingle(win), document);
 }

test/unit/test-activity.js

@@ -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) => {
@@ -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;
@@ -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;

test/unit/test-ampdoc.js

@@ -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);
@@ -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');
@@ -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(),
@@ -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');
@@ -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);
   });
 
@@ -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(() => {

test/unit/test-cid.js

@@ -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';
 
@@ -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);

test/unit/test-custom-element-registry.js

@@ -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';
@@ -278,6 +280,7 @@ describes.realWin('CustomElement register', {amp: true}, (env) => {
         },
         HTMLElement,
         __AMP_EXTENDED_ELEMENTS: {},
+        performance: new FakePerformance(window),
       };
       doc.defaultView = win;
 

test/unit/test-fixed-layer.js

@@ -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;
@@ -150,6 +154,7 @@ describes.sandboxed('FixedLayer', {}, (env) => {
         navigator: window.navigator,
         location: window.location,
         cookie: '',
+        performance: new FakePerformance(window),
       },
       createElement: (name) => {
         return createElement(name);

test/unit/test-storage.js

@@ -8,6 +8,8 @@ import {
 
 import {dev} from '#utils/log';
 
+import {FakePerformance} from '#testing/fake-dom';
+
 describes.sandboxed('Storage', {}, (env) => {
   let storage;
   let binding;
@@ -37,6 +39,7 @@ describes.sandboxed('Storage', {}, (env) => {
         windowApi = {
           document: {},
           location: 'https://acme.com/document1',
+          performance: new FakePerformance(window),
         };
         ampdoc = new AmpDocSingle(windowApi);