Support fixture tests for Windows embedder #35273

cbracken · 2022-08-09T17:29:50Z

This adds support for running end-to-end tests that use a live engine to
run Dart test fixtures. This enables testing the public Windows C API in
//flutter/shell/platform/windows/public/flutter_windows.h

This only adds support for a single test entrypoint (main). A followup
patch will add support for this. See:
flutter/flutter#93537

Issue: flutter/flutter#87299

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

cbracken · 2022-08-09T17:33:04Z

shell/platform/windows/testing/windows_test_config_builder.h

+struct ViewControllerDeleter {
+  typedef FlutterDesktopViewControllerRef pointer;
+  void operator()(FlutterDesktopViewControllerRef engine) {
+    FlutterDesktopViewControllerDestroy(engine);


Note that this doesn't return a boolean which is why there's no FML_CHECK here.

cbracken · 2022-08-09T17:37:03Z

shell/platform/windows/testing/windows_test_context.h

+
+ private:
+  std::wstring assets_path_;
+  std::wstring icu_data_path_ = L"icudtl.dat";


From an API point of view, it would have been nice to use std::string, but since we're passing these to the engine (via the Windows C API), they need to have a lifetime that meets or exceeds that of the engine, so converting to local std::wstrings in, e.g. WindowsTestConfigBuilder::GetEngineProperties, would mean the engine would hold pointers to garbage memory.

This adds support for running end-to-end tests that use a live engine to run Dart test fixtures. This enables testing the public Windows C API in //flutter/shell/platform/windows/public/flutter_windows.h This only adds support for a single test entrypoint (main). A followup patch will add support for this. See: flutter/flutter#93537 Issue: flutter/flutter#87299

loic-sharma

LGTM!

loic-sharma · 2022-08-09T17:39:21Z

shell/platform/windows/testing/windows_test_config_builder.cc

+}
+
+void WindowsConfigBuilder::InitializeCOM() const {
+  FML_CHECK(SUCCEEDED(::CoInitializeEx(nullptr, COINIT_MULTITHREADED)));


Should this be COINIT_APARTMENTTHREADED to match what the app uses?

See: https://github.com/flutter/flutter/blob/0fa8f765e437be84eadecb96d36a54083f015ceb/examples/api/windows/runner/main.cpp#L22

For the test, this is intentionally COINIT_MULTITHREADED to match what flutter_tester does:

engine/shell/testing/tester_main.cc

Lines 423 to 425 in d2489cd

#if defined(FML_OS_WIN)

CoInitializeEx(nullptr, COINIT_MULTITHREADED);

#endif // defined(FML_OS_WIN)

Honestly, this is primarily to allow me to be a little bit lazy with COM initialization in the tests, rather than trigger COM initialisation on each thread in the test context we create that uses COM.

)

@pragma

This adds a dart_entrypoint field to FlutterDesktopEngineProperties in the public C Windows API, which mirrors that in the embedder API. When a null or empty entrypoint is specified, a default entrypoint of 'main' is assumed. Otherwise, the app is launched at the top-level function specified, which must be annotated with @pragma('vm:entry-point') in the Dart source. This change is backward-compatible for existing users of the Windows C API and the C++ client wrapper API. To avoid breaking backward compatibility, this patch preserves the entry_point parameter to FlutterDesktopEngineRun in the public Windows C API as well as in the FlutterEngine::Run method in the C++ client wrapper API. The entrypoint can be specified in either the engine properties struct or via the parameter, but if conflicting non-empty values are specified, the engine launch will intentionally fail with an error message. This change has no effect on existing Flutter Windows desktop apps and no migration is required, because our app templates never specify a custom entrypoint, nor was the option to specify one via the old method particularly feasible, because the FlutterViewController class constructor immediately invokes FlutterViewControllerCreate which immediately launches the engine passed to it with a null entrypoint argument, so long as the engine is not already running. However, running the engine without a view controller previously resulted in errors due to failure to create a rendering surface. This is a followup patch to #35273 which added support for running Dart fixture tests with a live Windows embedder engine. Fixes: flutter/flutter#93537 Related: flutter/flutter#87299

This adds support for running end-to-end tests that use a live engine to run Dart test fixtures. This enables testing the public Windows C API in //flutter/shell/platform/windows/public/flutter_windows.h This only adds support for a single test entrypoint (main). A followup patch will add support for this. See: flutter/flutter#93537 Issue: flutter/flutter#87299

@pragma

This adds a dart_entrypoint field to FlutterDesktopEngineProperties in the public C Windows API, which mirrors that in the embedder API. When a null or empty entrypoint is specified, a default entrypoint of 'main' is assumed. Otherwise, the app is launched at the top-level function specified, which must be annotated with @pragma('vm:entry-point') in the Dart source. This change is backward-compatible for existing users of the Windows C API and the C++ client wrapper API. To avoid breaking backward compatibility, this patch preserves the entry_point parameter to FlutterDesktopEngineRun in the public Windows C API as well as in the FlutterEngine::Run method in the C++ client wrapper API. The entrypoint can be specified in either the engine properties struct or via the parameter, but if conflicting non-empty values are specified, the engine launch will intentionally fail with an error message. This change has no effect on existing Flutter Windows desktop apps and no migration is required, because our app templates never specify a custom entrypoint, nor was the option to specify one via the old method particularly feasible, because the FlutterViewController class constructor immediately invokes FlutterViewControllerCreate which immediately launches the engine passed to it with a null entrypoint argument, so long as the engine is not already running. However, running the engine without a view controller previously resulted in errors due to failure to create a rendering surface. This is a followup patch to flutter#35273 which added support for running Dart fixture tests with a live Windows embedder engine. Fixes: flutter/flutter#93537 Related: flutter/flutter#87299

cbracken requested review from stuartmorgan, loic-sharma, gspencergoog and yaakovschectman August 9, 2022 17:29

flutter-dashboard bot added the platform-windows label Aug 9, 2022

cbracken force-pushed the win-c-api-testing branch from 2378ccc to 4decf31 Compare August 9, 2022 17:32

cbracken commented Aug 9, 2022

View reviewed changes

cbracken force-pushed the win-c-api-testing branch from 4decf31 to c60924e Compare August 9, 2022 17:39

cbracken force-pushed the win-c-api-testing branch from c60924e to 2bb1d10 Compare August 9, 2022 17:50

loic-sharma approved these changes Aug 9, 2022

View reviewed changes

cbracken merged commit 0f386d5 into flutter:main Aug 9, 2022

cbracken deleted the win-c-api-testing branch August 9, 2022 19:43

engine-flutter-autoroll mentioned this pull request Aug 9, 2022

Roll Flutter Engine from d2489cd91788 to 0f386d5c0cdd (2 revisions) flutter/flutter#109252

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 9, 2022

0f386d5 Support fixture tests for Windows embedder (flutter/engine#35273

37bbfd7

)

cbracken mentioned this pull request Aug 10, 2022

Support custom entrypoints in public Windows API #35285

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support fixture tests for Windows embedder #35273

Support fixture tests for Windows embedder #35273

cbracken commented Aug 9, 2022

cbracken Aug 9, 2022

cbracken Aug 9, 2022 •

edited

Loading

loic-sharma left a comment

loic-sharma Aug 9, 2022

cbracken Aug 9, 2022 •

edited

Loading

	#if defined(FML_OS_WIN)
	CoInitializeEx(nullptr, COINIT_MULTITHREADED);
	#endif // defined(FML_OS_WIN)

Support fixture tests for Windows embedder #35273

Support fixture tests for Windows embedder #35273

Conversation

cbracken commented Aug 9, 2022

Pre-launch Checklist

cbracken Aug 9, 2022

Choose a reason for hiding this comment

cbracken Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

loic-sharma left a comment

Choose a reason for hiding this comment

loic-sharma Aug 9, 2022

Choose a reason for hiding this comment

cbracken Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

cbracken Aug 9, 2022 •

edited

Loading

cbracken Aug 9, 2022 •

edited

Loading