Skip to content

Commit 09e3248

Browse files
authored
Fix microsoft#164704. comment thread memory leak. (microsoft#164734)
* Fix microsoft#164704. comment thread memory leak. * Dispose first. * spell check
1 parent b39fd56 commit 09e3248

File tree

2 files changed

+26
-14
lines changed

2 files changed

+26
-14
lines changed

src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,8 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
149149
}
150150

151151
updateCommentThread(commentThread: languages.CommentThread<T>) {
152-
if (this._commentThread !== commentThread) {
153-
dispose(this._commentThreadDisposables);
154-
}
155-
156152
this._commentThread = commentThread;
153+
dispose(this._commentThreadDisposables);
157154
this._commentThreadDisposables = [];
158155
this._bindCommentThreadListeners();
159156

src/vs/workbench/contrib/notebook/browser/view/cellParts/cellComments.ts

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import * as dom from 'vs/base/browser/dom';
76
import * as languages from 'vs/editor/common/languages';
87
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
98
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
@@ -89,7 +88,7 @@ export class CellComments extends CellPart {
8988

9089
this.commentTheadDisposables.add(this._commentThreadWidget.onDidResize(() => {
9190
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget) {
92-
this.currentElement.commentHeight = dom.getClientArea(this._commentThreadWidget.container).height;
91+
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
9392
}
9493
}));
9594
}
@@ -102,25 +101,41 @@ export class CellComments extends CellPart {
102101
this._createCommentTheadWidget(info.owner, info.thread);
103102
const layoutInfo = (this.currentElement as CodeCellViewModel).layoutInfo;
104103
this.container.style.top = `${layoutInfo.outputContainerOffset + layoutInfo.outputTotalHeight}px`;
105-
106-
this.currentElement.commentHeight = dom.getClientArea(this._commentThreadWidget!.container).height;
107-
104+
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget!.getDimensions().height);
108105
return;
109106
}
110107

111108
if (this._commentThreadWidget) {
112-
if (info) {
113-
this._commentThreadWidget.updateCommentThread(info.thread);
114-
this.currentElement.commentHeight = dom.getClientArea(this._commentThreadWidget.container).height;
115-
} else {
109+
if (!info) {
116110
this._commentThreadWidget.dispose();
117111
this.currentElement.commentHeight = 0;
112+
return;
113+
}
114+
if (this._commentThreadWidget.commentThread === info.thread) {
115+
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
116+
return;
118117
}
118+
119+
this._commentThreadWidget.updateCommentThread(info.thread);
120+
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
119121
}
120122
}
121123
}));
122124
}
123125

126+
private _calculateCommentThreadHeight(bodyHeight: number) {
127+
const layoutInfo = this.notebookEditor.getLayoutInfo();
128+
129+
const headHeight = Math.ceil(layoutInfo.fontInfo.lineHeight * 1.2);
130+
const lineHeight = layoutInfo.fontInfo.lineHeight;
131+
const arrowHeight = Math.round(lineHeight / 3);
132+
const frameThickness = Math.round(lineHeight / 9) * 2;
133+
134+
const computedHeight = headHeight + bodyHeight + arrowHeight + frameThickness + 8 /** margin bottom to avoid margin collapse */;
135+
return computedHeight;
136+
137+
}
138+
124139
private async _getCommentThreadForCell(element: ICellViewModel): Promise<{ thread: languages.CommentThread<ICellRange>; owner: string } | null> {
125140
if (this.notebookEditor.hasModel()) {
126141
const commentInfos = coalesce(await this.commentService.getNotebookComments(element.uri));
@@ -149,7 +164,7 @@ export class CellComments extends CellPart {
149164

150165
override prepareLayout(): void {
151166
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget) {
152-
this.currentElement.commentHeight = dom.getClientArea(this._commentThreadWidget.container).height;
167+
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
153168
}
154169
}
155170

0 commit comments

Comments
 (0)