Adopt isUpdating in pr webview view and throw if re-entrant. (#7979)

Author: alexr00

src/github/activityBarViewProvider.ts

@@ -23,6 +23,7 @@ import { ReviewManager } from '../view/reviewManager';
 export class PullRequestViewProvider extends WebviewViewBase implements vscode.WebviewViewProvider {
 	public override readonly viewType = 'github:activePullRequest';
 	private _existingReviewers: ReviewState[] = [];
+	private _isUpdating: boolean = false;
 
 	constructor(
 		extensionUri: vscode.Uri,
@@ -182,132 +183,143 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
 			disposeAll(this._prDisposables);
 		}
 		this._prDisposables = [];
-		this._prDisposables.push(pullRequestModel.onDidChange(() => this.updatePullRequest(pullRequestModel)));
+		this._prDisposables.push(pullRequestModel.onDidChange(e => {
+			if ((e.state || e.comments || e.reviewers) && !this._isUpdating) {
+				this.updatePullRequest(pullRequestModel);
+			}
+		}));
 		this._prDisposables.push(pullRequestModel.onDidChangePendingReviewState(() => this.updatePullRequest(pullRequestModel)));
 	}
 
 	private _updatePendingVisibility: vscode.Disposable | undefined = undefined;
 	public async updatePullRequest(pullRequestModel: PullRequestModel): Promise<void> {
-		if (this._view && !this._view.visible) {
-			this._updatePendingVisibility?.dispose();
-			this._updatePendingVisibility = this._view.onDidChangeVisibility(async () => {
-				this.updatePullRequest(pullRequestModel);
-				this._updatePendingVisibility?.dispose();
-			});
+		if (this._isUpdating) {
+			throw new Error('Already updating pull request view');
 		}
+		this._isUpdating = true;
 
-		if ((this._prDisposables === undefined) || (pullRequestModel.number !== this._item.number)) {
-			this.registerPrSpecificListeners(pullRequestModel);
-		}
-		this._item = pullRequestModel;
-		return Promise.all([
-			this._folderRepositoryManager.resolvePullRequest(
-				pullRequestModel.remote.owner,
-				pullRequestModel.remote.repositoryName,
-				pullRequestModel.number,
-			),
-			this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel),
-			pullRequestModel.getTimelineEvents(),
-			pullRequestModel.getReviewRequests(),
-			this._folderRepositoryManager.getBranchNameForPullRequest(pullRequestModel),
-			this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel),
-			this._folderRepositoryManager.getCurrentUser(pullRequestModel.githubRepository),
-			pullRequestModel.canEdit(),
-			pullRequestModel.validateDraftMode(),
-			ensureEmojis(this._folderRepositoryManager.context)
-		])
-			.then(result => {
-				const [pullRequest, repositoryAccess, timelineEvents, requestedReviewers, branchInfo, defaultBranch, currentUser, viewerCanEdit, hasReviewDraft] = result;
-				if (!pullRequest) {
-					throw new Error(
-						`Fail to resolve Pull Request #${pullRequestModel.number} in ${pullRequestModel.remote.owner}/${pullRequestModel.remote.repositoryName}`,
-					);
-				}
+		try {
+			if (this._view && !this._view.visible) {
+				this._updatePendingVisibility?.dispose();
+				this._updatePendingVisibility = this._view.onDidChangeVisibility(async () => {
+					this.updatePullRequest(pullRequestModel);
+					this._updatePendingVisibility?.dispose();
+				});
+			}
 
-				this._item = pullRequest;
-				if (!this._view) {
-					// If the there is no PR webview, then there is nothing else to update.
-					return;
-				}
+			if ((this._prDisposables === undefined) || (pullRequestModel.number !== this._item.number)) {
+				this.registerPrSpecificListeners(pullRequestModel);
+			}
+			this._item = pullRequestModel;
+			const [pullRequest, repositoryAccess, timelineEvents, requestedReviewers, branchInfo, defaultBranch, currentUser, viewerCanEdit, hasReviewDraft] = await Promise.all([
+				this._folderRepositoryManager.resolvePullRequest(
+					pullRequestModel.remote.owner,
+					pullRequestModel.remote.repositoryName,
+					pullRequestModel.number,
+				),
+				this._folderRepositoryManager.getPullRequestRepositoryAccessAndMergeMethods(pullRequestModel),
+				pullRequestModel.getTimelineEvents(),
+				pullRequestModel.getReviewRequests(),
+				this._folderRepositoryManager.getBranchNameForPullRequest(pullRequestModel),
+				this._folderRepositoryManager.getPullRequestRepositoryDefaultBranch(pullRequestModel),
+				this._folderRepositoryManager.getCurrentUser(pullRequestModel.githubRepository),
+				pullRequestModel.canEdit(),
+				pullRequestModel.validateDraftMode(),
+				ensureEmojis(this._folderRepositoryManager.context)
+			]);
+
+			if (!pullRequest) {
+				throw new Error(
+					`Fail to resolve Pull Request #${pullRequestModel.number} in ${pullRequestModel.remote.owner}/${pullRequestModel.remote.repositoryName}`,
+				);
+			}
 
-				try {
-					this._view.title = `${vscode.l10n.t('Review Pull Request')} #${pullRequestModel.number.toString()}`;
-				} catch (e) {
-					// If we ry to set the title of the webview too early it will throw an error.
-				}
+			this._item = pullRequest;
+			if (!this._view) {
+				// If the there is no PR webview, then there is nothing else to update.
+				return;
+			}
 
-				const isCurrentlyCheckedOut = pullRequestModel.equals(this._folderRepositoryManager.activePullRequest);
-				const hasWritePermission = repositoryAccess!.hasWritePermission;
-				const mergeMethodsAvailability = repositoryAccess!.mergeMethodsAvailability;
-				const canEdit = hasWritePermission || viewerCanEdit;
-				const defaultMergeMethod = getDefaultMergeMethod(mergeMethodsAvailability);
-				this._existingReviewers = parseReviewers(
-					requestedReviewers ?? [],
-					timelineEvents ?? [],
-					pullRequest.author,
-				);
+			try {
+				this._view.title = `${vscode.l10n.t('Review Pull Request')} #${pullRequestModel.number.toString()}`;
+			} catch (e) {
+				// If we ry to set the title of the webview too early it will throw an error.
+			}
 
-				const isCrossRepository =
-					pullRequest.base &&
-					pullRequest.head &&
-					!pullRequest.base.repositoryCloneUrl.equals(pullRequest.head.repositoryCloneUrl);
-
-				const continueOnGitHub = !!(isCrossRepository && isInCodespaces());
-				const reviewState = this.getCurrentUserReviewState(this._existingReviewers, currentUser);
-
-				const context: Partial<PullRequest> = {
-					number: pullRequest.number,
-					title: pullRequest.title,
-					url: pullRequest.html_url,
-					createdAt: pullRequest.createdAt,
-					body: pullRequest.body,
-					bodyHTML: pullRequest.bodyHTML,
-					labels: pullRequest.item.labels.map(label => ({ ...label, displayName: emojify(label.name) })),
-					author: {
-						login: pullRequest.author.login,
-						name: pullRequest.author.name,
-						avatarUrl: pullRequest.userAvatar,
-						url: pullRequest.author.url,
-						email: pullRequest.author.email,
-						id: pullRequest.author.id,
-						accountType: pullRequest.author.accountType,
-					},
-					state: pullRequest.state,
-					isCurrentlyCheckedOut: isCurrentlyCheckedOut,
-					isRemoteBaseDeleted: pullRequest.isRemoteBaseDeleted,
-					base: pullRequest.base.label,
-					isRemoteHeadDeleted: pullRequest.isRemoteHeadDeleted,
-					isLocalHeadDeleted: !branchInfo,
-					head: pullRequest.head?.label ?? '',
-					canEdit: canEdit,
-					hasWritePermission,
-					mergeable: pullRequest.item.mergeable,
-					isDraft: pullRequest.isDraft,
-					status: null,
-					reviewRequirement: null,
-					canUpdateBranch: pullRequest.item.viewerCanUpdate,
-					events: timelineEvents,
-					mergeMethodsAvailability,
-					defaultMergeMethod,
-					repositoryDefaultBranch: defaultBranch,
-					isIssue: false,
-					isAuthor: currentUser.login === pullRequest.author.login,
-					reviewers: this._existingReviewers,
-					continueOnGitHub,
-					isDarkTheme: vscode.window.activeColorTheme.kind === vscode.ColorThemeKind.Dark,
-					isEnterprise: pullRequest.githubRepository.remote.isEnterprise,
-					hasReviewDraft,
-					currentUserReviewState: reviewState
-				};
-
-				this._postMessage({
-					command: 'pr.initialize',
-					pullrequest: context,
-				});
-			})
-			.catch(e => {
-				vscode.window.showErrorMessage(`Error updating active pull request view: ${formatError(e)}`);
+			const isCurrentlyCheckedOut = pullRequestModel.equals(this._folderRepositoryManager.activePullRequest);
+			const hasWritePermission = repositoryAccess!.hasWritePermission;
+			const mergeMethodsAvailability = repositoryAccess!.mergeMethodsAvailability;
+			const canEdit = hasWritePermission || viewerCanEdit;
+			const defaultMergeMethod = getDefaultMergeMethod(mergeMethodsAvailability);
+			this._existingReviewers = parseReviewers(
+				requestedReviewers ?? [],
+				timelineEvents ?? [],
+				pullRequest.author,
+			);
+
+			const isCrossRepository =
+				pullRequest.base &&
+				pullRequest.head &&
+				!pullRequest.base.repositoryCloneUrl.equals(pullRequest.head.repositoryCloneUrl);
+
+			const continueOnGitHub = !!(isCrossRepository && isInCodespaces());
+			const reviewState = this.getCurrentUserReviewState(this._existingReviewers, currentUser);
+
+			const context: Partial<PullRequest> = {
+				number: pullRequest.number,
+				title: pullRequest.title,
+				url: pullRequest.html_url,
+				createdAt: pullRequest.createdAt,
+				body: pullRequest.body,
+				bodyHTML: pullRequest.bodyHTML,
+				labels: pullRequest.item.labels.map(label => ({ ...label, displayName: emojify(label.name) })),
+				author: {
+					login: pullRequest.author.login,
+					name: pullRequest.author.name,
+					avatarUrl: pullRequest.userAvatar,
+					url: pullRequest.author.url,
+					email: pullRequest.author.email,
+					id: pullRequest.author.id,
+					accountType: pullRequest.author.accountType,
+				},
+				state: pullRequest.state,
+				isCurrentlyCheckedOut: isCurrentlyCheckedOut,
+				isRemoteBaseDeleted: pullRequest.isRemoteBaseDeleted,
+				base: pullRequest.base.label,
+				isRemoteHeadDeleted: pullRequest.isRemoteHeadDeleted,
+				isLocalHeadDeleted: !branchInfo,
+				head: pullRequest.head?.label ?? '',
+				canEdit: canEdit,
+				hasWritePermission,
+				mergeable: pullRequest.item.mergeable,
+				isDraft: pullRequest.isDraft,
+				status: null,
+				reviewRequirement: null,
+				canUpdateBranch: pullRequest.item.viewerCanUpdate,
+				events: timelineEvents,
+				mergeMethodsAvailability,
+				defaultMergeMethod,
+				repositoryDefaultBranch: defaultBranch,
+				isIssue: false,
+				isAuthor: currentUser.login === pullRequest.author.login,
+				reviewers: this._existingReviewers,
+				continueOnGitHub,
+				isDarkTheme: vscode.window.activeColorTheme.kind === vscode.ColorThemeKind.Dark,
+				isEnterprise: pullRequest.githubRepository.remote.isEnterprise,
+				hasReviewDraft,
+				currentUserReviewState: reviewState
+			};
+
+			this._postMessage({
+				command: 'pr.initialize',
+				pullrequest: context,
 			});
+
+		} catch (e) {
+			vscode.window.showErrorMessage(`Error updating active pull request view: ${formatError(e)}`);
+		} finally {
+			this._isUpdating = false;
+		}
 	}
 
 	private close(message: IRequestMessage<string>): void {

src/github/pullRequestOverview.ts

@@ -217,6 +217,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
 	}
 
 	protected override async updateItem(pullRequestModel: PullRequestModel): Promise<void> {
+		if (this._isUpdating) {
+			throw new Error('Already updating pull request webview');
+		}
 		this._isUpdating = true;
 		try {
 			const [