The new `BUILD.gn` files in the Engine tree can't go under
`build/secondary` because Skia still has its own, and they'd be selected
first. So, this PR puts the new `BUILD.gn` files under `flutter/skia`.
## Description
Before we deprecate the `RawKeyEvent` code, it needs to provide parity in functionality. One thing that is missing is the `eventSource` field from the `RawKeyEventDataAndroid` class, which provides the device type for the event.
See https://developer.android.com/reference/android/view/InputDevice#SOURCE_KEYBOARD for an example.
This PR implements that support, and sets the source to `KeyEventDeviceType.keyboard` for platforms that don't provide this information. The main thing it does is add the enum `KeyEventDeviceType`, and a new field `KeyData.deviceType`.
## Related Issues
- https://github.com/flutter/flutter/issues/136419
## Tests
- Updated tests to also read/write/verify this property.
As part of eliminating the Flutter buildroot
(https://github.com/flutter/flutter/issues/67373), we are moving all
third-party dependencies from //third_party to //flutter/third_party.
Once all third-party dependencies have been migrated, tooling and config
will be moved and the buildroot will be eliminated altogether.
No tests changed because there is no semantic change to this PR. This is
simply relocating a dependency.
The tests for FlutterCompositor are not useful. The current tests test two things:
1. That the mocks we set up behave the way we set them up to behave.
2. That the implementation of FlutterCompositor is the current implementation of FlutterCompositor.
As an example, consider FlutterCompositorTest.TestPresent: 70317028f2/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm (L107-L137)
Ostensibly, this test verifies that the onPresent callback configured in our fake FlutterViewProvider implementation is called when FlutterCompositor::Present() is called.
However, taking a look at the mocking setup:
70317028f2/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm (L47-L85)
We do the following:
1. Mock out FlutterSurfaceManager such that when a surface is requested, we hand back a mock surface. A little gross since we're relying on some knowledge of implementation details of the compositor, but let's take this as reasonable for now.
2. We mock out `FlutterSurface asFlutterMetalTexture` to return a mock texture. Again, we're getting a bit deep into implementation details that the test shouldn't know about, but let's assume this gets us somewhere.
3. We mock out `FlutterSurfaceManager present:notify:` to actually call the `onPresent` callback if it's passed in.
In effect, we're testing that:
1. We configured our mock for `FlutterSurfaceManager present:notify:` to call onPresent.
2. That `FlutterCompositor::Present` actually calls `FlutterSurfaceManager present:notify:` despite that being a simple implementation detail of that call.
This removes these tests. I have filed the following issue to track refactoring this class for testability and adding tests: https://github.com/flutter/flutter/issues/137648
Encountered these tests as part of deflaking and cleaning up memory allocations throughout the macOS desktop tests.
Issue: https://github.com/flutter/flutter/issues/137648
Issue: https://github.com/flutter/flutter/issues/104789
Issue: https://github.com/flutter/flutter/issues/127441
Issue: https://github.com/flutter/flutter/issues/124840
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
As part of eliminating the Flutter buildroot
(https://github.com/flutter/flutter/issues/67373), we are moving all
third-party dependencies from //third_party to //flutter/third_party.
Once all third-party dependencies have been migrated, tooling and config
will be moved and the buildroot will be eliminated altogether.
No tests changed because there is no semantic change to this PR. This is
simply relocating a dependency.
When we enter hybrid composition mode we 'pause' the default RenderSurface (implemented by SurfaceView or TextureView) and swap to an ImageReader based RenderSurface.
When we return from hybrid composition mode we recreate and re-initialize the real RenderSurface as if it was being used for the first time.
This broke Platform Views in an internal app b/306122497 because we would incorrectly tell the texture to attach when it was never detached.
This CL changes the protocol so that when we return from hybrid composition mode we only swap the RenderSurface and do not re-create it. This avoids doing a bunch of unnecessary work and fixes the logic error of re-attaching textures that were never detached.
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Adds a gtest test fixture mixin and convenience class that instantiates an NSAutoreleasePool at the beginning of each test and flushes it at the end; this allows Objective-C tests using ARC to free allocations at the end of each test.
Adds a subclass for the macOS accessibility bridge tests that instantiates and re-uses an NSWindow* across any tests that use it. This is because instantiating, closing, and immediately collecting an NSWindow results in a crash.
Prior to this patch, tests started failing (on my machine) around the 855th iteration, and issued the following warning on the 101st iteration:
```
2023-10-26 13:02:45.390829-0700 flutter_desktop_darwin_unittests[40837:1509026] [Window] WARNING: NSWindow has detected an excessive live window count of 101. Window 0x1423 of class 'NSWindow' created after passing the threshold of 100. This window is not necessarily the cause, and this warning will only be shown once per window class. (
0 AppKit 0x0000000192820d28 -[NSWindow _setWindowNumber:] + 684
1 AppKit 0x00000001933050e4 _NXCreateWindow + 284
2 AppKit 0x0000000192901ae0 -[NSWindow _commonAwake] + 672
3 AppKit 0x000000019281ff00 -[NSWindow _commonInitFrame:styleMask:backing:defer:] + 972
4 AppKit 0x000000019281f798 -[NSWindow _initContent:styleMask:backing:defer:contentView:] + 796
5 AppKit 0x000000019281f470 -[NSWindow initWithContentRect:styleMask:backing:defer:] + 48
6 flutter_desktop_darwin_unittests 0x0000000100001e3c _ZN7flutter7testing89AccessibilityBridgeMacTest_SendsAccessibilityCreateNotificationToWindowOfFlutterView_Test8TestBodyEv + 328
```
See: http://www.openradar.me/FB13291861
Issue: https://github.com/flutter/flutter/issues/104789
Issue: https://github.com/flutter/flutter/issues/127441
Issue: https://github.com/flutter/flutter/issues/124840
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts flutter/engine#46171
Initiated by: jonahwilliams
This change reverts the following previous change:
Original Description:
The "rasterizer screenshot" VM service API uses the Skia software backend, even when running with impeller. At best, this leads to confusing half working screenshots that are missing images.
See https://github.com/flutter/flutter/issues/135052
We should disable this functionality when running with impeller. Perhaps this could be reworked to use picture toImage instead of the offscreen so that it uses the "real" backend regardless of the platform it is running on.
Fixes https://github.com/flutter/flutter/issues/135052
This refactors how high contrast is implemented on Windows:
1. Added a test to verify accessibility features are updated when a view is created. This prevents staleness issues as the Windows embedder isn't notified of accessibility changes while in headless mode.
1. Moved high contrast mode detection to `WindowsProcTable` from `FlutterWindow` to remove engine to view to window plumbing.
1. `FlutterWindow` and `FlutterWindowsEngine` now share their `WindowsProcTable` (which is used for mocking and polyfilling win32 APIs) to reduce redundant dynamic loading.
This pull request contains no functional changes.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The "rasterizer screenshot" VM service API uses the Skia software backend, even when running with impeller. At best, this leads to confusing half working screenshots that are missing images.
See https://github.com/flutter/flutter/issues/135052
We should disable this functionality when running with impeller. Perhaps this could be reworked to use picture toImage instead of the offscreen so that it uses the "real" backend regardless of the platform it is running on.
Fixes https://github.com/flutter/flutter/issues/135052
This PR relands https://github.com/flutter/engine/pull/44473.
The previous PR was immediately reverted after merging because we found that the PR could cause illegal renders to be skipped on debug builds but crash the app on release builds. This PR makes the `Animator::Render` skip illegal renders as well. This should not be the final shape of this feature, and thus a TODO is added.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
As far as I can tell, this functionality has _always_ been falling back to software rendering with Skia when Impeller was enabled. This is at best extremely misleading. Since I started putting Impeller specific objects in the display list it introduced crashes, and must be disabled.
Fixes https://github.com/flutter/flutter/issues/136847
Eliminate unnecessary calls to [NSViewController loadView]. To quote the [documentation][loadview] for this method: "Do not call this method. If you require this method to be called, access the view property."
In several of the existing tests, we do read viewController.view, and the other tests pass without this line regardless.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Trace GPU execution time on GLES using GL_EXT_disjoint_timer_query. This requires a per-app opt in from the Android Manifest with the key `"io.flutter.embedding.android.EnableOpenGLGPUTracing` set to true.
In the SendsAccessibilityCreateNotificationToWindowOfFlutterView test in AccessibilityBridgeMacTest, we look for an accessibility notification by name in a map stored in the test accessibility bridge. If there is no such notification, bail out immediately.
Previously we got a crash since we called `second` to retrieve the value associated with the map entry we find, without actually checking that we'd actually found an entry.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
There was an internal global change which turned `-Wc99-designator`. This causes the following error:
```
error: nested designators are a C99 extension [-Werror,-Wc99-designator]
65 | .metal.struct_size = sizeof(FlutterMetalRendererConfig),
| ^~~~~~~~~~~~~~~~~~
```
This seems like a trivial (?) fix, so perhaps we can fix it properly and possibly remove b9fbd310d6/build/config/compiler/BUILD.gn (L667) after this PR lands.
*List which issues are fixed by this PR. You must list at least one issue.*
b/300573112
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Fixes https://github.com/flutter/flutter/issues/130880 , by making sure when the text is cleared the `markedTextRange` is cleared with it.
`[UITextField replaceRange:withText:]` always sets the selection range to the end of the replacement text, and removes the current `markedTextRange`. This PR makes the input plugin do the same in the `replaceRange:withText:` implementation.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1. Using arbitrary struct passed as const reference to `OCMStub` now fails in OCMock. Down the line this will result with `object_getClass` being called in [`OCMArg.m`](https://github.com/erikdoe/ocmock/blob/master/Source/OCMock/OCMArg.m#L129-L133) with the address of the reference, which is not a valid class instance. This seems to have worked pre-sonoma, but it seems like a weird thing to rely on.
2. `NSResponder` mock can not be set to view controller anymore. The controller will try to access an ivar of the `NSResponder`, but mocked responder does not have the ivar of original objects which will result on invalid selector being called on a `NSMutableArray` one of the ivar of mock objects. Solution for this is to inherit from `NSResponder` and forward calls to mocked object.
3. Adding `flutter::kModifierFlagShiftLeft` to a modifier flag containing `kCGEventFlagMaskShift`. The assertion was introduced in https://github.com/flutter/engine/pull/46230 but i missed the test failure because of the problems above.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This reverts commit 670b5bd8e714a1fd2bd137aa2c5b36760b8ce89b.
With https://github.com/flutter/engine/pull/46911, the Platform.script API will return a URI for the compiled temporary wrapper script generated by "flutter test". This will cause issues for tests that expect Platform.script to reflect the directory where "flutter test" was launched.
This addresses the problem in https://github.com/flutter/flutter/issues/12847 which changed slightly over time.
Today, `Platform.script` does not give an empty `file` URI, it gives something like `file://path/to/package/main.dart` _regardless of how the file is actually named_.
After this change, it will give the absolute path to the file being run under test.
So before this change, the new test would have a URI like
```
file:///Users/dnfield/src/flutter/engine/src/main.dart
```
And now it has
```
file:///Users/dnfield/src/flutter/engine/src/out/host_debug_unopt_arm64/gen/platform_test.dart.dill
```
This is going to be helpful in generating relative paths from the test file.
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
- Refactor the fence waiting code to only wait on Android >= 33.
- Log a warning message once per image rendering target on Android >= 29 && < 33.
- Add a simple unit test of ImageReaderPlatformViewRenderTargets.
The Impeller ContextVK contains a ConcurrentMessageLoop whose threads may invoke Dart timeline APIs. The Dart APIs will create a thread-local object that will be deleted during thread shutdown. Therefore, these threads should not outlive the engine/Shell and Dart VM.
Previously, RunTester held the ImpellerVulkanContextHolder on the stack, and its reference to the ContextVK would be dropped while exiting the function after the Shell is destructed.
This PR moves the contents of the tester's ImpellerVulkanContextHolder out of the instance on the stack and into a lambda owned by the Shell.
It also reenables the flutter_tester Impeller tests in the run_tests script.
Follows up on #46389
That patch was too permissive in cases where a build system enables impeller but not vulkan. This change makes the build succeed in such systems.
