From b82080db6015bf3e4bea0acfade724c2b0ec0321 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Thu, 6 Apr 2023 01:58:03 -0400 Subject: [PATCH] Moves pending ipc handling to controller --- src/plus/webviews/focus/protocol.ts | 1 - src/plus/webviews/graph/graphWebview.ts | 149 +++++++-------------- src/plus/webviews/graph/protocol.ts | 13 +- src/webviews/commitDetails/commitDetailsWebview.ts | 4 - src/webviews/commitDetails/protocol.ts | 2 +- src/webviews/protocol.ts | 2 +- src/webviews/webviewController.ts | 73 ++++++++-- 7 files changed, 112 insertions(+), 132 deletions(-) diff --git a/src/plus/webviews/focus/protocol.ts b/src/plus/webviews/focus/protocol.ts index 8b92021..57c46b6 100644 --- a/src/plus/webviews/focus/protocol.ts +++ b/src/plus/webviews/focus/protocol.ts @@ -63,5 +63,4 @@ export interface DidChangeSubscriptionParams { } export const DidChangeSubscriptionNotificationType = new IpcNotificationType( 'graph/subscription/didChange', - true, ); diff --git a/src/plus/webviews/graph/graphWebview.ts b/src/plus/webviews/graph/graphWebview.ts index e3c4753..a52dd64 100644 --- a/src/plus/webviews/graph/graphWebview.ts +++ b/src/plus/webviews/graph/graphWebview.ts @@ -76,7 +76,7 @@ import { getSettledValue } from '../../../system/promise'; import { isDarkTheme, isLightTheme } from '../../../system/utils'; import { isWebviewItemContext, isWebviewItemGroupContext, serializeWebviewItemContext } from '../../../system/webview'; import { RepositoryFolderNode } from '../../../views/nodes/viewNode'; -import type { IpcMessage, IpcMessageParams, IpcNotificationType } from '../../../webviews/protocol'; +import type { IpcMessage, IpcNotificationType } from '../../../webviews/protocol'; import { onIpc } from '../../../webviews/protocol'; import type { WebviewController, WebviewProvider } from '../../../webviews/webviewController'; import type { SubscriptionChangeEvent } from '../../subscription/subscriptionService'; @@ -205,7 +205,17 @@ export class GraphWebviewProvider implements WebviewProvider { private _etagRepository?: number; private _firstSelection = true; private _graph?: GitGraph; - private _pendingIpcNotifications = new Map Promise)>(); + private readonly _ipcNotificationMap = new Map, () => Promise>([ + [DidChangeColumnsNotificationType, this.notifyDidChangeColumns], + [DidChangeGraphConfigurationNotificationType, this.notifyDidChangeConfiguration], + [DidChangeNotificationType, this.notifyDidChangeState], + [DidChangeRefsVisibilityNotificationType, this.notifyDidChangeRefsVisibility], + [DidChangeSelectionNotificationType, this.notifyDidChangeSelection], + [DidChangeSubscriptionNotificationType, this.notifyDidChangeSubscription], + [DidChangeWorkingTreeNotificationType, this.notifyDidChangeWorkingTree], + [DidChangeWindowFocusNotificationType, this.notifyDidChangeWindowFocus], + [DidFetchNotificationType, this.notifyDidFetch], + ]); private _refsMetadata: Map | null | undefined; private _search: GitSearch | undefined; private _searchCancellation: CancellationTokenSource | undefined; @@ -294,7 +304,6 @@ export class GraphWebviewProvider implements WebviewProvider { onRefresh(force?: boolean) { if (force) { this.resetRepositoryState(); - this._pendingIpcNotifications.clear(); } } @@ -388,10 +397,6 @@ export class GraphWebviewProvider implements WebviewProvider { void this.notifyDidChangeWindowFocus(); } - onReady(): void { - this.sendPendingIpcNotifications(); - } - onFocusChanged(focused: boolean): void { if (!focused || this.activeSelection == null || !this.container.commitDetailsView.visible) { this._showActiveSelectionDetailsDebounced?.cancel(); @@ -417,7 +422,7 @@ export class GraphWebviewProvider implements WebviewProvider { if (visible) { if (this.host.ready) { - this.sendPendingIpcNotifications(); + this.host.sendPendingIpcNotifications(); } const { activeSelection } = this; @@ -734,7 +739,7 @@ export class GraphWebviewProvider implements WebviewProvider { } } - void this.notify(DidEnsureRowNotificationType, { id: id }, completionId); + void this.host.notify(DidEnsureRowNotificationType, { id: id }, completionId); } private async onGetMissingAvatars(e: GetMissingAvatarsParams) { @@ -910,7 +915,7 @@ export class GraphWebviewProvider implements WebviewProvider { this._search = search; void (await this.ensureSearchStartsInRange(this._graph!, search)); - void this.notify( + void this.host.notify( DidSearchNotificationType, { results: @@ -953,7 +958,7 @@ export class GraphWebviewProvider implements WebviewProvider { } catch (ex) { this._search = undefined; - void this.notify( + void this.host.notify( DidSearchNotificationType, { results: { @@ -967,7 +972,7 @@ export class GraphWebviewProvider implements WebviewProvider { if (cancellation.token.isCancellationRequested) { if (completionId != null) { - void this.notify(DidSearchNotificationType, { results: undefined }, completionId); + void this.host.notify(DidSearchNotificationType, { results: undefined }, completionId); } return; } @@ -985,7 +990,7 @@ export class GraphWebviewProvider implements WebviewProvider { this.setSelectedRows(firstResult); } - void this.notify( + void this.host.notify( DidSearchNotificationType, { results: @@ -1103,7 +1108,7 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private updateState(immediate: boolean = false) { - this._pendingIpcNotifications.clear(); + this.host.clearPendingIpcNotifications(); if (immediate) { void this.notifyDidChangeState(); @@ -1120,11 +1125,11 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeWindowFocus(): Promise { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeWindowFocusNotificationType); + this.host.addPendingIpcNotification(DidChangeWindowFocusNotificationType, this._ipcNotificationMap, this); return false; } - return this.notify(DidChangeWindowFocusNotificationType, { + return this.host.notify(DidChangeWindowFocusNotificationType, { focused: this.isWindowFocused, }); } @@ -1151,7 +1156,7 @@ export class GraphWebviewProvider implements WebviewProvider { if (this._graph == null) return; const data = this._graph; - return this.notify(DidChangeAvatarsNotificationType, { + return this.host.notify(DidChangeAvatarsNotificationType, { avatars: Object.fromEntries(data.avatars), }); } @@ -1176,7 +1181,7 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeRefsMetadata() { - return this.notify(DidChangeRefsMetadataNotificationType, { + return this.host.notify(DidChangeRefsMetadataNotificationType, { metadata: this._refsMetadata != null ? Object.fromEntries(this._refsMetadata) : this._refsMetadata, }); } @@ -1184,13 +1189,13 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeColumns() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeColumnsNotificationType); + this.host.addPendingIpcNotification(DidChangeColumnsNotificationType, this._ipcNotificationMap, this); return false; } const columns = this.getColumns(); const columnSettings = this.getColumnSettings(columns); - return this.notify(DidChangeColumnsNotificationType, { + return this.host.notify(DidChangeColumnsNotificationType, { columns: columnSettings, context: this.getColumnHeaderContext(columnSettings), }); @@ -1199,11 +1204,15 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeRefsVisibility() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeRefsVisibilityNotificationType); + this.host.addPendingIpcNotification( + DidChangeRefsVisibilityNotificationType, + this._ipcNotificationMap, + this, + ); return false; } - return this.notify(DidChangeRefsVisibilityNotificationType, { + return this.host.notify(DidChangeRefsVisibilityNotificationType, { excludeRefs: this.getExcludedRefs(this._graph), excludeTypes: this.getExcludedTypes(this._graph), includeOnlyRefs: this.getIncludeOnlyRefs(this._graph), @@ -1213,11 +1222,15 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeConfiguration() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeGraphConfigurationNotificationType); + this.host.addPendingIpcNotification( + DidChangeGraphConfigurationNotificationType, + this._ipcNotificationMap, + this, + ); return false; } - return this.notify(DidChangeGraphConfigurationNotificationType, { + return this.host.notify(DidChangeGraphConfigurationNotificationType, { config: this.getComponentConfig(), }); } @@ -1225,12 +1238,12 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidFetch() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidFetchNotificationType); + this.host.addPendingIpcNotification(DidFetchNotificationType, this._ipcNotificationMap, this); return false; } const lastFetched = await this.repository!.getLastFetched(); - return this.notify(DidFetchNotificationType, { + return this.host.notify(DidFetchNotificationType, { lastFetched: new Date(lastFetched), }); } @@ -1240,7 +1253,7 @@ export class GraphWebviewProvider implements WebviewProvider { if (this._graph == null) return; const data = this._graph; - return this.notify( + return this.host.notify( DidChangeRowsNotificationType, { rows: data.rows, @@ -1260,11 +1273,11 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeWorkingTree() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeWorkingTreeNotificationType); + this.host.addPendingIpcNotification(DidChangeWorkingTreeNotificationType, this._ipcNotificationMap, this); return false; } - return this.notify(DidChangeWorkingTreeNotificationType, { + return this.host.notify(DidChangeWorkingTreeNotificationType, { stats: (await this.getWorkingTreeStats()) ?? { added: 0, deleted: 0, modified: 0 }, }); } @@ -1272,11 +1285,11 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeSelection() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeSelectionNotificationType); + this.host.addPendingIpcNotification(DidChangeSelectionNotificationType, this._ipcNotificationMap, this); return false; } - return this.notify(DidChangeSelectionNotificationType, { + return this.host.notify(DidChangeSelectionNotificationType, { selection: this._selectedRows ?? {}, }); } @@ -1284,12 +1297,12 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeSubscription() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeSubscriptionNotificationType); + this.host.addPendingIpcNotification(DidChangeSubscriptionNotificationType, this._ipcNotificationMap, this); return false; } const [access] = await this.getGraphAccess(); - return this.notify(DidChangeSubscriptionNotificationType, { + return this.host.notify(DidChangeSubscriptionNotificationType, { subscription: access.subscription.current, allowed: access.allowed !== false, }); @@ -1298,77 +1311,11 @@ export class GraphWebviewProvider implements WebviewProvider { @debug() private async notifyDidChangeState() { if (!this.host.ready || !this.host.visible) { - this.addPendingIpcNotification(DidChangeNotificationType); + this.host.addPendingIpcNotification(DidChangeNotificationType, this._ipcNotificationMap, this); return false; } - return this.notify(DidChangeNotificationType, { state: await this.getState() }); - } - - private async notify>( - type: T, - params: IpcMessageParams, - completionId?: string, - ): Promise { - const msg: IpcMessage = { - id: this.host.nextIpcId(), - method: type.method, - params: params, - completionId: completionId, - }; - const success = await this.host.postMessage(msg); - if (success) { - this._pendingIpcNotifications.clear(); - } else { - this.addPendingIpcNotification(type, msg); - } - return success; - } - - private readonly _ipcNotificationMap = new Map, () => Promise>([ - [DidChangeColumnsNotificationType, this.notifyDidChangeColumns], - [DidChangeGraphConfigurationNotificationType, this.notifyDidChangeConfiguration], - [DidChangeNotificationType, this.notifyDidChangeState], - [DidChangeRefsVisibilityNotificationType, this.notifyDidChangeRefsVisibility], - [DidChangeSelectionNotificationType, this.notifyDidChangeSelection], - [DidChangeSubscriptionNotificationType, this.notifyDidChangeSubscription], - [DidChangeWorkingTreeNotificationType, this.notifyDidChangeWorkingTree], - [DidChangeWindowFocusNotificationType, this.notifyDidChangeWindowFocus], - [DidFetchNotificationType, this.notifyDidFetch], - ]); - - private addPendingIpcNotification(type: IpcNotificationType, msg?: IpcMessage) { - if (type === DidChangeNotificationType) { - this._pendingIpcNotifications.clear(); - } else if (type.overwriteable) { - this._pendingIpcNotifications.delete(type); - } - - let msgOrFn: IpcMessage | (() => Promise) | undefined; - if (msg == null) { - msgOrFn = this._ipcNotificationMap.get(type)?.bind(this); - if (msgOrFn == null) { - debugger; - return; - } - } else { - msgOrFn = msg; - } - this._pendingIpcNotifications.set(type, msgOrFn); - } - - private sendPendingIpcNotifications() { - if (this._pendingIpcNotifications.size === 0) return; - - const ipcs = new Map(this._pendingIpcNotifications); - this._pendingIpcNotifications.clear(); - for (const msgOrFn of ipcs.values()) { - if (typeof msgOrFn === 'function') { - void msgOrFn(); - } else { - void this.host.postMessage(msgOrFn); - } - } + return this.host.notify(DidChangeNotificationType, { state: await this.getState() }); } private ensureRepositorySubscriptions(force?: boolean) { diff --git a/src/plus/webviews/graph/protocol.ts b/src/plus/webviews/graph/protocol.ts index acdb93e..f19231c 100644 --- a/src/plus/webviews/graph/protocol.ts +++ b/src/plus/webviews/graph/protocol.ts @@ -308,7 +308,6 @@ export interface DidChangeGraphConfigurationParams { } export const DidChangeGraphConfigurationNotificationType = new IpcNotificationType( 'graph/configuration/didChange', - true, ); export interface DidChangeSubscriptionParams { @@ -317,7 +316,6 @@ export interface DidChangeSubscriptionParams { } export const DidChangeSubscriptionNotificationType = new IpcNotificationType( 'graph/subscription/didChange', - true, ); export interface DidChangeAvatarsParams { @@ -325,7 +323,6 @@ export interface DidChangeAvatarsParams { } export const DidChangeAvatarsNotificationType = new IpcNotificationType( 'graph/avatars/didChange', - true, ); export interface DidChangeRefsMetadataParams { @@ -333,7 +330,6 @@ export interface DidChangeRefsMetadataParams { } export const DidChangeRefsMetadataNotificationType = new IpcNotificationType( 'graph/refs/didChangeMetadata', - true, ); export interface DidChangeColumnsParams { @@ -342,7 +338,6 @@ export interface DidChangeColumnsParams { } export const DidChangeColumnsNotificationType = new IpcNotificationType( 'graph/columns/didChange', - true, ); export interface DidChangeWindowFocusParams { @@ -350,7 +345,6 @@ export interface DidChangeWindowFocusParams { } export const DidChangeWindowFocusNotificationType = new IpcNotificationType( 'graph/window/focus/didChange', - true, ); export interface DidChangeRefsVisibilityParams { @@ -360,7 +354,6 @@ export interface DidChangeRefsVisibilityParams { } export const DidChangeRefsVisibilityNotificationType = new IpcNotificationType( 'graph/refs/didChangeVisibility', - true, ); export interface DidChangeRowsParams { @@ -378,7 +371,6 @@ export interface DidChangeSelectionParams { } export const DidChangeSelectionNotificationType = new IpcNotificationType( 'graph/selection/didChange', - true, ); export interface DidChangeWorkingTreeParams { @@ -386,7 +378,6 @@ export interface DidChangeWorkingTreeParams { } export const DidChangeWorkingTreeNotificationType = new IpcNotificationType( 'graph/workingTree/didChange', - true, ); export interface DidEnsureRowParams { @@ -408,12 +399,12 @@ export interface DidSearchParams { results: GraphSearchResults | GraphSearchResultsError | undefined; selectedRows?: GraphSelectedRows; } -export const DidSearchNotificationType = new IpcNotificationType('graph/didSearch', true); +export const DidSearchNotificationType = new IpcNotificationType('graph/didSearch'); export interface DidFetchParams { lastFetched: Date; } -export const DidFetchNotificationType = new IpcNotificationType('graph/didFetch', true); +export const DidFetchNotificationType = new IpcNotificationType('graph/didFetch'); export interface ShowInCommitGraphCommandArgs { ref: GitReference; diff --git a/src/webviews/commitDetails/commitDetailsWebview.ts b/src/webviews/commitDetails/commitDetailsWebview.ts index 255324b..92bca9b 100644 --- a/src/webviews/commitDetails/commitDetailsWebview.ts +++ b/src/webviews/commitDetails/commitDetailsWebview.ts @@ -653,8 +653,6 @@ export class CommitDetailsWebviewProvider implements WebviewProvider void> | undefined = undefined; private updateState(immediate: boolean = false) { - if (!this.host.ready || !this.host.visible) return; - if (immediate) { void this.notifyDidChangeState(); return; @@ -668,8 +666,6 @@ export class CommitDetailsWebviewProvider implements WebviewProvider('comm export interface DidChangeParams { state: Serialized; } -export const DidChangeNotificationType = new IpcNotificationType('commit/didChange'); +export const DidChangeNotificationType = new IpcNotificationType('commit/didChange', true); export type DidChangeRichStateParams = { formattedMessage?: string; diff --git a/src/webviews/protocol.ts b/src/webviews/protocol.ts index 1b8dbac..1a4979c 100644 --- a/src/webviews/protocol.ts +++ b/src/webviews/protocol.ts @@ -10,7 +10,7 @@ export interface IpcMessage { abstract class IpcMessageType { _?: Params; // Required for type inferencing to work properly - constructor(public readonly method: string, public readonly overwriteable: boolean = false) {} + constructor(public readonly method: string, public readonly reset: boolean = false) {} } export type IpcMessageParams = T extends IpcMessageType ? P : never; diff --git a/src/webviews/webviewController.ts b/src/webviews/webviewController.ts index 8d742d2..4b66ce3 100644 --- a/src/webviews/webviewController.ts +++ b/src/webviews/webviewController.ts @@ -117,7 +117,6 @@ export class WebviewController< public readonly id: Descriptor['id']; private _ready: boolean = false; - private _suspended: boolean = false; get ready() { return this._ready; } @@ -174,7 +173,6 @@ export class WebviewController< this.provider?.onVisibilityChanged?.(false); this._ready = false; - this._suspended = false; this._onDidDispose.fire(); this.disposable?.dispose(); @@ -271,11 +269,13 @@ export class WebviewController< @debug() async refresh(force?: boolean): Promise { + if (force) { + this.clearPendingIpcNotifications(); + } this.provider.onRefresh?.(force); // Mark the webview as not ready, until we know if we are changing the html this._ready = false; - this._suspended = false; const html = await this.getHtml(this.webview); if (force) { @@ -307,7 +307,7 @@ export class WebviewController< case WebviewReadyCommandType.method: onIpc(WebviewReadyCommandType, e, () => { this._ready = true; - this._suspended = false; + this.sendPendingIpcNotifications(); this.provider.onReady?.(); }); @@ -348,14 +348,11 @@ export class WebviewController< private onParentVisibilityChanged(visible: boolean, active?: boolean) { if (this.descriptor.webviewHostOptions?.retainContextWhenHidden !== true) { if (visible) { - if (this._suspended) { - this._ready = true; - this._suspended = false; - this.provider.onReady?.(); + if (this._ready) { + this.sendPendingIpcNotifications(); } } else { this._ready = false; - this._suspended = true; } } @@ -438,17 +435,24 @@ export class WebviewController< return nextIpcId(); } - notify>( + async notify>( type: T, params: IpcMessageParams, completionId?: string, ): Promise { - return this.postMessage({ + const msg: IpcMessage = { id: this.nextIpcId(), method: type.method, params: params, completionId: completionId, - }); + }; + const success = await this.postMessage(msg); + if (success) { + this._pendingIpcNotifications.clear(); + } else { + this.addPendingIpcNotificationCore(type, msg); + } + return success; } @serialize() @@ -457,7 +461,7 @@ export class WebviewController< 0: m => `{"id":${m.id},"method":${m.method}${m.completionId ? `,"completionId":${m.completionId}` : ''}}`, }, }) - async postMessage(message: IpcMessage): Promise { + private async postMessage(message: IpcMessage): Promise { if (!this._ready) return Promise.resolve(false); // It looks like there is a bug where `postMessage` can sometimes just hang infinitely. Not sure why, but ensure we don't hang @@ -467,6 +471,49 @@ export class WebviewController< ]); return success; } + + private _pendingIpcNotifications = new Map Promise)>(); + + addPendingIpcNotification( + type: IpcNotificationType, + mapping: Map, () => Promise>, + thisArg: any, + ) { + this.addPendingIpcNotificationCore(type, mapping.get(type)?.bind(thisArg)); + } + + private addPendingIpcNotificationCore( + type: IpcNotificationType, + msgOrFn: IpcMessage | (() => Promise) | undefined, + ) { + if (type.reset) { + this._pendingIpcNotifications.clear(); + } + + if (msgOrFn == null) { + debugger; + return; + } + this._pendingIpcNotifications.set(type, msgOrFn); + } + + clearPendingIpcNotifications() { + this._pendingIpcNotifications.clear(); + } + + sendPendingIpcNotifications() { + if (this._pendingIpcNotifications.size === 0) return; + + const ipcs = new Map(this._pendingIpcNotifications); + this._pendingIpcNotifications.clear(); + for (const msgOrFn of ipcs.values()) { + if (typeof msgOrFn === 'function') { + void msgOrFn(); + } else { + void this.postMessage(msgOrFn); + } + } + } } export function replaceWebviewHtmlTokens(