Refactor Effen to make explicit the distinction between render & non-render nodes.

Refactor Effen to make explicit the distinction between render & non-render nodes.

All Effen which can directly create a sky.Node are now derived from RenderNode. In contrast, Component now derives from the (now) base Node class which simply represents a position in the Effen hierarchy.

A fair amount of clean-up & refactoring went into this change, simplifying & unifying the "sync" logic between Element & Component as well as making the Component.didMount/didUnmount signals async WRT component building.

BUG=
R=abarth@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/9ba1c916702e3e5b53706e1fa67281534c21081e

[rafaelw, 2015-03-18 19:00:36]

rafaelw@chromium.org changed reviewers:
+ abarth@chromium.org

[abarth-chromium, 2015-03-18 19:26:43]

lgtm

https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart
File sky/framework/fn.dart (right):

https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart#n...
sky/framework/fn.dart:541: _unmountedComponents.clear();
Should we move these collections into local variables before iterating them?

https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart#n...
sky/framework/fn.dart:614: bool _willSync(Node old) {
Can you add a comment to the declaration of _willSync in the base class explain
what it's return value means?

[rafaelw, 2015-03-18 19:59:01]

Committed patchset #5 (id:80001) manually as
9ba1c916702e3e5b53706e1fa67281534c21081e (presubmit successful).

[rafaelw, 2015-03-18 20:00:02]

https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart
File sky/framework/fn.dart (right):

https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart#n...
sky/framework/fn.dart:541: _unmountedComponents.clear();
On 2015/03/18 19:26:43, abarth wrote:
> Should we move these collections into local variables before iterating them?

You can check my math here, but the intent of these collections was to make
building & signalling mount/unmount entirely disjoint. Built cannot be trigged
from mount/unmount and unmount/mount cannot be  fired during build.

Thus, there should be no danger that these collections are mutated while they
are iterating.

https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart#n...
sky/framework/fn.dart:614: bool _willSync(Node old) {
On 2015/03/18 19:26:43, abarth wrote:
> Can you add a comment to the declaration of _willSync in the base class
explain
> what it's return value means?

Done. Also added comment to  _syncChild

[abarth-chromium, 2015-03-18 20:25:57]

On 2015/03/18 at 20:00:02, rafaelw wrote:
> https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart
> File sky/framework/fn.dart (right):
> 
>
https://codereview.chromium.org/1009543008/diff/60001/sky/framework/fn.dart#n...
> sky/framework/fn.dart:541: _unmountedComponents.clear();
> On 2015/03/18 19:26:43, abarth wrote:
> > Should we move these collections into local variables before iterating them?
> 
> You can check my math here, but the intent of these collections was to make
building & signalling mount/unmount entirely disjoint. Built cannot be trigged
from mount/unmount and unmount/mount cannot be  fired during build.
> 
> Thus, there should be no danger that these collections are mutated while they
are iterating.

Can we add an assert to that effect?  I'm just paranoid about this problem
because it always ends up getting me.  :)