Skip to content

Commit e257fad

Browse files
jriekenjrieken
committed
deco - some debt, add logging
1 parent c617db1 commit e257fad

2 files changed

Lines changed: 24 additions & 29 deletions

File tree

‎src/vs/workbench/services/decorations/browser/decorationsService.ts‎

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { URI } from 'vs/base/common/uri';
77
import { Event, Emitter } from 'vs/base/common/event';
88
import { IDecorationsService, IDecoration, IResourceDecorationChangeEvent, IDecorationsProvider, IDecorationData } from './decorations';
99
import { TernarySearchTree } from 'vs/base/common/map';
10-
import { Disposable, IDisposable, toDisposable, dispose } from 'vs/base/common/lifecycle';
10+
import { IDisposable, toDisposable, DisposableStore } from 'vs/base/common/lifecycle';
1111
import { isThenable } from 'vs/base/common/async';
1212
import { LinkedList } from 'vs/base/common/linkedList';
1313
import { createStyleSheet, createCSSRule, removeCSSRulesContainingSelector } from 'vs/base/browser/dom';
@@ -19,6 +19,7 @@ import { localize } from 'vs/nls';
1919
import { isPromiseCanceledError } from 'vs/base/common/errors';
2020
import { CancellationTokenSource } from 'vs/base/common/cancellation';
2121
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
22+
import { ILogService } from 'vs/platform/log/common/log';
2223

2324
class DecorationRule {
2425

@@ -96,25 +97,21 @@ class DecorationRule {
9697
}
9798
}
9899

99-
class DecorationStyles extends Disposable {
100+
class DecorationStyles {
100101

101102
private readonly _styleElement = createStyleSheet();
102103
private readonly _decorationRules = new Map<string, DecorationRule>();
104+
private readonly _dispoables = new DisposableStore();
103105

104106
constructor(
105107
private _themeService: IThemeService,
106108
) {
107-
super();
108-
this._register(this._themeService.onThemeChange(this._onThemeChange, this));
109+
this._themeService.onThemeChange(this._onThemeChange, this, this._dispoables);
109110
}
110111

111112
dispose(): void {
112-
super.dispose();
113-
114-
const parent = this._styleElement.parentElement;
115-
if (parent) {
116-
parent.removeChild(this._styleElement);
117-
}
113+
this._dispoables.dispose();
114+
this._styleElement.remove();
118115
}
119116

120117
asDecoration(data: IDecorationData[], onlyChildren: boolean): IDecoration {
@@ -224,11 +221,11 @@ class DecorationProviderWrapper {
224221
private readonly _dispoable: IDisposable;
225222

226223
constructor(
227-
private readonly _provider: IDecorationsProvider,
224+
readonly provider: IDecorationsProvider,
228225
private readonly _uriEmitter: Emitter<URI | URI[]>,
229226
private readonly _flushEmitter: Emitter<IResourceDecorationChangeEvent>
230227
) {
231-
this._dispoable = this._provider.onDidChange(uris => {
228+
this._dispoable = this.provider.onDidChange(uris => {
232229
if (!uris) {
233230
// flush event -> drop all data, can affect everything
234231
this.data.clear();
@@ -292,7 +289,7 @@ class DecorationProviderWrapper {
292289
}
293290

294291
const source = new CancellationTokenSource();
295-
const dataOrThenable = this._provider.provideDecorations(uri, source.token);
292+
const dataOrThenable = this.provider.provideDecorations(uri, source.token);
296293
if (!isThenable<IDecorationData | Promise<IDecorationData | undefined> | undefined>(dataOrThenable)) {
297294
// sync -> we have a result now
298295
return this._keepItem(uri, dataOrThenable);
@@ -325,15 +322,14 @@ class DecorationProviderWrapper {
325322
}
326323
}
327324

328-
export class FileDecorationsService implements IDecorationsService {
325+
export class DecorationsService implements IDecorationsService {
329326

330327
_serviceBrand: undefined;
331328

332329
private readonly _data = new LinkedList<DecorationProviderWrapper>();
333330
private readonly _onDidChangeDecorationsDelayed = new Emitter<URI | URI[]>();
334331
private readonly _onDidChangeDecorations = new Emitter<IResourceDecorationChangeEvent>();
335332
private readonly _decorationStyles: DecorationStyles;
336-
private readonly _disposables: IDisposable[];
337333

338334
readonly onDidChangeDecorations: Event<IResourceDecorationChangeEvent> = Event.any(
339335
this._onDidChangeDecorations.event,
@@ -345,29 +341,25 @@ export class FileDecorationsService implements IDecorationsService {
345341
);
346342

347343
constructor(
348-
@IThemeService themeService: IThemeService
344+
@IThemeService themeService: IThemeService,
345+
@ILogService private readonly _logService: ILogService,
349346
) {
350347
this._decorationStyles = new DecorationStyles(themeService);
351348

352349
// every so many events we check if there are
353350
// css styles that we don't need anymore
354351
let count = 0;
355-
let reg = this.onDidChangeDecorations(() => {
352+
this.onDidChangeDecorations(() => {
356353
if (++count % 17 === 0) {
357354
this._decorationStyles.cleanUp(this._data.iterator());
358355
}
359356
});
360-
361-
this._disposables = [
362-
reg,
363-
this._decorationStyles
364-
];
365357
}
366358

367359
dispose(): void {
368-
dispose(this._disposables);
369-
dispose(this._onDidChangeDecorations);
370-
dispose(this._onDidChangeDecorationsDelayed);
360+
this._decorationStyles.dispose();
361+
this._onDidChangeDecorations.dispose();
362+
this._onDidChangeDecorationsDelayed.dispose();
371363
}
372364

373365
registerDecorationsProvider(provider: IDecorationsProvider): IDisposable {
@@ -397,10 +389,12 @@ export class FileDecorationsService implements IDecorationsService {
397389
let data: IDecorationData[] = [];
398390
let containsChildren: boolean = false;
399391
for (let iter = this._data.iterator(), next = iter.next(); !next.done; next = iter.next()) {
392+
const { label } = next.value.provider;
400393
next.value.getOrRetrieve(uri, includeChildren, (deco, isChild) => {
401394
if (!isChild || deco.bubble) {
402395
data.push(deco);
403396
containsChildren = isChild || containsChildren;
397+
this._logService.trace('DecorationsService#getDecoration#getOrRetrieve', label, deco, isChild, uri);
404398
}
405399
});
406400
}
@@ -420,4 +414,4 @@ function getColor(theme: ITheme, color: string | undefined) {
420414
return 'inherit';
421415
}
422416

423-
registerSingleton(IDecorationsService, FileDecorationsService);
417+
registerSingleton(IDecorationsService, DecorationsService, true);

‎src/vs/workbench/services/decorations/test/browser/decorationsService.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,23 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as assert from 'assert';
7-
import { FileDecorationsService } from 'vs/workbench/services/decorations/browser/decorationsService';
7+
import { DecorationsService } from 'vs/workbench/services/decorations/browser/decorationsService';
88
import { IDecorationsProvider, IDecorationData } from 'vs/workbench/services/decorations/browser/decorations';
99
import { URI } from 'vs/base/common/uri';
1010
import { Event, Emitter } from 'vs/base/common/event';
1111
import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService';
1212
import { CancellationToken } from 'vs/base/common/cancellation';
13+
import { ConsoleLogService } from 'vs/platform/log/common/log';
1314

1415
suite('DecorationsService', function () {
1516

16-
let service: FileDecorationsService;
17+
let service: DecorationsService;
1718

1819
setup(function () {
1920
if (service) {
2021
service.dispose();
2122
}
22-
service = new FileDecorationsService(new TestThemeService());
23+
service = new DecorationsService(new TestThemeService(), new ConsoleLogService());
2324
});
2425

2526
test('Async provider, async/evented result', function () {

0 commit comments

Comments
 (0)