Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed watchdog crash after immediately refreshing application #15981

Merged
merged 6 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions packages/ckeditor5-watchdog/src/editorwatchdog.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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