Skip to content

Commit

Permalink
fix(extension): do not provide fork to closed tabs (#838)
Browse files Browse the repository at this point in the history
Relates to #820

Handler to update the redirect rules might not have been called because
the listener chain crapped out on this bug.
  • Loading branch information
frontendphil authored Feb 14, 2025
1 parent 11be336 commit 9d43609
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 4 deletions.
18 changes: 17 additions & 1 deletion deployables/extension/src/background/companionEnablement.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
type CompanionResponseMessage,
type Message,
} from '@zodiac/messages'
import { mockActiveTab, mockTab } from '@zodiac/test-utils/chrome'
import { mockActiveTab, mockTab, mockTabClose } from '@zodiac/test-utils/chrome'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { companionEnablement } from './companionEnablement'
import { trackRequests } from './rpcTracking'
Expand Down Expand Up @@ -68,6 +68,22 @@ describe('Companion Enablement', () => {
forkUrl: 'http://test-rpc.com',
})
})

it('does not notify the companion app when the tab has been closed', async () => {
await startPilotSession({ windowId: 1, tabId: 2 })

const tab = await connectCompanionApp({ id: 2, windowId: 1 })

await mockTabClose(tab.id)

await startSimulation({
windowId: 1,
chainId: Chain.ETH,
rpcUrl: 'http://test-rpc.com',
})

expect(chromeMock.tabs.sendMessage).toHaveBeenCalledTimes(1)
})
})

describe('Connection status', () => {
Expand Down
14 changes: 13 additions & 1 deletion deployables/extension/src/background/companionEnablement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const companionEnablement = (

console.debug('Companion App connected!')

onSimulationUpdate.addListener(async (fork) => {
const dispose = onSimulationUpdate.addListener(async (fork) => {
invariant(tab.id != null, 'Tab needs an ID')

console.debug('Sending updated fork to companion app', { fork })
Expand All @@ -52,6 +52,18 @@ export const companionEnablement = (
captureLastError()
})

const handleTabClose = (tabId: number) => {
if (tabId !== tab.id) {
return
}

chrome.tabs.onRemoved.removeListener(handleTabClose)

dispose()
}

chrome.tabs.onRemoved.addListener(handleTabClose)

break
}

Expand Down
4 changes: 4 additions & 0 deletions deployables/extension/src/background/createEventListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export const createEventListener = <
return {
addListener(listener) {
listeners.add(listener)

return () => {
listeners.delete(listener)
}
},
removeAllListeners() {
listeners.clear()
Expand Down
4 changes: 3 additions & 1 deletion deployables/extension/src/background/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ export interface Fork {

export type EventFn = (...args: any) => void

type DisposeFn = () => void

export type Event<T extends EventFn = () => void> = {
addListener: (listener: T) => void
addListener: (listener: T) => DisposeFn
removeListener: (listener: T) => void
removeAllListeners: () => void
}
6 changes: 5 additions & 1 deletion deployables/extension/test-utils/connectCompanionApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@ import { callListeners, chromeMock, createMockTab } from './chrome'
export const connectCompanionApp = async (
tab: Partial<chrome.tabs.Tab> = {},
) => {
const connectedTab = createMockTab(tab)

await callListeners(
chromeMock.runtime.onMessage,
{
type: CompanionAppMessageType.REQUEST_FORK_INFO,
} satisfies CompanionAppMessage,
{ id: chrome.runtime.id, tab: createMockTab(tab) },
{ id: chrome.runtime.id, tab: connectedTab },
vi.fn(),
)

return connectedTab
}
1 change: 1 addition & 0 deletions packages/test-utils/src/chrome/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export * from './creators'
export { mockActiveTab } from './mockActiveTab'
export { mockRuntimeConnect } from './mockRuntimeConnect'
export { mockTab } from './mockTab'
export { mockTabClose } from './mockTabClose'
export { mockTabConnect } from './mockTabConnect'
export { mockTabSwitch } from './mockTabSwitch'
export { createStorageMock } from './storageMock'
8 changes: 8 additions & 0 deletions packages/test-utils/src/chrome/mockTabClose.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { callListeners } from './callListeners'
import { chromeMock } from './chromeMock'

export const mockTabClose = (tabId: number) =>
callListeners(chromeMock.tabs.onRemoved, tabId, {
isWindowClosing: false,
windowId: 1,
})

0 comments on commit 9d43609

Please sign in to comment.