Skip to content

Commit

Permalink
[Windows] Ensure window is shown (flutter#127046)
Browse files Browse the repository at this point in the history
## 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 flutter#119415

See this thread for alternatives that were considered: flutter/engine#42061 (comment)
  • Loading branch information
loic-sharma authored May 19, 2023
1 parent 41e4c58 commit ce61eda
Show file tree
Hide file tree
Showing 15 changed files with 348 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions dev/integration_tests/ui/windows/runner/flutter_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions dev/manual_tests/windows/runner/flutter_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions examples/flutter_view/windows/runner/flutter_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions examples/hello_world/windows/runner/flutter_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions examples/layers/windows/runner/flutter_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions examples/platform_channel/windows/runner/flutter_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions packages/flutter_tools/lib/src/cmake_project.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 2 additions & 0 deletions packages/flutter_tools/lib/src/windows/build_windows.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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);
}
}
}
27 changes: 27 additions & 0 deletions packages/flutter_tools/lib/src/windows/migrations/utils.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit ce61eda

Please sign in to comment.