Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Commit

Permalink
Keep dirty manipulations private to Element base class (#109401)
Browse files Browse the repository at this point in the history
  • Loading branch information
goderbauer committed Aug 12, 2022
1 parent 28de3d4 commit a624cb7
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 38 deletions.
Expand Up @@ -1016,9 +1016,6 @@ class _NullElement extends Element {

@override
bool get debugDoingBuild => throw UnimplementedError();

@override
void performRebuild() { }
}

class _NullWidget extends Widget {
Expand Down
42 changes: 23 additions & 19 deletions packages/flutter/lib/src/widgets/framework.dart
Expand Up @@ -4507,6 +4507,10 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
}

/// Returns true if the element has been marked as needing rebuilding.
///
/// The flag is true when the element is first created and after
/// [markNeedsBuild] has been called. The flag is reset to false in the
/// [performRebuild] implementation.
bool get dirty => _dirty;
bool _dirty = true;

Expand Down Expand Up @@ -4580,10 +4584,14 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
/// Called by the [BuildOwner] when [BuildOwner.scheduleBuildFor] has been
/// called to mark this element dirty, by [mount] when the element is first
/// built, and by [update] when the widget has changed.
///
/// The method will only rebuild if [dirty] is true. To rebuild irregardless
/// of the [dirty] flag, set `force` to true. Forcing a rebuild is convenient
/// from [update], during which [dirty] is false.
@pragma('vm:prefer-inline')
void rebuild() {
void rebuild({bool force = false}) {
assert(_lifecycleState != _ElementLifecycle.initial);
if (_lifecycleState != _ElementLifecycle.active || !_dirty) {
if (_lifecycleState != _ElementLifecycle.active || (!_dirty && !force)) {
return;
}
assert(() {
Expand Down Expand Up @@ -4618,8 +4626,13 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
/// Cause the widget to update itself.
///
/// Called by [rebuild] after the appropriate checks have been made.
///
/// The base implementation only clears the [dirty] flag.
@protected
void performRebuild();
@mustCallSuper
void performRebuild() {
_dirty = false;
}
}

class _ElementDiagnosticableTreeNode extends DiagnosticableTreeNode {
Expand Down Expand Up @@ -4901,7 +4914,7 @@ abstract class ComponentElement extends Element {
} finally {
// We delay marking the element as clean until after calling build() so
// that attempts to markNeedsBuild() during build() will be ignored.
_dirty = false;
super.performRebuild(); // clears the "dirty" flag
}
try {
_child = updateChild(_child, built, slot);
Expand Down Expand Up @@ -4955,8 +4968,7 @@ class StatelessElement extends ComponentElement {
void update(StatelessWidget newWidget) {
super.update(newWidget);
assert(widget == newWidget);
_dirty = true;
rebuild();
rebuild(force: true);
}
}

Expand Down Expand Up @@ -5053,10 +5065,6 @@ class StatefulElement extends ComponentElement {
super.update(newWidget);
assert(widget == newWidget);
final StatefulWidget oldWidget = state._widget!;
// We mark ourselves as dirty before calling didUpdateWidget to
// let authors call setState from within didUpdateWidget without triggering
// asserts.
_dirty = true;
state._widget = widget as StatefulWidget;
final Object? debugCheckForReturnedFuture = state.didUpdateWidget(oldWidget) as dynamic;
assert(() {
Expand All @@ -5072,7 +5080,7 @@ class StatefulElement extends ComponentElement {
}
return true;
}());
rebuild();
rebuild(force: true);
}

@override
Expand Down Expand Up @@ -5217,8 +5225,7 @@ abstract class ProxyElement extends ComponentElement {
super.update(newWidget);
assert(widget == newWidget);
updated(oldWidget);
_dirty = true;
rebuild();
rebuild(force: true);
}

/// Called during build when the [widget] has changed.
Expand Down Expand Up @@ -5746,7 +5753,7 @@ abstract class RenderObjectElement extends Element {
}());
assert(_slot == newSlot);
attachRenderObject(newSlot);
_dirty = false;
super.performRebuild(); // clears the "dirty" flag
}

@override
Expand All @@ -5768,7 +5775,7 @@ abstract class RenderObjectElement extends Element {
}

@override
void performRebuild() {
void performRebuild() { // ignore: must_call_super, _performRebuild calls super.
_performRebuild(); // calls widget.updateRenderObject()
}

Expand All @@ -5783,7 +5790,7 @@ abstract class RenderObjectElement extends Element {
_debugDoingBuild = false;
return true;
}());
_dirty = false;
super.performRebuild(); // clears the "dirty" flag
}

/// Updates the children of this element to use new widgets.
Expand Down Expand Up @@ -6539,9 +6546,6 @@ class _NullElement extends Element {

@override
bool get debugDoingBuild => throw UnimplementedError();

@override
void performRebuild() => throw UnimplementedError();
}

class _NullWidget extends Widget {
Expand Down
3 changes: 0 additions & 3 deletions packages/flutter/test/cupertino/colors_test.dart
Expand Up @@ -591,9 +591,6 @@ class _NullElement extends Element {

@override
bool get debugDoingBuild => throw UnimplementedError();

@override
void performRebuild() { }
}

class _NullWidget extends Widget {
Expand Down
5 changes: 0 additions & 5 deletions packages/flutter/test/foundation/diagnostics_json_test.dart
Expand Up @@ -220,11 +220,6 @@ void main() {
class _TestElement extends Element {
_TestElement() : super(const Placeholder());

@override
void performRebuild() {
// Intentionally left empty.
}

@override
bool get debugDoingBuild => throw UnimplementedError();
}
Expand Down
70 changes: 70 additions & 0 deletions packages/flutter/test/widgets/did_update_widget_test.dart
@@ -0,0 +1,70 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
testWidgets('Can call setState from didUpdateWidget', (WidgetTester tester) async {
await tester.pumpWidget(const Directionality(
textDirection: TextDirection.ltr,
child: WidgetUnderTest(text: 'hello'),
));

expect(find.text('hello'), findsOneWidget);
expect(find.text('world'), findsNothing);
final _WidgetUnderTestState state = tester.state<_WidgetUnderTestState>(find.byType(WidgetUnderTest));
expect(state.setStateCalled, 0);
expect(state.didUpdateWidgetCalled, 0);

await tester.pumpWidget(const Directionality(
textDirection: TextDirection.ltr,
child: WidgetUnderTest(text: 'world'),
));

expect(find.text('world'), findsOneWidget);
expect(find.text('hello'), findsNothing);
expect(state.setStateCalled, 1);
expect(state.didUpdateWidgetCalled, 1);
});
}

class WidgetUnderTest extends StatefulWidget {
const WidgetUnderTest({super.key, required this.text});

final String text;

@override
State<WidgetUnderTest> createState() => _WidgetUnderTestState();
}

class _WidgetUnderTestState extends State<WidgetUnderTest> {
late String text = widget.text;

int setStateCalled = 0;
int didUpdateWidgetCalled = 0;

@override
void didUpdateWidget(WidgetUnderTest oldWidget) {
super.didUpdateWidget(oldWidget);
didUpdateWidgetCalled += 1;
if (oldWidget.text != widget.text) {
// This setState is load bearing for the test.
setState(() {
text = widget.text;
});
}
}

@override
void setState(VoidCallback fn) {
super.setState(fn);
setStateCalled += 1;
}

@override
Widget build(BuildContext context) {
return Text(text);
}
}
10 changes: 2 additions & 8 deletions packages/flutter/test/widgets/framework_test.dart
Expand Up @@ -1861,9 +1861,6 @@ class DirtyElementWithCustomBuildOwner extends Element {

final BuildOwner _owner;

@override
void performRebuild() {}

@override
BuildOwner get owner => _owner;

Expand Down Expand Up @@ -1968,9 +1965,9 @@ class StatefulElementSpy extends StatefulElement {
_Stateful get _statefulWidget => widget as _Stateful;

@override
void rebuild() {
void rebuild({bool force = false}) {
_statefulWidget.onElementRebuild?.call(this);
super.rebuild();
super.rebuild(force: force);
}
}

Expand Down Expand Up @@ -2115,9 +2112,6 @@ class _EmptyElement extends Element {

@override
bool get debugDoingBuild => false;

@override
void performRebuild() {}
}

class _TestLeaderLayerWidget extends SingleChildRenderObjectWidget {
Expand Down

0 comments on commit a624cb7

Please sign in to comment.