Change call-sites in trace viewer to use generators instead of iteration functions.

tracing/tracing/model/event_container.html

Index: tracing/tracing/model/event_container.html
diff --git a/tracing/tracing/model/event_container.html b/tracing/tracing/model/event_container.html
index b9baa16bccde789eb0e874f318dfe74ab2bf8729..856933bf3b0133ecf7d74a75afd1ff85d37faafd 100644
--- a/tracing/tracing/model/event_container.html
+++ b/tracing/tracing/model/event_container.html
@@ -53,7 +53,7 @@ tr.exportTo('tr.model', function() {
     },
 
     // TODO(charliea): A default implementation of this method could likely be
-    // provided that uses 'iterateAllEvents'.
+    // provided that iterates throuch descendantEvents.
     /**
      * Updates the bounds of the event container. After updating, this.bounds
      * will describe the range of timestamps of all ancestor events.
@@ -63,7 +63,7 @@ tr.exportTo('tr.model', function() {
     },
 
     // TODO(charliea): A default implementation of this method could likely be
-    // provided that uses 'iterateAllEvents'.
+    // provided that iterates through descendantEvents.
     /**
      * Shifts the timestamps for ancestor events by 'amount' milliseconds.
      */
@@ -71,75 +71,29 @@ tr.exportTo('tr.model', function() {
       throw new Error('Not implemented');
     },
 
-    /**
-     * Iterates over all child events.
-     */
-    iterateAllEventsInThisContainer: function(eventTypePredicate,
-                                              callback, opt_this) {
-      // TODO(alexandermont): We need this.childEvents() to take an
-      // eventTypePredicate because it doesn't look possible to directly get the
-      // type object from an object, so we have to do the filtering before
-      // the iteration. Probably it would be better if we find a workaround
-      // for this.
-      for (var event of this.childEvents(eventTypePredicate, opt_this))
-        callback.call(opt_this, event);
-    },
-
-    /**
-     * Iterates over all child containers.
+     /**
+     * Gets list of all events in this and descendant event containers.

[charliea (OOO until 10-5), 2016/06/29 18:08:44]

s/Gets list/Returns an iterable

[alexandermont, 2016/06/29 20:47:11]

Done

      */
-    iterateAllChildEventContainers: function(callback, opt_this) {
+    descendantEvents: function*() {

[charliea (OOO until 10-5), 2016/06/29 18:08:44]

Is there no default implementation of childEvents and childEventContainers?
There probably should be (that just does nothing)

[charliea (OOO until 10-5), 2016/06/29 18:08:44]

I think that this should be getDescendantEvents() rather than
descendantEvents(). (Hopefully this isn't too hard to fix with a simple search
and replace)

I think that, ideally, we'd be able to make this a getter so we could do:

get* descendantEvents() {
  yield *this.childEvents;
  ... 
}

for (var event of this.descendantEvents)

... but apparently JS doesn't have good support for ES5 getters/setters +
generator functions yet :-(

[alexandermont, 2016/06/29 20:47:11]

Added default implementation

+      yield * this.childEvents();
       for (var container of this.childEventContainers())
-        callback.call(opt_this, container);
+        yield * container.descendantEvents();
     },
 
     /**
-     * Iterates over all events in this container and in descendant
-     * event containers.
+     * Yields this and all descendant event containers.

[charliea (OOO until 10-5), 2016/06/29 18:08:44]

s/Yields/Returns an iterable containing

[alexandermont, 2016/06/29 20:47:11]

Done

      */
-    iterateAllEvents: function(callback, opt_this) {
-      // If new-style iteration has been implemented for this container, use
-      // it for iterateAllEvents.
-      if (this.childEvents && this.childEventContainers) {
-        for (var event of this.childEvents(x => true, opt_this))
-          callback.call(opt_this, event);
-        for (var container of this.childEventContainers())
-          container.iterateAllEvents(callback, opt_this);
-      } else {
-        // Otherwise, just do it the old way.
-        this.iterateAllEventContainers(function(ec) {
-          ec.iterateAllEventsInThisContainer(x => true, callback, opt_this);
-        });
-      }
-    },
-
-    /**
-     * Iterates over this container and all descendant containers.
-     */
-    iterateAllEventContainers: function(callback, opt_this) {
-      // If new-style iteration has been implemented for this container, use
-      // it for iterateAllEventContainers.
-      if (this.childEvents && this.childEventContainers) {
-        callback.call(opt_this, this);
-        for (var container of this.childEventContainers())
-          container.iterateAllEventContainers(callback, opt_this);
-      } else {
-        // Otherwise, just do it the old way.
-        function visit(ec) {
-          callback.call(opt_this, ec);
-          ec.iterateAllChildEventContainers(visit);
-        }
-        visit(this);
-      }
+    descendantEventContainers: function*() {

[charliea (OOO until 10-5), 2016/06/29 18:08:44]

same here (getDescendantEventContainers instead of descendantEventContainers)

+      yield this;
+      for (var container of this.childEventContainers())
+        yield * container.descendantEventContainers();
     },
 
     /**
      * Finds topmost slices in this container (see docstring for
      * findTopmostSlices).
      */
-    findTopmostSlicesInThisContainer: function(eventPredicate, callback,
-                                               opt_this) {
-      throw new Error('Not implemented.');

[charliea (OOO until 10-5), 2016/06/29 18:08:44]

Did you mean to delete the not implemented error?

[alexandermont, 2016/06/29 20:47:11]

Yes. The blank function is now the default behavior, enabling is to remove other
copies of the blank function from elsewhere in the code.

+    findTopmostSlicesInThisContainer: function*(eventPredicate, opt_this) {
     },
 
     /**
@@ -156,14 +110,13 @@ tr.exportTo('tr.model', function() {
      * the  left is not the topmost C, and the right one is, even though it is
      * not itself a top-level slice.
      */
-    findTopmostSlices: function(eventPredicate, callback, opt_this) {
-      this.iterateAllEventContainers(function(ec) {
-        ec.findTopmostSlicesInThisContainer(eventPredicate, callback, opt_this);
-      });
+    findTopmostSlices: function*(eventPredicate) {
+      for (var ec of this.descendantEventContainers())
+        yield * ec.findTopmostSlicesInThisContainer(eventPredicate);
     },
 
-    findTopmostSlicesNamed: function(name, callback, opt_this) {
-      this.findTopmostSlices(e => e.title === name, callback, opt_this);
+    findTopmostSlicesNamed: function*(name) {
+      yield * this.findTopmostSlices(e => e.title === name);
     }
   };