Skip to content

Commit

Permalink
Use only DOMHighResTimeStamp to calculate duration
Browse files Browse the repository at this point in the history
Previously, we were combining Date.now() and DOMHighResTimeStamp values to calculate duration. These two value types use different clocks, so we should avoid combining the two value types where possible, since using two different clocks leaves us vulnerable to asymetrical issues. The native intersection observer uses DOMHighResTimeStamp, so we should use that type wherever possible in calculations.

This change also removes the native-* files, which are superseded by USE_NATIVE_IO and never part of the public API.

Thanks to @xg-wang for pointing out some potential issues affecting one clock and not the other, which could in turn cause asymmetrical bugs:
w3c/hr-time#115
mdn/content#4713
  • Loading branch information
asakusuma committed Jul 16, 2021
1 parent cdd3bfc commit 8116085
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 59 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"scripts": {
"test": "tsc && yarn run build && testem ci && node test/headless/run",
"serve": "yarn run build && node test/headless/server/app",
"test:headless": "mocha --require @babel/register test/headless/specs/**/*.js --exit",
"test:headless": "mocha --require @babel/register test/headless/specs/**/*.js test/headless/specs/*.js --exit --timeout 5000",
"watch": "broccoli-timepiece exports",
"build": "./scripts/build.sh",
"stats": "node scripts/size-calc",
Expand Down
61 changes: 53 additions & 8 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,36 +28,81 @@ export interface SpanielObserverInit {
USE_NATIVE_IO?: boolean;
}

export interface TimeCompat {
highResTime: number;
unixTime: number;
}

export interface SpanielRecord {
target: SpanielTrackedElement;
payload: any;
thresholdStates: SpanielThresholdState[];
lastSeenEntry: IntersectionObserverEntry | null;
lastSeenEntry: MaybeInternalIntersectionObserverEntry | null;
}

export interface SpanielThresholdState {
lastSatisfied: Boolean;
lastEntry: IntersectionObserverEntry | null;
lastEntry: MaybeInternalIntersectionObserverEntry | null;
threshold: SpanielThreshold;
lastVisible: number;
lastVisible: TimeCompat;
visible: boolean;
timeoutId?: number;
}

export interface SpanielIntersectionObserverEntryInit {
time: DOMHighResTimeStamp;
rootBounds: ClientRect;
boundingClientRect: ClientRect;
intersectionRect: ClientRect;
highResTime: DOMHighResTimeStamp;
unixTime: number;
rootBounds: SpanielRect;
boundingClientRect: SpanielRect;
intersectionRect: SpanielRect & ClientRect;
target: SpanielTrackedElement;
}

export interface SpanielObserverEntry extends IntersectionObserverEntryInit {
export interface SpanielRect extends Partial<DOMRectReadOnly> {
readonly height: number;
readonly width: number;
readonly x: number;
readonly y: number;
}

export interface SpanielObserverEntry {
isIntersecting: boolean;
duration: number;
visibleTime: number;
intersectionRatio: number;
entering: boolean;
label?: string;
payload?: any;
unixTime: number;
highResTime: number;
time: number;
target: Element;
boundingClientRect: SpanielRect;
intersectionRect: SpanielRect;
rootBounds: SpanielRect | null;
}

export interface InternalIntersectionObserverEntry {
time: number;
highResTime: DOMHighResTimeStamp;
target: Element;
boundingClientRect: SpanielRect;
intersectionRect: SpanielRect;
rootBounds: SpanielRect | null;
intersectionRatio: number;
isIntersecting: boolean;
}

export interface MaybeInternalIntersectionObserverEntry {
time: number;
unixTime?: number;
highResTime?: DOMHighResTimeStamp;
target: Element;
boundingClientRect: SpanielRect;
intersectionRect: SpanielRect & ClientRect;
rootBounds: SpanielRect | null;
intersectionRatio: number;
isIntersecting: boolean;
}

export interface IntersectionObserverClass {
Expand Down
53 changes: 34 additions & 19 deletions src/intersection-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,27 @@ import {
DOMRectReadOnly,
IntersectionObserverInit,
DOMMargin,
SpanielIntersectionObserverEntryInit
SpanielIntersectionObserverEntryInit,
InternalIntersectionObserverEntry,
SpanielRect
} from './interfaces';

interface EntryEvent {
entry: IntersectionObserverEntry;
entry: InternalIntersectionObserverEntry;
numSatisfiedThresholds: number;
}

function marginToRect(margin: DOMMargin): ClientRect {
function marginToRect(margin: DOMMargin): ClientRect & SpanielRect {
let { left, right, top, bottom } = margin;
return {
left,
top,
bottom,
right,
width: right - left,
height: bottom - top
height: bottom - top,
x: left,
y: top
};
}

Expand Down Expand Up @@ -140,13 +144,14 @@ export class SpanielIntersectionObserver implements IntersectionObserver {
}
}

function addRatio(entryInit: SpanielIntersectionObserverEntryInit): IntersectionObserverEntry {
const { time, rootBounds, boundingClientRect, intersectionRect, target } = entryInit;
function addRatio(entryInit: SpanielIntersectionObserverEntryInit): InternalIntersectionObserverEntry {
const { unixTime, highResTime, rootBounds, boundingClientRect, intersectionRect, target } = entryInit;
const boundingArea = boundingClientRect.height * boundingClientRect.width;
const intersectionRatio = boundingArea > 0 ? (intersectionRect.width * intersectionRect.height) / boundingArea : 0;

return {
time,
time: unixTime,
highResTime,
rootBounds,
boundingClientRect,
intersectionRect,
Expand All @@ -156,7 +161,7 @@ function addRatio(entryInit: SpanielIntersectionObserverEntryInit): Intersection
};
}

function emptyRect(): ClientRect | DOMRect {
function emptyRect(): ClientRect & SpanielRect {
return {
bottom: 0,
height: 0,
Expand All @@ -174,26 +179,31 @@ export function generateEntry(
clientRect: DOMRectReadOnly,
el: HTMLElement,
rootMargin: DOMMargin
): IntersectionObserverEntry {
): InternalIntersectionObserverEntry {
if (el.style.display === 'none') {
return {
time: frame.dateNow,
highResTime: frame.highResTime,
boundingClientRect: emptyRect(),
intersectionRatio: 0,
intersectionRect: emptyRect(),
isIntersecting: false,
rootBounds: emptyRect(),
target: el,
time: frame.timestamp
target: el
};
}
let { bottom, right } = clientRect;
let rootBounds: ClientRect = {
left: frame.left + rootMargin.left,
top: frame.top + rootMargin.top,
const left = frame.left + rootMargin.left;
const top = frame.top + rootMargin.top;
let rootBounds: SpanielRect & ClientRect = {
left,
top,
bottom: rootMargin.bottom,
right: rootMargin.right,
width: frame.width - (rootMargin.right + rootMargin.left),
height: frame.height - (rootMargin.bottom + rootMargin.top)
height: frame.height - (rootMargin.bottom + rootMargin.top),
y: top,
x: left
};

let intersectX = Math.max(rootBounds.left, clientRect.left);
Expand All @@ -202,17 +212,22 @@ export function generateEntry(
let width = Math.min(rootBounds.left + rootBounds.width, clientRect.right) - intersectX;
let height = Math.min(rootBounds.top + rootBounds.height, clientRect.bottom) - intersectY;

let intersectionRect: ClientRect = {
left: width >= 0 ? intersectX : 0,
top: intersectY >= 0 ? intersectY : 0,
const interLeft = width >= 0 ? intersectX : 0;
const interTop = intersectY >= 0 ? intersectY : 0;
let intersectionRect: ClientRect & SpanielRect = {
left: interLeft,
top: interTop,
x: interLeft,
y: interTop,
width,
height,
right,
bottom
};

return addRatio({
time: frame.timestamp,
unixTime: frame.dateNow,
highResTime: frame.highResTime,
rootBounds,
target: <SpanielTrackedElement>el,
boundingClientRect: marginToRect(clientRect),
Expand Down
3 changes: 2 additions & 1 deletion src/metal/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ export interface ElementSchedulerInterface extends BaseSchedulerInterface {
}

export interface FrameInterface {
timestamp: number;
dateNow: number;
highResTime: DOMHighResTimeStamp;
scrollTop: number;
scrollLeft: number;
width: number;
Expand Down
4 changes: 3 additions & 1 deletion src/metal/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ let tokenCounter = 0;

export class Frame implements FrameInterface {
constructor(
public timestamp: number,
public dateNow: number,
public highResTime: number,
public scrollTop: number,
public scrollLeft: number,
public width: number,
Expand All @@ -47,6 +48,7 @@ export class Frame implements FrameInterface {
const rootMeta = this.revalidateRootMeta(root);
return new Frame(
Date.now(),
performance.now(),
rootMeta.scrollTop,
rootMeta.scrollLeft,
rootMeta.width,
Expand Down
Loading

0 comments on commit 8116085

Please sign in to comment.