Jelajahi Sumber

Improved how the context is destroyed by the editor when the editor has created the context.

Oskar Wróbel 6 tahun lalu
induk
melakukan
33e642a63b

+ 75 - 56
packages/ckeditor5-core/src/context.js

@@ -83,14 +83,6 @@ export default class Context {
 		 */
 		this.t = this.locale.t;
 
-		/**
-		 * `true` when the context is created by an editor `false` otherwise.
-		 *
-		 * @readonly
-		 * @type {Boolean}
-		 */
-		this.wasCreatedByEditor = false;
-
 		/**
 		 * List of editors to which this context instance is injected.
 		 *
@@ -98,44 +90,17 @@ export default class Context {
 		 * @type {Set<module:core/editor/editor~Editor>}
 		 */
 		this._editors = new Set();
-	}
-
-	/**
-	 * Adds a reference to the editor which is used with this context.
-	 *
-	 * When the context is created by the editor it is additionally
-	 * marked as a {@link ~Context#wasCreatedByEditor} what is used
-	 * in the destroy chain.
-	 *
-	 * @param {module:core/editor/editor~Editor} editor
-	 * @param {Boolean} isContextOwner
-	 */
-	addEditor( editor, isContextOwner ) {
-		if ( this.wasCreatedByEditor ) {
-			/**
-			 * Cannot add multiple editors to the context which is created by the editor.
-			 *
-			 * @error context-addEditor-private-context
-			 */
-			throw new CKEditorError(
-				'context-addEditor-private-context: Cannot add multiple editors to the context which is created by the editor.'
-			);
-		}
-
-		this._editors.add( editor );
-
-		if ( isContextOwner ) {
-			this.wasCreatedByEditor = true;
-		}
-	}
 
-	/**
-	 * Removes a reference to the editor which was used with this context.
-	 *
-	 * @param {module:core/editor/editor~Editor} editor
-	 */
-	removeEditor( editor ) {
-		return this._editors.delete( editor );
+		/**
+		 * Reference to the editor which created the context.
+		 * Null when the context was created outside of the editor.
+		 *
+		 * It is used to destroy the context when removing the editor that created the context.
+		 *
+		 * @private
+		 * @type {module:core/editor/editor~Editor|null}
+		 */
+		this._contextOwner = null;
 	}
 
 	/**
@@ -178,15 +143,79 @@ export default class Context {
 		return this.plugins.init( plugins );
 	}
 
+	/**
+	 * Destroys the context instance, releasing all resources used by it.
+	 *
+	 * @returns {Promise} A promise that resolves once the context instance is fully destroyed.
+	 */
+	destroy() {
+		return Promise.all( Array.from( this._editors, editor => editor.destroy() ) )
+			.then( () => this.plugins.destroy() );
+	}
+
+	/**
+	 * Adds a reference to the editor which is used with this context.
+	 *
+	 * When the given editor has created the context then the reference to this editor will be stored
+	 * as a {@link ~Context#_contextOwner}.
+	 *
+	 * This method should be used only by the editor.
+	 *
+	 * @protected
+	 * @param {module:core/editor/editor~Editor} editor
+	 * @param {Boolean} isContextOwner
+	 */
+	_addEditor( editor, isContextOwner ) {
+		if ( this._contextOwner ) {
+			/**
+			 * Cannot add multiple editors to the context which is created by the editor.
+			 *
+			 * @error context-addEditor-private-context
+			 */
+			throw new CKEditorError(
+				'context-addEditor-private-context: Cannot add multiple editors to the context which is created by the editor.'
+			);
+		}
+
+		this._editors.add( editor );
+
+		if ( isContextOwner ) {
+			this._contextOwner = editor;
+		}
+	}
+
+	/**
+	 * Removes a reference to the editor which was used with this context.
+	 * When the context was created by the given editor then the context will be destroyed.
+	 *
+	 * This method should be used only by the editor.
+	 *
+	 * @protected
+	 * @param {module:core/editor/editor~Editor} editor
+	 * @return {Promise} A promise that resolves once the editor is removed from the context or when the context has been destroyed.
+	 */
+	_removeEditor( editor ) {
+		this._editors.delete( editor );
+
+		if ( this._contextOwner === editor ) {
+			return this.destroy();
+		}
+
+		return Promise.resolve();
+	}
+
 	/**
 	 * Returns context configuration which will be copied to editors created using this context.
 	 *
 	 * The configuration returned by this method has removed plugins configuration - plugins are shared with all editors
 	 * through another mechanism.
 	 *
+	 * This method should be used only by the editor.
+	 *
+	 * @protected
 	 * @returns {Object} Configuration as a plain object.
 	 */
-	getConfigForEditor() {
+	_getConfigForEditor() {
 		const result = {};
 
 		for ( const name of this.config.names() ) {
@@ -198,16 +227,6 @@ export default class Context {
 		return result;
 	}
 
-	/**
-	 * Destroys the context instance, releasing all resources used by it.
-	 *
-	 * @returns {Promise} A promise that resolves once the context instance is fully destroyed.
-	 */
-	destroy() {
-		return Promise.all( Array.from( this._editors, editor => editor.destroy() ) )
-			.then( () => this.plugins.destroy() );
-	}
-
 	/**
 	 * Creates and initializes a new context instance.
 	 *

+ 5 - 11
packages/ckeditor5-core/src/editor/editor.js

@@ -59,7 +59,7 @@ export default class Editor {
 		 * @type {module:core/context~Context}
 		 */
 		this._context = config.context || new Context( { language: config.language } );
-		this._context.addEditor( this, !config.context );
+		this._context._addEditor( this, !config.context );
 
 		const availablePlugins = this.constructor.builtinPlugins;
 
@@ -74,7 +74,7 @@ export default class Editor {
 		 */
 		this.config = new Config( config, this.constructor.defaultConfig );
 		this.config.define( 'plugins', availablePlugins );
-		this.config.define( this._context.getConfigForEditor() );
+		this.config.define( this._context._getConfigForEditor() );
 
 		/**
 		 * The plugins loaded and in use by this editor instance.
@@ -252,15 +252,9 @@ export default class Editor {
 
 		return readyPromise
 			.then( () => {
-				// Remove the editor from the context to avoid destroying it
-				// one more time when context will be destroyed.
-				this._context.removeEditor( this );
-
-				// When the editor was an owner of the context then
-				// the context should be destroyed along with the editor.
-				if ( this._context.wasCreatedByEditor ) {
-					return this._context.destroy();
-				}
+				// Remove the editor from the context.
+				// When the context was created by this editor then then the context will be destroyed.
+				return this._context._removeEditor( this );
 			} ).then( () => {
 				this.fire( 'destroy' );
 				this.stopListening();

+ 2 - 2
packages/ckeditor5-core/tests/context.js

@@ -26,7 +26,7 @@ describe( 'Context', () => {
 		} );
 	} );
 
-	describe( 'getConfigForEditor()', () => {
+	describe( '_getConfigForEditor()', () => {
 		it( 'should return the configuration without plugin config', () => {
 			class FooPlugin extends ContextPlugin {}
 			class BarPlugin extends ContextPlugin {}
@@ -41,7 +41,7 @@ describe( 'Context', () => {
 				bar: 'bom'
 			} );
 
-			expect( context.getConfigForEditor() ).to.be.deep.equal( {
+			expect( context._getConfigForEditor() ).to.be.deep.equal( {
 				language: { ui: 'pl', content: 'ar' },
 				foo: 1,
 				bar: 'bom'

+ 0 - 2
packages/ckeditor5-core/tests/editor/editor.js

@@ -186,7 +186,6 @@ describe( 'Editor', () => {
 			const editor = new TestEditor();
 
 			expect( editor._context ).to.be.an.instanceof( Context );
-			expect( editor._context.wasCreatedByEditor ).to.true;
 		} );
 
 		it( 'should use context given through configuration when is defined', async () => {
@@ -194,7 +193,6 @@ describe( 'Editor', () => {
 			const editor = new TestEditor( { context } );
 
 			expect( editor._context ).to.equal( context );
-			expect( editor._context.wasCreatedByEditor ).to.false;
 		} );
 
 		it( 'should throw when try to use context created by one editor with the other editor', () => {