Fix #4500 Untitled notebook reopen doesn't show dirty (#5005)

* Fix 2 notebook issues
- Do not create notebook model twice on start
- Do not cause disposed warnings due to markdown cell deserialization

* Fix notebook dirty on open issue
Before model is resolved we weren't getting dirty events.
Solution is to use backing text model until it's ready.
Must hook to the dirty event & notify to get the dot to appear

src/sql/workbench/parts/notebook/cellViews/code.component.ts

@@ -4,7 +4,7 @@
 *--------------------------------------------------------------------------------------------*/
 import 'vs/css!./code';
 
-import { OnInit, Component, Input, Inject, ElementRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange } from '@angular/core';
+import { OnInit, Component, Input, Inject, ElementRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange, forwardRef, ChangeDetectorRef } from '@angular/core';
 
 import { AngularDisposable } from 'sql/base/node/lifecycle';
 import { QueryTextEditor } from 'sql/parts/modelComponents/queryTextEditor';
@@ -101,7 +101,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 		@Inject(IInstantiationService) private _instantiationService: IInstantiationService,
 		@Inject(IModelService) private _modelService: IModelService,
 		@Inject(IModeService) private _modeService: IModeService,
-		@Inject(IConfigurationService) private _configurationService: IConfigurationService
+		@Inject(IConfigurationService) private _configurationService: IConfigurationService,
+		@Inject(forwardRef(() => ChangeDetectorRef)) private _changeRef: ChangeDetectorRef
 	) {
 		super();
 		this._cellToggleMoreActions = this._instantiationService.createInstance(CellToggleMoreActions);
@@ -159,7 +160,14 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 			&& this.cellModel.cellUri;
 	}
 
+	private get destroyed(): boolean{
+		return !!(this._changeRef['destroyed']);
+	}
+
 	ngAfterContentInit(): void {
+		if (this.destroyed) {
+			return;
+		}
 		this.createEditor();
 		this._register(DOM.addDisposableListener(window, DOM.EventType.RESIZE, e => {
 			this._layoutEmitter.fire();
@@ -197,6 +205,14 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 		let overrideEditorSetting = this._configurationService.getValue<boolean>(OVERRIDE_EDITOR_THEMING_SETTING);
 		this._editor.hideLineNumbers = (overrideEditorSetting && this.cellModel.cellType === CellTypes.Markdown);
 
+
+		if (this.destroyed) {
+			// At this point, we may have been disposed (scenario: restoring markdown cell in preview mode).
+			// Exiting early to avoid warnings on registering already disposed items, which causes some churning
+			// due to re-disposing things.
+			// There's no negative impact as at this point the component isn't visible (it was removed from the DOM)
+			return;
+		}
 		this._register(this._editor);
 		this._register(this._editorInput);
 		this._register(this._editorModel.onDidChangeContent(e => {

src/sql/workbench/parts/notebook/notebookInput.ts

@@ -26,6 +26,7 @@ import { LocalContentManager } from 'sql/workbench/services/notebook/node/localC
 import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
 import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorInput';
 import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
+import { IDisposable } from 'vs/base/common/lifecycle';
 
 export type ModeViewSaveHandler = (handle: number) => Thenable<boolean>;
 
@@ -61,7 +62,7 @@ export class NotebookEditorModel extends EditorModel {
 		return model.getValue();
 	}
 
-	get isDirty(): boolean {
+	isDirty(): boolean {
 		return this.textEditorModel.isDirty();
 	}
 
@@ -135,6 +136,7 @@ export class NotebookInput extends EditorInput {
 	private _untitledEditorModel: UntitledEditorModel;
 	private _contentManager: IContentManager;
 	private _providersLoaded: Promise<void>;
+	private _dirtyListener: IDisposable;
 
 	constructor(private _title: string,
 		private resource: URI,
@@ -148,6 +150,9 @@ export class NotebookInput extends EditorInput {
 		this.resource = resource;
 		this._standardKernels = [];
 		this._providersLoaded = this.assignProviders();
+		if (this._textInput) {
+			this.hookDirtyListener(this._textInput.onDidChangeDirty, () => this._onDidChangeDirty.fire());
+		}
 	}
 
 	public get textInput(): UntitledEditorInput {
@@ -256,7 +261,7 @@ export class NotebookInput extends EditorInput {
 	}
 
 	async resolve(): Promise<NotebookEditorModel> {
-		if (this._model && this._model.isModelCreated()) {
+		if (this._model) {
 			return Promise.resolve(this._model);
 		} else {
 			let textOrUntitledEditorModel: UntitledEditorModel | IEditorModel;
@@ -268,11 +273,27 @@ export class NotebookInput extends EditorInput {
 				textOrUntitledEditorModel = await textEditorModelReference.object.load();
 			}
 			this._model = this.instantiationService.createInstance(NotebookEditorModel, this.resource, textOrUntitledEditorModel);
-			this._model.onDidChangeDirty(() => this._onDidChangeDirty.fire());
+			this.hookDirtyListener(this._model.onDidChangeDirty, () => this._onDidChangeDirty.fire());
 			return this._model;
 		}
 	}
 
+	private hookDirtyListener(dirtyEvent: Event<void>, listener: (e: any) => void): void {
+		let disposable = dirtyEvent(listener);
+		if (this._dirtyListener) {
+			this._dirtyListener.dispose();
+		} else {
+			this._register({
+				dispose: () => {
+					if (this._dirtyListener) {
+						this._dirtyListener.dispose();
+					}
+				}
+			});
+		}
+		this._dirtyListener = disposable;
+	}
+
 	private async assignProviders(): Promise<void> {
 		await this.extensionService.whenInstalledExtensionsRegistered();
 		let providerIds: string[] = getProvidersForFileName(this._title, this.notebookService);
@@ -318,7 +339,9 @@ export class NotebookInput extends EditorInput {
 	 */
 	isDirty(): boolean {
 		if (this._model) {
-			return this._model.isDirty;
+			return this._model.isDirty();
+		} else if (this._textInput) {
+			return this._textInput.isDirty();
 		}
 		return false;
 	}