Skip to content

Commit

Permalink
Remove IntersectionObserver
Browse files Browse the repository at this point in the history 
I’ve noticed that tracking all clickable elements with
`IntersectionObserver` does not seem to speed things up noticeably.
Instead, it just runs the risk of slowing down the browser. This commit
removes the `IntersectionObserver`. We had support for it not running
anyway, since it makes scrolling laggy on massive pages – now we use
that approach everywhere.

Not that we still use `IntersectionObserver` for frames, and in the
renderer.
  • Loading branch information
@lydell
lydell committed Aug 24, 2024
1 parent 636ac1f commit 0b8d42b
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 113 deletions.
8 changes: 2 additions & 6 deletions docs/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ const SECTIONS = [
</p>
<p>
Link Hints keeps track of all clickable elements in the background
when your browser is idle. This makes{" "}
<strong>hints appear quickly regardless of page size.</strong>
when your browser is idle, in an attempt to make hints appear more
quickly.
</p>
<p>
Other than accurately finding clickable elements, Link Hints also
Expand Down Expand Up @@ -122,10 +122,6 @@ const SECTIONS = [
<a href="https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver">
MutationObserver
</a>
,{" "}
<a href="https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserver">
IntersectionObserver
</a>{" "}
and{" "}
<a href="https://developer.mozilla.org/en-US/docs/Web/API/Window/requestIdleCallback">
requestIdleCallback
Expand Down
6 changes: 1 addition & 5 deletions src/background/Program.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2668,11 +2668,7 @@ function assignHints(
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
b.weight - a.weight ||
// If the weights are the same, sort by on-screen position, left to
// right and then top to bottom (reading order in LTR languages). If you
// scroll _down_ to a list of same-weight links they usually end up in
// the order naturally, but if you scroll _up_ to the same list the
// IntersectionObserver fires in a different order, so it’s important
// not to rely on that to get consistent hints.
// right and then top to bottom (reading order in LTR languages).
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
comparePositions(a.hintMeasurements, b.hintMeasurements) ||
// `hintsState.elementsWithHints` changes order as
Expand Down
14 changes: 0 additions & 14 deletions src/options/Perf.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,7 @@ function sumStats(
url: title,
numTotalElements: sum(({ numTotalElements }) => numTotalElements),
numTrackedElements: sum(({ numTrackedElements }) => numTrackedElements),
numVisibleElements: sum(({ numVisibleElements }) => numVisibleElements),
numVisibleFrames: sum(({ numVisibleFrames }) => numVisibleFrames),
bailed: sum(({ bailed }) => bailed),
durations: sumDurations(stats.map(({ durations }) => durations)),
},
];
Expand Down Expand Up @@ -268,17 +266,13 @@ function statsToRows(allStats: Array<Array<Stats>>): Array<{
? {
numTotalElements: match.numTotalElements.toString(),
numTrackedElements: match.numTrackedElements.toString(),
numVisibleElements: match.numVisibleElements.toString(),
numVisibleFrames: match.numVisibleFrames.toString(),
bailed: match.bailed.toString(),
durations: match.durations,
}
: {
numTotalElements: "-",
numTrackedElements: "-",
numVisibleElements: "-",
numVisibleFrames: "-",
bailed: "-",
durations: [],
};
});
Expand All @@ -295,18 +289,10 @@ function statsToRows(allStats: Array<Array<Stats>>): Array<{
heading: "# tracked elements",
values: allData.map(({ numTrackedElements }) => numTrackedElements),
},
{
heading: "# visible elements",
values: allData.map(({ numVisibleElements }) => numVisibleElements),
},
{
heading: "# visible frames",
values: allData.map(({ numVisibleFrames }) => numVisibleFrames),
},
{
heading: "# bailed",
values: allData.map(({ bailed }) => bailed),
},
],
};
});
Expand Down
2 changes: 0 additions & 2 deletions src/shared/perf.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ export const Stats = fieldsAuto({
url: string,
numTotalElements: number,
numTrackedElements: number,
numVisibleElements: number,
numVisibleFrames: number,
bailed: number,
durations: Durations,
});

Expand Down
89 changes: 3 additions & 86 deletions src/worker/ElementManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,6 @@ const ATTRIBUTES_NOT_CLICKABLE = new Set<string>([
]);

export const t = {
// The single-page HTML specification has over 70K links! If trying to track all
// of those with `IntersectionObserver`, scrolling is noticeably laggy. On my
// computer, the lag starts somewhere between 10K and 20K tracked links.
// Tracking at most 10K should be enough for regular sites.
MAX_INTERSECTION_OBSERVED_ELEMENTS: unsignedInt(10e3),

// If `.getVisibleElements` is taking too long, skip remaining elements.
// Chrome’s implementation of `document.elementFromPoint` is not optimized for
// elements with thousands of children, which is rare in practice but present
Expand Down Expand Up @@ -280,8 +274,6 @@ export default class ElementManager {

elements = new Map<HTMLElement, ElementType>();

visibleElements = new Set<HTMLElement>();

visibleFrames = new Set<HTMLFrameElement | HTMLIFrameElement>();

elementsWithClickListeners = new WeakSet<HTMLElement>();
Expand All @@ -290,16 +282,10 @@ export default class ElementManager {

idleCallbackId: IdleCallbackID | undefined = undefined;

bailed = false;

secretElementResets = new Resets();

windowListenerResets = new Resets();

intersectionObserver = new IntersectionObserver(
this.onIntersection.bind(this)
);

frameIntersectionObserver = new IntersectionObserver(
this.onFrameIntersection.bind(this)
);
Expand Down Expand Up @@ -357,15 +343,13 @@ export default class ElementManager {
cancelIdleCallback(this.idleCallbackId);
}

this.intersectionObserver.disconnect();
this.frameIntersectionObserver.disconnect();
this.mutationObserver.disconnect();
this.removalObserver.disconnect();
this.queue = makeEmptyQueue();
this.injectedHasQueue = false;
this.injectedListeners = new Map<string, Array<() => unknown>>();
this.elements.clear();
this.visibleElements.clear();
this.visibleFrames.clear();
// `WeakSet`s don’t have a `.clear()` method.
this.elementsWithClickListeners = new WeakSet();
Expand All @@ -376,28 +360,6 @@ export default class ElementManager {
this.sendInjectedEvent(RESET_EVENT);
}

// Stop using the intersection observer for everything except frames. The
// reason to still track frames is because it saves more than half a second
// when generating hints on the single-page HTML specification.
bail(): void {
if (this.bailed) {
return;
}

const { size } = this.elements;

this.intersectionObserver.disconnect();
this.visibleElements.clear();
this.bailed = true;

log(
"warn",
"ElementManager#bail",
size,
t.MAX_INTERSECTION_OBSERVED_ELEMENTS
);
}

addWindowListeners(): void {
// When adding new event listeners, consider also subscribing in
// `onRegisterSecretElement` and `setShadowRoot`.
Expand Down Expand Up @@ -474,9 +436,7 @@ export default class ElementManager {
url: window.location.href,
numTotalElements: Array.from(this.getAllElements(document)).length,
numTrackedElements: this.elements.size,
numVisibleElements: this.visibleElements.size,
numVisibleFrames: this.visibleFrames.size,
bailed: this.bailed ? 1 : 0,
durations,
};
}
Expand Down Expand Up @@ -545,16 +505,6 @@ export default class ElementManager {
}
}

onIntersection(entries: Array<IntersectionObserverEntry>): void {
for (const entry of entries) {
if (entry.isIntersecting) {
this.visibleElements.add(entry.target as HTMLElement);
} else {
this.visibleElements.delete(entry.target as HTMLElement);
}
}
}

onFrameIntersection(entries: Array<IntersectionObserverEntry>): void {
for (const entry of entries) {
const element = entry.target;
Expand Down Expand Up @@ -815,10 +765,6 @@ export default class ElementManager {
) {
switch (mutationType) {
case "added":
// In theory, this can lead to more than
// `maxIntersectionObservedElements` frames being tracked by the
// intersection observer, but in practice there are never that many
// frames. YAGNI.
this.frameIntersectionObserver.observe(element);
break;
case "removed":
Expand All @@ -837,44 +783,19 @@ export default class ElementManager {
if (type === undefined) {
if (mutationType !== "added") {
this.elements.delete(element);
// Removing an element from the DOM also triggers the
// IntersectionObserver (removing it from `this.visibleElements`), but
// changing an attribute of an element so that it isn't considered
// clickable anymore requires a manual deletion from
// `this.visibleElements` since the element might still be on-screen.
this.visibleElements.delete(element);
this.intersectionObserver.unobserve(element);
// The element must not be removed from `elementsWithClickListeners`
// or `elementsWithScrollbars` (if `mutationType === "removed"`), even
// (if `mutationType === "removed"`), even
// though it might seem logical at first. But the element (or one of
// its parents) could temporarily be removed from the paged and then
// its parents) could temporarily be removed from the page and then be
// re-inserted. Then it would still have its click listener, but we
// wouldn’t know. So instead of removing `element` here a `WeakSet` is
// used, to avoid memory leaks. An example of this is the sortable
// table headings on Wikipedia:
// <https://en.wikipedia.org/wiki/Help:Sorting>
// this.elementsWithClickListeners.delete(element);
// this.elementsWithScrollbars.delete(element);
}
} else {
const alreadyIntersectionObserved = this.elements.has(element);
this.elements.set(element, type);
if (!alreadyIntersectionObserved && !this.bailed) {
// We won’t know if this element is visible or not until the next time
// intersection observers are run. If we enter hints mode before that,
// we would miss this element. This happens a lot on Gmail. First, the
// intersection observer fires on the splash screen. Then _a lot_ of DOM
// nodes appear at once when the inbox renders. The mutation observer
// fires kind of straight away, but the intersection observer is slow.
// If hints mode was entered before that, no elements would be found.
// But the elements are clearly visible on screen! For this reason we
// consider _all_ new elements as visible until proved otherwise.
this.visibleElements.add(element);
this.intersectionObserver.observe(element);
if (this.elements.size > t.MAX_INTERSECTION_OBSERVED_ELEMENTS.value) {
this.bail();
}
}
}

if (mutationType === "added") {
Expand Down Expand Up @@ -1140,16 +1061,12 @@ export default class ElementManager {
this.flushQueue(infiniteDeadline);
}

this.onIntersection(this.intersectionObserver.takeRecords());

const candidates =
passedCandidates !== undefined
? passedCandidates
: types === "selectable"
? this.getAllElements(document)
: this.bailed
? this.elements.keys()
: this.visibleElements;
: this.elements.keys();
const range = document.createRange();
const deduper = new Deduper();

Expand Down

0 comments on commit 0b8d42b

Please sign in to comment.