Only process signals created by an operation after execution is complete

This ensures the operation cannot be accidentally interrupted by an external consumer of an event.
webodf · Sep 18, 2014 · 662c2c8 · 662c2c8
1 parent db2096f
commit 662c2c8
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 7 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);
+        });
     };
 
     /**
@@ -963,6 +971,18 @@ ops.OdtDocument = function OdtDocument(odfCanvas) {
         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}
      */

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());
         }
-Original file line number
+Diff line change
@@ Expand Up @@
                     if (timedOp.execute(odtDocument)) {
                         odtDocument.emit(ops.OdtDocument.signalOperationEnd, timedOp);
                     }
+                    odtDocument.processPendingSignals();
                 });
             };
@@ Expand Down @@