Skip to content

Commit

Permalink
remove CentralIpcHandler and use webContents.ipc
Browse files Browse the repository at this point in the history
our bookkeeping is not necessary anymore since each webContents has
its own scoped IpcMain instance now.

close #4848
  • Loading branch information
ganthern committed Feb 23, 2023
1 parent 93a551a commit 8816c12
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 99 deletions.
2 changes: 0 additions & 2 deletions src/desktop/ApplicationWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,6 @@ export class ApplicationWindow {
}

async reload(queryParams: Record<string, string | boolean>) {
// do this immediately as to not get the window destroyed on us
this.remoteBridge.destroyBridge(this)
await this.closeDb()
this.userId = null
this.initFacades()
Expand Down
4 changes: 2 additions & 2 deletions src/desktop/DesktopMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { DateProviderImpl } from "../calendar/date/CalendarUtils"
import { DesktopThemeFacade } from "./DesktopThemeFacade"
import { BuildConfigKey, DesktopConfigKey } from "./config/ConfigKeys"
import { DesktopNativeCredentialsFacade } from "./credentials/DesktopNativeCredentialsFacade.js"
import { webauthnIpcHandler, WebDialogController } from "./WebDialog.js"
import { WebDialogController } from "./WebDialog.js"
import path from "path"
import { DesktopContextMenu } from "./DesktopContextMenu.js"
import { DesktopNativePushFacade } from "./sse/DesktopNativePushFacade.js"
Expand Down Expand Up @@ -188,7 +188,7 @@ async function createComponents(): Promise<Components> {
log.error("Could not reschedule alarms", e)
return sse.resetStoredState()
})
const webDialogController = new WebDialogController(webauthnIpcHandler)
const webDialogController = new WebDialogController()

tray.setWindowManager(wm)
const sse = new DesktopSseClient(app, conf, notifier, wm, desktopAlarmScheduler, desktopNet, desktopCrypto, alarmStorage, lang)
Expand Down
15 changes: 4 additions & 11 deletions src/desktop/DesktopSqlCipher.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Database, default as sqlite } from "better-sqlite3"
import { Database, default as Sqlite } from "better-sqlite3"
import { CryptoError } from "@tutao/tutanota-crypto"
import { mapNullable, uint8ArrayToBase64 } from "@tutao/tutanota-utils"
import { SqlCipherFacade } from "../native/common/generatedipc/SqlCipherFacade.js"
Expand All @@ -17,11 +17,13 @@ export class DesktopSqlCipher implements SqlCipherFacade {

/**
* @param nativeBindingPath the path to the sqlite native module
* @param dbPath the path to the database file to use
* @param integrityCheck whether to check the integrity of the db file during initialization
*/
constructor(private readonly nativeBindingPath: string, private readonly dbPath: string, private readonly integrityCheck: boolean) {}

async openDb(userId: string, dbKey: Uint8Array): Promise<void> {
this._db = new sqlite(this.dbPath, {
this._db = new Sqlite(this.dbPath, {
// Remove ts-ignore once proper definition of Options exists, see https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/59049#
// @ts-ignore missing type
nativeBinding: this.nativeBindingPath,
Expand Down Expand Up @@ -137,12 +139,3 @@ export class DesktopSqlCipher implements SqlCipherFacade {
}
}
}

function boolToSqlite(bool: boolean): SqliteBool {
return bool ? SqliteBool.TRUE : SqliteBool.FALSE
}

enum SqliteBool {
TRUE = 1,
FALSE = 0,
}
19 changes: 2 additions & 17 deletions src/desktop/WebDialog.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { app, BrowserWindow, ipcMain, WebContents } from "electron"
import { app, BrowserWindow, WebContents } from "electron"
import path from "path"
import { defer } from "@tutao/tutanota-utils"
import { ElectronWebContentsTransport } from "./ipc/ElectronWebContentsTransport.js"
import { NativeToWebRequest, WebToNativeRequest } from "../native/main/WebauthnNativeBridge.js"
import { MessageDispatcher } from "../api/common/MessageDispatcher.js"
import { exposeRemote } from "../api/common/WorkerProxy.js"
import { CancelledError } from "../api/common/error/CancelledError.js"
import { CentralIpcHandler } from "./ipc/CentralIpcHandler.js"
import { register } from "./electron-localshortcut/LocalShortcut.js"
import { ProgrammingError } from "../api/common/error/ProgrammingError.js"

Expand All @@ -16,10 +15,6 @@ export const webauthnIpcConfig = Object.freeze({
})

export type WebDialogIpcConfig = typeof webauthnIpcConfig
export type WebDialogIpcHandler = CentralIpcHandler<WebDialogIpcConfig>

// Singleton
export const webauthnIpcHandler: WebDialogIpcHandler = new CentralIpcHandler(ipcMain, webauthnIpcConfig)

/** A dialog which was already loaded. Allows sending one request or closing it. */
export class WebDialog<FacadeType extends object> {
Expand Down Expand Up @@ -53,8 +48,6 @@ export class WebDialog<FacadeType extends object> {
* * returns the result of the call
*/
export class WebDialogController {
constructor(private readonly ipcHandler: WebDialogIpcHandler) {}

async create<FacadeType extends object>(parentWindowId: number, urlToOpen: URL): Promise<WebDialog<FacadeType>> {
const bw = await this.createBrowserWindow(parentWindowId)
// Holding a separate reference on purpose. When BrowserWindow is destroyed and WebContents fire "destroyed" event, we can't get WebContents from
Expand All @@ -78,10 +71,6 @@ export class WebDialogController {
const facadePromise = this.initRemoteFacade<FacadeType>(bw.webContents)
await bw.loadURL(urlToOpen.toString())

webContents.on("destroyed", () => {
this.uninitRemoteFacade(webContents)
})

// We can confidently await here because we're sure that the bridge will be finished being setup in the webDialog
const facade = await facadePromise
return new WebDialog(facade, closeDefer.promise, bw)
Expand Down Expand Up @@ -146,7 +135,7 @@ export class WebDialogController {
*/
private async initRemoteFacade<FacadeType>(webContents: WebContents): Promise<FacadeType> {
const deferred = defer<void>()
const transport = new ElectronWebContentsTransport<WebDialogIpcConfig, "facade", "init">(webContents, this.ipcHandler)
const transport = new ElectronWebContentsTransport<WebDialogIpcConfig, "facade", "init">(webContents, webauthnIpcConfig)
const dispatcher = new MessageDispatcher<NativeToWebRequest, WebToNativeRequest>(transport, {
init: () => {
deferred.resolve()
Expand All @@ -157,8 +146,4 @@ export class WebDialogController {
await deferred.promise
return facade
}

private async uninitRemoteFacade(webContents: WebContents) {
this.ipcHandler.removeHandler(webContents)
}
}
44 changes: 0 additions & 44 deletions src/desktop/ipc/CentralIpcHandler.ts

This file was deleted.

14 changes: 10 additions & 4 deletions src/desktop/ipc/ElectronWebContentsTransport.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import type { Message, Transport } from "../../api/common/MessageDispatcher.js"
import type { CentralIpcHandler, IpcConfig } from "./CentralIpcHandler.js"
import type { WebContents } from "electron"

export interface IpcConfig<RenderToMainEvent extends string, MainToRenderEvent extends string> {
renderToMainEvent: RenderToMainEvent
mainToRenderEvent: MainToRenderEvent
}

/**
* Implementation of Transport which delegates to CenterIpcHandler/WebContents.
* Should be instantiated per WebContents.
Expand All @@ -12,13 +16,15 @@ export class ElectronWebContentsTransport<
IncomingRequestType extends string,
> implements Transport<OutgoingRequestType, IncomingRequestType>
{
constructor(private readonly webContents: WebContents, private readonly ipcHandler: CentralIpcHandler<IpcConfigType>) {}
constructor(private readonly webContents: WebContents, private readonly config: IpcConfigType) {}

postMessage(message: Message<OutgoingRequestType>): void {
this.ipcHandler.sendTo(this.webContents, message)
if (this.webContents.isDestroyed()) return
this.webContents.send(this.config.mainToRenderEvent, message)
}

setMessageHandler(handler: (message: Message<IncomingRequestType>) => unknown): void {
this.ipcHandler.addHandler(this.webContents, handler)
if (this.webContents.isDestroyed()) return
this.webContents.ipc.handle(this.config.renderToMainEvent, (ev, arg) => handler(arg))
}
}
15 changes: 2 additions & 13 deletions src/desktop/ipc/RemoteBridge.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import * as electron from "electron"
import { DesktopFacade } from "../../native/common/generatedipc/DesktopFacade.js"
import { CommonNativeFacade } from "../../native/common/generatedipc/CommonNativeFacade.js"
import { ApplicationWindow } from "../ApplicationWindow.js"
import { ElectronWebContentsTransport } from "./ElectronWebContentsTransport.js"
import { ElectronWebContentsTransport, IpcConfig } from "./ElectronWebContentsTransport.js"
import { DesktopGlobalDispatcher } from "../../native/common/generatedipc/DesktopGlobalDispatcher.js"
import { MessageDispatcher, Request } from "../../api/common/MessageDispatcher.js"
import { DesktopFacadeSendDispatcher } from "../../native/common/generatedipc/DesktopFacadeSendDispatcher.js"
import { CommonNativeFacadeSendDispatcher } from "../../native/common/generatedipc/CommonNativeFacadeSendDispatcher.js"
import { CentralIpcHandler, IpcConfig } from "./CentralIpcHandler.js"
import { DesktopCommonSystemFacade } from "../DesktopCommonSystemFacade.js"
import { InterWindowEventFacadeSendDispatcher } from "../../native/common/generatedipc/InterWindowEventFacadeSendDispatcher.js"

Expand All @@ -21,8 +19,6 @@ const primaryIpcConfig: IpcConfig<"to-main", "to-renderer"> = {
renderToMainEvent: "to-main",
mainToRenderEvent: "to-renderer",
} as const
// Must be created only once
const primaryIpcHandler = new CentralIpcHandler(electron.ipcMain, primaryIpcConfig)

export type DispatcherFactory = (window: ApplicationWindow) => { desktopCommonSystemFacade: DesktopCommonSystemFacade; dispatcher: DesktopGlobalDispatcher }
export type FacadeHandler = (message: Request<"facade">) => Promise<any>
Expand All @@ -33,11 +29,10 @@ export class RemoteBridge {

createBridge(window: ApplicationWindow): SendingFacades {
const webContents = window._browserWindow.webContents
webContents.on("destroyed", () => primaryIpcHandler.removeHandler(webContents))
const { desktopCommonSystemFacade, dispatcher } = this.dispatcherFactory(window)
const facadeHandler = this.facadeHandlerFactory(window)

const transport = new ElectronWebContentsTransport<typeof primaryIpcConfig, JsRequestType, NativeRequestType>(webContents, primaryIpcHandler)
const transport = new ElectronWebContentsTransport<typeof primaryIpcConfig, JsRequestType, NativeRequestType>(webContents, primaryIpcConfig)
const messageDispatcher = new MessageDispatcher<JsRequestType, NativeRequestType>(transport, {
facade: facadeHandler,
ipc: async ({ args }) => {
Expand All @@ -57,10 +52,4 @@ export class RemoteBridge {
interWindowEventSender: new InterWindowEventFacadeSendDispatcher(nativeInterface),
}
}

destroyBridge(window: ApplicationWindow) {
// in this case, we can't get a ref to the wc anymore
if (!window._browserWindow || window._browserWindow.isDestroyed()) return
primaryIpcHandler.removeHandler(window._browserWindow.webContents)
}
}
7 changes: 3 additions & 4 deletions src/gui/base/List.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import {
addAll,
arrayEquals,
assertNotNull,
clear as clearArr,
debounceStart,
defer,
last,
lastThrow,
mapLazily,
neverNull,
remove,
clear as clearArr,
} from "@tutao/tutanota-utils"
import ColumnEmptyMessageBox from "./ColumnEmptyMessageBox"
import { progressIcon } from "./Icon"
Expand All @@ -33,7 +33,6 @@ import { firstBiggerThanSecond, GENERATED_MAX_ID, getElementId, getLetId } from
import { assertMainOrNode } from "../../api/common/Env"
import { Button, ButtonType } from "./Button.js"
import { LoadingState, LoadingStateTracker } from "../../offline/LoadingState"
import { lang } from "../../misc/LanguageViewModel"
import { isOfflineError } from "../../api/common/utils/ErrorCheckUtils.js"
import { PosRect } from "./Dropdown.js"

Expand Down Expand Up @@ -667,7 +666,7 @@ export class List<ElementType extends ListElement, RowType extends VirtualRow<El
shiftPressed = false
}

if (shiftPressed && this.lastMultiSelectWasKeyUp === true && this.selectedEntities.length > 1) {
if (shiftPressed && this.lastMultiSelectWasKeyUp && this.selectedEntities.length > 1) {
// we have to remove the selection from the top
this.selectedEntities.splice(0, 1)

Expand Down Expand Up @@ -701,7 +700,7 @@ export class List<ElementType extends ListElement, RowType extends VirtualRow<El
shiftPressed = false
}

if (shiftPressed && this.lastMultiSelectWasKeyUp === false && this.selectedEntities.length > 1) {
if (shiftPressed && !this.lastMultiSelectWasKeyUp && this.selectedEntities.length > 1) {
// we have to remove the selection from the bottom
this.selectedEntities.splice(-1, 1)

Expand Down
2 changes: 0 additions & 2 deletions test/tests/desktop/ApplicationWindowTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,8 +570,6 @@ o.spec("ApplicationWindow Test", function () {
// Reset IPC
const initialized = defer<void>()

remoteBridge.destroyBridge(w)

bwInstance.webContents.callbacks["did-finish-load"]()
// Init IPC
initialized.resolve()
Expand Down

0 comments on commit 8816c12

Please sign in to comment.