Unverified Commit c8f07aad authored by Todd Volkert's avatar Todd Volkert Committed by GitHub

Remove duplicate code in Element.rebuild() and BuildOwner.buildScope() (#71940)

Element.rebuild() and BuildOwner.buildScope() both had code to
manager the `BuildOwner._debugCurrentBuildTarget` state variable.
Unfortunately, only one of the two places was using a try/finally
to ensure that the state variable was restored even if an error
was thrown.

This creates a private method to unify the copy/pasted code.
parent a109fe68
...@@ -75,6 +75,7 @@ class PointerRouter { ...@@ -75,6 +75,7 @@ class PointerRouter {
/// ///
/// This is valid in debug builds only. In release builds, this will throw an /// This is valid in debug builds only. In release builds, this will throw an
/// [UnsupportedError]. /// [UnsupportedError].
@visibleForTesting
int get debugGlobalRouteCount { int get debugGlobalRouteCount {
int? count; int? count;
assert(() { assert(() {
......
...@@ -2651,6 +2651,26 @@ class BuildOwner { ...@@ -2651,6 +2651,26 @@ class BuildOwner {
/// Only valid when asserts are enabled. /// Only valid when asserts are enabled.
bool get debugBuilding => _debugBuilding; bool get debugBuilding => _debugBuilding;
bool _debugBuilding = false; bool _debugBuilding = false;
/// The element currently being built, or null if no element is actively
/// being built.
///
/// This is valid in debug builds only. In release builds, this will throw an
/// [UnsupportedError].
@visibleForTesting
Element? get debugCurrentBuildTarget {
Element? result;
bool isSupportedOperation = false;
assert(() {
result = _debugCurrentBuildTarget;
isSupportedOperation = true;
return true;
}());
if (isSupportedOperation) {
return result;
}
throw UnsupportedError('debugCurrentBuildTarget is not supported in release builds');
}
Element? _debugCurrentBuildTarget; Element? _debugCurrentBuildTarget;
/// Establishes a scope in which calls to [State.setState] are forbidden, and /// Establishes a scope in which calls to [State.setState] are forbidden, and
...@@ -2676,6 +2696,25 @@ class BuildOwner { ...@@ -2676,6 +2696,25 @@ class BuildOwner {
assert(_debugStateLockLevel >= 0); assert(_debugStateLockLevel >= 0);
} }
void _runWithCurrentBuildTarget(Element element, VoidCallback callback) {
assert(_debugStateLocked);
Element? debugPreviousBuildTarget;
assert(() {
debugPreviousBuildTarget = _debugCurrentBuildTarget;
_debugCurrentBuildTarget = element;
return true;
}());
try {
callback();
} finally {
assert(() {
assert(_debugCurrentBuildTarget == element);
_debugCurrentBuildTarget = debugPreviousBuildTarget;
return true;
}());
}
}
/// Establishes a scope for updating the widget tree, and calls the given /// Establishes a scope for updating the widget tree, and calls the given
/// `callback`, if any. Then, builds all the elements that were marked as /// `callback`, if any. Then, builds all the elements that were marked as
/// dirty using [scheduleBuildFor], in depth order. /// dirty using [scheduleBuildFor], in depth order.
...@@ -2718,12 +2757,9 @@ class BuildOwner { ...@@ -2718,12 +2757,9 @@ class BuildOwner {
try { try {
_scheduledFlushDirtyElements = true; _scheduledFlushDirtyElements = true;
if (callback != null) { if (callback != null) {
assert(_debugStateLocked); _runWithCurrentBuildTarget(context, () {
Element? debugPreviousBuildTarget;
assert(() { assert(() {
context._debugSetAllowIgnoredCallsToMarkNeedsBuild(true); context._debugSetAllowIgnoredCallsToMarkNeedsBuild(true);
debugPreviousBuildTarget = _debugCurrentBuildTarget;
_debugCurrentBuildTarget = context;
return true; return true;
}()); }());
_dirtyElementsNeedsResorting = false; _dirtyElementsNeedsResorting = false;
...@@ -2732,12 +2768,11 @@ class BuildOwner { ...@@ -2732,12 +2768,11 @@ class BuildOwner {
} finally { } finally {
assert(() { assert(() {
context._debugSetAllowIgnoredCallsToMarkNeedsBuild(false); context._debugSetAllowIgnoredCallsToMarkNeedsBuild(false);
assert(_debugCurrentBuildTarget == context);
_debugCurrentBuildTarget = debugPreviousBuildTarget;
_debugElementWasRebuilt(context); _debugElementWasRebuilt(context);
return true; return true;
}()); }());
} }
});
} }
_dirtyElements.sort(Element._sort); _dirtyElements.sort(Element._sort);
_dirtyElementsNeedsResorting = false; _dirtyElementsNeedsResorting = false;
...@@ -4369,19 +4404,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext { ...@@ -4369,19 +4404,7 @@ abstract class Element extends DiagnosticableTree implements BuildContext {
return true; return true;
}()); }());
assert(_lifecycleState == _ElementLifecycle.active); assert(_lifecycleState == _ElementLifecycle.active);
assert(owner!._debugStateLocked); owner!._runWithCurrentBuildTarget(this, performRebuild);
Element? debugPreviousBuildTarget;
assert(() {
debugPreviousBuildTarget = owner!._debugCurrentBuildTarget;
owner!._debugCurrentBuildTarget = this;
return true;
}());
performRebuild();
assert(() {
assert(owner!._debugCurrentBuildTarget == this);
owner!._debugCurrentBuildTarget = debugPreviousBuildTarget;
return true;
}());
assert(!_dirty); assert(!_dirty);
} }
...@@ -5586,6 +5609,7 @@ abstract class RenderObjectElement extends Element { ...@@ -5586,6 +5609,7 @@ abstract class RenderObjectElement extends Element {
} }
@override @override
@mustCallSuper
void performRebuild() { void performRebuild() {
assert(() { assert(() {
_debugDoingBuild = true; _debugDoingBuild = true;
......
...@@ -1494,6 +1494,44 @@ void main() { ...@@ -1494,6 +1494,44 @@ void main() {
expect(GestureBinding.instance!.pointerRouter.debugGlobalRouteCount, pointerRouterCount); expect(GestureBinding.instance!.pointerRouter.debugGlobalRouteCount, pointerRouterCount);
expect(RawKeyboard.instance.keyEventHandler, same(rawKeyEventHandler)); expect(RawKeyboard.instance.keyEventHandler, same(rawKeyEventHandler));
}); });
testWidgets('Errors in build', (WidgetTester tester) async {
final ErrorWidgetBuilder oldErrorBuilder = ErrorWidget.builder;
ErrorWidget.builder = (FlutterErrorDetails detail) => throw AssertionError();
try {
final FlutterExceptionHandler? oldErrorHandler = FlutterError.onError;
FlutterError.onError = (FlutterErrorDetails detail) {};
try {
int buildCount = 0;
void handleBuild() => buildCount++;
expect(tester.binding.buildOwner!.debugCurrentBuildTarget, isNull);
await tester.pumpWidget(_WidgetThatCanThrowInBuild(onBuild: handleBuild));
expect(buildCount, 1);
await tester.pumpWidget(_WidgetThatCanThrowInBuild(onBuild: handleBuild, shouldThrow: true));
tester.takeException();
expect(buildCount, 2);
expect(tester.binding.buildOwner!.debugCurrentBuildTarget, isNull);
} finally {
FlutterError.onError = oldErrorHandler;
}
} finally {
ErrorWidget.builder = oldErrorBuilder;
}
});
}
class _WidgetThatCanThrowInBuild extends StatelessWidget {
const _WidgetThatCanThrowInBuild({required this.onBuild, this.shouldThrow = false});
final VoidCallback onBuild;
final bool shouldThrow;
@override
Widget build(BuildContext context) {
onBuild();
assert(!shouldThrow);
return Container();
}
} }
class _FakeFocusManager implements FocusManager { class _FakeFocusManager implements FocusManager {
......
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