Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #44 from ckeditor/t/ckeditor5/1341
Browse files Browse the repository at this point in the history
Fix: There should be no memory leaks when the editor is created and destroyed (see ckeditor/ckeditor5#1341).
  • Loading branch information
oleq authored Jan 22, 2019
2 parents ead6e41 + 65b6ac0 commit dd2bb90
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 0 deletions.
15 changes: 15 additions & 0 deletions tests/inlineeditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import ElementApiMixin from '@ckeditor/ckeditor5-core/src/editor/utils/elementap
import RootElement from '@ckeditor/ckeditor5-engine/src/model/rootelement';

import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
import { describeMemoryUsage, testMemoryUsage } from '@ckeditor/ckeditor5-core/tests/_utils/memory';

describe( 'InlineEditor', () => {
let editor, editorElement;
Expand Down Expand Up @@ -294,4 +296,17 @@ describe( 'InlineEditor', () => {
.then( newEditor => newEditor.destroy() );
} );
} );

describeMemoryUsage( () => {
testMemoryUsage(
'should not grow on multiple create/destroy',
() => InlineEditor
.create( document.querySelector( '#mem-editor' ), {
plugins: [ ArticlePluginSet ],
toolbar: [ 'heading', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote' ],
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]
}
} ) );
} );
} );
28 changes: 28 additions & 0 deletions tests/manual/memory.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<p>
<button id="destroyEditors">Destroy editor</button>
<button id="initEditors">Init editors</button>
</p>

<div id="editor-1" contenteditable="true" class="custom-class" custom-attr="foo">
<h2>Editor 1</h2>
<p>This is an editor instance. And there's <a href="http://ckeditor.com">some link</a>.</p>
</div>

<div id="editor-2" class="custom-class" custom-attr="foo">
<h2>Editor 2</h2>
<p>This is another editor instance.</p>
<img src="sample.jpg" />
<p>
Unlike Editor 1 it doesn't have contenteditable=true initially.
Check if it's editable after initializing the editors and back to non-editable after destroying them.
</p>
</div>

<style>
.custom-class {
margin-top: 100px;
margin-left: 100px;
margin-bottom: 100px;
width: 450px;
}
</style>
80 changes: 80 additions & 0 deletions tests/manual/memory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals console:false, document, window */

import InlineEditor from '../../src/inlineeditor';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';

window.editors = {};

/*
* Memory-leak safe version of inline 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 initEditors() {
init( '#editor-1' );
init( '#editor-2' );

document.getElementById( 'destroyEditors' ).addEventListener( 'click', destroyEditors );

function init( selector ) {
InlineEditor
.create( document.querySelector( selector ), {
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( editor => {
window.editors[ selector ] = editor;
} )
.catch( err => {
console.error( err.stack );
} );
}

function destroyEditors() {
for ( const selector in window.editors ) {
window.editors[ selector ].destroy().then( () => {
window.editors[ selector ] = undefined;
} );
}

document.getElementById( 'destroyEditors' ).removeEventListener( 'click', destroyEditors );
}
}

document.getElementById( 'initEditors' ).addEventListener( 'click', initEditors );
31 changes: 31 additions & 0 deletions tests/manual/memory.md
Original file line number Diff line number Diff line change
@@ -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).
Binary file added tests/manual/sample.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit dd2bb90

Please sign in to comment.