feat(scope): move domWrite and domRead from RootScope to Scope

Closes dart-archive#1161

Author: vsavkin

lib/core/scope.dart

@@ -55,6 +55,32 @@ class ScopeEvent {
   }
 }
 
+/**
+ * [ConcurrentScopeModificationException] is thrown when `domWrite` or `domRead` is called from
+ * one of the ascendants of the scope.
+ *
+ * The following examples will both throw [ConcurrentScopeModificationException]:
+ *
+ *   rootScope.domWrite(() {
+ *     childScope.domWrite((){});
+ *   });
+ *
+ *   rootScope.domRead(() {
+ *     childScope.domRead((){});
+ *   });
+ *
+ * Calling `domWrite` or `domRead` from the descendants is allowed. So the following will run:
+ *
+ *   childScope.domWrite(() {
+ *     rootScope.domWrite((){});
+ *   });
+ *
+ */
+class ConcurrentScopeModificationException implements Exception {
+  String toString() =>
+      "Either `domWrite` or `domRead` is called from one of the ascendants of the current scope";
+}
+
 /**
  * Allows the configuration of [Scope.digest] iteration maximum time-to-live value. Digest keeps
  * checking the state of the watcher getters until it can execute one full iteration with no
@@ -127,6 +153,11 @@ class Scope {
 
   Scope _parentScope;
 
+  _FunctionChain _domReadHead, _domReadTail;
+  _FunctionChain _domWriteHead, _domWriteTail;
+  bool _scheduledDomWrite = false;
+  bool _scheduledDomRead = false;
+
   Scope get parentScope => _parentScope;
 
   final ScopeStats _stats;
@@ -413,6 +444,107 @@ class Scope {
     }
     return counts;
   }
+
+  /**
+   * Internal. Use [View.domWrite] instead.
+   */
+  void domWrite(fn()) {
+    checkForConcurrentModificationOnDomWrite();
+
+    var chain = new _FunctionChain(fn);
+    if (_domWriteHead == null) {
+      _domWriteHead = _domWriteTail = chain;
+    } else {
+      _domWriteTail = _domWriteTail._next = chain;
+    }
+
+    _setScheduledDomWrite();
+  }
+
+  /**
+   * Internal. Use [View.domRead] instead.
+   */
+  void domRead(fn()) {
+    checkForConcurrentModificationOnDomRead();
+
+    var chain = new _FunctionChain(fn);
+    if (_domReadHead == null) {
+      _domReadHead = _domReadTail = chain;
+    } else {
+      _domReadTail = _domReadTail._next = chain;
+    }
+
+    _setScheduledDomRead();
+  }
+
+  void _runDomWrites() {
+    Scope child = _childHead;
+    while (child != null) {
+      child._runDomWrites();
+      child = child._next;
+    }
+
+    _callFunctionsInChain(_domWriteHead);
+    _domWriteHead = _domWriteTail = null;
+    _scheduledDomWrite = false;
+  }
+
+  void _runDomReads() {
+    Scope child = _childHead;
+    while (child != null) {
+      child._runDomReads();
+      child = child._next;
+    }
+
+    _callFunctionsInChain(_domReadHead);
+    _domReadHead = _domReadTail = null;
+    _scheduledDomRead = false;
+  }
+
+  void _callFunctionsInChain(_FunctionChain chain) {
+    while (chain != null) {
+      try {
+        chain.fn();
+      } catch (e, s) {
+        _exceptionHandler(e, s);
+      }
+      chain = chain._next;
+    }
+  }
+
+  void checkForConcurrentModificationOnDomWrite() {
+    if (_scheduledDomWrite) return;
+
+    for(var scope = _parentScope; scope != null; scope = scope._parentScope) {
+      if (scope._scheduledDomWrite) throw new ConcurrentScopeModificationException();
+    }
+  }
+
+  void checkForConcurrentModificationOnDomRead() {
+    if (_scheduledDomRead) return;
+
+    for(var scope = _parentScope; scope != null; scope = scope._parentScope) {
+      if (scope._scheduledDomRead) throw new ConcurrentScopeModificationException();
+    }
+  }
+
+  void _setScheduledDomWrite() {
+    if (_scheduledDomWrite) return;
+
+    for(var scope = this; scope != null; scope = scope._parentScope) {
+      scope._scheduledDomWrite = true;
+    }
+  }
+
+  void _setScheduledDomRead() {
+    if (_scheduledDomRead) return;
+
+    for(var scope = this; scope != null; scope = scope._parentScope) {
+      scope._scheduledDomRead = true;
+    }
+  }
+
+  ExceptionHandler get _exceptionHandler => rootScope._exceptionHandler;
 }
 
 _mapEqual(Map a, Map b) => a.length == b.length &&
@@ -587,8 +719,6 @@ class RootScope extends Scope {
   final Map<String, AST> astCache = new HashMap<String, AST>();
 
   _FunctionChain _runAsyncHead, _runAsyncTail;
-  _FunctionChain _domWriteHead, _domWriteTail;
-  _FunctionChain _domReadHead, _domReadTail;
 
   final ScopeStats _scopeStats;
 
@@ -734,37 +864,25 @@ class RootScope extends Scope {
     bool runObservers = true;
     try {
       do {
-        if (_domWriteHead != null) _stats.domWriteStart();
-        while (_domWriteHead != null) {
-          try {
-            _domWriteHead.fn();
-          } catch (e, s) {
-            _exceptionHandler(e, s);
-          }
-          _domWriteHead = _domWriteHead._next;
-          if (_domWriteHead == null) _stats.domWriteEnd();
+        if (_scheduledDomWrite) {
+          _stats.domWriteStart();
+          _runDomWrites();
+          _stats.domWriteEnd();
         }
-        _domWriteTail = null;
         if (runObservers) {
           runObservers = false;
           readOnlyGroup.detectChanges(exceptionHandler:_exceptionHandler,
               fieldStopwatch: _scopeStats.fieldStopwatch,
               evalStopwatch: _scopeStats.evalStopwatch,
               processStopwatch: _scopeStats.processStopwatch);
         }
-        if (_domReadHead != null) _stats.domReadStart();
-        while (_domReadHead != null) {
-          try {
-            _domReadHead.fn();
-          } catch (e, s) {
-            _exceptionHandler(e, s);
-          }
-          _domReadHead = _domReadHead._next;
-          if (_domReadHead == null) _stats.domReadEnd();
+        if (_scheduledDomRead) {
+          _stats.domReadStart();
+          _runDomReads();
+          _stats.domReadEnd();
         }
-        _domReadTail = null;
         _runAsyncFns();
-      } while (_domWriteHead != null || _domReadHead != null || _runAsyncHead != null);
+      } while (_scheduledDomWrite || _scheduledDomRead || _runAsyncHead != null);
       _stats.flushEnd();
       assert((() {
         _stats.flushAssertStart();
@@ -822,24 +940,6 @@ class RootScope extends Scope {
     return count;
   }
 
-  void domWrite(fn()) {
-    var chain = new _FunctionChain(fn);
-    if (_domWriteHead == null) {
-      _domWriteHead = _domWriteTail = chain;
-    } else {
-      _domWriteTail = _domWriteTail._next = chain;
-    }
-  }
-
-  void domRead(fn()) {
-    var chain = new _FunctionChain(fn);
-    if (_domReadHead == null) {
-      _domReadHead = _domReadTail = chain;
-    } else {
-      _domReadTail = _domReadTail._next = chain;
-    }
-  }
-
   void destroy() {}
 
   void _transitionState(String from, String to) {

lib/core_dom/view.dart

@@ -21,6 +21,14 @@ class View {
   void registerEvent(String eventName) {
     eventHandler.register(eventName);
   }
+
+  void domWrite(fn()) {
+    scope.domWrite(fn);
+  }
+
+  void domRead(fn()) {
+    scope.domRead(fn);
+  }
 }
 
 /**

test/core/scope_spec.dart

@@ -1558,7 +1558,7 @@ void main() {
         module.bind(ExceptionHandler, toImplementation: LoggingExceptionHandler);
       });
 
-      it(r'should run writes before reads', (RootScope rootScope, Logger logger, ExceptionHandler e) {
+      it('should run writes before reads', (RootScope rootScope, Logger logger, ExceptionHandler e) {
         LoggingExceptionHandler exceptionHandler = e as LoggingExceptionHandler;
         rootScope.domWrite(() {
           logger('write1');
@@ -1579,6 +1579,68 @@ void main() {
         expect(exceptionHandler.errors[0].error).toEqual('write1');
         expect(exceptionHandler.errors[1].error).toEqual('read1');
       });
+
+      it("should run writes of child scopes first", (RootScope rootScope, Logger logger) {
+        final childScope = rootScope.createChild({});
+        childScope.domWrite(() {
+          logger("child1");
+        });
+        rootScope.domWrite(() {
+          logger("root");
+        });
+        childScope.domWrite(() {
+          logger("child2");
+        });
+
+        rootScope.flush();
+
+        expect(logger).toEqual(['child1', 'child2', 'root']);
+      });
+
+      it("should run reads of child scopes first", (RootScope rootScope, Logger logger) {
+        final childScope = rootScope.createChild({});
+        childScope.domRead(() {
+          logger("child1");
+        });
+        rootScope.domRead(() {
+          logger("root");
+        });
+        childScope.domRead(() {
+          logger("child2");
+        });
+
+        rootScope.flush();
+
+        expect(logger).toEqual(['child1', 'child2', 'root']);
+      });
+
+      it("should throw when calling dom write on a child scope",
+          (RootScope rootScope, Logger logger, ExceptionHandler e) {
+        LoggingExceptionHandler handler = e as LoggingExceptionHandler;
+
+        final childScope = rootScope.createChild({});
+        rootScope.domWrite(() {
+          childScope.domWrite((){});
+        });
+
+        rootScope.flush();
+
+        expect(handler.errors[0].error).toBeAnInstanceOf(ConcurrentScopeModificationException);
+      });
+
+      it("should throw when calling dom read on a child scope",
+          (RootScope rootScope, Logger logger, ExceptionHandler e) {
+        LoggingExceptionHandler handler = e as LoggingExceptionHandler;
+
+        final childScope = rootScope.createChild({});
+        rootScope.domRead(() {
+          childScope.domRead((){});
+        });
+
+        rootScope.flush();
+
+        expect(handler.errors[0].error).toBeAnInstanceOf(ConcurrentScopeModificationException);
+      });
     });
 
     describe('exceptionHander', () {