Use 'Directory.watch' on Windows in pkg/watcher, instead of pooling.

pkg/watcher/lib/src/directory_watcher/windows.dart

Index: pkg/watcher/lib/src/directory_watcher/windows.dart
diff --git a/pkg/watcher/lib/src/directory_watcher/mac_os.dart b/pkg/watcher/lib/src/directory_watcher/windows.dart
similarity index 52%
copy from pkg/watcher/lib/src/directory_watcher/mac_os.dart
copy to pkg/watcher/lib/src/directory_watcher/windows.dart
index e3efa2dd64bc034aaeed4b1e9369c67171a036c1..ddcf897d361217b7529c66a7a6fcab9bd341fbea 100644
--- a/pkg/watcher/lib/src/directory_watcher/mac_os.dart
+++ b/pkg/watcher/lib/src/directory_watcher/windows.dart
@@ -1,497 +1,419 @@
-// Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-library watcher.directory_watcher.mac_os;
-
-import 'dart:async';
-import 'dart:io';
-
-import 'package:path/path.dart' as p;
-import 'package:stack_trace/stack_trace.dart';
-
-import '../constructable_file_system_event.dart';
-import '../path_set.dart';
-import '../utils.dart';
-import '../watch_event.dart';
-import 'resubscribable.dart';
-
-/// Uses the FSEvents subsystem to watch for filesystem events.
-///
-/// FSEvents has two main idiosyncrasies that this class works around. First, it
-/// will occasionally report events that occurred before the filesystem watch
-/// was initiated. Second, if multiple events happen to the same file in close
-/// succession, it won't report them in the order they occurred. See issue
-/// 14373.
-///
-/// This also works around issues 16003 and 14849 in the implementation of
-/// [Directory.watch].
-class MacOSDirectoryWatcher extends ResubscribableDirectoryWatcher {
-  // TODO(nweiz): remove these when issue 15042 is fixed.
-  static var logDebugInfo = false;
-  static var _count = 0;
-
-  MacOSDirectoryWatcher(String directory)
-      : super(directory, () => new _MacOSDirectoryWatcher(directory, _count++));
-}
-
-class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher {
-  // TODO(nweiz): remove these when issue 15042 is fixed.
-  static var _count = 0;
-  final String _id;
-
-  final String directory;
-
-  Stream<WatchEvent> get events => _eventsController.stream;
-  final _eventsController = new StreamController<WatchEvent>.broadcast();
-
-  bool get isReady => _readyCompleter.isCompleted;
-
-  Future get ready => _readyCompleter.future;
-  final _readyCompleter = new Completer();
-
-  /// The set of files that are known to exist recursively within the watched
-  /// directory.
-  ///
-  /// The state of files on the filesystem is compared against this to determine
-  /// the real change that occurred when working around issue 14373. This is
-  /// also used to emit REMOVE events when subdirectories are moved out of the
-  /// watched directory.
-  final PathSet _files;
-
-  /// The subscription to the stream returned by [Directory.watch].
-  ///
-  /// This is separate from [_subscriptions] because this stream occasionally
-  /// needs to be resubscribed in order to work around issue 14849.
-  StreamSubscription<FileSystemEvent> _watchSubscription;
-
-  /// The subscription to the [Directory.list] call for the initial listing of
-  /// the directory to determine its initial state.
-  StreamSubscription<FileSystemEntity> _initialListSubscription;
-
-  /// The subscription to the [Directory.list] call for listing the contents of
-  /// a subdirectory that was moved into the watched directory.
-  StreamSubscription<FileSystemEntity> _listSubscription;
-
-  /// The timer for tracking how long we wait for an initial batch of bogus
-  /// events (see issue 14373).
-  Timer _bogusEventTimer;
-
-  _MacOSDirectoryWatcher(String directory, int parentId)
-      : directory = directory,
-        _files = new PathSet(directory),
-        _id = "$parentId/${_count++}" {
-    _startWatch();
-
-    // Before we're ready to emit events, wait for [_listDir] to complete and
-    // for enough time to elapse that if bogus events (issue 14373) would be
-    // emitted, they will be.
-    //
-    // If we do receive a batch of events, [_onBatch] will ensure that these
-    // futures don't fire and that the directory is re-listed.
-    Future.wait([
-      _listDir().then((_) {
-        if (MacOSDirectoryWatcher.logDebugInfo) {
-          print("[$_id] finished initial directory list");
-        }
-      }),
-      _waitForBogusEvents()
-    ]).then((_) {
-      if (MacOSDirectoryWatcher.logDebugInfo) {
-        print("[$_id] watcher is ready, known files:");
-        for (var file in _files.toSet()) {
-          print("[$_id]   ${p.relative(file, from: directory)}");
-        }
-      }
-      _readyCompleter.complete();
-    });
-  }
-
-  void close() {
-    if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] watcher is closed\n${new Chain.current().terse}");
-    }
-    if (_watchSubscription != null) _watchSubscription.cancel();
-    if (_initialListSubscription != null) _initialListSubscription.cancel();
-    if (_listSubscription != null) _listSubscription.cancel();
-    _watchSubscription = null;
-    _initialListSubscription = null;
-    _listSubscription = null;
-    _eventsController.close();
-  }
-
-  /// The callback that's run when [Directory.watch] emits a batch of events.
-  void _onBatch(List<FileSystemEvent> batch) {
-    if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] ======== batch:");
-      for (var event in batch) {
-        print("[$_id]   ${_formatEvent(event)}");
-      }
-
-      print("[$_id] known files:");
-      for (var file in _files.toSet()) {
-        print("[$_id]   ${p.relative(file, from: directory)}");
-      }
-    }
-
-    // If we get a batch of events before we're ready to begin emitting events,
-    // it's probable that it's a batch of pre-watcher events (see issue 14373).
-    // Ignore those events and re-list the directory.
-    if (!isReady) {
-      if (MacOSDirectoryWatcher.logDebugInfo) {
-        print("[$_id] not ready to emit events, re-listing directory");
-      }
-
-      // Cancel the timer because bogus events only occur in the first batch, so
-      // we can fire [ready] as soon as we're done listing the directory.
-      _bogusEventTimer.cancel();
-      _listDir().then((_) {
-        if (MacOSDirectoryWatcher.logDebugInfo) {
-          print("[$_id] watcher is ready, known files:");
-          for (var file in _files.toSet()) {
-            print("[$_id]   ${p.relative(file, from: directory)}");
-          }
-        }
-        _readyCompleter.complete();
-      });
-      return;
-    }
-
-    _sortEvents(batch).forEach((path, events) {
-      var relativePath = p.relative(path, from: directory);
-      if (MacOSDirectoryWatcher.logDebugInfo) {
-        print("[$_id] events for $relativePath:");
-        for (var event in events) {
-          print("[$_id]   ${_formatEvent(event)}");
-        }
-      }
-
-      var canonicalEvent = _canonicalEvent(events);
-      events = canonicalEvent == null ?
-          _eventsBasedOnFileSystem(path) : [canonicalEvent];
-      if (MacOSDirectoryWatcher.logDebugInfo) {
-        print("[$_id] canonical event for $relativePath: "
-            "${_formatEvent(canonicalEvent)}");
-        print("[$_id] actionable events for $relativePath: "
-            "${events.map(_formatEvent)}");
-      }
-
-      for (var event in events) {
-        if (event is FileSystemCreateEvent) {
-          if (!event.isDirectory) {
-            // Don't emit ADD events for files or directories that we already
-            // know about. Such an event comes from FSEvents reporting an add
-            // that happened prior to the watch beginning.
-            if (_files.contains(path)) continue;
-
-            _emitEvent(ChangeType.ADD, path);
-            _files.add(path);
-            continue;
-          }
-
-          if (_files.containsDir(path)) continue;
-
-          var stream = Chain.track(new Directory(path).list(recursive: true));
-          _listSubscription = stream.listen((entity) {
-            if (entity is Directory) return;
-            if (_files.contains(path)) return;
-
-            _emitEvent(ChangeType.ADD, entity.path);
-            _files.add(entity.path);
-          }, onError: (e, stackTrace) {
-            if (MacOSDirectoryWatcher.logDebugInfo) {
-              print("[$_id] got error listing $relativePath: $e");
-            }
-            _emitError(e, stackTrace);
-          }, cancelOnError: true);
-        } else if (event is FileSystemModifyEvent) {
-          assert(!event.isDirectory);
-          _emitEvent(ChangeType.MODIFY, path);
-        } else {
-          assert(event is FileSystemDeleteEvent);
-          for (var removedPath in _files.remove(path)) {
-            _emitEvent(ChangeType.REMOVE, removedPath);
-          }
-        }
-      }
-    });
-
-    if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] ======== batch complete");
-    }
-  }
-
-  /// Sort all the events in a batch into sets based on their path.
-  ///
-  /// A single input event may result in multiple events in the returned map;
-  /// for example, a MOVE event becomes a DELETE event for the source and a
-  /// CREATE event for the destination.
-  ///
-  /// The returned events won't contain any [FileSystemMoveEvent]s, nor will it
-  /// contain any events relating to [directory].
-  Map<String, Set<FileSystemEvent>> _sortEvents(List<FileSystemEvent> batch) {
-    var eventsForPaths = {};
-
-    // FSEvents can report past events, including events on the root directory
-    // such as it being created. We want to ignore these. If the directory is
-    // really deleted, that's handled by [_onDone].
-    batch = batch.where((event) => event.path != directory).toList();
-
-    // Events within directories that already have events are superfluous; the
-    // directory's full contents will be examined anyway, so we ignore such
-    // events. Emitting them could cause useless or out-of-order events.
-    var directories = unionAll(batch.map((event) {
-      if (!event.isDirectory) return new Set();
-      if (event is! FileSystemMoveEvent) return new Set.from([event.path]);
-      return new Set.from([event.path, event.destination]);
-    }));
-
-    isInModifiedDirectory(path) =>
-        directories.any((dir) => path != dir && path.startsWith(dir));
-
-    addEvent(path, event) {
-      if (isInModifiedDirectory(path)) return;
-      var set = eventsForPaths.putIfAbsent(path, () => new Set());
-      set.add(event);
-    }
-
-    for (var event in batch) {
-      // The Mac OS watcher doesn't emit move events. See issue 14806.
-      assert(event is! FileSystemMoveEvent);
-      addEvent(event.path, event);
-    }
-
-    return eventsForPaths;
-  }
-
-  /// Returns the canonical event from a batch of events on the same path, if
-  /// one exists.
-  ///
-  /// If [batch] doesn't contain any contradictory events (e.g. DELETE and
-  /// CREATE, or events with different values for [isDirectory]), this returns a
-  /// single event that describes what happened to the path in question.
-  ///
-  /// If [batch] does contain contradictory events, this returns `null` to
-  /// indicate that the state of the path on the filesystem should be checked to
-  /// determine what occurred.
-  FileSystemEvent _canonicalEvent(Set<FileSystemEvent> batch) {
-    // An empty batch indicates that we've learned earlier that the batch is
-    // contradictory (e.g. because of a move).
-    if (batch.isEmpty) return null;
-
-    var type = batch.first.type;
-    var isDir = batch.first.isDirectory;
-    var hadModifyEvent = false;
-
-    for (var event in batch.skip(1)) {
-      // If one event reports that the file is a directory and another event
-      // doesn't, that's a contradiction.
-      if (isDir != event.isDirectory) return null;
-
-      // Modify events don't contradict either CREATE or REMOVE events. We can
-      // safely assume the file was modified after a CREATE or before the
-      // REMOVE; otherwise there will also be a REMOVE or CREATE event
-      // (respectively) that will be contradictory.
-      if (event is FileSystemModifyEvent) {
-        hadModifyEvent = true;
-        continue;
-      }
-      assert(event is FileSystemCreateEvent || event is FileSystemDeleteEvent);
-
-      // If we previously thought this was a MODIFY, we now consider it to be a
-      // CREATE or REMOVE event. This is safe for the same reason as above.
-      if (type == FileSystemEvent.MODIFY) {
-        type = event.type;
-        continue;
-      }
-
-      // A CREATE event contradicts a REMOVE event and vice versa.
-      assert(type == FileSystemEvent.CREATE || type == FileSystemEvent.DELETE);
-      if (type != event.type) return null;
-    }
-
-    // If we got a CREATE event for a file we already knew about, that comes
-    // from FSEvents reporting an add that happened prior to the watch
-    // beginning. If we also received a MODIFY event, we want to report that,
-    // but not the CREATE.
-    if (type == FileSystemEvent.CREATE && hadModifyEvent &&
-        _files.contains(batch.first.path)) {
-      type = FileSystemEvent.MODIFY;
-    }
-
-    switch (type) {
-      case FileSystemEvent.CREATE:
-        // Issue 16003 means that a CREATE event for a directory can indicate
-        // that the directory was moved and then re-created.
-        // [_eventsBasedOnFileSystem] will handle this correctly by producing a
-        // DELETE event followed by a CREATE event if the directory exists.
-        if (isDir) return null;
-        return new ConstructableFileSystemCreateEvent(batch.first.path, false);
-      case FileSystemEvent.DELETE:
-        return new ConstructableFileSystemDeleteEvent(batch.first.path, isDir);
-      case FileSystemEvent.MODIFY:
-        return new ConstructableFileSystemModifyEvent(
-            batch.first.path, isDir, false);
-      default: assert(false);
-    }
-  }
-
-  /// Returns one or more events that describe the change between the last known
-  /// state of [path] and its current state on the filesystem.
-  ///
-  /// This returns a list whose order should be reflected in the events emitted
-  /// to the user, unlike the batched events from [Directory.watch]. The
-  /// returned list may be empty, indicating that no changes occurred to [path]
-  /// (probably indicating that it was created and then immediately deleted).
-  List<FileSystemEvent> _eventsBasedOnFileSystem(String path) {
-    var fileExisted = _files.contains(path);
-    var dirExisted = _files.containsDir(path);
-    var fileExists = new File(path).existsSync();
-    var dirExists = new Directory(path).existsSync();
-
-    if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] checking file system for "
-          "${p.relative(path, from: directory)}");
-      print("[$_id]   file existed: $fileExisted");
-      print("[$_id]   dir existed: $dirExisted");
-      print("[$_id]   file exists: $fileExists");
-      print("[$_id]   dir exists: $dirExists");
-    }
-
-    var events = [];
-    if (fileExisted) {
-      if (fileExists) {
-        events.add(new ConstructableFileSystemModifyEvent(path, false, false));
-      } else {
-        events.add(new ConstructableFileSystemDeleteEvent(path, false));
-      }
-    } else if (dirExisted) {
-      if (dirExists) {
-        // If we got contradictory events for a directory that used to exist and
-        // still exists, we need to rescan the whole thing in case it was
-        // replaced with a different directory.
-        events.add(new ConstructableFileSystemDeleteEvent(path, true));
-        events.add(new ConstructableFileSystemCreateEvent(path, true));
-      } else {
-        events.add(new ConstructableFileSystemDeleteEvent(path, true));
-      }
-    }
-
-    if (!fileExisted && fileExists) {
-      events.add(new ConstructableFileSystemCreateEvent(path, false));
-    } else if (!dirExisted && dirExists) {
-      events.add(new ConstructableFileSystemCreateEvent(path, true));
-    }
-
-    return events;
-  }
-
-  /// The callback that's run when the [Directory.watch] stream is closed.
-  void _onDone() {
-    if (MacOSDirectoryWatcher.logDebugInfo) print("[$_id] stream closed");
-
-    _watchSubscription = null;
-
-    // If the directory still exists and we're still expecting bogus events,
-    // this is probably issue 14849 rather than a real close event. We should
-    // just restart the watcher.
-    if (!isReady && new Directory(directory).existsSync()) {
-      if (MacOSDirectoryWatcher.logDebugInfo) {
-        print("[$_id] fake closure (issue 14849), re-opening stream");
-      }
-      _startWatch();
-      return;
-    }
-
-    // FSEvents can fail to report the contents of the directory being removed
-    // when the directory itself is removed, so we need to manually mark the
-    // files as removed.
-    for (var file in _files.toSet()) {
-      _emitEvent(ChangeType.REMOVE, file);
-    }
-    _files.clear();
-    close();
-  }
-
-  /// Start or restart the underlying [Directory.watch] stream.
-  void _startWatch() {
-    // Batch the FSEvent changes together so that we can dedup events.
-    var innerStream =
-        Chain.track(new Directory(directory).watch(recursive: true))
-        .transform(new BatchedStreamTransformer<FileSystemEvent>());
-    _watchSubscription = innerStream.listen(_onBatch,
-        onError: _eventsController.addError,
-        onDone: _onDone);
-  }
-
-  /// Starts or restarts listing the watched directory to get an initial picture
-  /// of its state.
-  Future _listDir() {
-    assert(!isReady);
-    if (_initialListSubscription != null) _initialListSubscription.cancel();
-
-    _files.clear();
-    var completer = new Completer();
-    var stream = Chain.track(new Directory(directory).list(recursive: true));
-    _initialListSubscription = stream.listen((entity) {
-      if (entity is! Directory) _files.add(entity.path);
-    },
-        onError: _emitError,
-        onDone: completer.complete,
-        cancelOnError: true);
-    return completer.future;
-  }
-
-  /// Wait 200ms for a batch of bogus events (issue 14373) to come in.
-  ///
-  /// 200ms is short in terms of human interaction, but longer than any Mac OS
-  /// watcher tests take on the bots, so it should be safe to assume that any
-  /// bogus events will be signaled in that time frame.
-  Future _waitForBogusEvents() {
-    var completer = new Completer();
-    _bogusEventTimer = new Timer(
-        new Duration(milliseconds: 200),
-        completer.complete);
-    return completer.future;
-  }
-
-  /// Emit an event with the given [type] and [path].
-  void _emitEvent(ChangeType type, String path) {
-    if (!isReady) return;
-
-    if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] emitting $type ${p.relative(path, from: directory)}");
-    }
-
-    _eventsController.add(new WatchEvent(type, path));
-  }
-
-  /// Emit an error, then close the watcher.
-  void _emitError(error, StackTrace stackTrace) {
-    if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] emitting error: $error\n" +
-          "${new Chain.forTrace(stackTrace).terse}");
-    }
-    _eventsController.addError(error, stackTrace);
-    close();
-  }
-
-  // TODO(nweiz): remove this when issue 15042 is fixed.
-  /// Return a human-friendly string representation of [event].
-  String _formatEvent(FileSystemEvent event) {
-    if (event == null) return 'null';
-
-    var path = p.relative(event.path, from: directory);
-    var type = event.isDirectory ? 'directory' : 'file';
-    if (event is FileSystemCreateEvent) {
-      return "create $type $path";
-    } else if (event is FileSystemDeleteEvent) {
-      return "delete $type $path";
-    } else if (event is FileSystemModifyEvent) {
-      return "modify $type $path";
-    } else if (event is FileSystemMoveEvent) {
-      return "move $type $path to "
-          "${p.relative(event.destination, from: directory)}";
-    }
-  }
-}
+// Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// TODO(rnystrom): Merge with mac_os version.
+
+library watcher.directory_watcher.windows;
+
+import 'dart:async';
+import 'dart:collection';
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:stack_trace/stack_trace.dart';
+
+import '../constructable_file_system_event.dart';
+import '../path_set.dart';
+import '../utils.dart';
+import '../watch_event.dart';
+import 'resubscribable.dart';
+
+class WindowsDirectoryWatcher extends ResubscribableDirectoryWatcher {
+  WindowsDirectoryWatcher(String directory)
+      : super(directory, () => new _WindowsDirectoryWatcher(directory));
+}
+
+class _EventBatcher {
+  static const Duration _BATCH_DELAY = const Duration(milliseconds: 100);
+  final List<FileSystemEvent> events = [];
+  Timer timer;
+
+  void addEvent(FileSystemEvent event) {
+    events.add(event);
+  }
+
+  void startTimer(void callback()) {
+    if (timer != null) {
+      timer.cancel();
+    }
+    timer = new Timer(_BATCH_DELAY, callback);
+  }

[Bob Nystrom, 2014/06/04 15:48:29]

Let's combine these two methods. You never want to add an event without
starting/resetting the timer.

[Anders Johnsen, 2014/06/10 06:47:14]

Done.

+
+  void cancelTimer() {
+    timer.cancel();
+  }
+}
+
+class _WindowsDirectoryWatcher implements ManuallyClosedDirectoryWatcher {
+  final String directory;
+
+  Stream<WatchEvent> get events => _eventsController.stream;
+  final _eventsController = new StreamController<WatchEvent>.broadcast();
+
+  bool get isReady => _readyCompleter.isCompleted;
+
+  Future get ready => _readyCompleter.future;
+  final _readyCompleter = new Completer();
+
+  final Map<String, _EventBatcher> _eventBatchers =
+      new HashMap<String, _EventBatcher>();
+
+  /// The set of files that are known to exist recursively within the watched
+  /// directory.
+  ///
+  /// The state of files on the filesystem is compared against this to determine
+  /// the real change that occurred. This is also used to emit REMOVE events
+  /// when subdirectories are moved out of the watched directory.
+  final PathSet _files;
+
+  /// The subscription to the stream returned by [Directory.watch].
+  StreamSubscription<FileSystemEvent> _watchSubscription;
+
+  /// The subscription to the stream returned by [Directory.watch] of the
+  /// parent directory to [directory]. This is needed to detect changes to
+  /// [directory], as they are not included on Windows.
+  StreamSubscription<FileSystemEvent> _parentWatchSubscription;
+
+  /// The subscription to the [Directory.list] call for the initial listing of
+  /// the directory to determine its initial state.
+  StreamSubscription<FileSystemEntity> _initialListSubscription;
+
+  /// The subscriptions to the [Directory.list] call for listing the contents of

[Bob Nystrom, 2014/06/04 15:48:29]

"call" -> "calls".

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+  /// subdirectories that was moved into the watched directory.

[Bob Nystrom, 2014/06/04 15:48:29]

"was" -> "were".

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+  final Set<StreamSubscription<FileSystemEntity>> _listSubscriptions
+      = new HashSet<StreamSubscription<FileSystemEntity>>();
+
+  _WindowsDirectoryWatcher(String directory)
+      : directory = directory, _files = new PathSet(directory) {
+    _startWatch();
+    _startParentWatcher();
+
+    // Before we're ready to emit events, wait for [_listDir] to complete.
+    _listDir().then(_readyCompleter.complete);
+  }
+
+  void close() {
+    if (_watchSubscription != null) _watchSubscription.cancel();
+    if (_parentWatchSubscription != null) _parentWatchSubscription.cancel();
+    if (_initialListSubscription != null) _initialListSubscription.cancel();
+    for (var sub in _listSubscriptions) {
+      sub.cancel();
+    }
+    _listSubscriptions.clear();
+    for (var batcher in _eventBatchers.values) {
+      batcher.cancelTimer();
+    }
+    _eventBatchers.clear();
+    _watchSubscription = null;
+    _parentWatchSubscription = null;
+    _initialListSubscription = null;
+    _eventsController.close();
+  }
+
+  /// On Windows, if [directory] is deleted, we will not receive any event.

[Bob Nystrom, 2014/06/04 15:48:29]

Blank "///" line after this.

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+  /// Instead, we add a watcher on the parent folder (if any), that can notify
+  /// us about [directory].
+  /// This also includes events such as moves.

[Bob Nystrom, 2014/06/04 15:48:29]

This can be part of the previous paragraph.

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+  void _startParentWatcher() {
+    var absoluteDir = p.absolute(directory);
+    var parent = p.dirname(absoluteDir);
+    // Check if we [directory] is already the root directory.

[Bob Nystrom, 2014/06/04 15:48:29]

Remove "we".

[Anders Johnsen, 2014/06/10 06:47:14]

Done.

+    if (FileSystemEntity.identicalSync(parent, directory)) return;
+    var parentStream = Chain.track(
+        new Directory(parent).watch(recursive: false));
+    _parentWatchSubscription = parentStream.listen((event) {
+      // Only look at events for 'directory'.
+      if (p.basename(event.path) != p.basename(absoluteDir)) return;
+      // Test if the directory is removed. FileSystemEntity.typeSync will
+      // return NOT_FOUND if it's unable to decide upon the type, including
+      // access denied issues, which may happen when the directory is deleted.
+      // FileSystemMoveEvent and FileSystemDeleteEvent events will always mean
+      // the directory is now gone.
+      if (event is FileSystemMoveEvent ||
+          event is FileSystemDeleteEvent ||
+          (FileSystemEntity.typeSync(directory) ==
+           FileSystemEntityType.NOT_FOUND)) {
+        for (var path in _files.toSet()) {

[Bob Nystrom, 2014/06/04 15:48:29]

The .toSet() call here could be expensive. Is it needed?

[Anders Johnsen, 2014/06/10 06:47:15]

This is needed for some reason.

+          _emitEvent(ChangeType.REMOVE, path);
+        }
+        _files.clear();
+        close();
+      }
+    }, onError: (error) {
+      // Ignore errors, simply close the stream.

[Bob Nystrom, 2014/06/04 15:48:29]

You answered in the code review, but can you leave a comment here explaining why
the error can be ignored?

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+      _parentWatchSubscription.cancel();
+      _parentWatchSubscription = null;
+    });
+  }
+
+  void _onEvent(FileSystemEvent event) {
+    // If we get a event before we're ready to begin emitting events,
+    // ignore those events and re-list the directory.

[Bob Nystrom, 2014/06/04 15:48:29]

Why not just defer calling _startWatch() until after the listing is done?

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+    if (!isReady) {
+      _listDir().then((_) {
+       _readyCompleter.complete();
+      });
+      return;
+    }
+
+    _EventBatcher batcher = _eventBatchers.putIfAbsent(

[Bob Nystrom, 2014/06/04 15:48:29]

Use var here.

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+        event.path, () => new _EventBatcher());
+    batcher.addEvent(event);
+    batcher.startTimer(() {
+      _eventBatchers.remove(event.path);
+      _onBatch(batcher.events);
+    });
+  }
+
+  /// The callback that's run when [Directory.watch] emits a batch of events.
+  void _onBatch(List<FileSystemEvent> batch) {
+    _sortEvents(batch).forEach((path, events) {
+      var relativePath = p.relative(path, from: directory);
+
+      var canonicalEvent = _canonicalEvent(events);
+      events = canonicalEvent == null ?
+          _eventsBasedOnFileSystem(path) : [canonicalEvent];
+
+      for (var event in events) {
+        if (event is FileSystemCreateEvent) {
+          if (!event.isDirectory) {
+            if (_files.contains(path)) continue;
+
+            _emitEvent(ChangeType.ADD, path);
+            _files.add(path);
+            continue;
+          }
+
+          if (_files.containsDir(path)) continue;
+
+          var stream = Chain.track(new Directory(path).list(recursive: true));
+          var sub;
+          sub = stream.listen((entity) {
+            if (entity is Directory) return;
+            if (_files.contains(path)) return;
+
+            _emitEvent(ChangeType.ADD, entity.path);
+            _files.add(entity.path);
+          }, onDone: () {
+            _listSubscriptions.remove(sub);
+          }, onError: (e, stackTrace) {
+            _listSubscriptions.remove(sub);
+            _emitError(e, stackTrace);
+          }, cancelOnError: true);
+          _listSubscriptions.add(sub);
+        } else if (event is FileSystemModifyEvent) {
+          if (!event.isDirectory) {
+            _emitEvent(ChangeType.MODIFY, path);
+          }
+        } else {
+          assert(event is FileSystemDeleteEvent);
+          for (var removedPath in _files.remove(path)) {
+            _emitEvent(ChangeType.REMOVE, removedPath);
+          }
+        }
+      }
+    });
+  }
+
+  /// Sort all the events in a batch into sets based on their path.
+  ///
+  /// A single input event may result in multiple events in the returned map;
+  /// for example, a MOVE event becomes a DELETE event for the source and a
+  /// CREATE event for the destination.
+  ///
+  /// The returned events won't contain any [FileSystemMoveEvent]s, nor will it
+  /// contain any events relating to [directory].
+  Map<String, Set<FileSystemEvent>> _sortEvents(List<FileSystemEvent> batch) {
+    var eventsForPaths = {};
+
+    // Events within directories that already have events are superfluous; the
+    // directory's full contents will be examined anyway, so we ignore such
+    // events. Emitting them could cause useless or out-of-order events.
+    var directories = unionAll(batch.map((event) {
+      if (!event.isDirectory) return new Set();
+      if (event is! FileSystemMoveEvent) return new Set.from([event.path]);
+      return new Set.from([event.path, event.destination]);
+    }));
+
+    isInModifiedDirectory(path) =>
+        directories.any((dir) => path != dir && path.startsWith(dir));
+
+    addEvent(path, event) {
+      if (isInModifiedDirectory(path)) return;
+      var set = eventsForPaths.putIfAbsent(path, () => new Set());
+      set.add(event);
+    }
+
+    for (var event in batch) {
+      if (event is FileSystemMoveEvent) {
+        FileSystemMoveEvent moveEvent = event;

[Bob Nystrom, 2014/06/04 15:48:29]

Ditch this line.

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+        addEvent(moveEvent.destination, event);
+      }
+      addEvent(event.path, event);
+    }
+
+    return eventsForPaths;
+  }
+
+  /// Returns the canonical event from a batch of events on the same path, if
+  /// one exists.
+  ///
+  /// If [batch] doesn't contain any contradictory events (e.g. DELETE and
+  /// CREATE, or events with different values for [isDirectory]), this returns a
+  /// single event that describes what happened to the path in question.
+  ///
+  /// If [batch] does contain contradictory events, this returns `null` to
+  /// indicate that the state of the path on the filesystem should be checked to
+  /// determine what occurred.
+  FileSystemEvent _canonicalEvent(Set<FileSystemEvent> batch) {
+    // An empty batch indicates that we've learned earlier that the batch is
+    // contradictory (e.g. because of a move).
+    if (batch.isEmpty) return null;
+
+    var type = batch.first.type;
+    var isDir = batch.first.isDirectory;
+    var hadModifyEvent = false;
+
+    for (var event in batch.skip(1)) {
+      // If one event reports that the file is a directory and another event
+      // doesn't, that's a contradiction.
+      if (isDir != event.isDirectory) return null;
+
+      // Modify events don't contradict either CREATE or REMOVE events. We can
+      // safely assume the file was modified after a CREATE or before the
+      // REMOVE; otherwise there will also be a REMOVE or CREATE event
+      // (respectively) that will be contradictory.
+      if (event is FileSystemModifyEvent) {
+        hadModifyEvent = true;
+        continue;
+      }
+      assert(event is FileSystemCreateEvent ||
+             event is FileSystemDeleteEvent ||
+             event is FileSystemMoveEvent);
+
+      // If we previously thought this was a MODIFY, we now consider it to be a
+      // CREATE or REMOVE event. This is safe for the same reason as above.
+      if (type == FileSystemEvent.MODIFY) {
+        type = event.type;
+        continue;
+      }
+
+      // A CREATE event contradicts a REMOVE event and vice versa.
+      assert(type == FileSystemEvent.CREATE ||
+             type == FileSystemEvent.DELETE ||
+             type == FileSystemEvent.MOVE);
+      if (type != event.type) return null;
+    }
+
+    switch (type) {
+      case FileSystemEvent.CREATE:
+        return new ConstructableFileSystemCreateEvent(batch.first.path, isDir);
+      case FileSystemEvent.DELETE:
+        return new ConstructableFileSystemDeleteEvent(batch.first.path, isDir);
+      case FileSystemEvent.MODIFY:
+        return new ConstructableFileSystemModifyEvent(
+            batch.first.path, isDir, false);
+      case FileSystemEvent.MOVE:
+        return null;
+      default: assert(false);
+    }
+  }
+
+  /// Returns one or more events that describe the change between the last known
+  /// state of [path] and its current state on the filesystem.
+  ///
+  /// This returns a list whose order should be reflected in the events emitted
+  /// to the user, unlike the batched events from [Directory.watch]. The
+  /// returned list may be empty, indicating that no changes occurred to [path]
+  /// (probably indicating that it was created and then immediately deleted).
+  List<FileSystemEvent> _eventsBasedOnFileSystem(String path) {
+    var fileExisted = _files.contains(path);
+    var dirExisted = _files.containsDir(path);
+    var fileExists = new File(path).existsSync();
+    var dirExists = new Directory(path).existsSync();
+
+    var events = [];
+    if (fileExisted) {
+      if (fileExists) {
+        events.add(new ConstructableFileSystemModifyEvent(path, false, false));
+      } else {
+        events.add(new ConstructableFileSystemDeleteEvent(path, false));
+      }
+    } else if (dirExisted) {
+      if (dirExists) {
+        // If we got contradictory events for a directory that used to exist and
+        // still exists, we need to rescan the whole thing in case it was
+        // replaced with a different directory.
+        events.add(new ConstructableFileSystemDeleteEvent(path, true));
+        events.add(new ConstructableFileSystemCreateEvent(path, true));
+      } else {
+        events.add(new ConstructableFileSystemDeleteEvent(path, true));
+      }
+    }
+
+    if (!fileExisted && fileExists) {
+      events.add(new ConstructableFileSystemCreateEvent(path, false));
+    } else if (!dirExisted && dirExists) {
+      events.add(new ConstructableFileSystemCreateEvent(path, true));
+    }
+
+    return events;
+  }
+
+  /// The callback that's run when the [Directory.watch] stream is closed.
+  /// Note that this is unlikely to happen on Windows, unless the system itself
+  /// closes the handle.
+  void _onDone() {
+    _watchSubscription = null;
+
+    // Emit remove-events for any remaining files.

[Bob Nystrom, 2014/06/04 15:48:29]

Remove the "-".

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+    for (var file in _files.toSet()) {
+      _emitEvent(ChangeType.REMOVE, file);
+    }
+    _files.clear();
+    close();
+  }
+
+  /// Start or restart the underlying [Directory.watch] stream.
+  void _startWatch() {
+    // Batch the events changes together so that we can dedup events.

[Bob Nystrom, 2014/06/04 15:48:29]

Remove "changes".

[Anders Johnsen, 2014/06/10 06:47:15]

Done.

+    var innerStream =
+        Chain.track(new Directory(directory).watch(recursive: true));
+    _watchSubscription = innerStream.listen(_onEvent,
+        onError: _eventsController.addError,
+        onDone: _onDone);
+  }
+
+  /// Starts or restarts listing the watched directory to get an initial picture
+  /// of its state.
+  Future _listDir() {
+    assert(!isReady);
+    if (_initialListSubscription != null) _initialListSubscription.cancel();
+
+    _files.clear();
+    var completer = new Completer();
+    var stream = Chain.track(new Directory(directory).list(recursive: true));
+    void handleEntity(entity) {
+      if (entity is! Directory) _files.add(entity.path);
+    }
+    _initialListSubscription = stream.listen(
+        handleEntity,
+        onError: _emitError,
+        onDone: completer.complete,
+        cancelOnError: true);
+    return completer.future;
+  }
+
+  /// Emit an event with the given [type] and [path].
+  void _emitEvent(ChangeType type, String path) {
+    if (!isReady) return;
+
+    _eventsController.add(new WatchEvent(type, path));
+  }
+
+  /// Emit an error, then close the watcher.
+  void _emitError(error, StackTrace stackTrace) {
+    _eventsController.addError(error, stackTrace);
+    close();
+  }
+}