Skip to content

Commit

Permalink
Fixed watchdog crash after immediately refreshing application (#15981)
Browse files Browse the repository at this point in the history
Fix (watchdog): `EditorWatchdog` will no longer crash when the application is refreshed before completing the editor initialization or destruction. Closes #15980.
  • Loading branch information
DawidKossowski committed Mar 26, 2024
1 parent 7df13e9 commit 90aeb27
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 2 deletions.
20 changes: 18 additions & 2 deletions packages/ckeditor5-watchdog/src/editorwatchdog.ts
Expand Up @@ -38,6 +38,14 @@ export default class EditorWatchdog<TEditor extends Editor = Editor> extends Wat
*/
private _editor: TEditor | null = null;

/**
* A promise associated with the life cycle of the editor (creation or destruction processes).
*
* It is used to prevent the initialization of the editor if the previous instance has not been destroyed yet,
* and conversely, to prevent the destruction of the editor if it has not been initialized.
*/
private _lifecyclePromise: Promise<unknown> | null = null;

/**
* Throttled save method. The `save()` method is called the specified `saveInterval` after `throttledSave()` is called,
* unless a new action happens in the meantime.
Expand Down Expand Up @@ -249,7 +257,7 @@ export default class EditorWatchdog<TEditor extends Editor = Editor> extends Wat
config: EditorConfig = this._config!,
context?: Context
): Promise<unknown> {
return Promise.resolve()
this._lifecyclePromise = Promise.resolve( this._lifecyclePromise )
.then( () => {
super._startErrorHandling();

Expand Down Expand Up @@ -282,7 +290,11 @@ export default class EditorWatchdog<TEditor extends Editor = Editor> extends Wat

this.state = 'ready';
this._fire( 'stateChange' );
} ).finally( () => {
this._lifecyclePromise = null;
} );

return this._lifecyclePromise;
}

/**
Expand All @@ -291,15 +303,19 @@ export default class EditorWatchdog<TEditor extends Editor = Editor> extends Wat
* It also sets the state to `destroyed`.
*/
public override destroy(): Promise<unknown> {
return Promise.resolve()
this._lifecyclePromise = Promise.resolve( this._lifecyclePromise )
.then( () => {
this.state = 'destroyed';
this._fire( 'stateChange' );

super.destroy();

return this._destroy();
} ).finally( () => {
this._lifecyclePromise = null;
} );

return this._lifecyclePromise;
}

private _destroy(): Promise<unknown> {
Expand Down
43 changes: 43 additions & 0 deletions packages/ckeditor5-watchdog/tests/editorwatchdog.js
Expand Up @@ -211,6 +211,34 @@ describe( 'EditorWatchdog', () => {

await watchdog.destroy();
} );

it( 'should create an editor instance after the ongoing destruction process has been finished', async () => {
const watchdog = new EditorWatchdog( ClassicTestEditor );

// It simulates the process of creating a new instance of the editor and immediately destroying it.
const simulateRefreshApp = async () => {
watchdog.create( element, {
initialData: '<p>foo</p>',
plugins: [ Paragraph ]
} );

watchdog.destroy();
};

// We do not wait for the completion of this process, which simulates the real case when the app is immediately
// restarted and the initial process of the new app is called without waiting for the previous process to finish.
simulateRefreshApp();

await watchdog.create( element, {
initialData: '<p>foo</p>',
plugins: [ Paragraph ]
} );

expect( watchdog.editor ).to.not.be.null;
expect( watchdog.state ).to.equal( 'ready' );

await watchdog.destroy();
} );
} );

describe( 'editor', () => {
Expand Down Expand Up @@ -1247,6 +1275,21 @@ describe( 'EditorWatchdog', () => {

sinon.assert.calledTwice( spy );
} );

it( 'should destroy the editor after finishing the ongoing creation process', async () => {
const watchdog = new EditorWatchdog( ClassicTestEditor );

// Do not wait for the creation process to finish.
watchdog.create( element, {
initialData: '<p>foo</p>',
plugins: [ Paragraph ]
} );

await watchdog.destroy();

expect( watchdog.editor ).to.equal( null );
expect( watchdog.state ).to.equal( 'destroyed' );
} );
} );

describe( 'crashes', () => {
Expand Down

0 comments on commit 90aeb27

Please sign in to comment.