From b648abc8d90d84cbd0c8f232278734087610e75f Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Thu, 9 Jul 2015 15:29:36 -0700
Subject: [PATCH] [AutoLayout] Address further concerns raised in
 https://codereview.chromium.org/1230583003

R=ianh@google.com

Review URL: https://codereview.chromium.org/1230033002 .
---
 sdk/example/rendering/simple_autolayout.dart | 10 +++---
 sdk/lib/rendering/auto_layout.dart           | 32 ++++++++++++--------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/sdk/example/rendering/simple_autolayout.dart b/sdk/example/rendering/simple_autolayout.dart
index 3c8d6038a90..23817fc9cf0 100644
--- a/sdk/example/rendering/simple_autolayout.dart
+++ b/sdk/example/rendering/simple_autolayout.dart
@@ -10,23 +10,23 @@ import 'package:sky/rendering/auto_layout.dart';
 import 'package:cassowary/cassowary.dart' as al;
 
 void main() {
-  var c1 = new RenderDecoratedBox(
+  RenderDecoratedBox c1 = new RenderDecoratedBox(
     decoration: new BoxDecoration(backgroundColor: const Color(0xFFFF0000))
   );
 
-  var c2 = new RenderDecoratedBox(
+  RenderDecoratedBox c2 = new RenderDecoratedBox(
     decoration: new BoxDecoration(backgroundColor: const Color(0xFF00FF00))
   );
 
-  var c3 = new RenderDecoratedBox(
+  RenderDecoratedBox c3 = new RenderDecoratedBox(
     decoration: new BoxDecoration(backgroundColor: const Color(0xFF0000FF))
   );
 
-  var c4 = new RenderDecoratedBox(
+  RenderDecoratedBox c4 = new RenderDecoratedBox(
     decoration: new BoxDecoration(backgroundColor: const Color(0xFFFFFFFF))
   );
 
-  var root = new RenderAutoLayout(children: [c1, c2, c3, c4]);
+  RenderAutoLayout root = new RenderAutoLayout(children: [c1, c2, c3, c4]);
 
   AutoLayoutParentData p1 = c1.parentData;
   AutoLayoutParentData p2 = c2.parentData;
diff --git a/sdk/lib/rendering/auto_layout.dart b/sdk/lib/rendering/auto_layout.dart
index d5424ba7f4b..4f30097605c 100644
--- a/sdk/lib/rendering/auto_layout.dart
+++ b/sdk/lib/rendering/auto_layout.dart
@@ -53,7 +53,15 @@ abstract class _AutoLayoutParamMixin {
     solver.suggestValueForVariable(_rightEdge.variable, size.width);
   }
 
+  /// Called when the solver has updated at least one of the layout parameters
+  /// of this object. The object is now responsible for applying this update to
+  /// it other properties (if necessary)
   void _applyAutolayoutParameterUpdates();
+
+  /// Returns the set of implicit constraints that need to be applied to all
+  /// instances of this class when they are moved into a render object with an
+  /// active solver. If no implicit constraints needs to be applied, the object
+  /// may return null.
   List<al.Constraint> _constructImplicitConstraints();
 
   void _setupImplicitConstraints(al.Solver solver) {
@@ -79,17 +87,18 @@ abstract class _AutoLayoutParamMixin {
 
     _implicitConstraints = null;
   }
+
 }
 
 class AutoLayoutParentData extends BoxParentData
     with ContainerParentDataMixin<RenderBox>, _AutoLayoutParamMixin {
 
-  final RenderBox _renderBox;
-
   AutoLayoutParentData(this._renderBox) {
     _setupLayoutParameters(this);
   }
 
+  final RenderBox _renderBox;
+
   @override
   void _applyAutolayoutParameterUpdates() {
     BoxConstraints box = new BoxConstraints.tightFor(
@@ -117,26 +126,23 @@ class RenderAutoLayout extends RenderBox
          RenderBoxContainerDefaultsMixin<RenderBox, AutoLayoutParentData>,
          _AutoLayoutParamMixin {
 
-  final al.Solver _solver = new al.Solver();
-  List<al.Constraint> _explicitConstraints = new List<al.Constraint>();
-
-  RenderAutoLayout({List<RenderBox> children}) {
+  RenderAutoLayout({ List<RenderBox> children }) {
     _setupLayoutParameters(this);
     _setupEditVariablesInSolver(_solver, al.Priority.required - 1);
-
     addAll(children);
   }
 
+  final al.Solver _solver = new al.Solver();
+  List<al.Constraint> _explicitConstraints = new List<al.Constraint>();
+
   /// Adds all the given constraints to the solver. Either all constraints are
   /// added or none
   al.Result addConstraints(List<al.Constraint> constraints) {
     al.Result result = _solver.addConstraints(constraints);
-
     if (result == al.Result.success) {
       markNeedsLayout();
       _explicitConstraints.addAll(constraints);
     }
-
     return result;
   }
 
@@ -166,9 +172,8 @@ class RenderAutoLayout extends RenderBox
 
   @override
   void setupParentData(RenderObject child) {
-    if (child.parentData is! AutoLayoutParentData) {
+    if (child.parentData is! AutoLayoutParentData)
       child.parentData = new AutoLayoutParentData(child);
-    }
   }
 
   @override
@@ -213,13 +218,14 @@ class RenderAutoLayout extends RenderBox
   void dropChild(RenderObject child) {
     child.parentData._collectImplicitConstraints(_solver);
 
-    // Call super last as this collects parent data
     super.dropChild(child);
   }
 
   @override
   List<al.Constraint> _constructImplicitConstraints() {
-    // Only edits are present on layout containers
+    // Only edits variables are present on layout containers. If, in the future,
+    // implicit constraints (for say margins, padding, etc.) need to be added,
+    // they must be returned from here.
     return null;
   }
 }