Merge pull request #24 from ckeditor/t/ckeditor5/1341

Fix: There should be no memory leaks when the editor is created and destroyed (see ckeditor/ckeditor5#1341).

Author: oleq

tests/decouplededitor.js

@@ -18,6 +18,8 @@ import DataApiMixin from '@ckeditor/ckeditor5-core/src/editor/utils/dataapimixin
 import RootElement from '@ckeditor/ckeditor5-engine/src/model/rootelement';
 
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+import { describeMemoryUsage, testMemoryUsage } from '@ckeditor/ckeditor5-core/tests/_utils/memory';
+import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
 
 const editorData = '<p><strong>foo</strong> bar</p>';
 
@@ -303,4 +305,17 @@ describe( 'DecoupledEditor', () => {
 			} );
 		}
 	} );
+
+	describeMemoryUsage( () => {
+		testMemoryUsage(
+			'should not grow on multiple create/destroy',
+			() => DecoupledEditor
+				.create( document.querySelector( '#mem-editor' ), {
+					plugins: [ ArticlePluginSet ],
+					toolbar: [ 'heading', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote' ],
+					image: {
+						toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]
+					}
+				} ) );
+	} );
 } );

tests/manual/memory.html

@@ -0,0 +1,49 @@
+<p>
+	<button id="destroyEditor">Destroy editor</button>
+	<button id="initEditor">Init editor</button>
+</p>
+
+<h2>The toolbar</h2>
+<div class="toolbar-container"></div>
+
+<h2>The editable</h2>
+<div class="editable-container"></div>
+
+<style>
+	.editable-container,
+	.toolbar-container {
+		position: relative;
+		border: 1px solid red;
+	}
+
+	.editable-container::after,
+	.toolbar-container::after {
+		content: attr(class);
+		position: absolute;
+		background: red;
+		color: #fff;
+		top: 0;
+		right: 0;
+		font: 10px/2 monospace;
+		padding: .1em .3em;
+	}
+
+	.toolbar-container{
+		padding: 1em;
+	}
+
+	.editable-container {
+		padding: 3em;
+		overflow-y: scroll;
+		max-height: 300px;
+	}
+
+	.editable-container .ck-editor__editable {
+		min-height: 21cm;
+		padding: 2em;
+		border: 1px #D3D3D3 solid;
+		border-radius: var(--ck-border-radius);
+		background: white;
+		box-shadow: 0 0 5px rgba(0, 0, 0, 0.1);
+	}
+</style>

tests/manual/memory.js

@@ -0,0 +1,74 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals console:false, document, window */
+
+import DecoupledEditor from '../../src/decouplededitor';
+import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
+
+const editorData = '<h2>Hello world</h2><p>This is the decoupled editor.</p><img src="sample.jpg" />';
+
+/*
+ * Memory-leak safe version of decoupled editor manual test does not:
+ * - define global variables (such as let editor; in main file scope)
+ * - console.log() objects
+ * - add event listeners with () => {} methods which reference other
+ */
+function initEditor() {
+	DecoupledEditor
+		.create( editorData, {
+			plugins: [ ArticlePluginSet ],
+			toolbar: {
+				items: [
+					'heading',
+					'|',
+					'bold',
+					'italic',
+					'link',
+					'bulletedList',
+					'numberedList',
+					'blockQuote',
+					'insertTable',
+					'mediaEmbed',
+					'undo',
+					'redo'
+				]
+			},
+			image: {
+				toolbar: [
+					'imageStyle:full',
+					'imageStyle:side',
+					'|',
+					'imageTextAlternative'
+				]
+			},
+			table: {
+				contentToolbar: [
+					'tableColumn',
+					'tableRow',
+					'mergeTableCells'
+				]
+			}
+		} )
+		.then( newEditor => {
+			window.editor = newEditor;
+
+			document.querySelector( '.toolbar-container' ).appendChild( newEditor.ui.view.toolbar.element );
+			document.querySelector( '.editable-container' ).appendChild( newEditor.ui.view.editable.element );
+
+			document.getElementById( 'destroyEditor' ).addEventListener( 'click', destroyEditor );
+		} )
+		.catch( err => {
+			console.error( err.stack );
+		} );
+
+	function destroyEditor() {
+		window.editor.destroy().then( () => console.log( 'Editor was destroyed' ) );
+		window.editor = null;
+		document.getElementById( 'destroyEditor' ).removeEventListener( 'click', destroyEditor );
+	}
+}
+
+document.getElementById( 'initEditor' ).addEventListener( 'click', initEditor );

tests/manual/memory.md

@@ -0,0 +1,31 @@
+1. Open DevTools on Chrome.
+2. Go to Memory tab.
+3. Select "Heap snapshot" profiling type.
+4. Click "Collect Garbage" (trash icon) and "Clear all profiles" (don't go icon).
+5. Record heap snapshot ("Take heap snapshot" a record icon).
+6. Repeat multiple times:
+    - click "Init editor",
+    - wait to editor show up ,
+    - click "Destroy editor".
+7. Record heap snapshot again.
+8. Compare Snapshot 1 with Snapshot 2 and look for:
+    - "detached" - HTML Elements/DOM nodes
+    - "EventListener" - there should be only 1
+    - Any CKEditor5 classes instances like "Editor" or "LivePosition"
+
+## Notes:
+
+* Browser extensions might attach event listeners to the DOM so the safest way to run this test is by running Chrome from command line:
+
+    ```
+    google-chrome \
+        --enable-precise-memory-info \
+        --disable-extensions \
+        --disable-plugins \
+        --incognito \
+        http://localhost:8125 
+    ```
+
+    The above will run Chrome without extensions or plugins in incognito mode and open manual tests page.
+
+* Close any running Chrome instance beforehand because otherwise it will open a tab in currently opened Chrome (look for "Opening in existing browser session." in command line).