Implement measurement using futures

client/layout/GridLayout.dart

Index: client/layout/GridLayout.dart
diff --git a/client/layout/GridLayout.dart b/client/layout/GridLayout.dart
index 872f53e941b0add64155d8991355ecd561d18dcb..a66766b6e8cddc9f0e0bccc117a21c9fbe9f9e46 100644
--- a/client/layout/GridLayout.dart
+++ b/client/layout/GridLayout.dart
@@ -85,23 +85,30 @@ class GridLayout extends ViewLayout {
     _columnTracks = columns != null ? columns.tracks : new List<GridTrack>();
   }
 
+
   int get currentWidth() => _gridWidth;
   int get currentHeight() => _gridHeight;
 
-  /** The main entry point for layout computation. */
-  bool measureLayout(int width, int height) {
-    _gridWidth = width;
-    _gridHeight = height;
+  void cacheExistingBrowserLayout() {
+    // We don't need to do anything as we don't rely on the _cachedViewRect
+    // when the grid layout is used.
+  }
 
+  /** The main entry point for layout computation. */
+  void measureLayout(Future<Size> size, Completer<bool> changed) {

[nweiz, 2011/10/28 03:58:38]

Passing in a Completer seems wrong... isn't the pattern for these sorts of
things to return a Future instead?

[Jacob, 2011/10/31 22:09:50]

I agree this is a bit odd looking.
However, for this case, returning futures would be much much uglier as you end
up having non-leaf views listening on the futures returned by each of their
subviews and then aggregating that into a new future.
This way things are far simpler and more efficient as we only care that at least
one of the leaf views set complete to true.

[nweiz, 2011/11/01 00:49:22]

I would argue that the correct pattern here is to return a future, then have a
library function that aggregates futures in the way you describe (it takes a
list of futures and returns a new future that will fire once any of the input
futures fire). That way this function has a type signature that matches its
semantics, and doesn't build in knowledge of the surrounding code structure.

Another thing: this function never returns false, so the bool value seems
out-of-place.

[Jacob, 2011/11/01 02:42:39]

Btw, the bool value is required as the completer is set to false if it was not
set to true before the next call to requestLayoutFrame. However, this suggests
the code isn't very readable as written. I've added a TODO to simplify the
logic.

     _ensureAllTracks();
-
-    if (_rowTracks.length < 1 || _columnTracks.length < 1) {
-      return false; // nothing to do
-    }
-
-    _measureTracks();
-    _setBoundsOfChildren();
-    return true;
+    window.requestLayoutFrame(() {
+      _gridWidth = size.value.width;
+      _gridHeight = size.value.height;
+
+      if (_rowTracks.length > 0 && _columnTracks.length > 0) {
+        _measureTracks();
+        _setBoundsOfChildren();
+        if (null != changed) {
+          changed.complete(true);
+        }
+      }
+    });
   }
 
   /**
@@ -418,6 +425,7 @@ class GridLayout extends ViewLayout {
         _ensureTrack(_columnTracks, columnSizing, p.column, p.columnSpan);
         child.layoutParams = p;
       }
+      child.cacheExistingBrowserLayout();
     }
   }