Fixed IDE diff markers being cached too long

3 files changed, 37 insertions, 13 deletions

diff --git a/app/assets/javascripts/ide/lib/common/model.js b/app/assets/javascripts/ide/lib/common/model.js
index e47adae99ed..b9d5c0be70b 100644
--- a/app/assets/javascripts/ide/lib/common/model.js
+++ b/app/assets/javascripts/ide/lib/common/model.js
@@ -31,7 +31,7 @@ export default class Model {
       );
     }
 
-    this.events = new Map();
+    this.events = new Set();
 
     this.updateContent = this.updateContent.bind(this);
     this.dispose = this.dispose.bind(this);
@@ -73,10 +73,11 @@ export default class Model {
   }
 
   onChange(cb) {
-    this.events.set(
-      this.path,
-      this.disposable.add(this.model.onDidChangeContent(e => cb(this, e))),
-    );
+    this.events.add(this.disposable.add(this.model.onDidChangeContent(e => cb(this, e))));
+  }
+
+  onDispose(cb) {
+    this.events.add(cb);
   }
 
   updateContent(content) {
@@ -86,6 +87,11 @@ export default class Model {
 
   dispose() {
     this.disposable.dispose();
+
+    this.events.forEach(cb => {
+      if (typeof cb === 'function') cb();
+    });
+
     this.events.clear();
 
     eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose);
diff --git a/app/assets/javascripts/ide/lib/decorations/controller.js b/app/assets/javascripts/ide/lib/decorations/controller.js
index 42904774747..13d477bb2cf 100644
--- a/app/assets/javascripts/ide/lib/decorations/controller.js
+++ b/app/assets/javascripts/ide/lib/decorations/controller.js
@@ -38,6 +38,15 @@ export default class DecorationsController {
     );
   }
 
+  hasDecorations(model) {
+    return this.decorations.has(model.url);
+  }
+
+  removeDecorations(model) {
+    this.decorations.delete(model.url);
+    this.editorDecorations.delete(model.url);
+  }
+
   dispose() {
     this.decorations.clear();
     this.editorDecorations.clear();
diff --git a/app/assets/javascripts/ide/lib/diff/controller.js b/app/assets/javascripts/ide/lib/diff/controller.js
index b136545ad11..f579424cf33 100644
--- a/app/assets/javascripts/ide/lib/diff/controller.js
+++ b/app/assets/javascripts/ide/lib/diff/controller.js
@@ -3,7 +3,7 @@ import { throttle } from 'underscore';
 import DirtyDiffWorker from './diff_worker';
 import Disposable from '../common/disposable';
 
-export const getDiffChangeType = (change) => {
+export const getDiffChangeType = change => {
   if (change.modified) {
     return 'modified';
   } else if (change.added) {
@@ -16,12 +16,7 @@ export const getDiffChangeType = (change) => {
 };
 
 export const getDecorator = change => ({
-  range: new monaco.Range(
-    change.lineNumber,
-    1,
-    change.endLineNumber,
-    1,
-  ),
+  range: new monaco.Range(change.lineNumber, 1, change.endLineNumber, 1),
   options: {
     isWholeLine: true,
     linesDecorationsClassName: `dirty-diff dirty-diff-${getDiffChangeType(change)}`,
@@ -31,6 +26,7 @@ export const getDecorator = change => ({
 export default class DirtyDiffController {
   constructor(modelManager, decorationsController) {
     this.disposable = new Disposable();
+    this.models = new Map();
     this.editorSimpleWorker = null;
     this.modelManager = modelManager;
     this.decorationsController = decorationsController;
@@ -42,7 +38,15 @@ export default class DirtyDiffController {
   }
 
   attachModel(model) {
+    if (this.models.has(model.url)) return;
+
     model.onChange(() => this.throttledComputeDiff(model));
+    model.onDispose(() => {
+      this.decorationsController.removeDecorations(model);
+      this.models.delete(model.url);
+    });
+
+    this.models.set(model.url, model);
   }
 
   computeDiff(model) {
@@ -54,7 +58,11 @@ export default class DirtyDiffController {
   }
 
   reDecorate(model) {
-    this.decorationsController.decorate(model);
+    if (this.decorationsController.hasDecorations(model)) {
+      this.decorationsController.decorate(model);
+    } else {
+      this.computeDiff(model);
+    }
   }
 
   decorate({ data }) {
@@ -65,6 +73,7 @@ export default class DirtyDiffController {
 
   dispose() {
     this.disposable.dispose();
+    this.models.clear();
 
     this.dirtyDiffWorker.removeEventListener('message', this.decorate);
     this.dirtyDiffWorker.terminate();