Commit 8945e011 authored by Hixie's avatar Hixie

Changing themes caused crash

The root cause was that we crawled the tree to mark anyone who depended
on the updated theme dirty _after_ we crawled it to rebuild it. Thus, if
anyone was already marked dirty when the process started, then got
marked clean by the first (rebuild) walk, then got marked dirty again by
the notification, they'd be clean when they got the notification,
despite already being in the dirty list, which would cause an assertion.

Also IconTheme didn't have an operator==, so it was independently too
aggressive about updates.
parent 10b99137
...@@ -7,6 +7,8 @@ import 'dart:collection'; ...@@ -7,6 +7,8 @@ import 'dart:collection';
import 'package:sky/rendering.dart'; import 'package:sky/rendering.dart';
// KEYS
/// A Key is an identifier for [Widget]s and [Element]s. A new Widget will only /// A Key is an identifier for [Widget]s and [Element]s. A new Widget will only
/// be used to reconfigure an existing Element if its Key is the same as its /// be used to reconfigure an existing Element if its Key is the same as its
/// original Widget's Key. /// original Widget's Key.
...@@ -171,6 +173,8 @@ class GlobalObjectKey extends GlobalKey { ...@@ -171,6 +173,8 @@ class GlobalObjectKey extends GlobalKey {
} }
// WIDGETS
/// A Widget object describes the configuration for an [Element]. /// A Widget object describes the configuration for an [Element].
/// Widget subclasses should be immutable with const constructors. /// Widget subclasses should be immutable with const constructors.
/// Widgets form a tree that is then inflated into an Element tree. /// Widgets form a tree that is then inflated into an Element tree.
...@@ -193,6 +197,8 @@ abstract class Widget { ...@@ -193,6 +197,8 @@ abstract class Widget {
void debugFillDescription(List<String> description) { } void debugFillDescription(List<String> description) { }
} }
// TODO(ianh): move the next four classes to below InheritedWidget
/// RenderObjectWidgets provide the configuration for [RenderObjectElement]s, /// RenderObjectWidgets provide the configuration for [RenderObjectElement]s,
/// which wrap [RenderObject]s, which provide the actual rendering of the /// which wrap [RenderObject]s, which provide the actual rendering of the
/// application. /// application.
...@@ -382,16 +388,14 @@ abstract class State<T extends StatefulComponent> { ...@@ -382,16 +388,14 @@ abstract class State<T extends StatefulComponent> {
} }
} }
abstract class ProxyWidget extends StatelessComponent { abstract class ProxyComponent extends Widget {
const ProxyWidget({ Key key, Widget this.child }) : super(key: key); const ProxyComponent({ Key key, this.child }) : super(key: key);
final Widget child; final Widget child;
Widget build(BuildContext context) => child;
} }
abstract class ParentDataWidget extends ProxyWidget { abstract class ParentDataWidget extends ProxyComponent {
ParentDataWidget({ Key key, Widget child }) const ParentDataWidget({ Key key, Widget child })
: super(key: key, child: child); : super(key: key, child: child);
ParentDataElement createElement() => new ParentDataElement(this); ParentDataElement createElement() => new ParentDataElement(this);
...@@ -405,7 +409,7 @@ abstract class ParentDataWidget extends ProxyWidget { ...@@ -405,7 +409,7 @@ abstract class ParentDataWidget extends ProxyWidget {
void applyParentData(RenderObject renderObject); void applyParentData(RenderObject renderObject);
} }
abstract class InheritedWidget extends ProxyWidget { abstract class InheritedWidget extends ProxyComponent {
const InheritedWidget({ Key key, Widget child }) const InheritedWidget({ Key key, Widget child })
: super(key: key, child: child); : super(key: key, child: child);
...@@ -414,6 +418,9 @@ abstract class InheritedWidget extends ProxyWidget { ...@@ -414,6 +418,9 @@ abstract class InheritedWidget extends ProxyWidget {
bool updateShouldNotify(InheritedWidget oldWidget); bool updateShouldNotify(InheritedWidget oldWidget);
} }
// ELEMENTS
bool _canUpdate(Widget oldWidget, Widget newWidget) { bool _canUpdate(Widget oldWidget, Widget newWidget) {
return oldWidget.runtimeType == newWidget.runtimeType && return oldWidget.runtimeType == newWidget.runtimeType &&
oldWidget.key == newWidget.key; oldWidget.key == newWidget.key;
...@@ -637,6 +644,7 @@ abstract class Element<T extends Widget> implements BuildContext { ...@@ -637,6 +644,7 @@ abstract class Element<T extends Widget> implements BuildContext {
assert(_debugLifecycleState == _ElementLifecycle.active); assert(_debugLifecycleState == _ElementLifecycle.active);
assert(widget != null); assert(widget != null);
assert(newWidget != null); assert(newWidget != null);
assert(newWidget != widget);
assert(depth != null); assert(depth != null);
assert(_active); assert(_active);
assert(_canUpdate(widget, newWidget)); assert(_canUpdate(widget, newWidget));
...@@ -970,8 +978,8 @@ abstract class BuildableElement<T extends Widget> extends Element<T> { ...@@ -970,8 +978,8 @@ abstract class BuildableElement<T extends Widget> extends Element<T> {
typedef Widget WidgetBuilder(BuildContext context); typedef Widget WidgetBuilder(BuildContext context);
/// Base class for the instantiation of StatelessComponent and StatefulComponent /// Base class for the instantiation of StatelessComponent, StatefulComponent,
/// widgets. /// and ProxyComponent widgets.
abstract class ComponentElement<T extends Widget> extends BuildableElement<T> { abstract class ComponentElement<T extends Widget> extends BuildableElement<T> {
ComponentElement(T widget) : super(widget); ComponentElement(T widget) : super(widget);
...@@ -1114,7 +1122,26 @@ class StatefulComponentElement<T extends StatefulComponent, U extends State<T>> ...@@ -1114,7 +1122,26 @@ class StatefulComponentElement<T extends StatefulComponent, U extends State<T>>
} }
} }
class ParentDataElement extends StatelessComponentElement<ParentDataWidget> { abstract class ProxyElement<T extends ProxyComponent> extends ComponentElement<T> {
ProxyElement(T widget) : super(widget) {
_builder = (BuildContext context) => this.widget.child;
}
void update(T newWidget) {
T oldWidget = widget;
assert(widget != null);
assert(widget != newWidget);
super.update(newWidget);
assert(widget == newWidget);
notifyDescendants(oldWidget);
_dirty = true;
rebuild();
}
void notifyDescendants(T oldWidget);
}
class ParentDataElement extends ProxyElement<ParentDataWidget> {
ParentDataElement(ParentDataWidget widget) : super(widget); ParentDataElement(ParentDataWidget widget) : super(widget);
void mount(Element parent, dynamic slot) { void mount(Element parent, dynamic slot) {
...@@ -1134,15 +1161,7 @@ class ParentDataElement extends StatelessComponentElement<ParentDataWidget> { ...@@ -1134,15 +1161,7 @@ class ParentDataElement extends StatelessComponentElement<ParentDataWidget> {
super.mount(parent, slot); super.mount(parent, slot);
} }
void update(ParentDataWidget newWidget) { void notifyDescendants(ParentDataWidget oldWidget) {
ParentDataWidget oldWidget = widget;
super.update(newWidget);
assert(widget == newWidget);
if (widget != oldWidget)
_notifyDescendants();
}
void _notifyDescendants() {
void notifyChildren(Element child) { void notifyChildren(Element child) {
if (child is RenderObjectElement) if (child is RenderObjectElement)
child.updateParentData(widget); child.updateParentData(widget);
...@@ -1153,20 +1172,14 @@ class ParentDataElement extends StatelessComponentElement<ParentDataWidget> { ...@@ -1153,20 +1172,14 @@ class ParentDataElement extends StatelessComponentElement<ParentDataWidget> {
} }
} }
class InheritedElement extends StatelessComponentElement<InheritedWidget> {
InheritedElement(InheritedWidget widget) : super(widget);
void update(StatelessComponent newWidget) {
InheritedWidget oldWidget = widget;
super.update(newWidget);
assert(widget == newWidget);
if (widget.updateShouldNotify(oldWidget)) {
assert(widget != oldWidget);
_notifyDescendants();
}
}
void _notifyDescendants() { class InheritedElement extends ProxyElement<InheritedWidget> {
InheritedElement(InheritedWidget widget) : super(widget);
void notifyDescendants(InheritedWidget oldWidget) {
if (!widget.updateShouldNotify(oldWidget))
return;
final Type ourRuntimeType = widget.runtimeType; final Type ourRuntimeType = widget.runtimeType;
void notifyChildren(Element child) { void notifyChildren(Element child) {
if (child._dependencies != null && if (child._dependencies != null &&
......
...@@ -14,6 +14,9 @@ enum IconThemeColor { white, black } ...@@ -14,6 +14,9 @@ enum IconThemeColor { white, black }
class IconThemeData { class IconThemeData {
const IconThemeData({ this.color }); const IconThemeData({ this.color });
final IconThemeColor color; final IconThemeColor color;
bool operator==(other) => other.runtimeType == runtimeType && other.color == color;
int get hashCode => color.hashCode;
} }
class IconTheme extends InheritedWidget { class IconTheme extends InheritedWidget {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment