Unverified Commit ce61eda7 authored by Loïc Sharma's avatar Loïc Sharma Committed by GitHub

[Windows] Ensure window is shown (#127046)

## Background

The Windows runner has a race at startup:

1. **Platform thread**: creates a hidden window
2. **Platform thread**: launches the Flutter engine
3. **UI/Raster threads**: renders the first frame
4. **Platform thread**: Registers a callback to show the window once the next frame has been rendered.

Steps 3 and 4 happen in parallel and it is possible for step 3 to complete before step 4 starts. In this scenario, the next frame callback is never called and the window is never shown.

As a result the `windows_startup_test`'s test, which [verifies that the "show window" callback is called](https://github.com/flutter/flutter/blob/1f09a8662dad3bb1959b24e9124e05e2b9dbff1d/dev/integration_tests/windows_startup_test/windows/runner/flutter_window.cpp#L60-L64), can flake if the first frame is rendered before the show window callback has been registered.

## Solution

This change makes the runner schedule a frame after it registers the next frame callback. If step 3 hasn't completed yet, this no-ops as a frame is already scheduled. If step 3 has already completed, a new frame will be rendered, which will call the next frame callback and show the window.

Part of https://github.com/flutter/flutter/issues/119415

See this thread for alternatives that were considered: https://github.com/flutter/engine/pull/42061#issuecomment-1550080722
parent 41e4c586
......@@ -35,6 +35,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -35,6 +35,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -63,6 +63,11 @@ bool FlutterWindow::OnCreate() {
visible = true;
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
// Create a method channel to check the window's visibility.
flutter::MethodChannel<> channel(
flutter_controller_->engine()->messenger(), "tests.flutter.dev/windows_startup_test",
......
......@@ -35,6 +35,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -35,6 +35,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -35,6 +35,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -35,6 +35,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -98,6 +98,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
......@@ -64,6 +64,9 @@ class WindowsProject extends FlutterProjectPlatform implements CmakeBasedProject
/// The native entrypoint's CMake specification.
File get runnerCmakeFile => runnerDirectory.childFile('CMakeLists.txt');
/// The native entrypoint's file that adds Flutter to the window.
File get runnerFlutterWindowFile => runnerDirectory.childFile('flutter_window.cpp');
/// The native entrypoint's resource file. Used to configure things
/// like the application icon, name, and version.
File get runnerResourceFile => runnerDirectory.childFile('Runner.rc');
......
......@@ -18,6 +18,7 @@ import '../convert.dart';
import '../flutter_plugins.dart';
import '../globals.dart' as globals;
import '../migrations/cmake_custom_command_migration.dart';
import 'migrations/show_window_migration.dart';
import 'migrations/version_migration.dart';
import 'visual_studio.dart';
......@@ -54,6 +55,7 @@ Future<void> buildWindows(WindowsProject windowsProject, BuildInfo buildInfo, {
final List<ProjectMigrator> migrators = <ProjectMigrator>[
CmakeCustomCommandMigration(windowsProject, globals.logger),
VersionMigration(windowsProject, globals.logger),
ShowWindowMigration(windowsProject, globals.logger),
];
final ProjectMigration migration = ProjectMigration(migrators);
......
// 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 '../../base/file_system.dart';
import '../../base/project_migrator.dart';
import '../../cmake_project.dart';
import 'utils.dart';
const String _before = r'''
flutter_controller_->engine()->SetNextFrameCallback([&]() {
this->Show();
});
return true;
''';
const String _after = r'''
flutter_controller_->engine()->SetNextFrameCallback([&]() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
''';
/// Migrates Windows apps to ensure the window is shown.
///
/// This prevents a race condition between Flutter rendering the first frame
/// and the app registering the callback to show the window on the first frame.
/// See https://github.com/flutter/flutter/issues/119415.
class ShowWindowMigration extends ProjectMigrator {
ShowWindowMigration(WindowsProject project, super.logger)
: _file = project.runnerFlutterWindowFile;
final File _file;
@override
void migrate() {
// Skip this migration if the affected file does not exist. This indicates
// the app has done non-trivial changes to its runner and this migration
// might not work as expected if applied.
if (!_file.existsSync()) {
logger.printTrace('''
windows/runner/flutter_window.cpp file not found, skipping show window migration.
This indicates non-trivial changes have been made to the Windows runner in the
"windows" folder. If needed, you can reset the Windows runner by deleting the
"windows" folder and then using the "flutter create --platforms=windows ." command.
''');
return;
}
// Migrate the windows/runner/flutter_window.cpp file.
final String originalContents = _file.readAsStringSync();
final String newContents = replaceFirst(
originalContents,
_before,
_after,
);
if (originalContents != newContents) {
logger.printStatus(
'windows/runner/flutter_window.cpp does not ensure the show window '
'callback is called, updating.'
);
_file.writeAsStringSync(newContents);
}
}
}
// 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.
/// Creates a new string with the first occurrence of [before] replaced by
/// [after].
///
/// If the [originalContents] uses CRLF line endings, the [before] and [after]
/// will be converted to CRLF line endings before the replacement is made.
/// This is necessary for users that have git autocrlf enabled.
///
/// Example:
/// ```dart
/// 'a\n'.replaceFirst('a\n', 'b\n'); // 'b\n'
/// 'a\r\n'.replaceFirst('a\n', 'b\n'); // 'b\r\n'
/// ```
String replaceFirst(String originalContents, String before, String after) {
final String result = originalContents.replaceFirst(before, after);
if (result != originalContents) {
return result;
}
final String beforeCrlf = before.replaceAll('\n', '\r\n');
final String afterCrlf = after.replaceAll('\n', '\r\n');
return originalContents.replaceFirst(beforeCrlf, afterCrlf);
}
......@@ -5,6 +5,7 @@
import '../../base/file_system.dart';
import '../../base/project_migrator.dart';
import '../../cmake_project.dart';
import 'utils.dart';
const String _cmakeFileBefore = r'''
# Apply the standard set of build settings. This can be removed for applications
......@@ -96,7 +97,7 @@ This indicates non-trivial changes have been made to the Windows runner in the
// Migrate the windows/runner/CMakeLists.txt file.
final String originalCmakeContents = _cmakeFile.readAsStringSync();
final String newCmakeContents = _replaceFirst(
final String newCmakeContents = replaceFirst(
originalCmakeContents,
_cmakeFileBefore,
_cmakeFileAfter,
......@@ -108,7 +109,7 @@ This indicates non-trivial changes have been made to the Windows runner in the
// Migrate the windows/runner/Runner.rc file.
final String originalResourceFileContents = _resourceFile.readAsStringSync();
final String newResourceFileContents = _replaceFirst(
final String newResourceFileContents = replaceFirst(
originalResourceFileContents,
_resourceFileBefore,
_resourceFileAfter,
......@@ -121,27 +122,3 @@ This indicates non-trivial changes have been made to the Windows runner in the
}
}
}
/// Creates a new string with the first occurrence of [before] replaced by
/// [after].
///
/// If the [originalContents] uses CRLF line endings, the [before] and [after]
/// will be converted to CRLF line endings before the replacement is made.
/// This is necessary for users that have git autocrlf enabled.
///
/// Example:
/// ```dart
/// 'a\n'.replaceFirst('a\n', 'b\n'); // 'b\n'
/// 'a\r\n'.replaceFirst('a\n', 'b\n'); // 'b\r\n'
/// ```
String _replaceFirst(String originalContents, String before, String after) {
final String result = originalContents.replaceFirst(before, after);
if (result != originalContents) {
return result;
}
final String beforeCrlf = before.replaceAll('\n', '\r\n');
final String afterCrlf = after.replaceAll('\n', '\r\n');
return originalContents.replaceFirst(beforeCrlf, afterCrlf);
}
......@@ -31,6 +31,11 @@ bool FlutterWindow::OnCreate() {
this->Show();
});
// Flutter can complete the first frame before the "show window" callback is
// registered. Ensure a frame is pending to ensure the window is shown.
// This no-ops if the first frame hasn't completed yet.
flutter_controller_->ForceRedraw();
return true;
}
......
// 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:file/file.dart';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/cmake_project.dart';
import 'package:flutter_tools/src/windows/migrations/show_window_migration.dart';
import 'package:test/fake.dart';
import '../../../src/common.dart';
void main () {
group('Windows Flutter show window migration', () {
late MemoryFileSystem memoryFileSystem;
late BufferLogger testLogger;
late FakeWindowsProject mockProject;
late File flutterWindowFile;
setUp(() {
memoryFileSystem = MemoryFileSystem.test();
flutterWindowFile = memoryFileSystem.file('flutter_window.cpp');
testLogger = BufferLogger(
terminal: Terminal.test(),
outputPreferences: OutputPreferences.test(),
);
mockProject = FakeWindowsProject(flutterWindowFile);
});
testWithoutContext('skipped if Flutter window file is missing', () {
final ShowWindowMigration migration = ShowWindowMigration(
mockProject,
testLogger,
);
migration.migrate();
expect(flutterWindowFile.existsSync(), isFalse);
expect(
testLogger.traceText,
contains(
'windows/runner/flutter_window.cpp file not found, '
'skipping show window migration'
),
);
expect(testLogger.statusText, isEmpty);
});
testWithoutContext('skipped if nothing to migrate', () {
const String flutterWindowContents = 'Nothing to migrate';
flutterWindowFile.writeAsStringSync(flutterWindowContents);
final DateTime updatedAt = flutterWindowFile.lastModifiedSync();
final ShowWindowMigration migration = ShowWindowMigration(
mockProject,
testLogger,
);
migration.migrate();
expect(flutterWindowFile.lastModifiedSync(), updatedAt);
expect(flutterWindowFile.readAsStringSync(), flutterWindowContents);
expect(testLogger.statusText, isEmpty);
});
testWithoutContext('skipped if already migrated', () {
const String flutterWindowContents =
' flutter_controller_->engine()->SetNextFrameCallback([&]() {\n'
' this->Show();\n'
' });\n'
'\n'
' // Flutter can complete the first frame before the "show window" callback is\n'
' // registered. Ensure a frame is pending to ensure the window is shown.\n'
" // This no-ops if the first frame hasn't completed yet.\n"
' flutter_controller_->ForceRedraw();\n'
'\n'
' return true;\n';
flutterWindowFile.writeAsStringSync(flutterWindowContents);
final DateTime updatedAt = flutterWindowFile.lastModifiedSync();
final ShowWindowMigration migration = ShowWindowMigration(
mockProject,
testLogger,
);
migration.migrate();
expect(flutterWindowFile.lastModifiedSync(), updatedAt);
expect(flutterWindowFile.readAsStringSync(), flutterWindowContents);
expect(testLogger.statusText, isEmpty);
});
testWithoutContext('skipped if already migrated (CRLF)', () {
const String flutterWindowContents =
' flutter_controller_->engine()->SetNextFrameCallback([&]() {\r\n'
' this->Show();\r\n'
' });\r\n'
'\r\n'
' // Flutter can complete the first frame before the "show window" callback is\r\n'
' // registered. Ensure a frame is pending to ensure the window is shown.\r\n'
" // This no-ops if the first frame hasn't completed yet.\r\n"
' flutter_controller_->ForceRedraw();\r\n'
'\r\n'
' return true;\r\n';
flutterWindowFile.writeAsStringSync(flutterWindowContents);
final DateTime updatedAt = flutterWindowFile.lastModifiedSync();
final ShowWindowMigration migration = ShowWindowMigration(
mockProject,
testLogger,
);
migration.migrate();
expect(flutterWindowFile.lastModifiedSync(), updatedAt);
expect(flutterWindowFile.readAsStringSync(), flutterWindowContents);
expect(testLogger.statusText, isEmpty);
});
testWithoutContext('migrates project to ensure window is shown', () {
flutterWindowFile.writeAsStringSync(
' flutter_controller_->engine()->SetNextFrameCallback([&]() {\n'
' this->Show();\n'
' });\n'
'\n'
' return true;\n'
);
final ShowWindowMigration migration = ShowWindowMigration(
mockProject,
testLogger,
);
migration.migrate();
expect(flutterWindowFile.readAsStringSync(),
' flutter_controller_->engine()->SetNextFrameCallback([&]() {\n'
' this->Show();\n'
' });\n'
'\n'
' // Flutter can complete the first frame before the "show window" callback is\n'
' // registered. Ensure a frame is pending to ensure the window is shown.\n'
" // This no-ops if the first frame hasn't completed yet.\n"
' flutter_controller_->ForceRedraw();\n'
'\n'
' return true;\n'
);
expect(testLogger.statusText, contains('windows/runner/flutter_window.cpp does not ensure the show window callback is called, updating.'));
});
testWithoutContext('migrates project to ensure window is shown (CRLF)', () {
flutterWindowFile.writeAsStringSync(
' flutter_controller_->engine()->SetNextFrameCallback([&]() {\r\n'
' this->Show();\r\n'
' });\r\n'
'\r\n'
' return true;\r\n'
);
final ShowWindowMigration migration = ShowWindowMigration(
mockProject,
testLogger,
);
migration.migrate();
expect(flutterWindowFile.readAsStringSync(),
' flutter_controller_->engine()->SetNextFrameCallback([&]() {\r\n'
' this->Show();\r\n'
' });\r\n'
'\r\n'
' // Flutter can complete the first frame before the "show window" callback is\r\n'
' // registered. Ensure a frame is pending to ensure the window is shown.\r\n'
" // This no-ops if the first frame hasn't completed yet.\r\n"
' flutter_controller_->ForceRedraw();\r\n'
'\r\n'
' return true;\r\n'
);
expect(testLogger.statusText, contains('windows/runner/flutter_window.cpp does not ensure the show window callback is called, updating.'));
});
});
}
class FakeWindowsProject extends Fake implements WindowsProject {
FakeWindowsProject(this.runnerFlutterWindowFile);
@override
final File runnerFlutterWindowFile;
}
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