Merge pull request #18 from ckeditor/t/17

Fix: The editor data will be saved correctly after the `destroy()` method is called. Added the protected `Watchdog#_now()` method that allows for time-based testing of the error handling mechanism. Closes #17. Closes #19.

src/watchdog.js

@@ -64,6 +64,14 @@ export default class Watchdog {
 		 */
 		this._crashNumberLimit = typeof config.crashNumberLimit === 'number' ? config.crashNumberLimit : 3;
 
+		/**
+		 * Returns the result of `Date.now()` call. It can be overridden in tests to mock time as the popular
+		 * approaches like `sinon.useFakeTimers()` does not work well with error handling.
+		 *
+		 * @protected
+		 */
+		this._now = Date.now;
+
 		/**
 		 * @private
 		 * @type {Number}
@@ -87,7 +95,10 @@ export default class Watchdog {
 		 * @private
 		 * @type {Function}
 		 */
-		this._throttledSave = throttle( this._save.bind( this ), config.saveInterval || 5000 );
+		this._throttledSave = throttle(
+			this._save.bind( this ),
+			typeof config.saveInterval === 'number' ? config.saveInterval : 5000
+		);
 
 		/**
 		 * The current editor instance.
@@ -258,6 +269,9 @@ export default class Watchdog {
 		window.removeEventListener( 'error', this._boundErrorHandler );
 		this.stopListening( this._editor.model.document, 'change:data', this._throttledSave );
 
+		// Save data if there is a remaining editor data change.
+		this._throttledSave.flush();
+
 		return Promise.resolve()
 			.then( () => this._destructor( this._editor ) )
 			.then( () => {
@@ -311,7 +325,7 @@ export default class Watchdog {
 				source: evt.source,
 				lineno: evt.lineno,
 				colno: evt.colno,
-				date: Date.now()
+				date: this._now()
 			} );
 
 			this.fire( 'error', { error: evt.error } );
@@ -374,7 +388,6 @@ export default class Watchdog {
 	 */
 	_restart() {
 		this.state = 'initializing';
-		this._throttledSave.flush();
 
 		return Promise.resolve()
 			.then( () => this._destroy() )

tests/watchdog.js

@@ -369,7 +369,7 @@ describe( 'Watchdog', () => {
 		} );
 
 		it( 'Watchdog should crash permanently if the `crashNumberLimit` is reached' +
-		' and the average time between errors is lower than `minimumNonErrorTimePeriod` (default values)', () => {
+			' and the average time between errors is lower than `minimumNonErrorTimePeriod` (default values)', () => {
 			const watchdog = new Watchdog();
 
 			watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) );
@@ -406,7 +406,7 @@ describe( 'Watchdog', () => {
 		} );
 
 		it( 'Watchdog should crash permanently if the `crashNumberLimit` is reached' +
-		' and the average time between errors is lower than `minimumNonErrorTimePeriod` (custom values)', () => {
+			' and the average time between errors is lower than `minimumNonErrorTimePeriod` (custom values)', () => {
 			const watchdog = new Watchdog( { crashNumberLimit: 2, minimumNonErrorTimePeriod: 1000 } );
 
 			watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) );
@@ -695,6 +695,44 @@ describe( 'Watchdog', () => {
 		} );
 	} );
 
+	describe( 'destroy()', () => {
+		// See #19.
+		it( 'should clean internal stuff', () => {
+			// 30ms should be enough to make the two data changes split into two data save actions.
+			// This will ensure that the second data save action will be put off in time.
+			const SAVE_INTERVAL = 30;
+
+			const watchdog = Watchdog.for( ClassicTestEditor, {
+				saveInterval: SAVE_INTERVAL,
+			} );
+
+			return watchdog.create( element, {
+				initialData: '<p>foo</p>',
+				plugins: [ Paragraph ]
+			} ).then( () => {
+				const doc = watchdog.editor.model.document;
+
+				watchdog.editor.model.change( writer => {
+					writer.insertText( 'bar', writer.createPositionAt( doc.getRoot(), 1 ) );
+				} );
+
+				watchdog.editor.model.change( writer => {
+					writer.insertText( 'foo', writer.createPositionAt( doc.getRoot(), 1 ) );
+				} );
+			} ).then( () => {
+				return watchdog.destroy();
+			} ).then( () => {
+				// Wait to ensure that the throttled save is cleared and won't be executed
+				// on the non-existing editor.
+				return new Promise( res => setTimeout( res, SAVE_INTERVAL ) );
+			} ).then( () => {
+				expect( watchdog.editor ).to.equal( null );
+				expect( watchdog.state ).to.equal( 'destroyed' );
+				expect( watchdog.crashes ).to.deep.equal( [] );
+			} );
+		} );
+	} );
+
 	describe( 'static for()', () => {
 		it( 'should be a shortcut method for creating the watchdog', () => {
 			const watchdog = Watchdog.for( ClassicTestEditor );