Merge pull request #823 from peitschie/delay-emit

Only process signals created by an operation after execution is complete
webodf · Sep 18, 2014 · ae2cfe1 · ae2cfe1
2 parents d761802 + 662c2c8
commit ae2cfe1
Show file tree

Hide file tree

Showing 15 changed files with 75 additions and 17 deletions.
diff --git a/webodf/lib/ops/OdtDocument.js b/webodf/lib/ops/OdtDocument.js
@@ -79,7 +79,9 @@ ops.OdtDocument = function OdtDocument(odfCanvas) {
         /**@const*/ SHOW_ALL = NodeFilter.SHOW_ALL,
         blacklistedNodes = new gui.BlacklistNamespaceNodeFilter(["urn:webodf:names:cursor", "urn:webodf:names:editinfo"]),
         odfTextBodyFilter = new gui.OdfTextBodyNodeFilter(),
-        defaultNodeFilter = new core.NodeFilterChain([blacklistedNodes, odfTextBodyFilter]);
+        defaultNodeFilter = new core.NodeFilterChain([blacklistedNodes, odfTextBodyFilter]),
+        /**@type{!Array.<!function():undefined>}*/
+        pendingSignals = [];
 
     /**
      *
@@ -890,12 +892,18 @@ ops.OdtDocument = function OdtDocument(odfCanvas) {
     };
 
     /**
+     * Emit a signal to interested subscribers. Note, signals are not emitted
+     * until *after* the current operation has completed execution in order to
+     * ensure operation atomicity.
+     *
      * @param {!string} eventid
      * @param {*} args
      * @return {undefined}
      */
     this.emit = function (eventid, args) {
-        eventNotifier.emit(eventid, args);
+        pendingSignals.push(function() {
+            eventNotifier.emit(eventid, args);
+        });
     };
 
     /**
@@ -941,15 +949,47 @@ ops.OdtDocument = function OdtDocument(odfCanvas) {
         callback();
     };
 
+    /**
+     * Process steps being inserted into the document. Will emit a steps inserted signal on
+     * behalf of the caller
+     * @param {!{position: !number}} args
+     * @return {undefined}
+     */
+    this.handleStepsInserted = function(args) {
+        stepsTranslator.handleStepsInserted(args);
+        self.emit(ops.OdtDocument.signalStepsInserted, args);
+    };
+
+    /**
+     * Process steps being removed from the document. Will emit a steps removed signal on
+     * behalf of the caller
+     * @param {!{position: !number}} args
+     * @return {undefined}
+     */
+    this.handleStepsRemoved = function(args) {
+        stepsTranslator.handleStepsRemoved(args);
+        self.emit(ops.OdtDocument.signalStepsRemoved, args);
+    };
+
+    /**
+     * Process all signals queued up during operation execution
+     * @return {undefined}
+     */
+    this.processPendingSignals = function() {
+        var signal = pendingSignals.shift();
+        while (signal) {
+            signal();
+            signal = pendingSignals.shift();
+        }
+    };
+
     /**
      * @return {undefined}
      */
     function init() {
         filter = new ops.TextPositionFilter();
         stepUtils = new odf.StepUtils();
         stepsTranslator = new ops.OdtStepsTranslator(getRootNode, createPositionIterator, filter, 500);
-        eventNotifier.subscribe(ops.OdtDocument.signalStepsInserted, stepsTranslator.handleStepsInserted);
-        eventNotifier.subscribe(ops.OdtDocument.signalStepsRemoved, stepsTranslator.handleStepsRemoved);
         eventNotifier.subscribe(ops.OdtDocument.signalOperationEnd, handleOperationExecuted);
         eventNotifier.subscribe(ops.OdtDocument.signalProcessingBatchEnd, core.Task.processTasks);
     }

diff --git a/webodf/lib/ops/OpAddAnnotation.js b/webodf/lib/ops/OpAddAnnotation.js
@@ -158,7 +158,7 @@ ops.OpAddAnnotation = function OpAddAnnotation() {
             insertNodeAtPosition(odtDocument, annotationEnd, position + length);
         }
         insertNodeAtPosition(odtDocument, annotation, position);
-        odtDocument.emit(ops.OdtDocument.signalStepsInserted, {position: position});
+        odtDocument.handleStepsInserted({position: position});
 
         // Move the cursor inside the new annotation,
         // by selecting the paragraph's range.

diff --git a/webodf/lib/ops/OpInsertImage.js b/webodf/lib/ops/OpInsertImage.js
@@ -97,7 +97,7 @@ ops.OpInsertImage = function OpInsertImage() {
             textNode.splitText(domPosition.offset) : textNode.nextSibling;
         frameElement = createFrameElement(odtDocument.getDOMDocument());
         textNode.parentNode.insertBefore(frameElement, refNode);
-        odtDocument.emit(ops.OdtDocument.signalStepsInserted, {position: position});
+        odtDocument.handleStepsInserted({position: position});
 
         // clean up any empty text node which was created by odtDocument.getTextNodeAtStep
         if (textNode.length === 0) {

diff --git a/webodf/lib/ops/OpInsertTable.js b/webodf/lib/ops/OpInsertTable.js
@@ -157,7 +157,7 @@ ops.OpInsertTable = function OpInsertTable() {
             previousSibling = odfUtils.getParagraphElement(domPosition.textNode);
             rootNode.insertBefore(tableNode, previousSibling.nextSibling);
             // The parent table counts for 1 position, and 1 paragraph is added per cell
-            odtDocument.emit(ops.OdtDocument.signalStepsInserted, {position: position});
+            odtDocument.handleStepsInserted({position: position});
 
             odtDocument.getOdfCanvas().refreshSize();
             odtDocument.emit(ops.OdtDocument.signalTableAdded, {

diff --git a/webodf/lib/ops/OpInsertText.js b/webodf/lib/ops/OpInsertText.js
@@ -186,7 +186,7 @@ ops.OpInsertText = function OpInsertText() {
                 previousNode.parentNode.removeChild(previousNode);
             }
 
-            odtDocument.emit(ops.OdtDocument.signalStepsInserted, {position: position});
+            odtDocument.handleStepsInserted({position: position});
 
             if (cursor && moveCursor) {
                 // Explicitly place the cursor in the desired position after insertion

diff --git a/webodf/lib/ops/OpMergeParagraph.js b/webodf/lib/ops/OpMergeParagraph.js
@@ -241,7 +241,7 @@ ops.OpMergeParagraph = function OpMergeParagraph() {
         collapseRules.mergeChildrenIntoParent(sourceParagraph);
 
         // Merging removes a single step between the boundary of the two paragraphs
-        odtDocument.emit(ops.OdtDocument.signalStepsRemoved, {position: sourceStartPosition - 1});
+        odtDocument.handleStepsRemoved({position: sourceStartPosition - 1});
 
         // Downgrade trailing spaces at the end of the destination paragraph, and the beginning of the source paragraph.
         // These are the only two places that might need downgrading as a result of the merge.

diff --git a/webodf/lib/ops/OpRemoveAnnotation.js b/webodf/lib/ops/OpRemoveAnnotation.js
@@ -90,7 +90,7 @@ ops.OpRemoveAnnotation = function OpRemoveAnnotation() {
             annotationEnd.parentNode.removeChild(annotationEnd);
         }
         // The specified position is the first walkable step in the annotation. The position is always just before the first point of change
-        odtDocument.emit(ops.OdtDocument.signalStepsRemoved, {position: position > 0 ? position - 1 : position});
+        odtDocument.handleStepsRemoved({position: position > 0 ? position - 1 : position});
 
         odtDocument.fixCursorPositions();
         odtDocument.getOdfCanvas().refreshAnnotations();

diff --git a/webodf/lib/ops/OpRemoveText.js b/webodf/lib/ops/OpRemoveText.js
@@ -91,7 +91,7 @@ ops.OpRemoveText = function OpRemoveText() {
             }
         });
 
-        odtDocument.emit(ops.OdtDocument.signalStepsRemoved, {position: position});
+        odtDocument.handleStepsRemoved({position: position});
         odtDocument.downgradeWhitespacesAtPosition(position);
         odtDocument.fixCursorPositions();
         odtDocument.getOdfCanvas().refreshSize();

diff --git a/webodf/lib/ops/OpSplitParagraph.js b/webodf/lib/ops/OpSplitParagraph.js
@@ -168,7 +168,7 @@ ops.OpSplitParagraph = function OpSplitParagraph() {
         if (domPosition.textNode.length === 0) {
             domPosition.textNode.parentNode.removeChild(domPosition.textNode);
         }
-        odtDocument.emit(ops.OdtDocument.signalStepsInserted, {position: position});
+        odtDocument.handleStepsInserted({position: position});
 
         if (cursor && moveCursor) {
             odtDocument.moveCursor(memberid, position + 1, 0);

diff --git a/webodf/lib/ops/Session.js b/webodf/lib/ops/Session.js
@@ -47,6 +47,7 @@ ops.Session = function Session(odfCanvas) {
      */
     function forwardBatchStart(args) {
         odtDocument.emit(ops.OdtDocument.signalProcessingBatchStart, args);
+        odtDocument.processPendingSignals();
     }
 
     /**
@@ -56,6 +57,7 @@ ops.Session = function Session(odfCanvas) {
      */
     function forwardBatchEnd(args) {
         odtDocument.emit(ops.OdtDocument.signalProcessingBatchEnd, args);
+        odtDocument.processPendingSignals();
     }
 
     /**
@@ -81,12 +83,14 @@ ops.Session = function Session(odfCanvas) {
         operationRouter.subscribe(ops.OperationRouter.signalProcessingBatchStart, forwardBatchStart);
         operationRouter.subscribe(ops.OperationRouter.signalProcessingBatchEnd, forwardBatchEnd);
         opRouter.setPlaybackFunction(function (op) {
+            var result = false;
             odtDocument.emit(ops.OdtDocument.signalOperationStart, op);
             if (op.execute(odtDocument)) {
                 odtDocument.emit(ops.OdtDocument.signalOperationEnd, op);
-                return true;
+                result = true;
             }
-            return false;
+            odtDocument.processPendingSignals();
+            return result;
         });
         opRouter.setOperationFactory(operationFactory);
     };

diff --git a/webodf/tests/gui/DirectFormattingControllerTests.js b/webodf/tests/gui/DirectFormattingControllerTests.js
@@ -89,7 +89,10 @@ gui.DirectFormattingControllerTests = function DirectFormattingControllerTests(r
 
         this.enqueue = function(ops) {
             self.operations.push.apply(self.operations, ops);
-            ops.forEach(function(op) { op.execute(odtDocument); });
+            ops.forEach(function(op) {
+                op.execute(odtDocument);
+                odtDocument.processPendingSignals();
+            });
         };
 
         this.reset = function() {
@@ -142,6 +145,7 @@ gui.DirectFormattingControllerTests = function DirectFormattingControllerTests(r
             });
             t.odtDocument.emit(ops.Document.signalCursorMoved, t.cursor);
         }
+        t.odtDocument.processPendingSignals();
         return node;
     }
 

diff --git a/webodf/tests/gui/MetadataControllerTests.js b/webodf/tests/gui/MetadataControllerTests.js
@@ -88,6 +88,7 @@ gui.MetadataControllerTests = function MetadataControllerTests(runner) {
                 if (timedOp.execute(odtDocument)) {
                     odtDocument.emit(ops.OdtDocument.signalOperationEnd, timedOp);
                 }
+                odtDocument.processPendingSignals();
             });
         };
 

diff --git a/webodf/tests/gui/SelectionControllerTests.js b/webodf/tests/gui/SelectionControllerTests.js
@@ -69,7 +69,10 @@ gui.SelectionControllerTests = function SelectionControllerTests(runner) {
 
         this.enqueue = function(ops) {
             self.operations.push.apply(self.operations, ops);
-            ops.forEach(function(op) { op.execute(odtDocument); });
+            ops.forEach(function(op) {
+                op.execute(odtDocument);
+                odtDocument.processPendingSignals();
+            });
         };
 
         this.reset = function() {
@@ -97,6 +100,7 @@ gui.SelectionControllerTests = function SelectionControllerTests(runner) {
         t.rangeToSelection = t.selectionController.rangeToSelection;
         t.cursor = new ops.OdtCursor(inputMemberId, t.odtDocument);
         t.odtDocument.addCursor(t.cursor);
+        t.odtDocument.processPendingSignals();
         return node;
     }
 

diff --git a/webodf/tests/gui/TextControllerTests.js b/webodf/tests/gui/TextControllerTests.js
@@ -72,7 +72,10 @@ gui.TextControllerTests = function TextControllerTests(runner) {
 
         this.enqueue = function(ops) {
             self.operations.push.apply(self.operations, ops);
-            ops.forEach(function(op) { op.execute(odtDocument); });
+            ops.forEach(function(op) {
+                op.execute(odtDocument);
+                odtDocument.processPendingSignals();
+            });
         };
 
         this.reset = function() {
@@ -132,6 +135,7 @@ gui.TextControllerTests = function TextControllerTests(runner) {
             range.setEndAfter(node.getElementsByTagNameNS(testns, "end")[0]);
             t.cursor.setSelectedRange(range, true);
         }
+        t.odtDocument.processPendingSignals();
         return node;
     }
 

diff --git a/webodf/tests/ops/OperationTests.js b/webodf/tests/ops/OperationTests.js
@@ -293,6 +293,7 @@ ops.OperationTests = function OperationTests(runner) {
             if (metabefore) {
                 t.odtDocument.emit(ops.OdtDocument.signalOperationEnd, op);
             }
+            t.odtDocument.processPendingSignals();
             checkForEmptyTextNodes(t.odtDocument.getCanvas().getElement());
         }