Ckeditor5: It looks like memory leaks tests are broken again

[jodator]

The first occurence of master-revision CI started to fail today: https://travis-ci.org/ckeditor/ckeditor5-editor-classic/jobs/524842961. The previous one is OK: https://travis-ci.org/ckeditor/ckeditor5/builds/523466495.

It looks like Chrome introduces some changes as previos Jobs were executed on Chrome 73.x and the failing one is on Chrome 74.x.

I've tested locally and all previous builds on master-revisions branch fails locally.

[mlewand]

To mitigate the issue for CI it make sense to temporarily use Chrome 73.x until this issue is resolved. Meanwhile this issue should be fixed asap. Once done we'll restore the latest Chrome version on our CI.

@pomek

[pomek]

https://docs.travis-ci.com/user/chrome

[…] you can’t select a specific numeric version.

So we need to find an image (archive), download it on CI and install Chrome from it. And this change must be done in all repositories.

[mlewand]

All right so ignoring the test sounds like a simpler idea, thanks for checking that.

[jodator]

My findings so far:

• removing all plugins but Essentials from ArticlePlugin set make tests pass (but it might be due to the fact of low number of features)
• removing toolbar (or even UI) from the editor (commenting out in base editor classes) also made the tests pass (but the reason might be similar)

I have a weird feeling that either Chrome messed something because I couldn't find any obvious leak (like detached event listeners). Also some retainers looks like webpack-related stuff - but again I didn't find anything that I'd be 100% sure about.

[mlewand]

Ok, for the time being the test were ignored with ckeditor/ckeditor5-core#172.

[mlewand]

@jodator for the time being please prioritize #1490 over this issue for the time being. We'd like to have this feature as soon as possible, while issue with Chrome's gc API already took some time and we would be able to resume to it once #1490 is ready for the review.

It's ok to push it to iteration 25th if we can't do it within this iteration.

[jodator]

I was investigating this issue for a while and I have mixed feelings.

I've started with Chrome 74 and running automated tests gave me some positive results (decreased "memory consumption" in those tests). Yesterday or day before the Chrome was updated to version 75 and it looked like the tests just went green on master (without my previous "fixes"). I've started to debug those issues and I cannot create a repeatable environment for tests.

Anyway my observation are:

• there are no leaks in current Chrome as far as I can test it manually
• we have to implement CKEditor5 inspector removal for those manual memory tests in order to test it reliably
• logging anything from the editor (which can hold reference back to the editor) will leak memory - ie console.log( editor.plugins ) will hold editor instance in the memory until the console is cleared.

I'm holding this issue to the end of iteration and will get to this later.

[jodator]

@Reinmar, unfortunately, it looks like the memory tests are still unreliable. I'm not sure if we're going to be able to have something stable enough for automatic testing.

[jodator]

I've revived the tests on memory-test branch. So far so good. I'd like to see if we have any false negatives there. The situation might get improved as we recently fixed some issues.

[jodator]

It would be nice to use performance.measureMemory() (now as a trial in Chrome 83) - but it requires a special flag to enable it as it is not released yet. Edit - it's trial ends on September this year so might not be feasible to use that yet.