Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
use pagehide instead of visibilitychange
Browse files Browse the repository at this point in the history
  • Loading branch information
richvdh committed Aug 16, 2023
1 parent e1008f9 commit 34a6a9e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
18 changes: 14 additions & 4 deletions src/utils/SessionLock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,23 @@ export async function getSessionLock(onNewInstance: () => Promise<void>): Promis
// Now add a listener for other claimants to the lock.
window.addEventListener("storage", onStorageEvent);

// also add a listener to clear our claims when our tab closes (provided we haven't released it already)
window.document.addEventListener("visibilitychange", (event) => {
if (window.document.visibilityState === "hidden" && lockServicer !== null) {
prefixedLogger.info("Unloading: clearing our claims");
// also add a listener to clear our claims when our tab closes or navigates away
window.addEventListener("pagehide", (event) => {

This comment has been minimized.

Copy link
@t3chguy

t3chguy Aug 16, 2023

Member

The pagehide event is sent to a Window when the browser hides the current page in the process of presenting a different page from the session's history.

That doesn't sound right

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2023

Author Member

empirically, it also fires on tab closure.

This comment has been minimized.

Copy link
@t3chguy

t3chguy Aug 16, 2023

Member

Did you test on Chrome, Firefox & Safari?

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 17, 2023

Author Member

I tested with Chrome and Firefox. I don't have access to Safari, so was unable to test there.

The flowchart at https://developer.chrome.com/blog/page-lifecycle-api/#overview-of-page-lifecycle-states-and-events makes it pretty clear that pagehide will fire on tab close too.

// only remove the ping if we still think we're the owner. Otherwise we could be removing someone else's claim!
if (lockServicer !== null) {
prefixedLogger.info("page hide: clearing our claim");
window.localStorage.removeItem(STORAGE_ITEM_PING);
window.localStorage.removeItem(STORAGE_ITEM_OWNER);
}

// It's worth noting that, according to the spec, the page might come back to life again after a pagehide.
//
// In practice that's unlikely because Element is unlikely to qualify for the bfcache, but if it does,
// this is probably the best we can do: we certainly don't want to stop the user loading any new tabs because
// Element happens to be in a bfcache somewhere.
//
// So, we just hope that we aren't in the middle of any crypto operations, and rely on `onStorageEvent` kicking
// in soon enough after we resume to tell us if another tab woke up while we were asleep.
});

return true;
Expand Down
5 changes: 2 additions & 3 deletions test/utils/SessionLock-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ describe("SessionLock", () => {
expect(await getSessionLock(onNewInstance1)).toBe(true);
expect(onNewInstance1).not.toHaveBeenCalled();

// ... and is closed
jest.spyOn(window.document, "visibilityState", "get").mockReturnValue("hidden");
window.document.dispatchEvent(new Event("visibilitychange", {}));
// ... and navigates away
window.dispatchEvent(new Event("pagehide", {}));

// second instance starts as normal
const onNewInstance2 = jest.fn();
Expand Down

0 comments on commit 34a6a9e

Please sign in to comment.