Merge pull request #4301 from Tyriar/cleanup_coreservice

Avoid passing circular callback into CoreService ctor

src/common/CoreTerminal.ts

@@ -109,7 +109,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
     this._instantiationService.setService(IBufferService, this._bufferService);
     this._logService = this.register(this._instantiationService.createInstance(LogService));
     this._instantiationService.setService(ILogService, this._logService);
-    this.coreService = this.register(this._instantiationService.createInstance(CoreService, () => this.scrollToBottom()));
+    this.coreService = this.register(this._instantiationService.createInstance(CoreService));
     this._instantiationService.setService(ICoreService, this.coreService);
     this.coreMouseService = this.register(this._instantiationService.createInstance(CoreMouseService));
     this._instantiationService.setService(ICoreMouseService, this.coreMouseService);
@@ -129,6 +129,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
     this.register(forwardEvent(this._bufferService.onResize, this._onResize));
     this.register(forwardEvent(this.coreService.onData, this._onData));
     this.register(forwardEvent(this.coreService.onBinary, this._onBinary));
+    this.register(this.coreService.onRequestScrollToBottom(() => this.scrollToBottom()));
     this.register(this.coreService.onUserInput(() =>  this._writeBuffer.handleUserInput()));
     this.register(this.optionsService.onSpecificOptionChange('windowsMode', e => this._handleWindowsModeOptionChange(e)));
     this.register(this._bufferService.onScroll(event => {

src/common/InputHandler.test.ts

@@ -65,7 +65,7 @@ describe('InputHandler', () => {
     optionsService = new MockOptionsService();
     bufferService = new BufferService(optionsService);
     bufferService.resize(80, 30);
-    coreService = new CoreService(() => { }, bufferService, new MockLogService(), optionsService);
+    coreService = new CoreService(bufferService, new MockLogService(), optionsService);
 
     inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService());
   });
@@ -2300,7 +2300,7 @@ describe('InputHandler - async handlers', () => {
     optionsService = new MockOptionsService();
     bufferService = new BufferService(optionsService);
     bufferService.resize(80, 30);
-    coreService = new CoreService(() => { }, bufferService, new MockLogService(), optionsService);
+    coreService = new CoreService(bufferService, new MockLogService(), optionsService);
     coreService.onData(data => { console.log(data); });
 
     inputHandler = new TestInputHandler(bufferService, new MockCharsetService(), coreService, new MockLogService(), optionsService, new MockOscLinkService(), new MockCoreMouseService(), new MockUnicodeService());

src/common/TestUtils.test.ts

@@ -95,6 +95,7 @@ export class MockCoreService implements ICoreService {
   public onData: IEvent<string> = new EventEmitter<string>().event;
   public onUserInput: IEvent<void> = new EventEmitter<void>().event;
   public onBinary: IEvent<string> = new EventEmitter<string>().event;
+  public onRequestScrollToBottom: IEvent<void> = new EventEmitter<void>().event;
   public reset(): void { }
   public triggerDataEvent(data: string, wasUserInput?: boolean): void { }
   public triggerBinaryEvent(data: string): void { }

src/common/services/CoreService.test.ts

@@ -13,7 +13,6 @@ describe('CoreService', () => {
 
   beforeEach(() => {
     coreService = new CoreService(
-      () => {},
       new MockBufferService(80, 30),
       new MockLogService(),
       new MockOptionsService());

src/common/services/CoreService.ts

@@ -31,26 +31,21 @@ export class CoreService extends Disposable implements ICoreService {
   public modes: IModes;
   public decPrivateModes: IDecPrivateModes;
 
-  // Circular dependency, this must be unset or memory will leak after Terminal.dispose
-  private _scrollToBottom: (() => void) | undefined;
-
   private readonly _onData = this.register(new EventEmitter<string>());
   public readonly onData = this._onData.event;
   private readonly _onUserInput = this.register(new EventEmitter<void>());
   public readonly onUserInput = this._onUserInput.event;
   private readonly _onBinary = this.register(new EventEmitter<string>());
   public readonly onBinary = this._onBinary.event;
+  private readonly _onRequestScrollToBottom = this.register(new EventEmitter<void>());
+  public readonly onRequestScrollToBottom = this._onRequestScrollToBottom.event;
 
   constructor(
-    // TODO: Move this into a service
-    scrollToBottom: () => void,
     @IBufferService private readonly _bufferService: IBufferService,
     @ILogService private readonly _logService: ILogService,
     @IOptionsService private readonly _optionsService: IOptionsService
   ) {
     super();
-    this._scrollToBottom = scrollToBottom;
-    this.register({ dispose: () => this._scrollToBottom = undefined });
     this.modes = clone(DEFAULT_MODES);
     this.decPrivateModes = clone(DEFAULT_DEC_PRIVATE_MODES);
   }
@@ -69,7 +64,7 @@ export class CoreService extends Disposable implements ICoreService {
     // Input is being sent to the terminal, the terminal should focus the prompt.
     const buffer = this._bufferService.buffer;
     if (wasUserInput && this._optionsService.rawOptions.scrollOnUserInput && buffer.ybase !== buffer.ydisp) {
-      this._scrollToBottom!();
+      this._onRequestScrollToBottom.fire();
     }
 
     // Fire onUserInput so listeners can react as well (eg. clear selection)

src/common/services/Services.ts

@@ -79,6 +79,7 @@ export interface ICoreService {
   readonly onData: IEvent<string>;
   readonly onUserInput: IEvent<void>;
   readonly onBinary: IEvent<string>;
+  readonly onRequestScrollToBottom: IEvent<void>;
 
   reset(): void;