From 01242a2f0d174b4bf8b51fd5627edced465757e9 Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Thu, 23 Sep 2021 12:44:49 +0900 Subject: Search a content from frames successfully loaded --- src/background/Application.ts | 20 ++++++++- src/background/di.ts | 6 ++- src/background/operators/impls/FindNextOperator.ts | 11 +++-- .../operators/impls/FindOperatorFactoryChain.ts | 10 ++--- src/background/operators/impls/FindPrevOperator.ts | 10 +++-- src/background/presenters/FramePresenter.ts | 12 ------ .../repositories/ReadyFrameRepository.ts | 49 ++++++++++++++++++++++ src/background/usecases/ReadyFrameUseCase.ts | 18 ++++++++ src/background/usecases/StartFindUseCase.ts | 12 ++++-- 9 files changed, 118 insertions(+), 30 deletions(-) delete mode 100644 src/background/presenters/FramePresenter.ts create mode 100644 src/background/repositories/ReadyFrameRepository.ts create mode 100644 src/background/usecases/ReadyFrameUseCase.ts (limited to 'src/background') diff --git a/src/background/Application.ts b/src/background/Application.ts index 2006965..87865e6 100644 --- a/src/background/Application.ts +++ b/src/background/Application.ts @@ -4,6 +4,7 @@ import SettingController from "./controllers/SettingController"; import VersionController from "./controllers/VersionController"; import SettingRepository from "./repositories/SettingRepository"; import FindRepositoryImpl from "./repositories/FindRepository"; +import ReadyFrameRepository from "./repositories/ReadyFrameRepository"; @injectable() export default class Application { @@ -14,7 +15,9 @@ export default class Application { @inject("SyncSettingRepository") private syncSettingRepository: SettingRepository, @inject("FindRepository") - private readonly findRepository: FindRepositoryImpl + private readonly findRepository: FindRepositoryImpl, + @inject("ReadyFrameRepository") + private readonly frameRepository: ReadyFrameRepository ) {} run() { @@ -22,6 +25,7 @@ export default class Application { browser.tabs.onUpdated.addListener((tabId: number, info) => { if (info.status == "loading") { + this.frameRepository.clearFrameIds(tabId); this.findRepository.deleteLocalState(tabId); } }); @@ -31,6 +35,20 @@ export default class Application { } this.versionController.notify(); }); + browser.webNavigation.onCompleted.addListener((detail) => { + // The console iframe embedded by Vim-Vixen has url starting with + // 'moz-extensions://'. The add-on should ignore it from search targets. + // + // When a browser blocks to load an iframe by x-frame options or a + // content security policy, the URL begins with 'about:neterror', and + // a background script fails to send a message to iframe. + if ( + detail.url.startsWith("http://") || + detail.url.startsWith("https://") + ) { + this.frameRepository.addFrameId(detail.tabId, detail.frameId); + } + }); this.contentMessageListener.run(); this.syncSettingRepository.onChange(() => { diff --git a/src/background/di.ts b/src/background/di.ts index e97c4a8..495de7c 100644 --- a/src/background/di.ts +++ b/src/background/di.ts @@ -18,10 +18,10 @@ import { BrowserSettingRepositoryImpl } from "./repositories/BrowserSettingRepos import { RepeatRepositoryImpl } from "./repositories/RepeatRepository"; import { ZoomPresenterImpl } from "./presenters/ZoomPresenter"; import { WindowPresenterImpl } from "./presenters/WindowPresenter"; -import { FramePresenterImpl } from "./presenters/FramePresenter"; import { FindClientImpl } from "./clients/FindClient"; import { ConsoleFrameClientImpl } from "./clients/ConsoleFrameClient"; import { FindRepositoryImpl } from "./repositories/FindRepository"; +import { ReadyFrameRepositoryImpl } from "./repositories/ReadyFrameRepository"; container.register("LocalSettingRepository", { useClass: LocalSettingRepository, @@ -43,10 +43,12 @@ container.register("TabRepository", { useClass: TabRepositoryImpl }); container.register("ZoomPresenter", { useClass: ZoomPresenterImpl }); container.register("TabPresenter", { useClass: TabPresenterImpl }); container.register("WindowPresenter", { useClass: WindowPresenterImpl }); -container.register("FramePresenter", { useClass: FramePresenterImpl }); container.register("FindRepository", { useClass: FindRepositoryImpl }); container.register("FindClient", { useClass: FindClientImpl }); container.register("NavigateClient", { useClass: NavigateClientImpl }); container.register("ConsoleClient", { useClass: ConsoleClientImpl }); container.register("ConsoleFrameClient", { useClass: ConsoleFrameClientImpl }); container.register("OperatorFactory", { useClass: OperatorFactoryImpl }); +container.register("ReadyFrameRepository", { + useClass: ReadyFrameRepositoryImpl, +}); diff --git a/src/background/operators/impls/FindNextOperator.ts b/src/background/operators/impls/FindNextOperator.ts index 241f71d..2aed9fb 100644 --- a/src/background/operators/impls/FindNextOperator.ts +++ b/src/background/operators/impls/FindNextOperator.ts @@ -3,7 +3,7 @@ import TabPresenter from "../../presenters/TabPresenter"; import FindRepository from "../../repositories/FindRepository"; import FindClient from "../../clients/FindClient"; import ConsoleClient from "../../infrastructures/ConsoleClient"; -import FramePresenter from "../../presenters/FramePresenter"; +import ReadyFrameRepository from "../../repositories/ReadyFrameRepository"; export default class FindNextOperator implements Operator { constructor( @@ -11,7 +11,7 @@ export default class FindNextOperator implements Operator { private readonly findRepository: FindRepository, private readonly findClient: FindClient, private readonly consoleClient: ConsoleClient, - private readonly framePresenter: FramePresenter + private readonly frameRepository: ReadyFrameRepository ) {} async run(): Promise { @@ -65,7 +65,12 @@ export default class FindNextOperator implements Operator { const keyword = await this.findRepository.getGlobalKeyword(); if (keyword) { - const frameIds = await this.framePresenter.getAllFrameIds(tabId); + const frameIds = await this.frameRepository.getFrameIds(tabId); + if (typeof frameIds === "undefined") { + // No frames are ready + return; + } + for (const frameId of frameIds) { await this.findClient.clearSelection(tabId, frameId); } diff --git a/src/background/operators/impls/FindOperatorFactoryChain.ts b/src/background/operators/impls/FindOperatorFactoryChain.ts index b71f032..cc169dd 100644 --- a/src/background/operators/impls/FindOperatorFactoryChain.ts +++ b/src/background/operators/impls/FindOperatorFactoryChain.ts @@ -7,8 +7,8 @@ import FindNextOperator from "./FindNextOperator"; import FindPrevOperator from "./FindPrevOperator"; import FindRepository from "../../repositories/FindRepository"; import FindClient from "../../clients/FindClient"; -import FramePresenter from "../../presenters/FramePresenter"; import ConsoleClient from "../../infrastructures/ConsoleClient"; +import ReadyFrameRepository from "../../repositories/ReadyFrameRepository"; @injectable() export default class FindOperatorFactoryChain implements OperatorFactoryChain { @@ -21,8 +21,8 @@ export default class FindOperatorFactoryChain implements OperatorFactoryChain { private readonly findClient: FindClient, @inject("ConsoleClient") private readonly consoleClient: ConsoleClient, - @inject("FramePresenter") - private readonly framePresenter: FramePresenter + @inject("ReadyFrameRepository") + private readonly frameRepository: ReadyFrameRepository ) {} create(op: operations.Operation): Operator | null { @@ -33,7 +33,7 @@ export default class FindOperatorFactoryChain implements OperatorFactoryChain { this.findRepository, this.findClient, this.consoleClient, - this.framePresenter + this.frameRepository ); case operations.FIND_PREV: return new FindPrevOperator( @@ -41,7 +41,7 @@ export default class FindOperatorFactoryChain implements OperatorFactoryChain { this.findRepository, this.findClient, this.consoleClient, - this.framePresenter + this.frameRepository ); } return null; diff --git a/src/background/operators/impls/FindPrevOperator.ts b/src/background/operators/impls/FindPrevOperator.ts index 822c386..3c4411d 100644 --- a/src/background/operators/impls/FindPrevOperator.ts +++ b/src/background/operators/impls/FindPrevOperator.ts @@ -3,7 +3,7 @@ import TabPresenter from "../../presenters/TabPresenter"; import FindRepository from "../../repositories/FindRepository"; import FindClient from "../../clients/FindClient"; import ConsoleClient from "../../infrastructures/ConsoleClient"; -import FramePresenter from "../../presenters/FramePresenter"; +import ReadyFrameRepository from "../../repositories/ReadyFrameRepository"; export default class FindPrevOperator implements Operator { constructor( @@ -11,7 +11,7 @@ export default class FindPrevOperator implements Operator { private readonly findRepository: FindRepository, private readonly findClient: FindClient, private readonly consoleClient: ConsoleClient, - private readonly framePresenter: FramePresenter + private readonly frameRepository: ReadyFrameRepository ) {} async run(): Promise { @@ -65,7 +65,11 @@ export default class FindPrevOperator implements Operator { const keyword = await this.findRepository.getGlobalKeyword(); if (keyword) { - const frameIds = await this.framePresenter.getAllFrameIds(tabId); + const frameIds = await this.frameRepository.getFrameIds(tabId); + if (typeof frameIds === "undefined") { + // No frames are ready + return; + } for (const frameId of frameIds) { await this.findClient.clearSelection(tabId, frameId); } diff --git a/src/background/presenters/FramePresenter.ts b/src/background/presenters/FramePresenter.ts deleted file mode 100644 index c94f8dd..0000000 --- a/src/background/presenters/FramePresenter.ts +++ /dev/null @@ -1,12 +0,0 @@ -export default interface FramePresenter { - getAllFrameIds(tabId: number): Promise>; -} - -export class FramePresenterImpl implements FramePresenter { - async getAllFrameIds(tabId: number): Promise> { - const frames = await browser.webNavigation.getAllFrames({ tabId: tabId }); - return frames - .filter((f) => !f.url.startsWith("moz-extension://")) - .map((f) => f.frameId); - } -} diff --git a/src/background/repositories/ReadyFrameRepository.ts b/src/background/repositories/ReadyFrameRepository.ts new file mode 100644 index 0000000..725f604 --- /dev/null +++ b/src/background/repositories/ReadyFrameRepository.ts @@ -0,0 +1,49 @@ +import MemoryStorage from "../infrastructures/MemoryStorage"; + +const REPOSITORY_KEY = "readyFrameRepository"; + +type State = { [tabId: number]: number[] }; + +export default interface ReadyFrameRepository { + clearFrameIds(tabId: number): Promise; + + addFrameId(tabId: number, fraemId: number): Promise; + + getFrameIds(tabId: number): Promise; +} + +export class ReadyFrameRepositoryImpl implements ReadyFrameRepository { + private cache: MemoryStorage; + + constructor() { + this.cache = new MemoryStorage(); + } + + clearFrameIds(tabId: number): Promise { + let state: State | undefined = this.cache.get(REPOSITORY_KEY); + if (typeof state === "undefined") { + state = {}; + } + delete state[tabId]; + this.cache.set(REPOSITORY_KEY, state); + return Promise.resolve(); + } + + addFrameId(tabId: number, fraemId: number): Promise { + let state: State | undefined = this.cache.get(REPOSITORY_KEY); + if (typeof state === "undefined") { + state = {}; + } + state[tabId] = (state[tabId] || []).concat(fraemId); + this.cache.set(REPOSITORY_KEY, state); + return Promise.resolve(); + } + + getFrameIds(tabId: number): Promise { + const state: State | undefined = this.cache.get(REPOSITORY_KEY); + if (typeof state === "undefined") { + return Promise.resolve(undefined); + } + return Promise.resolve(state[tabId]); + } +} diff --git a/src/background/usecases/ReadyFrameUseCase.ts b/src/background/usecases/ReadyFrameUseCase.ts new file mode 100644 index 0000000..81bee0c --- /dev/null +++ b/src/background/usecases/ReadyFrameUseCase.ts @@ -0,0 +1,18 @@ +import { inject, injectable } from "tsyringe"; +import ReadyFrameRepository from "../repositories/ReadyFrameRepository"; + +@injectable() +export default class ReadyFrameUseCase { + constructor( + @inject("ReadyFrameRepository") + private readonly frameRepository: ReadyFrameRepository + ) {} + + async addReadyFrame(tabId: number, frameId: number): Promise { + return this.frameRepository.addFrameId(tabId, frameId); + } + + async clearReadyFrame(tabId: number): Promise { + return this.frameRepository.clearFrameIds(tabId); + } +} diff --git a/src/background/usecases/StartFindUseCase.ts b/src/background/usecases/StartFindUseCase.ts index 066d930..a62462f 100644 --- a/src/background/usecases/StartFindUseCase.ts +++ b/src/background/usecases/StartFindUseCase.ts @@ -2,7 +2,7 @@ import { inject, injectable } from "tsyringe"; import ConsoleClient from "../infrastructures/ConsoleClient"; import FindRepositoryImpl from "../repositories/FindRepository"; import FindClient from "../clients/FindClient"; -import FramePresenter from "../presenters/FramePresenter"; +import ReadyFrameRepository from "../repositories/ReadyFrameRepository"; @injectable() export default class StartFindUseCase { @@ -13,8 +13,8 @@ export default class StartFindUseCase { private readonly findRepository: FindRepositoryImpl, @inject("ConsoleClient") private readonly consoleClient: ConsoleClient, - @inject("FramePresenter") - private readonly framePresenter: FramePresenter + @inject("ReadyFrameRepository") + private readonly frameRepository: ReadyFrameRepository ) {} async startFind(tabId: number, keyword?: string): Promise { @@ -31,7 +31,11 @@ export default class StartFindUseCase { this.findRepository.setGlobalKeyword(keyword); - const frameIds = await this.framePresenter.getAllFrameIds(tabId); + const frameIds = await this.frameRepository.getFrameIds(tabId); + if (typeof frameIds === "undefined") { + // No frames are ready + return; + } for (const frameId of frameIds) { await this.findClient.clearSelection(tabId, frameId); } -- cgit v1.2.3 From b72728bdabf140c77f165b35813595dcaa060eea Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Thu, 23 Sep 2021 23:23:55 +0900 Subject: Create find targets by port connections --- src/background/Application.ts | 41 ++++++---- src/background/infrastructures/FindPortListener.ts | 23 ++++++ src/background/operators/impls/FindNextOperator.ts | 94 ++++++++++------------ src/background/operators/impls/FindPrevOperator.ts | 94 ++++++++++------------ src/background/repositories/FindRepository.ts | 3 +- .../repositories/ReadyFrameRepository.ts | 22 ++++- src/background/usecases/StartFindUseCase.ts | 11 +-- src/content/Application.ts | 16 +++- test/background/mock/MockReadyFrameRepository.ts | 4 + .../operators/impls/FindNextOperator.test.ts | 24 +++--- .../operators/impls/FindPrevOperator.test.ts | 24 +++--- .../background/repositories/FindRepository.test.ts | 4 +- test/background/usecases/StartFindUseCase.test.ts | 8 +- 13 files changed, 207 insertions(+), 161 deletions(-) create mode 100644 src/background/infrastructures/FindPortListener.ts (limited to 'src/background') diff --git a/src/background/Application.ts b/src/background/Application.ts index 87865e6..c7bcc42 100644 --- a/src/background/Application.ts +++ b/src/background/Application.ts @@ -1,5 +1,6 @@ import { injectable, inject } from "tsyringe"; import ContentMessageListener from "./infrastructures/ContentMessageListener"; +import FindPortListener from "./infrastructures/FindPortListener"; import SettingController from "./controllers/SettingController"; import VersionController from "./controllers/VersionController"; import SettingRepository from "./repositories/SettingRepository"; @@ -20,6 +21,11 @@ export default class Application { private readonly frameRepository: ReadyFrameRepository ) {} + private readonly findPortListener = new FindPortListener( + this.onFindPortConnect.bind(this), + this.onFindPortDisconnect.bind(this) + ); + run() { this.settingController.reload(); @@ -35,24 +41,31 @@ export default class Application { } this.versionController.notify(); }); - browser.webNavigation.onCompleted.addListener((detail) => { - // The console iframe embedded by Vim-Vixen has url starting with - // 'moz-extensions://'. The add-on should ignore it from search targets. - // - // When a browser blocks to load an iframe by x-frame options or a - // content security policy, the URL begins with 'about:neterror', and - // a background script fails to send a message to iframe. - if ( - detail.url.startsWith("http://") || - detail.url.startsWith("https://") - ) { - this.frameRepository.addFrameId(detail.tabId, detail.frameId); - } - }); this.contentMessageListener.run(); this.syncSettingRepository.onChange(() => { this.settingController.reload(); }); + this.findPortListener.run(); + } + + private onFindPortConnect(port: browser.runtime.Port) { + const tabId = port.sender?.tab?.id; + const frameId = port.sender?.frameId; + if (typeof tabId === "undefined" || typeof frameId === "undefined") { + return; + } + + this.frameRepository.addFrameId(tabId, frameId); + } + + private onFindPortDisconnect(port: browser.runtime.Port) { + const tabId = port.sender?.tab?.id; + const frameId = port.sender?.frameId; + if (typeof tabId === "undefined" || typeof frameId === "undefined") { + return; + } + + this.frameRepository.removeFrameId(tabId, frameId); } } diff --git a/src/background/infrastructures/FindPortListener.ts b/src/background/infrastructures/FindPortListener.ts new file mode 100644 index 0000000..ca82439 --- /dev/null +++ b/src/background/infrastructures/FindPortListener.ts @@ -0,0 +1,23 @@ +import { injectable } from "tsyringe"; + +type OnConnectFunc = (port: browser.runtime.Port) => void; +type OnDisconnectFunc = (port: browser.runtime.Port) => void; + +@injectable() +export default class FindPortListener { + constructor( + private readonly onConnect: OnConnectFunc, + private readonly onDisconnect: OnDisconnectFunc + ) {} + + run(): void { + browser.runtime.onConnect.addListener((port) => { + if (port.name !== "vimvixen-find") { + return; + } + + port.onDisconnect.addListener(this.onDisconnect); + this.onConnect(port); + }); + } +} diff --git a/src/background/operators/impls/FindNextOperator.ts b/src/background/operators/impls/FindNextOperator.ts index 2aed9fb..99f1759 100644 --- a/src/background/operators/impls/FindNextOperator.ts +++ b/src/background/operators/impls/FindNextOperator.ts @@ -21,72 +21,64 @@ export default class FindNextOperator implements Operator { return; } + const frameIds = await this.frameRepository.getFrameIds(tabId); + if (typeof frameIds === "undefined") { + // No frames are ready + return; + } + const state = await this.findRepository.getLocalState(tabId); if (state) { - // Start to find the keyword from the current frame which last found on, - // and concat it to end of frame ids to perform a wrap-search - // - // ,- keyword should be in this frame - // | - // [100, 101, 0, 100] - // | - // `- continue from frame id 100 - // - const targetFrameIds = state.frameIds - .slice(state.framePos) - .concat( - state.frameIds.slice(0, state.framePos), - state.frameIds[state.framePos] - ); + const framePos = frameIds.indexOf(state.frameId); + if (framePos !== -1) { + // Start to find the keyword from the current frame which last found on, + // and concat it to end of frame ids to perform a wrap-search + // + // ,- keyword should be in this frame + // | + // [100, 101, 0, 100] + // | + // `- continue from frame id 100 + // + const targetFrameIds = frameIds + .slice(framePos) + .concat(frameIds.slice(0, framePos), frameIds[framePos]); - for (let i = 0; i < targetFrameIds.length; ++i) { - const found = await this.findClient.findNext( + for (const frameId of targetFrameIds) { + const found = await this.findClient.findNext( + tabId, + frameId, + state.keyword + ); + if (found) { + this.findRepository.setLocalState(tabId, { + keyword: state.keyword, + frameId, + }); + return; + } + this.findClient.clearSelection(tabId, frameId); + } + + // The keyword is gone. + this.consoleClient.showError( tabId, - targetFrameIds[i], - state.keyword + "Pattern not found: " + state.keyword ); - if (found) { - this.findRepository.setLocalState(tabId, { - ...state, - framePos: (i + state.framePos) % state.frameIds.length, // save current frame position or first - }); - return; - } - this.findClient.clearSelection(tabId, targetFrameIds[i]); + return; } - - // The keyword is gone. - this.consoleClient.showError( - tabId, - "Pattern not found: " + state.keyword - ); - return; } const keyword = await this.findRepository.getGlobalKeyword(); if (keyword) { - const frameIds = await this.frameRepository.getFrameIds(tabId); - if (typeof frameIds === "undefined") { - // No frames are ready - return; - } - for (const frameId of frameIds) { await this.findClient.clearSelection(tabId, frameId); } - for (let framePos = 0; framePos < frameIds.length; ++framePos) { - const found = await this.findClient.findNext( - tabId, - frameIds[framePos], - keyword - ); + for (const frameId of frameIds) { + const found = await this.findClient.findNext(tabId, frameId, keyword); if (found) { - await this.findRepository.setLocalState(tabId, { - frameIds, - framePos, - keyword, - }); + await this.findRepository.setLocalState(tabId, { frameId, keyword }); await this.consoleClient.showInfo(tabId, "Pattern found: " + keyword); return; } diff --git a/src/background/operators/impls/FindPrevOperator.ts b/src/background/operators/impls/FindPrevOperator.ts index 3c4411d..f8506b9 100644 --- a/src/background/operators/impls/FindPrevOperator.ts +++ b/src/background/operators/impls/FindPrevOperator.ts @@ -21,71 +21,65 @@ export default class FindPrevOperator implements Operator { return; } + let frameIds = await this.frameRepository.getFrameIds(tabId); + if (typeof frameIds === "undefined") { + // No frames are ready + return; + } + frameIds = frameIds.slice(0).reverse(); + const state = await this.findRepository.getLocalState(tabId); if (state) { - // Start to find the keyword from the current frame which last found on, - // and concat it to end of frame ids to perform a wrap-search - // - // ,- keyword should be in this frame - // | - // [100, 101, 0, 100] - // | - // `- continue from frame id 100 - // - const targetFrameIds = state.frameIds - .slice(state.framePos) - .concat( - state.frameIds.slice(0, state.framePos), - state.frameIds[state.framePos] - ); + const framePos = frameIds.indexOf(state.frameId); + if (framePos !== -1) { + // Start to find the keyword from the current frame which last found on, + // and concat it to end of frame ids to perform a wrap-search + // + // ,- keyword should be in this frame + // | + // [100, 101, 0, 100] + // | + // `- continue from frame id 100 + // + const targetFrameIds = frameIds + .slice(framePos) + .concat(frameIds.slice(0, framePos), frameIds[framePos]); - for (let i = targetFrameIds.length - 1; i >= 0; --i) { - const found = await this.findClient.findPrev( + for (const frameId of targetFrameIds) { + const found = await this.findClient.findPrev( + tabId, + frameId, + state.keyword + ); + if (found) { + this.findRepository.setLocalState(tabId, { + keyword: state.keyword, + frameId, + }); + return; + } + this.findClient.clearSelection(tabId, frameId); + } + + // The keyword is gone. + this.consoleClient.showError( tabId, - targetFrameIds[i], - state.keyword + "Pattern not found: " + state.keyword ); - if (found) { - this.findRepository.setLocalState(tabId, { - ...state, - framePos: (i + state.framePos) % state.frameIds.length, // save current frame position or first - }); - return; - } - this.findClient.clearSelection(tabId, targetFrameIds[i]); + return; } - - // The keyword is gone. - this.consoleClient.showError( - tabId, - "Pattern not found: " + state.keyword - ); - return; } const keyword = await this.findRepository.getGlobalKeyword(); if (keyword) { - const frameIds = await this.frameRepository.getFrameIds(tabId); - if (typeof frameIds === "undefined") { - // No frames are ready - return; - } for (const frameId of frameIds) { await this.findClient.clearSelection(tabId, frameId); } - for (let framePos = frameIds.length - 1; framePos >= 0; --framePos) { - const found = await this.findClient.findPrev( - tabId, - frameIds[framePos], - keyword - ); + for (const frameId of frameIds) { + const found = await this.findClient.findPrev(tabId, frameId, keyword); if (found) { - await this.findRepository.setLocalState(tabId, { - frameIds, - framePos, - keyword, - }); + await this.findRepository.setLocalState(tabId, { frameId, keyword }); await this.consoleClient.showInfo(tabId, "Pattern found: " + keyword); return; } diff --git a/src/background/repositories/FindRepository.ts b/src/background/repositories/FindRepository.ts index 46ee390..3492759 100644 --- a/src/background/repositories/FindRepository.ts +++ b/src/background/repositories/FindRepository.ts @@ -6,8 +6,7 @@ const FIND_LOCAL_KEYWORD_KEY = "find-local-keyword"; export type FindState = { keyword: string; - framePos: number; - frameIds: number[]; + frameId: number; }; export default interface FindRepository { diff --git a/src/background/repositories/ReadyFrameRepository.ts b/src/background/repositories/ReadyFrameRepository.ts index 725f604..72ae5a4 100644 --- a/src/background/repositories/ReadyFrameRepository.ts +++ b/src/background/repositories/ReadyFrameRepository.ts @@ -7,7 +7,9 @@ type State = { [tabId: number]: number[] }; export default interface ReadyFrameRepository { clearFrameIds(tabId: number): Promise; - addFrameId(tabId: number, fraemId: number): Promise; + addFrameId(tabId: number, frameId: number): Promise; + + removeFrameId(tabId: number, frameId: number): Promise; getFrameIds(tabId: number): Promise; } @@ -29,12 +31,26 @@ export class ReadyFrameRepositoryImpl implements ReadyFrameRepository { return Promise.resolve(); } - addFrameId(tabId: number, fraemId: number): Promise { + addFrameId(tabId: number, frameId: number): Promise { let state: State | undefined = this.cache.get(REPOSITORY_KEY); if (typeof state === "undefined") { state = {}; } - state[tabId] = (state[tabId] || []).concat(fraemId); + state[tabId] = (state[tabId] || []).concat(frameId); + this.cache.set(REPOSITORY_KEY, state); + return Promise.resolve(); + } + + removeFrameId(tabId: number, frameId: number): Promise { + const state: State | undefined = this.cache.get(REPOSITORY_KEY); + if (typeof state === "undefined") { + return Promise.resolve(); + } + const ids = state[tabId]; + if (typeof ids === "undefined") { + return Promise.resolve(); + } + state[tabId] = ids.filter((id) => id != frameId); this.cache.set(REPOSITORY_KEY, state); return Promise.resolve(); } diff --git a/src/background/usecases/StartFindUseCase.ts b/src/background/usecases/StartFindUseCase.ts index a62462f..6aad962 100644 --- a/src/background/usecases/StartFindUseCase.ts +++ b/src/background/usecases/StartFindUseCase.ts @@ -40,16 +40,11 @@ export default class StartFindUseCase { await this.findClient.clearSelection(tabId, frameId); } - for (let framePos = 0; framePos < frameIds.length; ++framePos) { - const found = await this.findClient.findNext( - tabId, - frameIds[framePos], - keyword - ); + for (const frameId of frameIds) { + const found = await this.findClient.findNext(tabId, frameId, keyword); if (found) { await this.findRepository.setLocalState(tabId, { - frameIds, - framePos, + frameId, keyword, }); await this.consoleClient.showInfo(tabId, "Pattern found: " + keyword); diff --git a/src/content/Application.ts b/src/content/Application.ts index a12c3c6..6b881fe 100644 --- a/src/content/Application.ts +++ b/src/content/Application.ts @@ -41,7 +41,21 @@ export default class Application { if (window.self === window.top) { this.routeMasterComponents(); } - return this.routeCommonComponents(); + this.routeCommonComponents(); + // Make sure the background script sends a message to the content script by + // establishing a connection. If the background script tries to send a + // message to a frame on which cannot run the content script, it fails with + // a message "Could not establish connection." + // + // The port is never used, and the messages are delivered via + // `browser.tabs.sendMessage` API because sending a message via port cannot + // receive returned value. + // + // /* on background script */ + // port.sendMessage({ type: "do something" }); <- returns void + // + browser.runtime.connect(undefined, { name: "vimvixen-find" }); + return Promise.resolve(); } private routeMasterComponents() { diff --git a/test/background/mock/MockReadyFrameRepository.ts b/test/background/mock/MockReadyFrameRepository.ts index 63b7cf4..a95b2e1 100644 --- a/test/background/mock/MockReadyFrameRepository.ts +++ b/test/background/mock/MockReadyFrameRepository.ts @@ -9,6 +9,10 @@ export default class MockReadyFrameRepository implements ReadyFrameRepository { throw new Error("not implemented"); } + removeFrameId(_tabId: number, _frameId: number): Promise { + throw new Error("not implemented"); + } + getFrameIds(_tabId: number): Promise { throw new Error("not implemented"); } diff --git a/test/background/operators/impls/FindNextOperator.test.ts b/test/background/operators/impls/FindNextOperator.test.ts index b5088df..7509ef4 100644 --- a/test/background/operators/impls/FindNextOperator.test.ts +++ b/test/background/operators/impls/FindNextOperator.test.ts @@ -32,6 +32,10 @@ describe("FindNextOperator", () => { active: true, }); currentTabId = currentTab.id!; + + sinon + .stub(frameRepository, "getFrameIds") + .returns(Promise.resolve(frameIds)); }); describe("#run", () => { @@ -54,8 +58,7 @@ describe("FindNextOperator", () => { sinon.stub(findRepository, "getLocalState").returns( Promise.resolve({ keyword, - frameIds, - framePos: 1, + frameId: 100, }) ); @@ -68,7 +71,7 @@ describe("FindNextOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 1 }); + .withArgs(currentTabId, { keyword, frameId: 100 }); await sut.run(); @@ -80,8 +83,7 @@ describe("FindNextOperator", () => { sinon.stub(findRepository, "getLocalState").returns( Promise.resolve({ keyword, - frameIds, - framePos: 1, + frameId: 100, }) ); @@ -102,7 +104,7 @@ describe("FindNextOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 2 }); + .withArgs(currentTabId, { keyword, frameId: 101 }); await sut.run(); @@ -114,8 +116,7 @@ describe("FindNextOperator", () => { sinon.stub(findRepository, "getLocalState").returns( Promise.resolve({ keyword, - frameIds, - framePos: 2, + frameId: 101, }) ); @@ -136,7 +137,7 @@ describe("FindNextOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 0 }); + .withArgs(currentTabId, { keyword, frameId: 0 }); await sut.run(); @@ -151,9 +152,6 @@ describe("FindNextOperator", () => { sinon .stub(findRepository, "getGlobalKeyword") .returns(Promise.resolve(keyword)); - sinon - .stub(frameRepository, "getFrameIds") - .returns(Promise.resolve(frameIds)); sinon.stub(consoleClient, "showInfo").returns(Promise.resolve()); const mockFindClient = sinon.mock(findClient); @@ -168,7 +166,7 @@ describe("FindNextOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 0 }); + .withArgs(currentTabId, { keyword, frameId: 0 }); await sut.run(); diff --git a/test/background/operators/impls/FindPrevOperator.test.ts b/test/background/operators/impls/FindPrevOperator.test.ts index 296f0dd..090f815 100644 --- a/test/background/operators/impls/FindPrevOperator.test.ts +++ b/test/background/operators/impls/FindPrevOperator.test.ts @@ -32,6 +32,10 @@ describe("FindPrevOperator", () => { active: true, }); currentTabId = currentTab.id!; + + sinon + .stub(frameRepository, "getFrameIds") + .returns(Promise.resolve(frameIds.slice(0))); }); describe("#run", () => { @@ -54,8 +58,7 @@ describe("FindPrevOperator", () => { sinon.stub(findRepository, "getLocalState").returns( Promise.resolve({ keyword, - frameIds, - framePos: 1, + frameId: 100, }) ); @@ -68,7 +71,7 @@ describe("FindPrevOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 1 }); + .withArgs(currentTabId, { keyword, frameId: 100 }); await sut.run(); @@ -80,8 +83,7 @@ describe("FindPrevOperator", () => { sinon.stub(findRepository, "getLocalState").returns( Promise.resolve({ keyword, - frameIds, - framePos: 1, + frameId: 100, }) ); @@ -102,7 +104,7 @@ describe("FindPrevOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 0 }); + .withArgs(currentTabId, { keyword, frameId: 0 }); await sut.run(); @@ -114,8 +116,7 @@ describe("FindPrevOperator", () => { sinon.stub(findRepository, "getLocalState").returns( Promise.resolve({ keyword, - frameIds, - framePos: 0, + frameId: 0, }) ); @@ -136,7 +137,7 @@ describe("FindPrevOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 2 }); + .withArgs(currentTabId, { keyword, frameId: 101 }); await sut.run(); @@ -151,9 +152,6 @@ describe("FindPrevOperator", () => { sinon .stub(findRepository, "getGlobalKeyword") .returns(Promise.resolve(keyword)); - sinon - .stub(frameRepository, "getFrameIds") - .returns(Promise.resolve(frameIds)); sinon.stub(consoleClient, "showInfo").returns(Promise.resolve()); const mockFindClient = sinon.mock(findClient); @@ -168,7 +166,7 @@ describe("FindPrevOperator", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { keyword, frameIds, framePos: 2 }); + .withArgs(currentTabId, { keyword, frameId: 101 }); await sut.run(); diff --git a/test/background/repositories/FindRepository.test.ts b/test/background/repositories/FindRepository.test.ts index a08dc6d..d8c9506 100644 --- a/test/background/repositories/FindRepository.test.ts +++ b/test/background/repositories/FindRepository.test.ts @@ -25,12 +25,12 @@ describe("background/repositories/FindRepositoryImpl", () => { await sut.setLocalState(10, { keyword: "Hello, world", - frameIds: [20, 21], - framePos: 0, + frameId: 11, }); const state = await sut.getLocalState(10); expect(state?.keyword).to.equal("Hello, world"); + expect(state?.frameId).to.equal(11); expect(await sut.getLocalState(20)).to.be.undefined; }); diff --git a/test/background/usecases/StartFindUseCase.test.ts b/test/background/usecases/StartFindUseCase.test.ts index 99ab508..24e1fdc 100644 --- a/test/background/usecases/StartFindUseCase.test.ts +++ b/test/background/usecases/StartFindUseCase.test.ts @@ -46,7 +46,7 @@ describe("StartFindUseCase", () => { const mockFindRepository = sinon.mock(findRepository); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { frameIds, framePos: 1, keyword }); + .withArgs(currentTabId, { keyword, frameId: 100 }); const mockConsoleClient = sinon.mock(consoleClient); mockConsoleClient .expects("showInfo") @@ -76,10 +76,10 @@ describe("StartFindUseCase", () => { mockFindRepository .expects("getLocalState") .withArgs(currentTabId) - .returns(Promise.resolve({ keyword, frameIds, framePos: 0 })); + .returns(Promise.resolve({ keyword, frameId: 0 })); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { frameIds, framePos: 1, keyword }); + .withArgs(currentTabId, { keyword, frameId: 100 }); const mockConsoleClient = sinon.mock(consoleClient); mockConsoleClient .expects("showInfo") @@ -115,7 +115,7 @@ describe("StartFindUseCase", () => { .returns(Promise.resolve(keyword)); mockFindRepository .expects("setLocalState") - .withArgs(currentTabId, { frameIds, framePos: 1, keyword }); + .withArgs(currentTabId, { keyword, frameId: 100 }); const mockConsoleClient = sinon.mock(consoleClient); mockConsoleClient .expects("showInfo") -- cgit v1.2.3 From fa3500e3bbd11caa0e7ad1633b85f95600cf962f Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Sat, 25 Sep 2021 16:10:08 +0900 Subject: Sort frame IDs --- src/background/repositories/ReadyFrameRepository.ts | 2 +- test/background/repositories/ReadyFrameRepository.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src/background') diff --git a/src/background/repositories/ReadyFrameRepository.ts b/src/background/repositories/ReadyFrameRepository.ts index 72ae5a4..aa5d986 100644 --- a/src/background/repositories/ReadyFrameRepository.ts +++ b/src/background/repositories/ReadyFrameRepository.ts @@ -36,7 +36,7 @@ export class ReadyFrameRepositoryImpl implements ReadyFrameRepository { if (typeof state === "undefined") { state = {}; } - state[tabId] = (state[tabId] || []).concat(frameId); + state[tabId] = (state[tabId] || []).concat(frameId).sort(); this.cache.set(REPOSITORY_KEY, state); return Promise.resolve(); } diff --git a/test/background/repositories/ReadyFrameRepository.test.ts b/test/background/repositories/ReadyFrameRepository.test.ts index d952a9b..888f3f5 100644 --- a/test/background/repositories/ReadyFrameRepository.test.ts +++ b/test/background/repositories/ReadyFrameRepository.test.ts @@ -12,10 +12,11 @@ describe("background/repositories/ReadyFrameRepositoryImpl", () => { expect(await sut.getFrameIds(1)).to.be.undefined; await sut.addFrameId(1, 10); + await sut.addFrameId(1, 12); await sut.addFrameId(1, 11); await sut.addFrameId(2, 20); - expect(await sut.getFrameIds(1)).to.deep.equal([10, 11]); + expect(await sut.getFrameIds(1)).to.deep.equal([10, 11, 12]); expect(await sut.getFrameIds(2)).to.deep.equal([20]); await sut.clearFrameIds(1); -- cgit v1.2.3 From a0f2c3ee107508e406a4a41cbda76ecd2d3f0d8f Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Sat, 25 Sep 2021 18:17:50 +0900 Subject: Discover frames on search by count of connections When a tab switches pages quickly, a disconnect event on top frame is sometime delivered after second connect event. In addition, `tabs.onUpdated()` event is independent on port connection event. Now the background script finds alive frames by only port connection. --- src/background/Application.ts | 1 - .../repositories/ReadyFrameRepository.ts | 37 ++++++++++++---------- src/background/usecases/ReadyFrameUseCase.ts | 18 ----------- test/background/mock/MockReadyFrameRepository.ts | 4 --- .../repositories/ReadyFrameRepository.test.ts | 13 ++++++-- 5 files changed, 31 insertions(+), 42 deletions(-) delete mode 100644 src/background/usecases/ReadyFrameUseCase.ts (limited to 'src/background') diff --git a/src/background/Application.ts b/src/background/Application.ts index c7bcc42..b439d19 100644 --- a/src/background/Application.ts +++ b/src/background/Application.ts @@ -31,7 +31,6 @@ export default class Application { browser.tabs.onUpdated.addListener((tabId: number, info) => { if (info.status == "loading") { - this.frameRepository.clearFrameIds(tabId); this.findRepository.deleteLocalState(tabId); } }); diff --git a/src/background/repositories/ReadyFrameRepository.ts b/src/background/repositories/ReadyFrameRepository.ts index aa5d986..c993858 100644 --- a/src/background/repositories/ReadyFrameRepository.ts +++ b/src/background/repositories/ReadyFrameRepository.ts @@ -2,11 +2,9 @@ import MemoryStorage from "../infrastructures/MemoryStorage"; const REPOSITORY_KEY = "readyFrameRepository"; -type State = { [tabId: number]: number[] }; +type State = { [tabId: number]: { [frameId: number]: number } }; export default interface ReadyFrameRepository { - clearFrameIds(tabId: number): Promise; - addFrameId(tabId: number, frameId: number): Promise; removeFrameId(tabId: number, frameId: number): Promise; @@ -21,22 +19,14 @@ export class ReadyFrameRepositoryImpl implements ReadyFrameRepository { this.cache = new MemoryStorage(); } - clearFrameIds(tabId: number): Promise { - let state: State | undefined = this.cache.get(REPOSITORY_KEY); - if (typeof state === "undefined") { - state = {}; - } - delete state[tabId]; - this.cache.set(REPOSITORY_KEY, state); - return Promise.resolve(); - } - addFrameId(tabId: number, frameId: number): Promise { let state: State | undefined = this.cache.get(REPOSITORY_KEY); if (typeof state === "undefined") { state = {}; } - state[tabId] = (state[tabId] || []).concat(frameId).sort(); + const tab = state[tabId] || {}; + tab[frameId] = (tab[frameId] || 0) + 1; + state[tabId] = tab; this.cache.set(REPOSITORY_KEY, state); return Promise.resolve(); } @@ -50,7 +40,15 @@ export class ReadyFrameRepositoryImpl implements ReadyFrameRepository { if (typeof ids === "undefined") { return Promise.resolve(); } - state[tabId] = ids.filter((id) => id != frameId); + const tab = state[tabId] || {}; + tab[frameId] = (tab[frameId] || 0) - 1; + if (tab[frameId] == 0) { + delete tab[frameId]; + } + if (Object.keys(tab).length === 0) { + delete state[tabId]; + } + this.cache.set(REPOSITORY_KEY, state); return Promise.resolve(); } @@ -60,6 +58,13 @@ export class ReadyFrameRepositoryImpl implements ReadyFrameRepository { if (typeof state === "undefined") { return Promise.resolve(undefined); } - return Promise.resolve(state[tabId]); + const tab = state[tabId]; + if (typeof tab === "undefined") { + return Promise.resolve(undefined); + } + const frameIds = Object.keys(tab) + .map((v) => Number(v)) + .sort(); + return Promise.resolve(frameIds); } } diff --git a/src/background/usecases/ReadyFrameUseCase.ts b/src/background/usecases/ReadyFrameUseCase.ts deleted file mode 100644 index 81bee0c..0000000 --- a/src/background/usecases/ReadyFrameUseCase.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { inject, injectable } from "tsyringe"; -import ReadyFrameRepository from "../repositories/ReadyFrameRepository"; - -@injectable() -export default class ReadyFrameUseCase { - constructor( - @inject("ReadyFrameRepository") - private readonly frameRepository: ReadyFrameRepository - ) {} - - async addReadyFrame(tabId: number, frameId: number): Promise { - return this.frameRepository.addFrameId(tabId, frameId); - } - - async clearReadyFrame(tabId: number): Promise { - return this.frameRepository.clearFrameIds(tabId); - } -} diff --git a/test/background/mock/MockReadyFrameRepository.ts b/test/background/mock/MockReadyFrameRepository.ts index a95b2e1..4a5ec52 100644 --- a/test/background/mock/MockReadyFrameRepository.ts +++ b/test/background/mock/MockReadyFrameRepository.ts @@ -1,10 +1,6 @@ import ReadyFrameRepository from "../../../src/background/repositories/ReadyFrameRepository"; export default class MockReadyFrameRepository implements ReadyFrameRepository { - clearFrameIds(_tabId: number): Promise { - throw new Error("not implemented"); - } - addFrameId(_tabId: number, _fraemId: number): Promise { throw new Error("not implemented"); } diff --git a/test/background/repositories/ReadyFrameRepository.test.ts b/test/background/repositories/ReadyFrameRepository.test.ts index 888f3f5..71f20af 100644 --- a/test/background/repositories/ReadyFrameRepository.test.ts +++ b/test/background/repositories/ReadyFrameRepository.test.ts @@ -15,12 +15,19 @@ describe("background/repositories/ReadyFrameRepositoryImpl", () => { await sut.addFrameId(1, 12); await sut.addFrameId(1, 11); await sut.addFrameId(2, 20); + await sut.addFrameId(2, 21); + await sut.addFrameId(2, 21); expect(await sut.getFrameIds(1)).to.deep.equal([10, 11, 12]); - expect(await sut.getFrameIds(2)).to.deep.equal([20]); + expect(await sut.getFrameIds(2)).to.deep.equal([20, 21]); - await sut.clearFrameIds(1); + await sut.removeFrameId(2, 21); + expect(await sut.getFrameIds(2)).to.deep.equal([20, 21]); - expect(await sut.getFrameIds(1)).to.be.undefined; + await sut.removeFrameId(2, 21); + expect(await sut.getFrameIds(2)).to.deep.equal([20]); + + await sut.removeFrameId(2, 20); + expect(await sut.getFrameIds(2)).to.be.undefined; }); }); -- cgit v1.2.3